Page MenuHomePhabricator

Context-free rendering of "T2 had blocking task T1 closed" is missing information
Closed, ResolvedPublic

Description

Since we updated to latest Phabricator version last week, I've noticed a couple of times that notifications refuse to mark themselves as "read" even after visiting the ticket or diff in question.

First few times this happened I manually cleared it with "mark all as read" which worked. I am certain that at least once this happened with a differential revision notification although I have no screen shots of those times.

Now it's come up twice more so thought I'd report it.

See: {F160802}

In this screen shot: you can see ticket 15392 appears twice in the list (with same timestamp/message). From the open tabs you can see that I have viewed it and I DID refresh notifications page afterwards.

The other unread one indicated in the badge is a totally different ticket which exhibits same behaviour - two identical entries in the notifications list one of which is marked as read and the other is not despite having reloaded both ticket and notifications page several times. The only difference is that in the other example further down the notification list. the one stuck as unread appears before the read one.

My wild guess is that the bug is that these tickets are causing double notifications for some reason and that the auto-mark-as-read functionality on view is only finding one of the records and marking that as read.

No one is online right now who can tell me exactly which version/commit we have deployed right now but it was updated to the head of master on Friday 23 May.

If you can think of any more info that might help debug please ask.

Thanks.

Event Timeline

banks raised the priority of this task from to Needs Triage.
banks updated the task description. (Show Details)
banks added a subscriber: banks.

I think this is a bad rendering of the feed story. That is, the story currently reads like this:

alincoln closed blocking task T1.

This rendering is appropriate when the event is rendered contextually (on a task detail page), but not when it is rendered noncontextually (in feed or notifications). The story should read more like this (albeit ideally with less awkward phrasing):

alincoln closed T1, a task blocking T2.

This story is about the task T2 ("it had a blocking task closed") but there's no link to T2 in the story rendering. Clicking the T1 link doesn't clear the notification because the story isn't about T1.

As an awkward workaround, if you use the notification menu (the flame in the main menu bar) and click the body of the notification (i.e., not the link) that should take you to T2 and clear the notification.

I'll fix the story rendering.

(These notifications need some polish in general: I'd like to always keep them out of the main feed, and not send you the T1 had blocker T2 closed notification if you get the T2 was closed notification. However, we don't have other notifications which follow either of those rules right now, so I need to tweak some internals to support those changes.)

epriestley renamed this task from Unread notification count gets stuck on certain items to Context-free rendering of "T2 had blocking task T1 closed" is missing information.May 26 2014, 12:49 PM

Oh, right, this is why:

case self::TYPE_UNBLOCK:

  // TODO: We should probably not show these in feed; they're highly
  // redundant. For now, just use the normal titles. Right now, we can't
  // publish something to noficiations without also publishing it to feed.
  // Fix that, then stop these from rendering in feed only.

  break;