diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ 'names' => array( 'conpherence.pkg.css' => 'e68cf1fa', 'conpherence.pkg.js' => '15191c65', - 'core.pkg.css' => '39061f68', + 'core.pkg.css' => 'c30f6eaa', 'core.pkg.js' => 'e1f0f7bd', 'differential.pkg.css' => '06dc617c', 'differential.pkg.js' => 'c2ca903a', @@ -38,7 +38,7 @@ 'rsrc/css/application/almanac/almanac.css' => 'dbb9b3af', 'rsrc/css/application/auth/auth.css' => '0877ed6e', 'rsrc/css/application/base/main-menu-view.css' => '1802a242', - 'rsrc/css/application/base/notification-menu.css' => '10685bd4', + 'rsrc/css/application/base/notification-menu.css' => 'ef480927', 'rsrc/css/application/base/phui-theme.css' => '9f261c6b', 'rsrc/css/application/base/standard-page-view.css' => '34ee718b', 'rsrc/css/application/chatlog/chatlog.css' => 'd295b020', @@ -768,7 +768,7 @@ 'phabricator-nav-view-css' => '694d7723', 'phabricator-notification' => '4f774dac', 'phabricator-notification-css' => '457861ec', - 'phabricator-notification-menu-css' => '10685bd4', + 'phabricator-notification-menu-css' => 'ef480927', 'phabricator-object-selector-css' => '85ee8ce6', 'phabricator-phtize' => 'd254d646', 'phabricator-prefab' => '77b0ae28', diff --git a/src/applications/notification/controller/PhabricatorNotificationPanelController.php b/src/applications/notification/controller/PhabricatorNotificationPanelController.php --- a/src/applications/notification/controller/PhabricatorNotificationPanelController.php +++ b/src/applications/notification/controller/PhabricatorNotificationPanelController.php @@ -6,6 +6,10 @@ public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); + $unread_count = $viewer->getUnreadNotificationCount(); + + $warning = $this->prunePhantomNotifications($unread_count); + $query = id(new PhabricatorNotificationQuery()) ->setViewer($viewer) ->withUserPHIDs(array($viewer->getPHID())) @@ -66,13 +70,12 @@ )); $content = hsprintf( - '%s%s%s', + '%s%s%s%s', $header, + $warning, $content, $connection_ui); - $unread_count = $viewer->getUnreadNotificationCount(); - $json = array( 'content' => $content, 'number' => (int)$unread_count, @@ -80,4 +83,81 @@ return id(new AphrontAjaxResponse())->setContent($json); } + + private function prunePhantomNotifications($unread_count) { + // See T8953. If you have an unread notification about an object you + // do not have permission to view, it isn't possible to clear it by + // visiting the object. Identify these notifications and mark them as + // read. + + $viewer = $this->getViewer(); + + if (!$unread_count) { + return null; + } + + $table = new PhabricatorFeedStoryNotification(); + $conn = $table->establishConnection('r'); + + $rows = queryfx_all( + $conn, + 'SELECT chronologicalKey, primaryObjectPHID FROM %T + WHERE userPHID = %s AND hasViewed = 0', + $table->getTableName(), + $viewer->getPHID()); + if (!$rows) { + return null; + } + + $map = array(); + foreach ($rows as $row) { + $map[$row['primaryObjectPHID']][] = $row['chronologicalKey']; + } + + $handles = $viewer->loadHandles(array_keys($map)); + $purge_keys = array(); + foreach ($handles as $handle) { + $phid = $handle->getPHID(); + if ($handle->isComplete()) { + continue; + } + + foreach ($map[$phid] as $chronological_key) { + $purge_keys[] = $chronological_key; + } + } + + if (!$purge_keys) { + return null; + } + + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); + + $conn = $table->establishConnection('w'); + queryfx( + $conn, + 'UPDATE %T SET hasViewed = 1 + WHERE userPHID = %s AND chronologicalKey IN (%Ls)', + $table->getTableName(), + $viewer->getPHID(), + $purge_keys); + + PhabricatorUserCache::clearCache( + PhabricatorUserNotificationCountCacheType::KEY_COUNT, + $viewer->getPHID()); + + unset($unguarded); + + return phutil_tag( + 'div', + array( + 'class' => 'phabricator-notification phabricator-notification-warning', + ), + pht( + '%s notification(s) about objects which no longer exist or which '. + 'you can no longer see were discarded.', + phutil_count($purge_keys))); + } + + } 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 @@ -76,31 +76,31 @@ return $stories; } - protected function buildWhereClause(AphrontDatabaseConnection $conn_r) { - $where = array(); + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); if ($this->userPHIDs !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'notif.userPHID IN (%Ls)', $this->userPHIDs); } if ($this->unread !== null) { $where[] = qsprintf( - $conn_r, + $conn, 'notif.hasViewed = %d', (int)!$this->unread); } if ($this->keys) { $where[] = qsprintf( - $conn_r, + $conn, 'notif.chronologicalKey IN (%Ls)', $this->keys); } - return $this->formatWhereClause($where); + return $where; } protected function getResultCursor($item) { diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1651,6 +1651,14 @@ 'Destroyed %s credentials of type "%s".', ), + '%s notification(s) about objects which no longer exist or which '. + 'you can no longer see were discarded.' => array( + 'One notification about an object which no longer exists or which '. + 'you can no longer see was discarded.', + '%s notifications about objects which no longer exist or which '. + 'you can no longer see were discarded.', + ), + ); } diff --git a/webroot/rsrc/css/application/base/notification-menu.css b/webroot/rsrc/css/application/base/notification-menu.css --- a/webroot/rsrc/css/application/base/notification-menu.css +++ b/webroot/rsrc/css/application/base/notification-menu.css @@ -68,6 +68,10 @@ color: {$lightgreytext}; } +.phabricator-notification-warning { + background: {$sh-yellowbackground}; +} + .phabricator-notification-list .phabricator-notification-unread, .phabricator-notification-menu .phabricator-notification-unread { background: {$hoverblue}; @@ -95,7 +99,7 @@ .phabricator-notification-unread .phabricator-notification-foot .phabricator-notification-status { font-size: 7px; - color: {$lightgreytext}; + color: {$lightbluetext}; position: absolute; display: inline-block; top: 6px;