From eec6cd865c854fd00c8de41b8ca8ed0da78760e8 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Fri, 24 Feb 2017 15:21:23 -0800 Subject: [PATCH] Miscellanous badge fixes Summary: Ref T12270. Add transaction validation for name, alias, award, revoke. Change auto subscribe for authors. Fix some typos. Test Plan: Add badge, award badge, revoke badge, edit badge. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T12270 Differential Revision: https://secure.phabricator.com/D17412 --- .../PhabricatorBadgesAwardController.php | 2 +- ...icatorBadgesRemoveRecipientsController.php | 2 +- .../PhabricatorBadgesViewController.php | 9 ------ .../editor/PhabricatorBadgesEditEngine.php | 2 +- .../badges/editor/PhabricatorBadgesEditor.php | 24 +++++++++++++++ .../query/PhabricatorBadgesSearchEngine.php | 2 +- .../badges/storage/PhabricatorBadgesBadge.php | 2 +- ...PhabricatorBadgesBadgeAwardTransaction.php | 28 ++++++++++++++++++ ...habricatorBadgesBadgeFlavorTransaction.php | 17 +++++++++++ .../PhabricatorBadgesBadgeNameTransaction.php | 11 +++++++ ...habricatorBadgesBadgeRevokeTransaction.php | 29 +++++++++++++++++++ 11 files changed, 114 insertions(+), 14 deletions(-) diff --git a/src/applications/badges/controller/PhabricatorBadgesAwardController.php b/src/applications/badges/controller/PhabricatorBadgesAwardController.php index 3fa474018e..e934e4f2a1 100644 --- a/src/applications/badges/controller/PhabricatorBadgesAwardController.php +++ b/src/applications/badges/controller/PhabricatorBadgesAwardController.php @@ -67,7 +67,7 @@ final class PhabricatorBadgesAwardController )))); $dialog = $this->newDialog() - ->setTitle(pht('Grant Badge')) + ->setTitle(pht('Award Badge')) ->appendForm($form) ->addCancelButton($view_uri) ->addSubmitButton(pht('Award')); diff --git a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php index b26a2c2159..e6638b745c 100644 --- a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php +++ b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php @@ -29,7 +29,7 @@ final class PhabricatorBadgesRemoveRecipientsController return new Aphront404Response(); } - $view_uri = $this->getApplicationURI('view/'.$badge->getID().'/'); + $view_uri = $this->getApplicationURI('recipients/'.$badge->getID().'/'); if ($request->isFormPost()) { $xactions = array(); diff --git a/src/applications/badges/controller/PhabricatorBadgesViewController.php b/src/applications/badges/controller/PhabricatorBadgesViewController.php index 3c434ff1d5..a5390ea5f4 100644 --- a/src/applications/badges/controller/PhabricatorBadgesViewController.php +++ b/src/applications/badges/controller/PhabricatorBadgesViewController.php @@ -90,7 +90,6 @@ final class PhabricatorBadgesViewController $id = $badge->getID(); $edit_uri = $this->getApplicationURI("/edit/{$id}/"); $archive_uri = $this->getApplicationURI("/archive/{$id}/"); - $award_uri = $this->getApplicationURI("/recipients/{$id}/add/"); $curtain = $this->newCurtainView($badge); @@ -119,14 +118,6 @@ final class PhabricatorBadgesViewController ->setHref($archive_uri)); } - $curtain->addAction( - id(new PhabricatorActionView()) - ->setName('Add Recipients') - ->setIcon('fa-users') - ->setDisabled(!$can_edit) - ->setWorkflow(true) - ->setHref($award_uri)); - return $curtain; } diff --git a/src/applications/badges/editor/PhabricatorBadgesEditEngine.php b/src/applications/badges/editor/PhabricatorBadgesEditEngine.php index 597cf470f6..596e70cd13 100644 --- a/src/applications/badges/editor/PhabricatorBadgesEditEngine.php +++ b/src/applications/badges/editor/PhabricatorBadgesEditEngine.php @@ -92,7 +92,7 @@ final class PhabricatorBadgesEditEngine ->setIsRequired(true), id(new PhabricatorTextEditField()) ->setKey('flavor') - ->setLabel(pht('Flavor text')) + ->setLabel(pht('Flavor Text')) ->setDescription(pht('Short description of the badge.')) ->setConduitTypeDescription(pht('New badge flavor.')) ->setValue($object->getFlavor()) diff --git a/src/applications/badges/editor/PhabricatorBadgesEditor.php b/src/applications/badges/editor/PhabricatorBadgesEditor.php index 65b2ca6cdf..48057f66e6 100644 --- a/src/applications/badges/editor/PhabricatorBadgesEditor.php +++ b/src/applications/badges/editor/PhabricatorBadgesEditor.php @@ -55,6 +55,30 @@ final class PhabricatorBadgesEditor return true; } + protected function expandTransactions( + PhabricatorLiskDAO $object, + array $xactions) { + + $actor = $this->getActor(); + $actor_phid = $actor->getPHID(); + + $results = parent::expandTransactions($object, $xactions); + + // Automatically subscribe the author when they create a badge. + if ($this->getIsNewObject()) { + if ($actor_phid) { + $results[] = id(new PhabricatorBadgesTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) + ->setNewValue( + array( + '+' => array($actor_phid => $actor_phid), + )); + } + } + + return $results; + } + protected function buildReplyHandler(PhabricatorLiskDAO $object) { return id(new PhabricatorBadgesReplyHandler()) ->setMailReceiver($object); diff --git a/src/applications/badges/query/PhabricatorBadgesSearchEngine.php b/src/applications/badges/query/PhabricatorBadgesSearchEngine.php index b57eae5910..4db0cdd979 100644 --- a/src/applications/badges/query/PhabricatorBadgesSearchEngine.php +++ b/src/applications/badges/query/PhabricatorBadgesSearchEngine.php @@ -147,7 +147,7 @@ final class PhabricatorBadgesSearchEngine ->setTitle(pht('Welcome to %s', $app_name)) ->setDescription( pht('Badges let you award and distinguish special users '. - 'throughout your instance.')) + 'throughout your install.')) ->addAction($create_button); return $view; diff --git a/src/applications/badges/storage/PhabricatorBadgesBadge.php b/src/applications/badges/storage/PhabricatorBadgesBadge.php index 4cdc4edf61..cf02f705ce 100644 --- a/src/applications/badges/storage/PhabricatorBadgesBadge.php +++ b/src/applications/badges/storage/PhabricatorBadgesBadge.php @@ -156,7 +156,7 @@ final class PhabricatorBadgesBadge extends PhabricatorBadgesDAO public function isAutomaticallySubscribed($phid) { - return ($this->creatorPHID == $phid); + return false; } diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php index 5f339fa368..5ecacdd441 100644 --- a/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php +++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php @@ -59,4 +59,32 @@ final class PhabricatorBadgesBadgeAwardTransaction return 'fa-user-plus'; } + public function validateTransactions($object, array $xactions) { + $errors = array(); + + foreach ($xactions as $xaction) { + $user_phids = $xaction->getNewValue(); + if (!$user_phids) { + $errors[] = $this->newRequiredError( + pht('Recipient is required.')); + continue; + } + + foreach ($user_phids as $user_phid) { + $user = id(new PhabricatorPeopleQuery()) + ->setViewer($this->getActor()) + ->withPHIDs(array($user_phid)) + ->executeOne(); + if (!$user) { + $errors[] = $this->newInvalidError( + pht( + 'Recipient PHID "%s" is not a valid user PHID.', + $user_phid)); + } + } + } + + return $errors; + } + } diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php index 069ec09603..2635aab1a6 100644 --- a/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php +++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeFlavorTransaction.php @@ -30,4 +30,21 @@ final class PhabricatorBadgesBadgeFlavorTransaction $this->renderNewValue()); } + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $max_length = $object->getColumnMaximumByteLength('flavor'); + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + $new_length = strlen($new_value); + if ($new_length > $max_length) { + $errors[] = $this->newRequiredError( + pht('The flavor text can be no longer than %s characters.', + new PhutilNumber($max_length))); + } + } + + return $errors; + } + } diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php index 4565c95fb7..1df7d89a70 100644 --- a/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php +++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeNameTransaction.php @@ -38,6 +38,17 @@ final class PhabricatorBadgesBadgeNameTransaction pht('Badges must have a name.')); } + $max_length = $object->getColumnMaximumByteLength('name'); + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + $new_length = strlen($new_value); + if ($new_length > $max_length) { + $errors[] = $this->newRequiredError( + pht('The name can be no longer than %s characters.', + new PhutilNumber($max_length))); + } + } + return $errors; } diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php index 704a518371..a3fa58a060 100644 --- a/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php +++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php @@ -51,4 +51,33 @@ final class PhabricatorBadgesBadgeRevokeTransaction return 'fa-user-times'; } + public function validateTransactions($object, array $xactions) { + $errors = array(); + + foreach ($xactions as $xaction) { + $award_phids = $xaction->getNewValue(); + if (!$award_phids) { + $errors[] = $this->newRequiredError( + pht('Recipient is required.')); + continue; + } + + foreach ($award_phids as $award_phid) { + $award = id(new PhabricatorBadgesAwardQuery()) + ->setViewer($this->getActor()) + ->withRecipientPHIDs(array($award_phid)) + ->withBadgePHIDs(array($object->getPHID())) + ->executeOne(); + if (!$award) { + $errors[] = $this->newInvalidError( + pht( + 'Recipient PHID "%s" has not been awarded.', + $award_phid)); + } + } + } + + return $errors; + } + }