diff --git a/src/applications/badges/controller/PhabricatorBadgesAwardController.php b/src/applications/badges/controller/PhabricatorBadgesAwardController.php --- a/src/applications/badges/controller/PhabricatorBadgesAwardController.php +++ b/src/applications/badges/controller/PhabricatorBadgesAwardController.php @@ -22,7 +22,6 @@ $badges = id(new PhabricatorBadgesQuery()) ->setViewer($viewer) ->withPHIDs($badge_phids) - ->needRecipients(true) ->requireCapabilities( array( PhabricatorPolicyCapability::CAN_EDIT, diff --git a/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php --- a/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php +++ b/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php @@ -11,7 +11,6 @@ $badge = id(new PhabricatorBadgesQuery()) ->setViewer($viewer) ->withIDs(array($id)) - ->needRecipients(true) ->requireCapabilities( array( PhabricatorPolicyCapability::CAN_EDIT, @@ -23,8 +22,6 @@ } $view_uri = $this->getApplicationURI('recipients/'.$badge->getID().'/'); - $awards = $badge->getAwards(); - $recipient_phids = mpull($awards, 'getRecipientPHID'); if ($request->isFormPost()) { $award_phids = array(); @@ -52,17 +49,6 @@ ->setURI($view_uri); } - $recipient_phids = array_reverse($recipient_phids); - $handles = $this->loadViewerHandles($recipient_phids); - - $state = array(); - foreach ($handles as $handle) { - $state[] = array( - 'phid' => $handle->getPHID(), - 'name' => $handle->getFullName(), - ); - } - $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, $badge, diff --git a/src/applications/badges/controller/PhabricatorBadgesRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesRecipientsController.php --- a/src/applications/badges/controller/PhabricatorBadgesRecipientsController.php +++ b/src/applications/badges/controller/PhabricatorBadgesRecipientsController.php @@ -14,14 +14,21 @@ $badge = id(new PhabricatorBadgesQuery()) ->setViewer($viewer) ->withIDs(array($id)) - ->needRecipients(true) ->executeOne(); if (!$badge) { return new Aphront404Response(); } - $this->setBadge($badge); + $awards = id(new PhabricatorBadgesAwardQuery()) + ->setViewer($viewer) + ->withBadgePHIDs(array($badge->getPHID())) + ->execute(); + + $recipient_phids = mpull($awards, 'getRecipientPHID'); + $recipient_phids = array_reverse($recipient_phids); + $handles = $this->loadViewerHandles($recipient_phids); + $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('Recipients')); $crumbs->setBorder(true); @@ -29,13 +36,9 @@ $header = $this->buildHeaderView(); - $awards = $badge->getAwards(); - $recipient_phids = mpull($awards, 'getRecipientPHID'); - $recipient_phids = array_reverse($recipient_phids); - $handles = $this->loadViewerHandles($recipient_phids); - $recipient_list = id(new PhabricatorBadgesRecipientsListView()) ->setBadge($badge) + ->setAwards($awards) ->setHandles($handles) ->setUser($viewer); diff --git a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php --- a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php +++ b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php @@ -10,7 +10,6 @@ $badge = id(new PhabricatorBadgesQuery()) ->setViewer($viewer) ->withIDs(array($id)) - ->needRecipients(true) ->requireCapabilities( array( PhabricatorPolicyCapability::CAN_VIEW, @@ -21,14 +20,7 @@ return new Aphront404Response(); } - $awards = $badge->getAwards(); - $recipient_phids = mpull($awards, 'getRecipientPHID'); $remove_phid = $request->getStr('phid'); - - if (!in_array($remove_phid, $recipient_phids)) { - return new Aphront404Response(); - } - $view_uri = $this->getApplicationURI('recipients/'.$badge->getID().'/'); if ($request->isFormPost()) { diff --git a/src/applications/badges/query/PhabricatorBadgesAwardQuery.php b/src/applications/badges/query/PhabricatorBadgesAwardQuery.php --- a/src/applications/badges/query/PhabricatorBadgesAwardQuery.php +++ b/src/applications/badges/query/PhabricatorBadgesAwardQuery.php @@ -9,21 +9,19 @@ protected function willFilterPage(array $awards) { + $badge_phids = array(); + foreach ($awards as $key => $award) { + $badge_phids[] = $award->getBadgePHID(); + } + $badges = id(new PhabricatorBadgesQuery()) ->setViewer($this->getViewer()) - ->withRecipientPHIDs(mpull($awards, null, 'getRecipientPHID')) + ->withPHIDs($badge_phids) ->execute(); $badges = mpull($badges, null, 'getPHID'); - foreach ($awards as $key => $award) { $award_badge = idx($badges, $award->getBadgePHID()); - if ($award_badge === null) { - $this->didRejectResult($award); - unset($awards[$key]); - continue; - } - $award->attachBadge($award_badge); } diff --git a/src/applications/badges/query/PhabricatorBadgesQuery.php b/src/applications/badges/query/PhabricatorBadgesQuery.php --- a/src/applications/badges/query/PhabricatorBadgesQuery.php +++ b/src/applications/badges/query/PhabricatorBadgesQuery.php @@ -7,9 +7,6 @@ private $phids; private $qualities; private $statuses; - private $recipientPHIDs; - - private $needRecipients; public function withIDs(array $ids) { $this->ids = $ids; @@ -31,22 +28,12 @@ return $this; } - public function withRecipientPHIDs(array $recipient_phids) { - $this->recipientPHIDs = $recipient_phids; - return $this; - } - public function withNameNgrams($ngrams) { return $this->withNgramsConstraint( id(new PhabricatorBadgesBadgeNameNgrams()), $ngrams); } - public function needRecipients($need_recipients) { - $this->needRecipients = $need_recipients; - return $this; - } - protected function loadPage() { return $this->loadStandardPage($this->newResultObject()); } @@ -59,24 +46,6 @@ return new PhabricatorBadgesBadge(); } - protected function didFilterPage(array $badges) { - if ($this->needRecipients) { - $query = id(new PhabricatorBadgesAwardQuery()) - ->setViewer($this->getViewer()) - ->withBadgePHIDs(mpull($badges, 'getPHID')) - ->execute(); - - $awards = mgroup($query, 'getBadgePHID'); - - foreach ($badges as $badge) { - $badge_awards = idx($awards, $badge->getPHID(), array()); - $badge->attachAwards($badge_awards); - } - } - - return $badges; - } - protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { $where = parent::buildWhereClauseParts($conn); diff --git a/src/applications/badges/storage/PhabricatorBadgesBadge.php b/src/applications/badges/storage/PhabricatorBadgesBadge.php --- a/src/applications/badges/storage/PhabricatorBadgesBadge.php +++ b/src/applications/badges/storage/PhabricatorBadgesBadge.php @@ -20,8 +20,6 @@ protected $status; protected $creatorPHID; - private $awards = self::ATTACHABLE; - const STATUS_ACTIVE = 'open'; const STATUS_ARCHIVED = 'closed'; @@ -84,15 +82,6 @@ return ($this->getStatus() == self::STATUS_ARCHIVED); } - public function attachAwards(array $awards) { - $this->awards = $awards; - return $this; - } - - public function getAwards() { - return $this->assertAttached($this->awards); - } - public function getViewURI() { return '/badges/view/'.$this->getID().'/'; } diff --git a/src/applications/badges/view/PhabricatorBadgesRecipientsListView.php b/src/applications/badges/view/PhabricatorBadgesRecipientsListView.php --- a/src/applications/badges/view/PhabricatorBadgesRecipientsListView.php +++ b/src/applications/badges/view/PhabricatorBadgesRecipientsListView.php @@ -3,6 +3,7 @@ final class PhabricatorBadgesRecipientsListView extends AphrontView { private $badge; + private $awards; private $handles; public function setBadge(PhabricatorBadgesBadge $badge) { @@ -10,6 +11,11 @@ return $this; } + public function setAwards(array $awards) { + $this->awards = $awards; + return $this; + } + public function setHandles(array $handles) { $this->handles = $handles; return $this; @@ -20,7 +26,7 @@ $badge = $this->badge; $handles = $this->handles; - $awards = mpull($badge->getAwards(), null, 'getRecipientPHID'); + $awards = mpull($this->awards, null, 'getRecipientPHID'); $can_edit = PhabricatorPolicyFilter::hasCapability( $viewer, diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php --- a/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php +++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeAwardTransaction.php @@ -6,25 +6,17 @@ const TRANSACTIONTYPE = 'badge.award'; public function generateOldValue($object) { - return mpull($object->getAwards(), 'getRecipientPHID'); + return null; } public function applyExternalEffects($object, $value) { - $awards = $object->getAwards(); - $awards = mpull($awards, null, 'getRecipientPHID'); - foreach ($value as $phid) { - $award = idx($awards, $phid); - if (!$award) { - $award = PhabricatorBadgesAward::initializeNewBadgesAward( - $this->getActor(), - $object, - $phid); - $award->save(); - $awards[] = $award; - } + $award = PhabricatorBadgesAward::initializeNewBadgesAward( + $this->getActor(), + $object, + $phid); + $award->save(); } - $object->attachAwards($awards); return; } @@ -71,6 +63,7 @@ } foreach ($user_phids as $user_phid) { + // Check if a valid user $user = id(new PhabricatorPeopleQuery()) ->setViewer($this->getActor()) ->withPHIDs(array($user_phid)) @@ -80,6 +73,20 @@ pht( 'Recipient PHID "%s" is not a valid user PHID.', $user_phid)); + continue; + } + + // Check if already awarded + $award = id(new PhabricatorBadgesAwardQuery()) + ->setViewer($this->getActor()) + ->withRecipientPHIDs(array($user_phid)) + ->withBadgePHIDs(array($object->getPHID())) + ->executeOne(); + if ($award) { + $errors[] = $this->newInvalidError( + pht( + '%s has already been awarded this badge.', + $user->getUsername())); } } } diff --git a/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php b/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php --- a/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php +++ b/src/applications/badges/xaction/PhabricatorBadgesBadgeRevokeTransaction.php @@ -10,13 +10,16 @@ } public function applyExternalEffects($object, $value) { - $awards = $object->getAwards(); + $awards = id(new PhabricatorBadgesAwardQuery()) + ->setViewer($this->getActor()) + ->withRecipientPHIDs($value) + ->withBadgePHIDs(array($object->getPHID())) + ->execute(); $awards = mpull($awards, null, 'getRecipientPHID'); foreach ($value as $phid) { $awards[$phid]->delete(); } - $object->attachAwards($awards); return; }