Looks like I'm going through https://.../notification/ then marking them all as read, if new notifications have arrived in between, they are also marked as read. Sounds easy to miss some notifications because of this (especially for people like me who disabled email notifications).
Description
Revisions and Commits
Event Timeline
Notifications have a sequential ordering, so limiting the update to just notifications which are older than some time is easy. However, making sure the "Mark All Read" link in the dropdown menu always has a timestamp which aligns with user expectations may not be as easy.
Hmmm, I think we need to alter one of the existing key here:
feed_storynotification | CREATE TABLE `feed_storynotification` ( `userPHID` varchar(64) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL, `primaryObjectPHID` varchar(64) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL, `chronologicalKey` bigint(20) unsigned NOT NULL, `hasViewed` tinyint(1) NOT NULL, UNIQUE KEY `userPHID` (`userPHID`,`chronologicalKey`), KEY `userPHID_2` (`userPHID`,`hasViewed`,`primaryObjectPHID`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8
The userPHID key should change to (userPHID, chronologicalKey, hasViewed). We need this so we don't update every notification ever before a certain chronologicalKey; just the ones where hasViewed = 0.
Sound right? If so, can I DROP KEY then ADD KEY in my migration script or is that going to be bogus?
On the good news front, slapping some ?cronoKey=X into the three spots we render this seems easy at the moment. :)
I think it's fine to just add a where chronologicalKey < x clause to the existing query. It can still use the userPHID, hasViewed parts of they key, which are way more important. The result set should be tiny after those parts of the key are used in all non-pathological cases.
The UI case I'm worried about is the notification menu: X should probably not be "the time when the page was loaded" there. I'm not sure if we re-render the menu every time you open it, or what we do if a notification comes in while you have the menu open.
Provided that we (a) render when you open the menu and (b) regenerate the whole menu when you re-open it after a notification comes in (I think we do?), then just using whatever the top notification was when we rendered the menu as X is probably fine. Using the most recent notification for the /notification/ page is definitely fine, I think. But if we're doing anything weird with the dropdown menu it might need a lighter touch.