Page MenuHomePhabricator

Fix an issue where internal paging of notifications could fail if some notifications are not visible
ClosedPublic

Authored by epriestley on Apr 22 2019, 2:27 PM.
Tags
None
Referenced Files
F18804005: D20455.id.diff
Sat, Oct 18, 5:24 AM
F18786717: D20455.diff
Tue, Oct 14, 8:06 PM
F18703835: D20455.id48809.diff
Sun, Sep 28, 5:12 AM
F18675436: D20455.id48809.diff
Thu, Sep 25, 5:45 PM
F18658654: D20455.id48825.diff
Sep 23 2025, 7:41 AM
F18653765: D20455.diff
Sep 21 2025, 5:33 PM
F18631558: D20455.id48825.diff
Sep 16 2025, 3:43 PM
F18623922: D20455.id48809.diff
Sep 15 2025, 6:06 PM
Subscribers

Details

Summary

Ref T13266. See https://discourse.phabricator-community.org/t/notification-page-throws-unrecoverable-fatal-error/2651/.

The "notifications" query currently uses offset paging for no apparent reason (just a legacy issue?), so some of the paging code is only reachable internally.

  • Stop it from using offset paging, since modern cursor paging is fine here (and Feed has used cursor paging for a long time).
  • Fix the non-offset paging to work like Feed.

Also:

  • Remove a couple of stub methods with no callsites after cursor refactoring.
Test Plan
  • Set things up so I had more than 100 notifications and some in the first 100 were policy filtered, to reproduce the issue (I just made FeedStory return NO_ONE as a visibility policy).
  • Applied the patch, notifications now page cleanly.
  • Verified that "Next Page" took me to the right place in the result list.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable