@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

Use new Differential bucketing logic on default (non-dashboard) homepage

Summary:
Ref T10939. If you haven't installed a dashboard, we show an "Active Revisions" panel on the homepage by default. I waited a bit to update this, but the new buckets don't seem to have caused any major problems so far.

Update this to use the new logic. I'm just showing "must review" + "should review", which is similar to the old beahvior.

Also replace the notification count with this same number. This is a little different from the old behavior, but simpler, and I think we should probably move toward getting rid of these counts completely.

Test Plan:
- Viewed homepage as logged-in user, saw my revisions (including revisions I have authority over only because of project membership).
- Saw consistent notification count.
- Grepped for removed method.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10939

Differential Revision: https://secure.phabricator.com/D15950

+85 -115
+61 -51
src/applications/differential/application/PhabricatorDifferentialApplication.php
··· 102 102 ); 103 103 } 104 104 105 - public function loadStatus(PhabricatorUser $user) { 106 - $limit = self::MAX_STATUS_ITEMS; 105 + public static function loadNeedAttentionRevisions(PhabricatorUser $viewer) { 106 + if (!$viewer->isLoggedIn()) { 107 + return array(); 108 + } 107 109 108 - $revisions = id(new DifferentialRevisionQuery()) 109 - ->setViewer($user) 110 - ->withResponsibleUsers(array($user->getPHID())) 110 + $viewer_phid = $viewer->getPHID(); 111 + 112 + $responsible_phids = id(new DifferentialResponsibleDatasource()) 113 + ->setViewer($viewer) 114 + ->evaluateTokens(array($viewer_phid)); 115 + 116 + $revision_query = id(new DifferentialRevisionQuery()) 117 + ->setViewer($viewer) 111 118 ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) 119 + ->withResponsibleUsers($responsible_phids) 120 + ->needReviewerStatus(true) 112 121 ->needRelationships(true) 113 - ->setLimit($limit) 114 - ->execute(); 115 - 116 - $status = array(); 117 - if (count($revisions) >= $limit) { 118 - $all_count = count($revisions); 119 - $all_count_str = pht( 120 - '%s+ Active Review(s)', 121 - new PhutilNumber($limit - 1)); 122 + ->needFlags(true) 123 + ->needDrafts(true) 124 + ->setLimit(self::MAX_STATUS_ITEMS); 122 125 123 - $type = PhabricatorApplicationStatusView::TYPE_WARNING; 124 - $status[] = id(new PhabricatorApplicationStatusView()) 125 - ->setType($type) 126 - ->setText($all_count_str) 127 - ->setCount($all_count); 128 - } else { 129 - list($blocking, $active, $waiting) = 130 - DifferentialRevisionQuery::splitResponsible( 131 - $revisions, 132 - array($user->getPHID())); 126 + $revisions = $revision_query->execute(); 133 127 134 - $blocking = count($blocking); 135 - $blocking_str = pht( 136 - '%s Review(s) Blocking Others', 137 - new PhutilNumber($blocking)); 128 + $query = id(new PhabricatorSavedQuery()) 129 + ->attachParameterMap( 130 + array( 131 + 'responsiblePHIDs' => $responsible_phids, 132 + )); 138 133 139 - $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; 140 - $status[] = id(new PhabricatorApplicationStatusView()) 141 - ->setType($type) 142 - ->setText($blocking_str) 143 - ->setCount($blocking); 134 + $groups = id(new DifferentialRevisionRequiredActionResultBucket()) 135 + ->setViewer($viewer) 136 + ->newResultGroups($query, $revisions); 144 137 145 - $active = count($active); 146 - $active_str = pht( 147 - '%s Review(s) Need Attention', 148 - new PhutilNumber($active)); 138 + $include = array(); 139 + foreach ($groups as $group) { 140 + switch ($group->getKey()) { 141 + case DifferentialRevisionRequiredActionResultBucket::KEY_MUSTREVIEW: 142 + case DifferentialRevisionRequiredActionResultBucket::KEY_SHOULDREVIEW: 143 + foreach ($group->getObjects() as $object) { 144 + $include[] = $object; 145 + } 146 + break; 147 + default: 148 + break; 149 + } 150 + } 149 151 150 - $type = PhabricatorApplicationStatusView::TYPE_WARNING; 151 - $status[] = id(new PhabricatorApplicationStatusView()) 152 - ->setType($type) 153 - ->setText($active_str) 154 - ->setCount($active); 152 + return $include; 153 + } 155 154 156 - $waiting = count($waiting); 157 - $waiting_str = pht( 158 - '%s Review(s) Waiting on Others', 159 - new PhutilNumber($waiting)); 155 + public function loadStatus(PhabricatorUser $user) { 156 + $revisions = self::loadNeedAttentionRevisions($user); 157 + $limit = self::MAX_STATUS_ITEMS; 160 158 161 - $type = PhabricatorApplicationStatusView::TYPE_INFO; 162 - $status[] = id(new PhabricatorApplicationStatusView()) 163 - ->setType($type) 164 - ->setText($waiting_str) 165 - ->setCount($waiting); 159 + if (count($revisions) >= $limit) { 160 + $display_count = ($limit - 1); 161 + $display_label = pht( 162 + '%s+ Active Review(s)', 163 + new PhutilNumber($display_count)); 164 + } else { 165 + $display_count = count($revisions); 166 + $display_label = pht( 167 + '%s Review(s) Need Attention', 168 + new PhutilNumber($display_count)); 166 169 } 170 + 171 + $status = array(); 172 + 173 + $status[] = id(new PhabricatorApplicationStatusView()) 174 + ->setType(PhabricatorApplicationStatusView::TYPE_WARNING) 175 + ->setText($display_label) 176 + ->setCount($display_count); 167 177 168 178 return $status; 169 179 }
-44
src/applications/differential/query/DifferentialRevisionQuery.php
··· 1033 1033 } 1034 1034 } 1035 1035 1036 - 1037 - public static function splitResponsible(array $revisions, array $user_phids) { 1038 - $blocking = array(); 1039 - $active = array(); 1040 - $waiting = array(); 1041 - $status_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW; 1042 - 1043 - // Bucket revisions into $blocking (revisions where you are blocking 1044 - // others), $active (revisions you need to do something about) and $waiting 1045 - // (revisions you're waiting on someone else to do something about). 1046 - foreach ($revisions as $revision) { 1047 - $needs_review = ($revision->getStatus() == $status_review); 1048 - $filter_is_author = in_array($revision->getAuthorPHID(), $user_phids); 1049 - if (!$revision->getReviewers()) { 1050 - $needs_review = false; 1051 - $author_is_reviewer = false; 1052 - } else { 1053 - $author_is_reviewer = in_array( 1054 - $revision->getAuthorPHID(), 1055 - $revision->getReviewers()); 1056 - } 1057 - 1058 - // If exactly one of "needs review" and "the user is the author" is 1059 - // true, the user needs to act on it. Otherwise, they're waiting on 1060 - // it. 1061 - if ($needs_review ^ $filter_is_author) { 1062 - if ($needs_review) { 1063 - array_unshift($blocking, $revision); 1064 - } else { 1065 - $active[] = $revision; 1066 - } 1067 - // User is author **and** reviewer. An exotic but configurable workflow. 1068 - // User needs to act on it double. 1069 - } else if ($needs_review && $author_is_reviewer) { 1070 - array_unshift($blocking, $revision); 1071 - $active[] = $revision; 1072 - } else { 1073 - $waiting[] = $revision; 1074 - } 1075 - } 1076 - 1077 - return array($blocking, $active, $waiting); 1078 - } 1079 - 1080 1036 private function loadReviewerAuthority( 1081 1037 array $revisions, 1082 1038 array $edges,
+5
src/applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php
··· 5 5 6 6 const BUCKETKEY = 'action'; 7 7 8 + const KEY_MUSTREVIEW = 'must-review'; 9 + const KEY_SHOULDREVIEW = 'should-review'; 10 + 8 11 private $objects; 9 12 10 13 public function getResultBucketName() { ··· 30 33 31 34 $groups[] = $this->newGroup() 32 35 ->setName(pht('Must Review')) 36 + ->setKey(self::KEY_MUSTREVIEW) 33 37 ->setNoDataString(pht('No revisions are blocked on your review.')) 34 38 ->setObjects($this->filterMustReview($phids)); 35 39 36 40 $groups[] = $this->newGroup() 37 41 ->setName(pht('Ready to Review')) 42 + ->setKey(self::KEY_SHOULDREVIEW) 38 43 ->setNoDataString(pht('No revisions are waiting on you to review them.')) 39 44 ->setObjects($this->filterShouldReview($phids)); 40 45
+9 -20
src/applications/home/controller/PhabricatorHomeMainController.php
··· 205 205 } 206 206 207 207 private function buildRevisionPanel() { 208 - $user = $this->getRequest()->getUser(); 209 - $user_phid = $user->getPHID(); 208 + $viewer = $this->getViewer(); 210 209 211 - $revision_query = id(new DifferentialRevisionQuery()) 212 - ->setViewer($user) 213 - ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) 214 - ->withResponsibleUsers(array($user_phid)) 215 - ->needRelationships(true) 216 - ->needFlags(true) 217 - ->needDrafts(true); 210 + $revisions = PhabricatorDifferentialApplication::loadNeedAttentionRevisions( 211 + $viewer); 218 212 219 - $revisions = $revision_query->execute(); 220 - 221 - list($blocking, $active) = DifferentialRevisionQuery::splitResponsible( 222 - $revisions, 223 - array($user_phid)); 224 - 225 - if (!$blocking && !$active) { 213 + if (!$revisions) { 226 214 return $this->renderMiniPanel( 227 215 pht('No Waiting Revisions'), 228 216 pht('No revisions are waiting on you.')); 229 217 } 230 218 231 219 $title = pht('Revisions Waiting on You'); 232 - $href = '/differential'; 220 + $href = '/differential/'; 233 221 $panel = new PHUIObjectBoxView(); 234 222 $panel->setHeader($this->renderSectionHeader($title, $href)); 235 223 236 224 $revision_view = id(new DifferentialRevisionListView()) 237 225 ->setHighlightAge(true) 238 - ->setRevisions(array_merge($blocking, $active)) 239 - ->setUser($user); 226 + ->setRevisions($revisions) 227 + ->setUser($viewer); 228 + 240 229 $phids = array_merge( 241 - array($user_phid), 230 + array($viewer->getPHID()), 242 231 $revision_view->getRequiredHandlePHIDs()); 243 232 $handles = $this->loadViewerHandles($phids); 244 233
+10
src/applications/search/buckets/PhabricatorSearchResultBucketGroup.php
··· 4 4 extends Phobject { 5 5 6 6 private $name; 7 + private $key; 7 8 private $noDataString; 8 9 private $objects; 9 10 ··· 23 24 24 25 public function getName() { 25 26 return $this->name; 27 + } 28 + 29 + public function setKey($key) { 30 + $this->key = $key; 31 + return $this; 32 + } 33 + 34 + public function getKey() { 35 + return $this->key; 26 36 } 27 37 28 38 public function setObjects(array $objects) {