From 0330ea575db78db76fb88bf25c5296ba26b9b3df Mon Sep 17 00:00:00 2001 From: lkassianik Date: Wed, 13 Jan 2016 11:33:46 -0800 Subject: [PATCH] Converting badge recipients from Edge to BadgeAward table Summary: Ref T8996, Convert badge recipients from Edges to actual BadgeAward objects Test Plan: Create badge, award it to recipient. Make sure adding/removing recipients works. (Still need to migrate exisiting recipients to new table and need to create activity feed blurbs) Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: chad, Korvin Maniphest Tasks: T8996 Differential Revision: https://secure.phabricator.com/D15014 --- .../sql/autopatches/20160102.badges.award.sql | 10 +++ .../sql/autopatches/20160323.badgemigrate.sql | 6 ++ src/__phutil_library_map__.php | 8 ++ .../PhabricatorBadgesEditController.php | 1 - ...bricatorBadgesEditRecipientsController.php | 24 ++--- ...icatorBadgesRemoveRecipientsController.php | 14 +-- .../PhabricatorBadgesViewController.php | 3 +- .../badges/editor/PhabricatorBadgesEditor.php | 40 +++++++++ .../query/PhabricatorBadgesAwardQuery.php | 87 +++++++++++++++++++ .../badges/query/PhabricatorBadgesQuery.php | 21 ++--- .../badges/storage/PhabricatorBadgesAward.php | 83 ++++++++++++++++++ .../badges/storage/PhabricatorBadgesBadge.php | 19 ++-- .../storage/PhabricatorBadgesTransaction.php | 2 + .../people/query/PhabricatorPeopleQuery.php | 21 ++--- src/view/phui/PHUITimelineView.php | 31 +++---- 15 files changed, 296 insertions(+), 74 deletions(-) create mode 100644 resources/sql/autopatches/20160102.badges.award.sql create mode 100644 resources/sql/autopatches/20160323.badgemigrate.sql create mode 100644 src/applications/badges/query/PhabricatorBadgesAwardQuery.php create mode 100644 src/applications/badges/storage/PhabricatorBadgesAward.php diff --git a/resources/sql/autopatches/20160102.badges.award.sql b/resources/sql/autopatches/20160102.badges.award.sql new file mode 100644 index 0000000000..d637c93650 --- /dev/null +++ b/resources/sql/autopatches/20160102.badges.award.sql @@ -0,0 +1,10 @@ +CREATE TABLE {$NAMESPACE}_badges.badges_award ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + badgePHID VARBINARY(64) NOT NULL, + recipientPHID VARBINARY(64) NOT NULL, + awarderPHID varbinary(64) NOT NULL, + dateCreated INT UNSIGNED NOT NULL, + dateModified INT UNSIGNED NOT NULL, + UNIQUE KEY `key_badge` (badgePHID, recipientPHID), + KEY `key_recipient` (recipientPHID) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/resources/sql/autopatches/20160323.badgemigrate.sql b/resources/sql/autopatches/20160323.badgemigrate.sql new file mode 100644 index 0000000000..c7341b287b --- /dev/null +++ b/resources/sql/autopatches/20160323.badgemigrate.sql @@ -0,0 +1,6 @@ +/* PhabricatorBadgeHasRecipientEdgeType::TYPECONST = 59 */ + +INSERT IGNORE INTO {$NAMESPACE}_badges.badges_award + (badgePHID, recipientPHID, awarderPHID, dateCreated, dateModified) + SELECT src, dst, 'PHID-VOID-00000000000000000000', dateCreated, dateCreated + FROM {$NAMESPACE}_badges.edge WHERE type = 59; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 150d2bd8cd..75541320c0 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1870,6 +1870,8 @@ phutil_register_library_map(array( 'PhabricatorBadgeHasRecipientEdgeType' => 'applications/badges/edge/PhabricatorBadgeHasRecipientEdgeType.php', 'PhabricatorBadgesApplication' => 'applications/badges/application/PhabricatorBadgesApplication.php', 'PhabricatorBadgesArchiveController' => 'applications/badges/controller/PhabricatorBadgesArchiveController.php', + 'PhabricatorBadgesAward' => 'applications/badges/storage/PhabricatorBadgesAward.php', + 'PhabricatorBadgesAwardQuery' => 'applications/badges/query/PhabricatorBadgesAwardQuery.php', 'PhabricatorBadgesBadge' => 'applications/badges/storage/PhabricatorBadgesBadge.php', 'PhabricatorBadgesCommentController' => 'applications/badges/controller/PhabricatorBadgesCommentController.php', 'PhabricatorBadgesController' => 'applications/badges/controller/PhabricatorBadgesController.php', @@ -6231,6 +6233,12 @@ phutil_register_library_map(array( 'PhabricatorBadgeHasRecipientEdgeType' => 'PhabricatorEdgeType', 'PhabricatorBadgesApplication' => 'PhabricatorApplication', 'PhabricatorBadgesArchiveController' => 'PhabricatorBadgesController', + 'PhabricatorBadgesAward' => array( + 'PhabricatorBadgesDAO', + 'PhabricatorDestructibleInterface', + 'PhabricatorPolicyInterface', + ), + 'PhabricatorBadgesAwardQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorBadgesBadge' => array( 'PhabricatorBadgesDAO', 'PhabricatorPolicyInterface', diff --git a/src/applications/badges/controller/PhabricatorBadgesEditController.php b/src/applications/badges/controller/PhabricatorBadgesEditController.php index 3ac1aebe58..725068d7a6 100644 --- a/src/applications/badges/controller/PhabricatorBadgesEditController.php +++ b/src/applications/badges/controller/PhabricatorBadgesEditController.php @@ -2,7 +2,6 @@ final class PhabricatorBadgesEditController extends PhabricatorBadgesController { - public function handleRequest(AphrontRequest $request) { return id(new PhabricatorBadgesEditEngine()) ->setController($this) diff --git a/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php index 70e3d8f029..a73c6777af 100644 --- a/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php +++ b/src/applications/badges/controller/PhabricatorBadgesEditRecipientsController.php @@ -6,6 +6,7 @@ final class PhabricatorBadgesEditRecipientsController public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); $id = $request->getURIData('id'); + $xactions = array(); $badge = id(new PhabricatorBadgesQuery()) ->setViewer($viewer) @@ -21,30 +22,23 @@ final class PhabricatorBadgesEditRecipientsController return new Aphront404Response(); } - $recipient_phids = $badge->getRecipientPHIDs(); $view_uri = $this->getApplicationURI('view/'.$badge->getID().'/'); + $awards = $badge->getAwards(); + $recipient_phids = mpull($awards, 'getRecipientPHID'); if ($request->isFormPost()) { - $recipient_spec = array(); - - $remove = $request->getStr('remove'); - if ($remove) { - $recipient_spec['-'] = array_fuse(array($remove)); - } + $award_phids = array(); $add_recipients = $request->getArr('phids'); if ($add_recipients) { - $recipient_spec['+'] = array_fuse($add_recipients); + foreach ($add_recipients as $phid) { + $award_phids[] = $phid; + } } - $type_recipient = PhabricatorBadgeHasRecipientEdgeType::EDGECONST; - - $xactions = array(); - $xactions[] = id(new PhabricatorBadgesTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) - ->setMetadataValue('edge:type', $type_recipient) - ->setNewValue($recipient_spec); + ->setTransactionType(PhabricatorBadgesTransaction::TYPE_AWARD) + ->setNewValue($award_phids); $editor = id(new PhabricatorBadgesEditor($badge)) ->setActor($viewer) diff --git a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php index bc467b703d..c77a5f33ea 100644 --- a/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php +++ b/src/applications/badges/controller/PhabricatorBadgesRemoveRecipientsController.php @@ -21,7 +21,8 @@ final class PhabricatorBadgesRemoveRecipientsController return new Aphront404Response(); } - $recipient_phids = $badge->getRecipientPHIDs(); + $awards = $badge->getAwards(); + $recipient_phids = mpull($awards, 'getRecipientPHID'); $remove_phid = $request->getStr('phid'); if (!in_array($remove_phid, $recipient_phids)) { @@ -31,17 +32,10 @@ final class PhabricatorBadgesRemoveRecipientsController $view_uri = $this->getApplicationURI('view/'.$badge->getID().'/'); if ($request->isFormPost()) { - $recipient_spec = array(); - $recipient_spec['-'] = array($remove_phid => $remove_phid); - - $type_recipient = PhabricatorBadgeHasRecipientEdgeType::EDGECONST; - $xactions = array(); - $xactions[] = id(new PhabricatorBadgesTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) - ->setMetadataValue('edge:type', $type_recipient) - ->setNewValue($recipient_spec); + ->setTransactionType(PhabricatorBadgesTransaction::TYPE_REVOKE) + ->setNewValue(array($remove_phid)); $editor = id(new PhabricatorBadgesEditor($badge)) ->setActor($viewer) diff --git a/src/applications/badges/controller/PhabricatorBadgesViewController.php b/src/applications/badges/controller/PhabricatorBadgesViewController.php index fac82ccc8d..0f3077de0b 100644 --- a/src/applications/badges/controller/PhabricatorBadgesViewController.php +++ b/src/applications/badges/controller/PhabricatorBadgesViewController.php @@ -50,7 +50,8 @@ final class PhabricatorBadgesViewController $badge, new PhabricatorBadgesTransactionQuery()); - $recipient_phids = $badge->getRecipientPHIDs(); + $awards = $badge->getAwards(); + $recipient_phids = mpull($awards, 'getRecipientPHID'); $recipient_phids = array_reverse($recipient_phids); $handles = $this->loadViewerHandles($recipient_phids); diff --git a/src/applications/badges/editor/PhabricatorBadgesEditor.php b/src/applications/badges/editor/PhabricatorBadgesEditor.php index fd0b14ad46..af3def28c5 100644 --- a/src/applications/badges/editor/PhabricatorBadgesEditor.php +++ b/src/applications/badges/editor/PhabricatorBadgesEditor.php @@ -20,6 +20,8 @@ final class PhabricatorBadgesEditor $types[] = PhabricatorBadgesTransaction::TYPE_ICON; $types[] = PhabricatorBadgesTransaction::TYPE_STATUS; $types[] = PhabricatorBadgesTransaction::TYPE_QUALITY; + $types[] = PhabricatorBadgesTransaction::TYPE_AWARD; + $types[] = PhabricatorBadgesTransaction::TYPE_REVOKE; $types[] = PhabricatorTransactions::TYPE_COMMENT; $types[] = PhabricatorTransactions::TYPE_EDGE; @@ -44,6 +46,11 @@ final class PhabricatorBadgesEditor return $object->getQuality(); case PhabricatorBadgesTransaction::TYPE_STATUS: return $object->getStatus(); + case PhabricatorBadgesTransaction::TYPE_AWARD: + $award_phids = mpull($object->getAwards(), 'getRecipientPHID'); + return $award_phids; + case PhabricatorBadgesTransaction::TYPE_REVOKE: + return null; } return parent::getCustomTransactionOldValue($object, $xaction); @@ -60,6 +67,8 @@ final class PhabricatorBadgesEditor case PhabricatorBadgesTransaction::TYPE_ICON: case PhabricatorBadgesTransaction::TYPE_STATUS: case PhabricatorBadgesTransaction::TYPE_QUALITY: + case PhabricatorBadgesTransaction::TYPE_AWARD: + case PhabricatorBadgesTransaction::TYPE_REVOKE: return $xaction->getNewValue(); } @@ -90,6 +99,9 @@ final class PhabricatorBadgesEditor case PhabricatorBadgesTransaction::TYPE_STATUS: $object->setStatus($xaction->getNewValue()); return; + case PhabricatorBadgesTransaction::TYPE_AWARD: + case PhabricatorBadgesTransaction::TYPE_REVOKE: + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -108,6 +120,34 @@ final class PhabricatorBadgesEditor case PhabricatorBadgesTransaction::TYPE_STATUS: case PhabricatorBadgesTransaction::TYPE_QUALITY: return; + case PhabricatorBadgesTransaction::TYPE_REVOKE: + $revoked_recipient_phids = $xaction->getNewValue(); + $awards = $object->getAwards(); + $awards = mpull($awards, null, 'getRecipientPHID'); + + foreach ($revoked_recipient_phids as $phid) { + $awards[$phid]->delete(); + } + $object->attachAwards($awards); + return; + case PhabricatorBadgesTransaction::TYPE_AWARD: + $recipient_phids = $xaction->getNewValue(); + $awards = $object->getAwards(); + $awards = mpull($awards, null, 'getRecipientPHID'); + + foreach ($recipient_phids as $phid) { + $award = idx($awards, $phid); + if (!$award) { + $award = PhabricatorBadgesAward::initializeNewBadgesAward( + $this->getActor(), + $object, + $phid); + $award->save(); + $awards[] = $award; + } + } + $object->attachAwards($awards); + return; } return parent::applyCustomExternalTransaction($object, $xaction); diff --git a/src/applications/badges/query/PhabricatorBadgesAwardQuery.php b/src/applications/badges/query/PhabricatorBadgesAwardQuery.php new file mode 100644 index 0000000000..355feec066 --- /dev/null +++ b/src/applications/badges/query/PhabricatorBadgesAwardQuery.php @@ -0,0 +1,87 @@ +setViewer($this->getViewer()) + ->withRecipientPHIDs(mpull($awards, null, 'getRecipientPHID')) + ->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); + } + + return $awards; + } + + public function withBadgePHIDs(array $phids) { + $this->badgePHIDs = $phids; + return $this; + } + + public function withRecipientPHIDs(array $phids) { + $this->recipientPHIDs = $phids; + return $this; + } + + public function withAwarderPHIDs(array $phids) { + $this->awarderPHIDs = $phids; + return $this; + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + public function newResultObject() { + return new PhabricatorBadgesAward(); + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->badgePHIDs !== null) { + $where[] = qsprintf( + $conn, + 'badgePHID IN (%Ls)', + $this->badgePHIDs); + } + + if ($this->recipientPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'recipientPHID IN (%Ls)', + $this->recipientPHIDs); + } + + if ($this->awarderPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'awarderPHID IN (%Ls)', + $this->awarderPHIDs); + } + + return $where; + } + + public function getQueryApplicationClass() { + return 'PhabricatorBadgesApplication'; + } + +} diff --git a/src/applications/badges/query/PhabricatorBadgesQuery.php b/src/applications/badges/query/PhabricatorBadgesQuery.php index 91d7111253..fce8b3731f 100644 --- a/src/applications/badges/query/PhabricatorBadgesQuery.php +++ b/src/applications/badges/query/PhabricatorBadgesQuery.php @@ -50,22 +50,17 @@ final class PhabricatorBadgesQuery } protected function didFilterPage(array $badges) { - if ($this->needRecipients) { - $edge_query = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs(mpull($badges, 'getPHID')) - ->withEdgeTypes( - array( - PhabricatorBadgeHasRecipientEdgeType::EDGECONST, - )); - $edge_query->execute(); + $query = id(new PhabricatorBadgesAwardQuery()) + ->setViewer($this->getViewer()) + ->withBadgePHIDs(mpull($badges, 'getPHID')) + ->execute(); + + $awards = mgroup($query, 'getBadgePHID'); foreach ($badges as $badge) { - $phids = $edge_query->getDestinationPHIDs( - array( - $badge->getPHID(), - )); - $badge->attachRecipientPHIDs($phids); + $badge_awards = idx($awards, $badge->getPHID(), array()); + $badge->attachAwards($badge_awards); } } diff --git a/src/applications/badges/storage/PhabricatorBadgesAward.php b/src/applications/badges/storage/PhabricatorBadgesAward.php new file mode 100644 index 0000000000..851ac87002 --- /dev/null +++ b/src/applications/badges/storage/PhabricatorBadgesAward.php @@ -0,0 +1,83 @@ +setRecipientPHID($recipient_phid) + ->setBadgePHID($badge->getPHID()) + ->setAwarderPHID($actor->getPHID()) + ->attachBadge($badge); + } + + protected function getConfiguration() { + return array( + self::CONFIG_KEY_SCHEMA => array( + 'key_badge' => array( + 'columns' => array('badgePHID', 'recipientPHID'), + 'unique' => true, + ), + 'key_recipient' => array( + 'columns' => array('recipientPHID'), + ), + ), + ) + parent::getConfiguration(); + } + + public function attachBadge(PhabricatorBadgesBadge $badge) { + $this->badge = $badge; + return $this; + } + + public function getBadge() { + return $this->assertAttached($this->badge); + } + + +/* -( PhabricatorDestructibleInterface )----------------------------------- */ + + + public function destroyObjectPermanently( + PhabricatorDestructionEngine $engine) { + + $this->openTransaction(); + $this->delete(); + $this->saveTransaction(); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + ); + } + + public function getPolicy($capability) { + return $this->getBadge()->getPolicy($capability); + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + + public function describeAutomaticCapability($capability) { + return null; + } + +} diff --git a/src/applications/badges/storage/PhabricatorBadgesBadge.php b/src/applications/badges/storage/PhabricatorBadgesBadge.php index d32a46a803..2959111dc4 100644 --- a/src/applications/badges/storage/PhabricatorBadgesBadge.php +++ b/src/applications/badges/storage/PhabricatorBadgesBadge.php @@ -19,7 +19,7 @@ final class PhabricatorBadgesBadge extends PhabricatorBadgesDAO protected $status; protected $creatorPHID; - private $recipientPHIDs = self::ATTACHABLE; + private $awards = self::ATTACHABLE; const STATUS_ACTIVE = 'open'; const STATUS_ARCHIVED = 'closed'; @@ -102,13 +102,13 @@ final class PhabricatorBadgesBadge extends PhabricatorBadgesDAO return ($this->getStatus() == self::STATUS_ARCHIVED); } - public function attachRecipientPHIDs(array $phids) { - $this->recipientPHIDs = $phids; + public function attachAwards(array $awards) { + $this->awards = $awards; return $this; } - public function getRecipientPHIDs() { - return $this->assertAttached($this->recipientPHIDs); + public function getAwards() { + return $this->assertAttached($this->awards); } public function getViewURI() { @@ -197,6 +197,15 @@ final class PhabricatorBadgesBadge extends PhabricatorBadgesDAO public function destroyObjectPermanently( PhabricatorDestructionEngine $engine) { + $awards = id(new PhabricatorBadgesAwardQuery()) + ->setViewer($engine->getViewer()) + ->withBadgePHIDs(array($this->getPHID())) + ->execute(); + + foreach ($awards as $award) { + $engine->destroyObjectPermanently($award); + } + $this->openTransaction(); $this->delete(); $this->saveTransaction(); diff --git a/src/applications/badges/storage/PhabricatorBadgesTransaction.php b/src/applications/badges/storage/PhabricatorBadgesTransaction.php index c2e934390d..f25c629c96 100644 --- a/src/applications/badges/storage/PhabricatorBadgesTransaction.php +++ b/src/applications/badges/storage/PhabricatorBadgesTransaction.php @@ -9,6 +9,8 @@ final class PhabricatorBadgesTransaction const TYPE_ICON = 'badges:icon'; const TYPE_STATUS = 'badges:status'; const TYPE_FLAVOR = 'badges:flavor'; + const TYPE_AWARD = 'badges:award'; + const TYPE_REVOKE = 'badges:revoke'; const MAILTAG_DETAILS = 'badges:details'; const MAILTAG_COMMENT = 'badges:comment'; diff --git a/src/applications/people/query/PhabricatorPeopleQuery.php b/src/applications/people/query/PhabricatorPeopleQuery.php index 2932d8c853..0bdb9fab2d 100644 --- a/src/applications/people/query/PhabricatorPeopleQuery.php +++ b/src/applications/people/query/PhabricatorPeopleQuery.php @@ -155,20 +155,17 @@ final class PhabricatorPeopleQuery } if ($this->needBadges) { - $edge_query = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs(mpull($users, 'getPHID')) - ->withEdgeTypes( - array( - PhabricatorRecipientHasBadgeEdgeType::EDGECONST, - )); - $edge_query->execute(); + $awards = id(new PhabricatorBadgesAwardQuery()) + ->setViewer($this->getViewer()) + ->withRecipientPHIDs(mpull($users, 'getPHID')) + ->execute(); + + $awards = mgroup($awards, 'getRecipientPHID'); foreach ($users as $user) { - $phids = $edge_query->getDestinationPHIDs( - array( - $user->getPHID(), - )); - $user->attachBadgePHIDs($phids); + $user_awards = idx($awards, $user->getPHID(), array()); + $badge_phids = mpull($user_awards, 'getBadgePHID'); + $user->attachBadgePHIDs($badge_phids); } } diff --git a/src/view/phui/PHUITimelineView.php b/src/view/phui/PHUITimelineView.php index 2718fbd9f2..729320ebd1 100644 --- a/src/view/phui/PHUITimelineView.php +++ b/src/view/phui/PHUITimelineView.php @@ -250,31 +250,28 @@ final class PHUITimelineView extends AphrontView { return; } - $edges = id(new PhabricatorEdgeQuery()) - ->withSourcePHIDs($user_phids) - ->withEdgeTypes(array($badge_edge_type)); - $edges->execute(); - $badge_phids = $edges->getDestinationPHIDs(); - if (!$badge_phids) { - return; - } - - $all_badges = id(new PhabricatorBadgesQuery()) - ->setViewer($viewer) - ->withPHIDs($badge_phids) - ->withStatuses(array(PhabricatorBadgesBadge::STATUS_ACTIVE)) + $awards = id(new PhabricatorBadgesAwardQuery()) + ->setViewer($this->getViewer()) + ->withRecipientPHIDs($user_phids) ->execute(); - $all_badges = mpull($all_badges, null, 'getPHID'); + + $awards = mgroup($awards, 'getRecipientPHID'); foreach ($events as $event) { - $author_phid = $event->getAuthorPHID(); - $event_phids = $edges->getDestinationPHIDs(array($author_phid)); - $badges = array_select_keys($all_badges, $event_phids); + $author_awards = idx($awards, $event->getAuthorPHID(), array()); + $badges = array(); + foreach ($author_awards as $award) { + $badge = $award->getBadge(); + if ($badge->getStatus() == PhabricatorBadgesBadge::STATUS_ACTIVE) { + $badges[$award->getBadgePHID()] = $badge; + } + } // TODO: Pick the "best" badges in some smart way. For now, just pick // the first two. $badges = array_slice($badges, 0, 2); + foreach ($badges as $badge) { $badge_view = id(new PHUIBadgeMiniView()) ->setIcon($badge->getIcon())