diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -7,7 +7,7 @@ private $changedPriorToCommitURI; private $isCloseByCommit; private $repositoryPHIDOverride = false; - private $expandedDone = false; + private $didExpandInlineState = false; public function getEditorApplicationClass() { return 'PhabricatorDifferentialApplication'; @@ -100,7 +100,6 @@ case PhabricatorTransactions::TYPE_EDIT_POLICY: case DifferentialTransaction::TYPE_ACTION: case DifferentialTransaction::TYPE_UPDATE: - case DifferentialTransaction::TYPE_INLINEDONE: return $xaction->getNewValue(); case DifferentialTransaction::TYPE_INLINE: return null; @@ -199,7 +198,6 @@ case PhabricatorTransactions::TYPE_SUBSCRIBERS: case PhabricatorTransactions::TYPE_COMMENT: case DifferentialTransaction::TYPE_INLINE: - case DifferentialTransaction::TYPE_INLINEDONE: return; case PhabricatorTransactions::TYPE_EDGE: return; @@ -522,13 +520,13 @@ break; } - if (!$this->expandedDone) { + if (!$this->didExpandInlineState) { switch ($xaction->getTransactionType()) { case PhabricatorTransactions::TYPE_COMMENT: case DifferentialTransaction::TYPE_ACTION: case DifferentialTransaction::TYPE_UPDATE: case DifferentialTransaction::TYPE_INLINE: - $this->expandedDone = true; + $this->didExpandInlineState = true; $actor_phid = $this->getActingAsPHID(); $actor_is_author = ($object->getAuthorPHID() == $actor_phid); @@ -536,12 +534,7 @@ break; } - $state_map = array( - PhabricatorInlineCommentInterface::STATE_DRAFT => - PhabricatorInlineCommentInterface::STATE_DONE, - PhabricatorInlineCommentInterface::STATE_UNDRAFT => - PhabricatorInlineCommentInterface::STATE_UNDONE, - ); + $state_map = PhabricatorTransactions::getInlineStateMap(); $inlines = id(new DifferentialDiffInlineCommentQuery()) ->setViewer($this->getActor()) @@ -560,7 +553,7 @@ } $results[] = id(new DifferentialTransaction()) - ->setTransactionType(DifferentialTransaction::TYPE_INLINEDONE) + ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE) ->setIgnoreOnNoEffect(true) ->setOldValue($old_value) ->setNewValue($new_value); @@ -590,18 +583,6 @@ $reply->setHasReplies(1)->save(); } return; - case DifferentialTransaction::TYPE_INLINEDONE: - $table = new DifferentialTransactionComment(); - $conn_w = $table->establishConnection('w'); - foreach ($xaction->getNewValue() as $phid => $state) { - queryfx( - $conn_w, - 'UPDATE %T SET fixedState = %s WHERE phid = %s', - $table->getTableName(), - $state, - $phid); - } - return; case DifferentialTransaction::TYPE_UPDATE: // Now that we're inside the transaction, do a final check. $diff = $this->requireDiff($xaction->getNewValue()); @@ -626,6 +607,28 @@ return parent::applyCustomExternalTransaction($object, $xaction); } + protected function applyBuiltinExternalTransaction( + PhabricatorLiskDAO $object, + PhabricatorApplicationTransaction $xaction) { + + switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_INLINESTATE: + $table = new DifferentialTransactionComment(); + $conn_w = $table->establishConnection('w'); + foreach ($xaction->getNewValue() as $phid => $state) { + queryfx( + $conn_w, + 'UPDATE %T SET fixedState = %s WHERE phid = %s', + $table->getTableName(), + $state, + $phid); + } + return; + } + + return parent::applyBuiltinExternalTransaction($object, $xaction); + } + protected function mergeEdgeData($type, array $u, array $v) { $result = parent::mergeEdgeData($type, $u, $v); diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php --- a/src/applications/differential/storage/DifferentialTransaction.php +++ b/src/applications/differential/storage/DifferentialTransaction.php @@ -18,7 +18,6 @@ const TYPE_UPDATE = 'differential:update'; const TYPE_ACTION = 'differential:action'; const TYPE_STATUS = 'differential:status'; - const TYPE_INLINEDONE = 'differential:inlinedone'; public function getApplicationName() { return 'differential'; @@ -36,15 +35,6 @@ return new DifferentialTransactionView(); } - public function shouldGenerateOldValue() { - switch ($this->getTransactionType()) { - case DifferentialTransaction::TYPE_INLINEDONE: - return false; - } - - return parent::shouldGenerateOldValue(); - } - public function shouldHide() { $old = $this->getOldValue(); $new = $this->getNewValue(); @@ -80,33 +70,13 @@ return parent::shouldHide(); } - public function shouldHideForMail(array $xactions) { - switch ($this->getTransactionType()) { - case self::TYPE_INLINE: - // Hide inlines when rendering mail transactions if any other - // transaction type exists. - foreach ($xactions as $xaction) { - if ($xaction->getTransactionType() != self::TYPE_INLINE) { - return true; - } - } - - // If only inline transactions exist, we just render the first one. - return ($this !== head($xactions)); - } - - return parent::shouldHideForMail($xactions); - } - - public function getBodyForMail() { + public function isInlineCommentTransaction() { switch ($this->getTransactionType()) { case self::TYPE_INLINE: - // Don't render inlines into the mail body; they render into a special - // section immediately after the body instead. - return null; + return true; } - return parent::getBodyForMail(); + return parent::isInlineCommentTransaction(); } public function getRequiredHandlePHIDs() { @@ -145,8 +115,6 @@ return 3; case self::TYPE_UPDATE: return 2; - case self::TYPE_INLINE: - return 0.25; } return parent::getActionStrength(); @@ -241,35 +209,6 @@ return pht( '%s added inline comments.', $author_handle); - case self::TYPE_INLINEDONE: - $done = 0; - $undone = 0; - foreach ($new as $phid => $state) { - if ($state == PhabricatorInlineCommentInterface::STATE_DONE) { - $done++; - } else { - $undone++; - } - } - if ($done && $undone) { - return pht( - '%s marked %s inline comment(s) as done and %s inline comment(s) '. - 'as not done.', - $author_handle, - new PhutilNumber($done), - new PhutilNumber($undone)); - } else if ($done) { - return pht( - '%s marked %s inline comment(s) as done.', - $author_handle, - new PhutilNumber($done)); - } else { - return pht( - '%s marked %s inline comment(s) as not done.', - $author_handle, - new PhutilNumber($undone)); - } - break; case self::TYPE_UPDATE: if ($this->getMetadataValue('isCommitUpdate')) { return pht( diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php --- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php @@ -550,6 +550,8 @@ break; } break; + case PhabricatorTransactions::TYPE_INLINESTATE: + return true; } return $this->shouldHide(); @@ -944,6 +946,10 @@ } public function getActionStrength() { + if ($this->isInlineCommentTransaction()) { + return 0.25; + } + switch ($this->getTransactionType()) { case PhabricatorTransactions::TYPE_COMMENT: return 0.5;