diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 9e5e4774ef..074cf6efca 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -306,20 +306,27 @@ abstract class PhabricatorApplicationTransactionEditor case PhabricatorTransactions::TYPE_SUBSCRIBERS: return array_values($this->subscribers); case PhabricatorTransactions::TYPE_VIEW_POLICY: + if ($this->getIsNewObject()) { + return null; + } return $object->getViewPolicy(); case PhabricatorTransactions::TYPE_EDIT_POLICY: + if ($this->getIsNewObject()) { + return null; + } return $object->getEditPolicy(); case PhabricatorTransactions::TYPE_JOIN_POLICY: + if ($this->getIsNewObject()) { + return null; + } return $object->getJoinPolicy(); case PhabricatorTransactions::TYPE_SPACE: + if ($this->getIsNewObject()) { + return null; + } + $space_phid = $object->getSpacePHID(); if ($space_phid === null) { - if ($this->getIsNewObject()) { - // In this case, just return `null` so we know this is the initial - // transaction and it should be hidden. - return null; - } - $default_space = PhabricatorSpacesNamespaceQuery::getDefaultSpace(); if ($default_space) { $space_phid = $default_space->getPHID(); diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php index ba7a92df0c..f3973f9213 100644 --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -481,14 +481,22 @@ abstract class PhabricatorApplicationTransaction // transactions like "alice set the task tile to: ...", which are // essentially never interesting. if ($this->getIsCreateTransaction()) { - $old = $this->getOldValue(); + switch ($this->getTransactionType()) { + case PhabricatorTransactions::TYPE_VIEW_POLICY: + case PhabricatorTransactions::TYPE_EDIT_POLICY: + case PhabricatorTransactions::TYPE_JOIN_POLICY: + case PhabricatorTransactions::TYPE_SPACE: + break; + default: + $old = $this->getOldValue(); - if (is_array($old) && !$old) { - return true; - } + if (is_array($old) && !$old) { + return true; + } - if (!strlen($old)) { - return true; + if (!strlen($old)) { + return true; + } } } @@ -510,6 +518,13 @@ abstract class PhabricatorApplicationTransaction case PhabricatorTransactions::TYPE_EDIT_POLICY: case PhabricatorTransactions::TYPE_JOIN_POLICY: case PhabricatorTransactions::TYPE_SPACE: + if ($this->getIsCreateTransaction()) { + break; + } + + // TODO: Remove this eventually, this is handling old changes during + // object creation prior to the introduction of "create" and "default" + // transaction display flags. if ($this->getOldValue() === null) { return true; } else { @@ -692,29 +707,57 @@ abstract class PhabricatorApplicationTransaction '%s added a comment.', $this->renderHandleLink($author_phid)); case PhabricatorTransactions::TYPE_VIEW_POLICY: - return pht( - '%s changed the visibility from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $this->renderPolicyName($old, 'old'), - $this->renderPolicyName($new, 'new')); + if ($this->getIsCreateTransaction()) { + return pht( + '%s created this object with visibility "%s".', + $this->renderHandleLink($author_phid), + $this->renderPolicyName($new, 'new')); + } else { + return pht( + '%s changed the visibility from "%s" to "%s".', + $this->renderHandleLink($author_phid), + $this->renderPolicyName($old, 'old'), + $this->renderPolicyName($new, 'new')); + } case PhabricatorTransactions::TYPE_EDIT_POLICY: - return pht( - '%s changed the edit policy from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $this->renderPolicyName($old, 'old'), - $this->renderPolicyName($new, 'new')); + if ($this->getIsCreateTransaction()) { + return pht( + '%s created this object with edit policy "%s".', + $this->renderHandleLink($author_phid), + $this->renderPolicyName($new, 'new')); + } else { + return pht( + '%s changed the edit policy from "%s" to "%s".', + $this->renderHandleLink($author_phid), + $this->renderPolicyName($old, 'old'), + $this->renderPolicyName($new, 'new')); + } case PhabricatorTransactions::TYPE_JOIN_POLICY: - return pht( - '%s changed the join policy from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $this->renderPolicyName($old, 'old'), - $this->renderPolicyName($new, 'new')); + if ($this->getIsCreateTransaction()) { + return pht( + '%s created this object with join policy "%s".', + $this->renderHandleLink($author_phid), + $this->renderPolicyName($new, 'new')); + } else { + return pht( + '%s changed the join policy from "%s" to "%s".', + $this->renderHandleLink($author_phid), + $this->renderPolicyName($old, 'old'), + $this->renderPolicyName($new, 'new')); + } case PhabricatorTransactions::TYPE_SPACE: - return pht( - '%s shifted this object from the %s space to the %s space.', - $this->renderHandleLink($author_phid), - $this->renderHandleLink($old), - $this->renderHandleLink($new)); + if ($this->getIsCreateTransaction()) { + return pht( + '%s created this object in space %s.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($new)); + } else { + return pht( + '%s shifted this object from the %s space to the %s space.', + $this->renderHandleLink($author_phid), + $this->renderHandleLink($old), + $this->renderHandleLink($new)); + } case PhabricatorTransactions::TYPE_SUBSCRIBERS: $add = array_diff($new, $old); $rem = array_diff($old, $new);