From 5cb0de1efc31bb9d9bf54d8bf8fc22284766366e Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 18 Dec 2015 10:14:37 -0800 Subject: [PATCH] Restore "Create" transactions Summary: Ref T10004. This restores "alice created this task." transactions, but in a generic way so we don't have to special case one of the other edits with an old `null` value. In most cases, creating an object now shows only an "alice created this thing." transaction, unless nonempty defaults (usually, policy or spaces) were adjusted. Test Plan: Created pastes, tasks, blogs, packages, and forms. Saw a single "alice created this thing." transaction. Reviewers: chad Reviewed By: chad Maniphest Tasks: T10004 Differential Revision: https://secure.phabricator.com/D14820 --- .../maniphest/storage/ManiphestTransaction.php | 5 +++++ .../storage/PhabricatorOwnersPackageTransaction.php | 11 ++++++++++- .../paste/storage/PhabricatorPasteTransaction.php | 9 ++++++++- .../phame/storage/PhameBlogTransaction.php | 8 +++++++- .../constants/PhabricatorTransactions.php | 1 + .../editengine/PhabricatorEditEngine.php | 12 ++++++++++++ .../PhabricatorApplicationTransactionEditor.php | 10 ++++++++++ .../storage/PhabricatorApplicationTransaction.php | 12 ++++++++++++ ...PhabricatorEditEngineConfigurationTransaction.php | 4 ++++ 9 files changed, 69 insertions(+), 3 deletions(-) diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php index f83b1faf10..dc1030265b 100644 --- a/src/applications/maniphest/storage/ManiphestTransaction.php +++ b/src/applications/maniphest/storage/ManiphestTransaction.php @@ -381,6 +381,11 @@ final class ManiphestTransaction $new = $this->getNewValue(); switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: + return pht( + '%s created this task.', + $this->renderHandleLink($author_phid)); + case self::TYPE_TITLE: if ($old === null) { return pht( diff --git a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php index 93ef9d419d..fa60a71adb 100644 --- a/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php +++ b/src/applications/owners/storage/PhabricatorOwnersPackageTransaction.php @@ -47,11 +47,16 @@ final class PhabricatorOwnersPackageTransaction switch ($this->getTransactionType()) { case self::TYPE_DESCRIPTION: - return ($old === null); + if ($old === null) { + return true; + } + break; case self::TYPE_PRIMARY: // TODO: Eventually, remove these transactions entirely. return true; } + + return parent::shouldHide(); } public function getTitle() { @@ -60,6 +65,10 @@ final class PhabricatorOwnersPackageTransaction $author_phid = $this->getAuthorPHID(); switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: + return pht( + '%s created this package.', + $this->renderHandleLink($author_phid)); case self::TYPE_NAME: if ($old === null) { return pht( diff --git a/src/applications/paste/storage/PhabricatorPasteTransaction.php b/src/applications/paste/storage/PhabricatorPasteTransaction.php index d6b962298b..1402f4c14c 100644 --- a/src/applications/paste/storage/PhabricatorPasteTransaction.php +++ b/src/applications/paste/storage/PhabricatorPasteTransaction.php @@ -41,7 +41,10 @@ final class PhabricatorPasteTransaction switch ($this->getTransactionType()) { case self::TYPE_TITLE: case self::TYPE_LANGUAGE: - return ($old === null); + if ($old === null) { + return true; + } + break; } return parent::shouldHide(); } @@ -77,6 +80,10 @@ final class PhabricatorPasteTransaction $type = $this->getTransactionType(); switch ($type) { + case PhabricatorTransactions::TYPE_CREATE: + return pht( + '%s created this paste.', + $this->renderHandleLink($author_phid)); case self::TYPE_CONTENT: if ($old === null) { return pht( diff --git a/src/applications/phame/storage/PhameBlogTransaction.php b/src/applications/phame/storage/PhameBlogTransaction.php index e7cf87a6f5..6a89b936f6 100644 --- a/src/applications/phame/storage/PhameBlogTransaction.php +++ b/src/applications/phame/storage/PhameBlogTransaction.php @@ -24,7 +24,9 @@ final class PhameBlogTransaction $old = $this->getOldValue(); switch ($this->getTransactionType()) { case self::TYPE_DESCRIPTION: - return ($old === null); + if ($old === null) { + return true; + } } return parent::shouldHide(); } @@ -98,6 +100,10 @@ final class PhameBlogTransaction $type = $this->getTransactionType(); switch ($type) { + case PhabricatorTransactions::TYPE_CREATE: + return pht( + '%s created this blog.', + $this->renderHandleLink($author_phid)); case self::TYPE_NAME: if ($old === null) { return pht( diff --git a/src/applications/transactions/constants/PhabricatorTransactions.php b/src/applications/transactions/constants/PhabricatorTransactions.php index 1422f21081..172f80ba31 100644 --- a/src/applications/transactions/constants/PhabricatorTransactions.php +++ b/src/applications/transactions/constants/PhabricatorTransactions.php @@ -13,6 +13,7 @@ final class PhabricatorTransactions extends Phobject { const TYPE_TOKEN = 'token:give'; const TYPE_INLINESTATE = 'core:inlinestate'; const TYPE_SPACE = 'core:space'; + const TYPE_CREATE = 'core:create'; const COLOR_RED = 'red'; const COLOR_ORANGE = 'orange'; diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index b628e1723a..2e96e60dd0 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -857,6 +857,12 @@ abstract class PhabricatorEditEngine } $xactions = array(); + + if ($this->getIsCreate()) { + $xactions[] = id(clone $template) + ->setTransactionType(PhabricatorTransactions::TYPE_CREATE); + } + foreach ($submit_fields as $key => $field) { $field_value = $field->getValueForTransaction(); @@ -1647,6 +1653,12 @@ abstract class PhabricatorEditEngine } $results = array(); + + if ($this->getIsCreate()) { + $results[] = id(clone $template) + ->setTransactionType(PhabricatorTransactions::TYPE_CREATE); + } + foreach ($xactions as $xaction) { $type = $types[$xaction['type']]; diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 074cf6efca..830a35ca44 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -258,6 +258,8 @@ abstract class PhabricatorApplicationTransactionEditor public function getTransactionTypes() { $types = array(); + $types[] = PhabricatorTransactions::TYPE_CREATE; + if ($this->object instanceof PhabricatorSubscribableInterface) { $types[] = PhabricatorTransactions::TYPE_SUBSCRIBERS; } @@ -303,6 +305,8 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: + return null; case PhabricatorTransactions::TYPE_SUBSCRIBERS: return array_values($this->subscribers); case PhabricatorTransactions::TYPE_VIEW_POLICY: @@ -371,6 +375,8 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorLiskDAO $object, PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: + return null; case PhabricatorTransactions::TYPE_SUBSCRIBERS: return $this->getPHIDTransactionNewValue($xaction); case PhabricatorTransactions::TYPE_VIEW_POLICY: @@ -422,6 +428,8 @@ abstract class PhabricatorApplicationTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: + return true; case PhabricatorTransactions::TYPE_COMMENT: return $xaction->hasComment(); case PhabricatorTransactions::TYPE_CUSTOMFIELD: @@ -484,6 +492,7 @@ abstract class PhabricatorApplicationTransactionEditor case PhabricatorTransactions::TYPE_CUSTOMFIELD: $field = $this->getCustomFieldForTransaction($object, $xaction); return $field->applyApplicationTransactionInternalEffects($xaction); + case PhabricatorTransactions::TYPE_CREATE: case PhabricatorTransactions::TYPE_BUILDABLE: case PhabricatorTransactions::TYPE_TOKEN: case PhabricatorTransactions::TYPE_VIEW_POLICY: @@ -534,6 +543,7 @@ abstract class PhabricatorApplicationTransactionEditor case PhabricatorTransactions::TYPE_CUSTOMFIELD: $field = $this->getCustomFieldForTransaction($object, $xaction); return $field->applyApplicationTransactionExternalEffects($xaction); + case PhabricatorTransactions::TYPE_CREATE: case PhabricatorTransactions::TYPE_EDGE: case PhabricatorTransactions::TYPE_BUILDABLE: case PhabricatorTransactions::TYPE_TOKEN: diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index f3973f9213..715bd0f261 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -482,6 +482,7 @@ abstract class PhabricatorApplicationTransaction // essentially never interesting. if ($this->getIsCreateTransaction()) { switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: case PhabricatorTransactions::TYPE_VIEW_POLICY: case PhabricatorTransactions::TYPE_EDIT_POLICY: case PhabricatorTransactions::TYPE_JOIN_POLICY: @@ -497,6 +498,7 @@ abstract class PhabricatorApplicationTransaction if (!strlen($old)) { return true; } + break; } } @@ -702,6 +704,10 @@ abstract class PhabricatorApplicationTransaction $new = $this->getNewValue(); switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: + return pht( + '%s created this object.', + $this->renderHandleLink($author_phid)); case PhabricatorTransactions::TYPE_COMMENT: return pht( '%s added a comment.', @@ -918,6 +924,11 @@ abstract class PhabricatorApplicationTransaction $new = $this->getNewValue(); switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_CREATE: + return pht( + '%s created %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($object_phid)); case PhabricatorTransactions::TYPE_COMMENT: return pht( '%s added a comment to %s.', @@ -1119,6 +1130,7 @@ abstract class PhabricatorApplicationTransaction // Make this weaker than TYPE_COMMENT. return 0.25; } + return 1.0; } diff --git a/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php b/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php index 582dfb8179..da5f0520f6 100644 --- a/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorEditEngineConfigurationTransaction.php @@ -34,6 +34,10 @@ final class PhabricatorEditEngineConfigurationTransaction $type = $this->getTransactionType(); switch ($type) { + case PhabricatorTransactions::TYPE_CREATE: + return pht( + '%s created this form configuration.', + $this->renderHandleLink($author_phid)); case self::TYPE_NAME: if (strlen($old)) { return pht(