From 1bff5309e667a8cf4a60f52492e68aa844b293b9 Mon Sep 17 00:00:00 2001 From: Austin McKinley Date: Thu, 18 May 2017 16:15:24 -0700 Subject: [PATCH] Migrate Project icons to modular transactions Test Plan: Unit tests pass. Changed some icons, observed expected timeline entries. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D17956 --- src/__phutil_library_map__.php | 2 + .../conduit/ProjectCreateConduitAPIMethod.php | 2 +- .../PhabricatorProjectTransactionEditor.php | 10 +---- .../engine/PhabricatorProjectEditEngine.php | 4 +- .../storage/PhabricatorProjectTransaction.php | 24 +---------- .../PhabricatorProjectIconTransaction.php | 42 +++++++++++++++++++ 6 files changed, 49 insertions(+), 35 deletions(-) create mode 100644 src/applications/project/xaction/PhabricatorProjectIconTransaction.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d43d756426..2516fd72c6 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3620,6 +3620,7 @@ phutil_register_library_map(array( 'PhabricatorProjectHeraldFieldGroup' => 'applications/project/herald/PhabricatorProjectHeraldFieldGroup.php', 'PhabricatorProjectHovercardEngineExtension' => 'applications/project/engineextension/PhabricatorProjectHovercardEngineExtension.php', 'PhabricatorProjectIconSet' => 'applications/project/icon/PhabricatorProjectIconSet.php', + 'PhabricatorProjectIconTransaction' => 'applications/project/xaction/PhabricatorProjectIconTransaction.php', 'PhabricatorProjectIconsConfigOptionType' => 'applications/project/config/PhabricatorProjectIconsConfigOptionType.php', 'PhabricatorProjectImageTransaction' => 'applications/project/xaction/PhabricatorProjectImageTransaction.php', 'PhabricatorProjectInterface' => 'applications/project/interface/PhabricatorProjectInterface.php', @@ -9035,6 +9036,7 @@ phutil_register_library_map(array( 'PhabricatorProjectHeraldFieldGroup' => 'HeraldFieldGroup', 'PhabricatorProjectHovercardEngineExtension' => 'PhabricatorHovercardEngineExtension', 'PhabricatorProjectIconSet' => 'PhabricatorIconSet', + 'PhabricatorProjectIconTransaction' => 'PhabricatorProjectTransactionType', 'PhabricatorProjectIconsConfigOptionType' => 'PhabricatorConfigJSONOptionType', 'PhabricatorProjectImageTransaction' => 'PhabricatorProjectTransactionType', 'PhabricatorProjectListController' => 'PhabricatorProjectController', diff --git a/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php b/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php index 896cc9a07b..407512de9a 100644 --- a/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php +++ b/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php @@ -52,7 +52,7 @@ final class ProjectCreateConduitAPIMethod extends ProjectConduitAPIMethod { if ($request->getValue('icon')) { $xactions[] = id(new PhabricatorProjectTransaction()) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_ICON) + ->setTransactionType(PhabricatorProjectIconTransaction::TRANSACTIONTYPE) ->setNewValue($request->getValue('icon')); } diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php index 101020970a..915807445a 100644 --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -30,7 +30,6 @@ final class PhabricatorProjectTransactionEditor $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; $types[] = PhabricatorTransactions::TYPE_JOIN_POLICY; - $types[] = PhabricatorProjectTransaction::TYPE_ICON; $types[] = PhabricatorProjectTransaction::TYPE_COLOR; $types[] = PhabricatorProjectTransaction::TYPE_LOCKED; $types[] = PhabricatorProjectTransaction::TYPE_PARENT; @@ -48,8 +47,6 @@ final class PhabricatorProjectTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_ICON: - return $object->getIcon(); case PhabricatorProjectTransaction::TYPE_COLOR: return $object->getColor(); case PhabricatorProjectTransaction::TYPE_LOCKED: @@ -75,7 +72,6 @@ final class PhabricatorProjectTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_ICON: case PhabricatorProjectTransaction::TYPE_COLOR: case PhabricatorProjectTransaction::TYPE_LOCKED: case PhabricatorProjectTransaction::TYPE_PARENT: @@ -101,9 +97,6 @@ final class PhabricatorProjectTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_ICON: - $object->setIcon($xaction->getNewValue()); - return; case PhabricatorProjectTransaction::TYPE_COLOR: $object->setColor($xaction->getNewValue()); return; @@ -143,7 +136,6 @@ final class PhabricatorProjectTransactionEditor $new = $xaction->getNewValue(); switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_ICON: case PhabricatorProjectTransaction::TYPE_COLOR: case PhabricatorProjectTransaction::TYPE_LOCKED: case PhabricatorProjectTransaction::TYPE_PARENT: @@ -329,7 +321,7 @@ final class PhabricatorProjectTransactionEditor case PhabricatorProjectNameTransaction::TRANSACTIONTYPE: case PhabricatorProjectStatusTransaction::TRANSACTIONTYPE: case PhabricatorProjectImageTransaction::TRANSACTIONTYPE: - case PhabricatorProjectTransaction::TYPE_ICON: + case PhabricatorProjectIconTransaction::TRANSACTIONTYPE: case PhabricatorProjectTransaction::TYPE_COLOR: PhabricatorPolicyFilter::requireCapability( $this->requireActor(), diff --git a/src/applications/project/engine/PhabricatorProjectEditEngine.php b/src/applications/project/engine/PhabricatorProjectEditEngine.php index b0e3384d4a..a4a97654cd 100644 --- a/src/applications/project/engine/PhabricatorProjectEditEngine.php +++ b/src/applications/project/engine/PhabricatorProjectEditEngine.php @@ -112,7 +112,7 @@ final class PhabricatorProjectEditEngine PhabricatorTransactions::TYPE_VIEW_POLICY, PhabricatorTransactions::TYPE_EDIT_POLICY, PhabricatorTransactions::TYPE_JOIN_POLICY, - PhabricatorProjectTransaction::TYPE_ICON, + PhabricatorProjectIconTransaction::TRANSACTIONTYPE, PhabricatorProjectTransaction::TYPE_COLOR, ); $unavailable = array_fuse($unavailable); @@ -244,7 +244,7 @@ final class PhabricatorProjectEditEngine id(new PhabricatorIconSetEditField()) ->setKey('icon') ->setLabel(pht('Icon')) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_ICON) + ->setTransactionType(PhabricatorProjectIconTransaction::TRANSACTIONTYPE) ->setIconSet(new PhabricatorProjectIconSet()) ->setDescription(pht('Project icon.')) ->setConduitDescription(pht('Change the project icon.')) diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php index d99cf4c5ce..9f707c3e94 100644 --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -3,7 +3,6 @@ final class PhabricatorProjectTransaction extends PhabricatorModularTransaction { - const TYPE_ICON = 'project:icon'; const TYPE_COLOR = 'project:color'; const TYPE_LOCKED = 'project:locked'; const TYPE_PARENT = 'project:parent'; @@ -99,8 +98,6 @@ final class PhabricatorProjectTransaction } else { return 'fa-unlock'; } - case self::TYPE_ICON: - return PhabricatorProjectIconSet::getIconIcon($new); case self::TYPE_MEMBERS: return 'fa-user'; } @@ -119,15 +116,6 @@ final class PhabricatorProjectTransaction '%s created this project.', $this->renderHandleLink($author_phid)); - case self::TYPE_ICON: - $set = new PhabricatorProjectIconSet(); - - return pht( - "%s set this project's icon to %s.", - $author_handle, - $set->getIconLabel($new)); - break; - case self::TYPE_COLOR: return pht( "%s set this project's color to %s.", @@ -226,16 +214,6 @@ final class PhabricatorProjectTransaction $new = $this->getNewValue(); switch ($this->getTransactionType()) { - - case self::TYPE_ICON: - $set = new PhabricatorProjectIconSet(); - - return pht( - '%s set the icon for %s to %s.', - $author_handle, - $object_handle, - $set->getIconLabel($new)); - case self::TYPE_COLOR: return pht( '%s set the color for %s to %s.', @@ -266,7 +244,7 @@ final class PhabricatorProjectTransaction case PhabricatorProjectNameTransaction::TRANSACTIONTYPE: case PhabricatorProjectSlugsTransaction::TRANSACTIONTYPE: case PhabricatorProjectImageTransaction::TRANSACTIONTYPE: - case self::TYPE_ICON: + case PhabricatorProjectIconTransaction::TRANSACTIONTYPE: case self::TYPE_COLOR: $tags[] = self::MAILTAG_METADATA; break; diff --git a/src/applications/project/xaction/PhabricatorProjectIconTransaction.php b/src/applications/project/xaction/PhabricatorProjectIconTransaction.php new file mode 100644 index 0000000000..932ce4bdc4 --- /dev/null +++ b/src/applications/project/xaction/PhabricatorProjectIconTransaction.php @@ -0,0 +1,42 @@ +getIcon(); + } + + public function applyInternalEffects($object, $value) { + $object->setIcon($value); + } + + public function getTitle() { + $set = new PhabricatorProjectIconSet(); + $new = $this->getNewValue(); + + return pht( + "%s set this project's icon to %s.", + $this->renderAuthor(), + $this->renderValue($set->getIconLabel($new))); + } + + public function getTitleForFeed() { + $set = new PhabricatorProjectIconSet(); + $new = $this->getNewValue(); + + return pht( + '%s set the icon for %s to %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderValue($set->getIconLabel($new))); + } + + public function getIcon() { + $new = $this->getNewValue(); + return PhabricatorProjectIconSet::getIconIcon($new); + } + +}