From 618cec23d8f05d1dcbc04dad26f11373120c7d7a Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 2 Dec 2015 11:35:24 -0800 Subject: [PATCH] Make notification counts properly translatable Summary: Ref T9132. When I've touched `PhabricatorApplication` I keep hitting this bad `pht()` junk. The warning is correct, these strings are not extactable and can not be translated. Fix it so they can be extracted and translated. Broadly, in all cases we want to render one of these: > 95 Things (for fewer than some limit) > 99+ Things (when we hit the limit) Test Plan: Looked at homepage status counts, moused over them, saw reasonable strings. Grepped for removed method. Reviewers: chad Reviewed By: chad Subscribers: joshuaspence Maniphest Tasks: T9132 Differential Revision: https://secure.phabricator.com/D14638 --- .../PhabricatorAuditApplication.php | 25 +++++---- .../base/PhabricatorApplication.php | 16 ------ .../PhabricatorDifferentialApplication.php | 38 ++++++------- .../PhabricatorFlagsApplication.php | 11 ++-- .../PhabricatorManiphestApplication.php | 14 +++-- .../view/PhabricatorApplicationLaunchView.php | 14 ++++- .../PhabricatorPeopleApplication.php | 18 ++++--- .../PhabricatorPhrequentApplication.php | 15 +++--- .../PhabricatorPonderApplication.php | 8 --- .../PhabricatorUSEnglishTranslation.php | 54 +++++++++---------- 10 files changed, 110 insertions(+), 103 deletions(-) diff --git a/src/applications/audit/application/PhabricatorAuditApplication.php b/src/applications/audit/application/PhabricatorAuditApplication.php index 858993e5aa..4f4c5a56d1 100644 --- a/src/applications/audit/application/PhabricatorAuditApplication.php +++ b/src/applications/audit/application/PhabricatorAuditApplication.php @@ -47,6 +47,7 @@ final class PhabricatorAuditApplication extends PhabricatorApplication { public function loadStatus(PhabricatorUser $user) { $status = array(); + $limit = self::MAX_STATUS_ITEMS; $phids = PhabricatorAuditCommentEditor::loadAuditPHIDsForUser($user); @@ -54,14 +55,16 @@ final class PhabricatorAuditApplication extends PhabricatorApplication { ->setViewer($user) ->withAuthorPHIDs(array($user->getPHID())) ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_CONCERN) - ->setLimit(self::MAX_STATUS_ITEMS); + ->setLimit($limit); $commits = $query->execute(); $count = count($commits); - $count_str = self::formatStatusCount( - $count, - '%s Problem Commits', - '%d Problem Commit(s)'); + if ($count >= $limit) { + $count_str = pht('%s+ Problem Commit(s)', new PhutilNumber($limit - 1)); + } else { + $count_str = pht('%s Problem Commit(s)', new PhutilNumber($count)); + } + $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) @@ -72,14 +75,16 @@ final class PhabricatorAuditApplication extends PhabricatorApplication { ->setViewer($user) ->withNeedsAuditByPHIDs($phids) ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_OPEN) - ->setLimit(self::MAX_STATUS_ITEMS); + ->setLimit($limit); $commits = $query->execute(); $count = count($commits); - $count_str = self::formatStatusCount( - $count, - '%s Commits Awaiting Audit', - '%d Commit(s) Awaiting Audit'); + if ($count >= $limit) { + $count_str = pht('%s+ Problem Commit(s)', new PhutilNumber($limit - 1)); + } else { + $count_str = pht('%s Problem Commit(s)', new PhutilNumber($count)); + } + $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php index dff4106324..c1cda07710 100644 --- a/src/applications/base/PhabricatorApplication.php +++ b/src/applications/base/PhabricatorApplication.php @@ -288,22 +288,6 @@ abstract class PhabricatorApplication return array(); } - /** - * @return string - * @task ui - */ - final 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 9203d0c522..e8113dc3bd 100644 --- a/src/applications/differential/application/PhabricatorDifferentialApplication.php +++ b/src/applications/differential/application/PhabricatorDifferentialApplication.php @@ -104,21 +104,23 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication { } public function loadStatus(PhabricatorUser $user) { + $limit = self::MAX_STATUS_ITEMS; + $revisions = id(new DifferentialRevisionQuery()) ->setViewer($user) ->withResponsibleUsers(array($user->getPHID())) ->withStatus(DifferentialRevisionQuery::STATUS_OPEN) ->needRelationships(true) - ->setLimit(self::MAX_STATUS_ITEMS) + ->setLimit($limit) ->execute(); $status = array(); - if (count($revisions) == self::MAX_STATUS_ITEMS) { + if (count($revisions) >= $limit) { $all_count = count($revisions); - $all_count_str = self::formatStatusCount( - $all_count, - '%s Active Reviews', - '%d Active Review(s)'); + $all_count_str = pht( + '%s+ Active Review(s)', + new PhutilNumber($limit - 1)); + $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) @@ -131,10 +133,10 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication { array($user->getPHID())); $blocking = count($blocking); - $blocking_str = self::formatStatusCount( - $blocking, - '%s Reviews Blocking Others', - '%d Review(s) Blocking Others'); + $blocking_str = pht( + '%s Review(s) Blocking Others', + new PhutilNumber($blocking)); + $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) @@ -142,10 +144,10 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication { ->setCount($blocking); $active = count($active); - $active_str = self::formatStatusCount( - $active, - '%s Reviews Need Attention', - '%d Review(s) Need Attention'); + $active_str = pht( + '%s Review(s) Need Attention', + new PhutilNumber($active)); + $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) @@ -153,10 +155,10 @@ final class PhabricatorDifferentialApplication extends PhabricatorApplication { ->setCount($active); $waiting = count($waiting); - $waiting_str = self::formatStatusCount( - $waiting, - '%s Reviews Waiting on Others', - '%d Review(s) Waiting on Others'); + $waiting_str = pht( + '%s Review(s) Waiting on Others', + new PhutilNumber($waiting)); + $type = PhabricatorApplicationStatusView::TYPE_INFO; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) diff --git a/src/applications/flag/application/PhabricatorFlagsApplication.php b/src/applications/flag/application/PhabricatorFlagsApplication.php index 232f6f4dca..fad4c0bc8e 100644 --- a/src/applications/flag/application/PhabricatorFlagsApplication.php +++ b/src/applications/flag/application/PhabricatorFlagsApplication.php @@ -34,6 +34,7 @@ final class PhabricatorFlagsApplication extends PhabricatorApplication { public function loadStatus(PhabricatorUser $user) { $status = array(); + $limit = self::MAX_STATUS_ITEMS; $flags = id(new PhabricatorFlagQuery()) ->setViewer($user) @@ -42,10 +43,12 @@ final class PhabricatorFlagsApplication extends PhabricatorApplication { ->execute(); $count = count($flags); - $count_str = self::formatStatusCount( - $count, - '%s Flagged Objects', - '%d Flagged Object(s)'); + if ($count >= $limit) { + $count_str = pht('%s+ Flagged Object(s)', new PhutilNumber($limit - 1)); + } else { + $count_str = pht('%s Flagged Object(s)', new PhutilNumber($count)); + } + $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) diff --git a/src/applications/maniphest/application/PhabricatorManiphestApplication.php b/src/applications/maniphest/application/PhabricatorManiphestApplication.php index 758d85f879..7edb1a12d9 100644 --- a/src/applications/maniphest/application/PhabricatorManiphestApplication.php +++ b/src/applications/maniphest/application/PhabricatorManiphestApplication.php @@ -80,16 +80,20 @@ final class PhabricatorManiphestApplication extends PhabricatorApplication { return $status; } + $limit = self::MAX_STATUS_ITEMS; + $query = id(new ManiphestTaskQuery()) ->setViewer($user) ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants()) ->withOwners(array($user->getPHID())) - ->setLimit(self::MAX_STATUS_ITEMS); + ->setLimit($limit); + $count = count($query->execute()); - $count_str = self::formatStatusCount( - $count, - '%s Assigned Tasks', - '%d Assigned Task(s)'); + if ($count >= $limit) { + $count_str = pht('%s+ Assigned Task(s)', new PhutilNumber($limit - 1)); + } else { + $count_str = pht('%s Assigned Task(s)', new PhutilNumber($count)); + } $type = PhabricatorApplicationStatusView::TYPE_WARNING; $status[] = id(new PhabricatorApplicationStatusView()) diff --git a/src/applications/meta/view/PhabricatorApplicationLaunchView.php b/src/applications/meta/view/PhabricatorApplicationLaunchView.php index 397838882e..c714e0daa6 100644 --- a/src/applications/meta/view/PhabricatorApplicationLaunchView.php +++ b/src/applications/meta/view/PhabricatorApplicationLaunchView.php @@ -72,7 +72,7 @@ final class PhabricatorApplicationLaunchView extends AphrontTagView { array( 'class' => 'phabricator-application-attention-count', ), - PhabricatorApplication::formatStatusCount($count)); + $this->formatStatusItemCount($count)); } @@ -82,8 +82,9 @@ final class PhabricatorApplicationLaunchView extends AphrontTagView { array( 'class' => 'phabricator-application-warning-count', ), - PhabricatorApplication::formatStatusCount($counts[$warning])); + $this->formatStatusItemCount($counts[$warning])); } + if (nonempty($count1) && nonempty($count2)) { $numbers = array($count1, ' / ', $count2); } else { @@ -132,4 +133,13 @@ final class PhabricatorApplicationLaunchView extends AphrontTagView { ); } + private function formatStatusItemCount($count) { + $limit = PhabricatorApplication::MAX_STATUS_ITEMS; + if ($count >= $limit) { + return pht('%s+', new PhutilNumber($limit - 1)); + } else { + return pht('%s', new PhutilNumber($count)); + } + } + } diff --git a/src/applications/people/application/PhabricatorPeopleApplication.php b/src/applications/people/application/PhabricatorPeopleApplication.php index b3a36687cb..9ab8a61bfd 100644 --- a/src/applications/people/application/PhabricatorPeopleApplication.php +++ b/src/applications/people/application/PhabricatorPeopleApplication.php @@ -95,14 +95,14 @@ final class PhabricatorPeopleApplication extends PhabricatorApplication { if (!$user->getIsAdmin()) { return array(); } + $limit = self::MAX_STATUS_ITEMS; $need_approval = id(new PhabricatorPeopleQuery()) ->setViewer($user) ->withIsApproved(false) ->withIsDisabled(false) - ->setLimit(self::MAX_STATUS_ITEMS) + ->setLimit($limit) ->execute(); - if (!$need_approval) { return array(); } @@ -110,10 +110,16 @@ 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'); + if ($count >= $limit) { + $count_str = pht( + '%s+ User(s) Need Approval', + new PhutilNumber($limit - 1)); + } else { + $count_str = pht( + '%s User(s) Need Approval', + new PhutilNumber($count)); + } + $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) diff --git a/src/applications/phrequent/application/PhabricatorPhrequentApplication.php b/src/applications/phrequent/application/PhabricatorPhrequentApplication.php index bc0c39994f..bded663ed9 100644 --- a/src/applications/phrequent/application/PhabricatorPhrequentApplication.php +++ b/src/applications/phrequent/application/PhabricatorPhrequentApplication.php @@ -48,17 +48,18 @@ final class PhabricatorPhrequentApplication extends PhabricatorApplication { public function loadStatus(PhabricatorUser $user) { $status = array(); + $limit = self::MAX_STATUS_ITEMS; // Show number of objects that are currently // being tracked for a user. - $count = PhrequentUserTimeQuery::getUserTotalObjectsTracked( - $user, - self::MAX_STATUS_ITEMS); - $count_str = self::formatStatusCount( - $count, - '%s Objects Tracked', - '%d Object(s) Tracked'); + $count = PhrequentUserTimeQuery::getUserTotalObjectsTracked($user, $limit); + if ($count >= $limit) { + $count_str = pht('%s+ Object(s) Tracked', new PhutilNumber($limit - 1)); + } else { + $count_str = pht('%s Object(s) Tracked', new PhutilNumber($count)); + } + $type = PhabricatorApplicationStatusView::TYPE_NEEDS_ATTENTION; $status[] = id(new PhabricatorApplicationStatusView()) ->setType($type) diff --git a/src/applications/ponder/application/PhabricatorPonderApplication.php b/src/applications/ponder/application/PhabricatorPonderApplication.php index 38546a8a7d..886fddcc05 100644 --- a/src/applications/ponder/application/PhabricatorPonderApplication.php +++ b/src/applications/ponder/application/PhabricatorPonderApplication.php @@ -28,14 +28,6 @@ final class PhabricatorPonderApplication extends PhabricatorApplication { return "\xE2\x97\xB3"; } - 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; - } - public function getRemarkupRules() { return array( new PonderRemarkupRule(), diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index 346e37977d..49aba80ade 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -55,49 +55,49 @@ final class PhabricatorUSEnglishTranslation 'There are %d aggregate facts in storage.', ), - '%d Commit(s) Awaiting Audit' => array( - '%d Commit Awaiting Audit', - '%d Commits Awaiting Audit', + '%s Commit(s) Awaiting Audit' => array( + '%s Commit Awaiting Audit', + '%s Commits Awaiting Audit', ), - '%d Problem Commit(s)' => array( - '%d Problem Commit', - '%d Problem Commits', + '%s Problem Commit(s)' => array( + '%s Problem Commit', + '%s Problem Commits', ), - '%d Review(s) Blocking Others' => array( - '%d Review Blocking Others', - '%d Reviews Blocking Others', + '%s Review(s) Blocking Others' => array( + '%s Review Blocking Others', + '%s Reviews Blocking Others', ), - '%d Review(s) Need Attention' => array( - '%d Review Needs Attention', - '%d Reviews Need Attention', + '%s Review(s) Need Attention' => array( + '%s Review Needs Attention', + '%s Reviews Need Attention', ), - '%d Review(s) Waiting on Others' => array( - '%d Review Waiting on Others', - '%d Reviews Waiting on Others', + '%s Review(s) Waiting on Others' => array( + '%s Review Waiting on Others', + '%s Reviews Waiting on Others', ), - '%d Active Review(s)' => array( - '%d Active Review', - '%d Active Reviews', + '%s Active Review(s)' => array( + '%s Active Review', + '%s Active Reviews', ), - '%d Flagged Object(s)' => array( - '%d Flagged Object', - '%d Flagged Objects', + '%s Flagged Object(s)' => array( + '%s Flagged Object', + '%s Flagged Objects', ), - '%d Object(s) Tracked' => array( - '%d Object Tracked', - '%d Objects Tracked', + '%s Object(s) Tracked' => array( + '%s Object Tracked', + '%s Objects Tracked', ), - '%d Assigned Task(s)' => array( - '%d Assigned Task', - '%d Assigned Tasks', + '%s Assigned Task(s)' => array( + '%s Assigned Task', + '%s Assigned Tasks', ), 'Show %d Lint Message(s)' => array(