Page MenuHomePhabricator
Files F15550138

D21603.id.diff
No OneTemporary
Actions

D21603.id.diff
View Options

diff --git a/src/applications/notification/query/PhabricatorNotificationQuery.php b/src/applications/notification/query/PhabricatorNotificationQuery.php
--- a/src/applications/notification/query/PhabricatorNotificationQuery.php
+++ b/src/applications/notification/query/PhabricatorNotificationQuery.php
@@ -63,25 +63,7 @@
$this->buildWhereClause($conn),
$this->buildLimitClause($conn));
- // See T13623. Although most queries for notifications return unique
- // stories, this isn't a guarantee.
- $story_map = ipull($data, null, 'chronologicalKey');
-
- $stories = PhabricatorFeedStory::loadAllFromRows(
- $story_map,
- $this->getViewer());
- $stories = mpull($stories, null, 'getChronologicalKey');
-
- $results = array();
- foreach ($data as $row) {
- $story_key = $row['chronologicalKey'];
- $has_viewed = $row['hasViewed'];
-
- $results[] = id(clone $stories[$story_key])
- ->setHasViewed($has_viewed);
- }
-
- return $results;
+ return $data;
}
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
@@ -111,14 +93,47 @@
return $where;
}
- protected function willFilterPage(array $stories) {
- foreach ($stories as $key => $story) {
+ protected function willFilterPage(array $rows) {
+ // See T13623. The policy model here is outdated and awkward.
+
+ // Users may have notifications about objects they can no longer see.
+ // Two ways this can arise: destroy an object; or change an object's
+ // view policy to exclude a user.
+
+ // "PhabricatorFeedStory::loadAllFromRows()" does its own policy filtering.
+ // This doesn't align well with modern query sequencing, but we should be
+ // able to get away with it by loading here.
+
+ // See T13623. Although most queries for notifications return unique
+ // stories, this isn't a guarantee.
+ $story_map = ipull($rows, null, 'chronologicalKey');
+
+ $viewer = $this->getViewer();
+ $stories = PhabricatorFeedStory::loadAllFromRows($story_map, $viewer);
+ $stories = mpull($stories, null, 'getChronologicalKey');
+
+ $results = array();
+ foreach ($rows as $row) {
+ $story_key = $row['chronologicalKey'];
+ $has_viewed = $row['hasViewed'];
+
+ if (!isset($stories[$story_key])) {
+ // NOTE: We can't call "didRejectResult()" here because we don't have
+ // a policy object to pass.
+ continue;
+ }
+
+ $story = id(clone $stories[$story_key])
+ ->setHasViewed($has_viewed);
+
if (!$story->isVisibleInNotifications()) {
- unset($stories[$key]);
+ continue;
}
+
+ $results[] = $story;
}
- return $stories;
+ return $results;
}
protected function getDefaultOrderVector() {

File Metadata

Mime Type
text/plain
Expires
Mon, Apr 28, 9:36 AM (6 h, 58 s)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7225020
Default Alt Text
D21603.id.diff (2 KB)

Event Timeline

Phabricator · Built by Phacility