From 2b99b4add8ae571cabfb0ec75ae08ba998795862 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Fri, 12 Dec 2014 12:02:25 -0800 Subject: [PATCH] Home - limit "status" queries to 100 and show 99+ if we hit that Summary: Fixes T6595. This diff has two issues as is... 1) the differential data fetching is pretty cheesey, but it looks like we can't just issue three separate databases to get the right data? 2) the translations break, since I am turning this into a string (and not an int) so the whole pluralization bit fails. I think 1 is okay as is and 2 needs to be fixed though I am not sure how to best do that... Test Plan: loaded home page and it looked nice...! Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T6595 Differential Revision: https://secure.phabricator.com/D10979 --- .../PhabricatorAuditApplication.php | 18 +++-- .../base/PhabricatorApplication.php | 18 +++++ .../PhabricatorDifferentialApplication.php | 71 +++++++++++++------ .../PhabricatorFlagsApplication.php | 7 +- .../PhabricatorManiphestApplication.php | 9 ++- .../view/PhabricatorApplicationLaunchView.php | 4 +- .../PhabricatorPeopleApplication.php | 7 +- .../PhabricatorPhrequentApplication.php | 10 ++- .../query/PhrequentUserTimeQuery.php | 9 ++- .../PhabricatorPonderApplication.php | 1 + .../PhabricatorBaseEnglishTranslation.php | 10 +++ 11 files changed, 126 insertions(+), 38 deletions(-) diff --git a/src/applications/audit/application/PhabricatorAuditApplication.php b/src/applications/audit/application/PhabricatorAuditApplication.php index cbdf2ad5a5..ba8be0c5ed 100644 --- a/src/applications/audit/application/PhabricatorAuditApplication.php +++ b/src/applications/audit/application/PhabricatorAuditApplication.php @@ -54,28 +54,38 @@ final class PhabricatorAuditApplication extends PhabricatorApplication { $query = id(new DiffusionCommitQuery()) ->setViewer($user) ->withAuthorPHIDs(array($user->getPHID())) - ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_CONCERN); + ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_CONCERN) + ->setLimit(self::MAX_STATUS_ITEMS); $commits = $query->execute(); $count = count($commits); + $count_str = self::formatStatusCount( + $count, + '%s Problem Commits', + '%d Problem Commit(s)'); $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) - ->setText(pht('%d Problem Commit(s)', $count)) + ->setText($count_str) ->setCount($count); $query = id(new DiffusionCommitQuery()) ->setViewer($user) ->withAuditorPHIDs($phids) ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_OPEN) - ->withAuditAwaitingUser($user); + ->withAuditAwaitingUser($user) + ->setLimit(self::MAX_STATUS_ITEMS); $commits = $query->execute(); $count = count($commits); + $count_str = self::formatStatusCount( + $count, + '%s Commits Awaiting Audit', + '%d Commit(s) Awaiting Audit'); $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) - ->setText(pht('%d Commit(s) Awaiting Audit', $count)) + ->setText($count_str) ->setCount($count); return $status; diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index c58ad98560..fd8a8ca43c 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -9,6 +9,8 @@ */ abstract class PhabricatorApplication implements PhabricatorPolicyInterface { + const MAX_STATUS_ITEMS = 100; + const GROUP_CORE = 'core'; const GROUP_UTILITIES = 'util'; const GROUP_ADMIN = 'admin'; @@ -231,6 +233,22 @@ abstract class PhabricatorApplication implements PhabricatorPolicyInterface { return array(); } + /** + * @return string + * @task ui + */ + public static function formatStatusCount( + $count, + $limit_string = '%s', + $base_string = '%d') { + if ($count == self::MAX_STATUS_ITEMS) { + $count_str = pht($limit_string, ($count - 1).'+'); + } else { + $count_str = pht($base_string, $count); + } + return $count_str; + } + /** * You can provide an optional piece of flavor text for the application. This diff --git a/src/applications/differential/application/PhabricatorDifferentialApplication.php b/src/applications/differential/application/PhabricatorDifferentialApplication.php index 43cb463e5c..5fe4b9db39 100644 --- a/src/applications/differential/application/PhabricatorDifferentialApplication.php +++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php @@ -100,35 +100,60 @@ EOTEXT ->withResponsibleUsers(array($user->getPHID())) ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) ->needRelationships(true) + ->setLimit(self::MAX_STATUS_ITEMS) ->execute(); - list($blocking, $active, $waiting) = - DifferentialRevisionQuery::splitResponsible( - $revisions, - array($user->getPHID())); - $status = array(); + if (count($revisions) == self::MAX_STATUS_ITEMS) { + $all_count = count($revisions); + $all_count_str = self::formatStatusCount( + $all_count, + '%s Active Reviews', + '%d Active Review(s)'); + $type = PhabricatorApplicationStatusView::TYPE_WARNING; + $status[] = id(new PhabricatorApplicationStatusView()) + ->setType($type) + ->setText($all_count_str) + ->setCount($all_count); + } else { + list($blocking, $active, $waiting) = + DifferentialRevisionQuery::splitResponsible( + $revisions, + array($user->getPHID())); - $blocking = count($blocking); - $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; - $status[] = id(new PhabricatorApplicationStatusView()) - ->setType($type) - ->setText(pht('%d Review(s) Blocking Others', $blocking)) - ->setCount($blocking); + $blocking = count($blocking); + $blocking_str = self::formatStatusCount( + $blocking, + '%s Reviews Blocking Others', + '%d Review(s) Blocking Others'); + $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; + $status[] = id(new PhabricatorApplicationStatusView()) + ->setType($type) + ->setText($blocking_str) + ->setCount($blocking); - $active = count($active); - $type = PhabricatorApplicationStatusView::TYPE_WARNING; - $status[] = id(new PhabricatorApplicationStatusView()) - ->setType($type) - ->setText(pht('%d Review(s) Need Attention', $active)) - ->setCount($active); + $active = count($active); + $active_str = self::formatStatusCount( + $active, + '%s Reviews Need Attention', + '%d Review(s) Need Attention'); + $type = PhabricatorApplicationStatusView::TYPE_WARNING; + $status[] = id(new PhabricatorApplicationStatusView()) + ->setType($type) + ->setText($active_str) + ->setCount($active); - $waiting = count($waiting); - $type = PhabricatorApplicationStatusView::TYPE_INFO; - $status[] = id(new PhabricatorApplicationStatusView()) - ->setType($type) - ->setText(pht('%d Review(s) Waiting on Others', $waiting)) - ->setCount($waiting); + $waiting = count($waiting); + $waiting_str = self::formatStatusCount( + $waiting, + '%s Reviews Waiting on Others', + '%d Review(s) Waiting on Others'); + $type = PhabricatorApplicationStatusView::TYPE_INFO; + $status[] = id(new PhabricatorApplicationStatusView()) + ->setType($type) + ->setText($waiting_str) + ->setCount($waiting); + } return $status; } diff --git a/src/applications/flag/application/PhabricatorFlagsApplication.php b/src/applications/flag/application/PhabricatorFlagsApplication.php index 076abd06cc..874a50024b 100644 --- a/src/applications/flag/application/PhabricatorFlagsApplication.php +++ b/src/applications/flag/application/PhabricatorFlagsApplication.php @@ -38,13 +38,18 @@ final class PhabricatorFlagsApplication extends PhabricatorApplication { $flags = id(new PhabricatorFlagQuery()) ->setViewer($user) ->withOwnerPHIDs(array($user->getPHID())) + ->setLimit(self::MAX_STATUS_ITEMS) ->execute(); $count = count($flags); + $count_str = self::formatStatusCount( + $count, + '%s Flagged Objects', + '%d Flagged Object(s)'); $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) - ->setText(pht('%d Flagged Object(s)', $count)) + ->setText($count_str) ->setCount($count); return $status; diff --git a/src/applications/maniphest/application/PhabricatorManiphestApplication.php b/src/applications/maniphest/application/PhabricatorManiphestApplication.php index f6de7fc6c0..00bbc83c60 100644 --- a/src/applications/maniphest/application/PhabricatorManiphestApplication.php +++ b/src/applications/maniphest/application/PhabricatorManiphestApplication.php @@ -80,13 +80,18 @@ final class PhabricatorManiphestApplication extends PhabricatorApplication { $query = id(new ManiphestTaskQuery()) ->setViewer($user) ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants()) - ->withOwners(array($user->getPHID())); + ->withOwners(array($user->getPHID())) + ->setLimit(self::MAX_STATUS_ITEMS); $count = count($query->execute()); + $count_str = self::formatStatusCount( + $count, + '%s Assigned Tasks', + '%d Assigned Task(s)'); $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) - ->setText(pht('%s Assigned Task(s)', new PhutilNumber($count))) + ->setText($count_str) ->setCount($count); return $status; diff --git a/src/applications/meta/view/PhabricatorApplicationLaunchView.php b/src/applications/meta/view/PhabricatorApplicationLaunchView.php index 0e951b0dcd..550d9920be 100644 --- a/src/applications/meta/view/PhabricatorApplicationLaunchView.php +++ b/src/applications/meta/view/PhabricatorApplicationLaunchView.php @@ -73,7 +73,7 @@ final class PhabricatorApplicationLaunchView extends AphrontTagView { array( 'class' => 'phabricator-application-attention-count', ), - $count); + PhabricatorApplication::formatStatusCount($count)); } @@ -83,7 +83,7 @@ final class PhabricatorApplicationLaunchView extends AphrontTagView { array( 'class' => 'phabricator-application-warning-count', ), - $counts[$warning]); + PhabricatorApplication::formatStatusCount($counts[$warning])); } if (nonempty($count1) && nonempty($count2)) { $numbers = array($count1, ' / ', $count2); diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php index a057c0b803..8d7421cb12 100644 --- a/src/applications/people/application/PhabricatorPeopleApplication.php +++ b/src/applications/people/application/PhabricatorPeopleApplication.php @@ -91,6 +91,7 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication { ->setViewer($user) ->withIsApproved(false) ->withIsDisabled(false) + ->setLimit(self::MAX_STATUS_ITEMS) ->execute(); if (!$need_approval) { @@ -100,10 +101,14 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication { $status = array(); $count = count($need_approval); + $count_str = self::formatStatusCount( + $count, + '%s Users Need Approval', + '%d User(s) Need Approval'); $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) - ->setText(pht('%d User(s) Need Approval', $count)) + ->setText($count_str) ->setCount($count); return $status; diff --git a/src/applications/phrequent/application/PhabricatorPhrequentApplication.php b/src/applications/phrequent/application/PhabricatorPhrequentApplication.php index 2d54ddfa09..d03d92e371 100644 --- a/src/applications/phrequent/application/PhabricatorPhrequentApplication.php +++ b/src/applications/phrequent/application/PhabricatorPhrequentApplication.php @@ -52,11 +52,17 @@ final class PhabricatorPhrequentApplication extends PhabricatorApplication { // Show number of objects that are currently // being tracked for a user. - $count = PhrequentUserTimeQuery::getUserTotalObjectsTracked($user); + $count = PhrequentUserTimeQuery::getUserTotalObjectsTracked( + $user, + self::MAX_STATUS_ITEMS); + $count_str = self::formatStatusCount( + $count, + '%s Objects Tracked', + '%d Object(s) Tracked'); $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) - ->setText(pht('%d Object(s) Tracked', $count)) + ->setText($count_str) ->setCount($count); return $status; diff --git a/src/applications/phrequent/query/PhrequentUserTimeQuery.php b/src/applications/phrequent/query/PhrequentUserTimeQuery.php index a27e8aca01..06a9028a13 100644 --- a/src/applications/phrequent/query/PhrequentUserTimeQuery.php +++ b/src/applications/phrequent/query/PhrequentUserTimeQuery.php @@ -255,7 +255,8 @@ final class PhrequentUserTimeQuery } public static function getUserTotalObjectsTracked( - PhabricatorUser $user) { + PhabricatorUser $user, + $limit = PHP_INT_MAX) { $usertime_dao = new PhrequentUserTime(); $conn = $usertime_dao->establishConnection('r'); @@ -264,9 +265,11 @@ final class PhrequentUserTimeQuery $conn, 'SELECT COUNT(usertime.id) N FROM %T usertime '. 'WHERE usertime.userPHID = %s '. - 'AND usertime.dateEnded IS NULL', + 'AND usertime.dateEnded IS NULL '. + 'LIMIT %d', $usertime_dao->getTableName(), - $user->getPHID()); + $user->getPHID(), + $limit); return $count['N']; } diff --git a/src/applications/ponder/application/PhabricatorPonderApplication.php b/src/applications/ponder/application/PhabricatorPonderApplication.php index 694fa86316..1598aa7c15 100644 --- a/src/applications/ponder/application/PhabricatorPonderApplication.php +++ b/src/applications/ponder/application/PhabricatorPonderApplication.php @@ -30,6 +30,7 @@ final class PhabricatorPonderApplication extends PhabricatorApplication { public function loadStatus(PhabricatorUser $user) { // replace with "x new unanswered questions" or some such + // make sure to use self::formatStatusCount and friends...! $status = array(); return $status; diff --git a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php index 40735a6651..0360a400c2 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php @@ -172,11 +172,21 @@ abstract class PhabricatorBaseEnglishTranslation '%d Reviews Waiting on Others', ), + '%d Active Review(s)' => array( + '%d Active Review', + '%d Active Reviews', + ), + '%d Flagged Object(s)' => array( '%d Flagged Object', '%d Flagged Objects', ), + '%d Object(s) Tracked' => array( + '%d Object Tracked', + '%d Objects Tracked', + ), + '%d Unbreak Now Task(s)!' => array( '%d Unbreak Now Task!', '%d Unbreak Now Tasks!',