From b5750412c75b85b30ad0932ed7ccca4c6378db3e Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 2 Aug 2014 14:45:39 -0700 Subject: [PATCH] Apply normal Audit actions directly with Transaction editor Summary: Ref T4896. This converts the last "CommentEditor" to a transaction editor and removes a large part of the old code. Test Plan: - Added comments. - Accepted / added auditors. - Added inline comments. Reviewers: joshuaspence, btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4896 Differential Revision: https://secure.phabricator.com/D10128 --- src/__phutil_library_map__.php | 2 - .../PhabricatorAuditAddCommentController.php | 49 +++-- .../editor/PhabricatorAuditCommentEditor.php | 58 ------ .../mail/PhabricatorAuditReplyHandler.php | 2 +- .../audit/storage/PhabricatorAuditComment.php | 197 ------------------ 5 files changed, 32 insertions(+), 276 deletions(-) delete mode 100644 src/applications/audit/storage/PhabricatorAuditComment.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e67271042d..1260289c10 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1136,7 +1136,6 @@ phutil_register_library_map(array( 'PhabricatorAuditActionConstants' => 'applications/audit/constants/PhabricatorAuditActionConstants.php', 'PhabricatorAuditAddCommentController' => 'applications/audit/controller/PhabricatorAuditAddCommentController.php', 'PhabricatorAuditApplication' => 'applications/audit/application/PhabricatorAuditApplication.php', - 'PhabricatorAuditComment' => 'applications/audit/storage/PhabricatorAuditComment.php', 'PhabricatorAuditCommentEditor' => 'applications/audit/editor/PhabricatorAuditCommentEditor.php', 'PhabricatorAuditCommitStatusConstants' => 'applications/audit/constants/PhabricatorAuditCommitStatusConstants.php', 'PhabricatorAuditController' => 'applications/audit/controller/PhabricatorAuditController.php', @@ -3931,7 +3930,6 @@ phutil_register_library_map(array( 'PhabricatorAsanaConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorAuditAddCommentController' => 'PhabricatorAuditController', 'PhabricatorAuditApplication' => 'PhabricatorApplication', - 'PhabricatorAuditComment' => 'PhabricatorMarkupInterface', 'PhabricatorAuditCommentEditor' => 'PhabricatorEditor', 'PhabricatorAuditController' => 'PhabricatorController', 'PhabricatorAuditDAO' => 'PhabricatorLiskDAO', diff --git a/src/applications/audit/controller/PhabricatorAuditAddCommentController.php b/src/applications/audit/controller/PhabricatorAuditAddCommentController.php index 9cf07058cb..92c9beb933 100644 --- a/src/applications/audit/controller/PhabricatorAuditAddCommentController.php +++ b/src/applications/audit/controller/PhabricatorAuditAddCommentController.php @@ -21,22 +21,20 @@ final class PhabricatorAuditAddCommentController return new Aphront404Response(); } - $phids = array($commit_phid); - - $comments = array(); + $xactions = array(); // make sure we only add auditors or ccs if the action matches $action = $request->getStr('action'); switch ($action) { case PhabricatorAuditActionConstants::ADD_AUDITORS: $auditors = $request->getArr('auditors'); - $comments[] = id(new PhabricatorAuditComment()) - ->setAction(PhabricatorAuditActionConstants::ADD_AUDITORS) + $xactions[] = id(new PhabricatorAuditTransaction()) + ->setTransactionType(PhabricatorAuditActionConstants::ADD_AUDITORS) ->setNewValue(array_fuse($auditors)); break; case PhabricatorAuditActionConstants::ADD_CCS: - $comments[] = id(new PhabricatorAuditComment()) - ->setAction(PhabricatorTransactions::TYPE_SUBSCRIBERS) + $xactions[] = id(new PhabricatorAuditTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS) ->setNewValue( array( '+' => $request->getArr('ccs'), @@ -46,25 +44,36 @@ final class PhabricatorAuditAddCommentController // We'll deal with this below. break; default: - $comments[] = id(new PhabricatorAuditComment()) - ->setAction($action); + $xactions[] = id(new PhabricatorAuditTransaction()) + ->setTransactionType(PhabricatorAuditActionConstants::ACTION) + ->setNewValue($action); break; } $content = $request->getStr('content'); if (strlen($content)) { - $comments[] = id(new PhabricatorAuditComment()) - ->setAction(PhabricatorAuditActionConstants::COMMENT) - ->setContent($content); + $xactions[] = id(new PhabricatorAuditTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) + ->attachComment( + id(new PhabricatorAuditTransactionComment()) + ->setCommitPHID($commit->getPHID()) + ->setContent($content)); } - id(new PhabricatorAuditCommentEditor($commit)) - ->setActor($user) - ->setAttachInlineComments(true) - ->addComments($comments); + $inlines = PhabricatorAuditInlineComment::loadDraftComments( + $user, + $commit->getPHID()); + foreach ($inlines as $inline) { + $xactions[] = id(new PhabricatorAuditTransaction()) + ->setTransactionType(PhabricatorAuditActionConstants::INLINE) + ->attachComment($inline->getTransactionComment()); + } - $handles = $this->loadViewerHandles($phids); - $uri = $handles[$commit_phid]->getURI(); + id(new PhabricatorAuditEditor()) + ->setActor($user) + ->setContentSourceFromRequest($request) + ->setContinueOnMissingFields(true) + ->applyTransactions($commit, $xactions); $draft = id(new PhabricatorDraft())->loadOneWhere( 'authorPHID = %s AND draftKey = %s', @@ -74,6 +83,10 @@ final class PhabricatorAuditAddCommentController $draft->delete(); } + $monogram = $commit->getRepository()->getMonogram(); + $identifier = $commit->getCommitIdentifier(); + $uri = '/'.$monogram.$identifier; + return id(new AphrontRedirectResponse())->setURI($uri); } diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php index 9842d08b90..5567af5774 100644 --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -2,64 +2,6 @@ final class PhabricatorAuditCommentEditor extends PhabricatorEditor { - private $commit; - private $attachInlineComments; - - public function __construct(PhabricatorRepositoryCommit $commit) { - $this->commit = $commit; - return $this; - } - - public function setAttachInlineComments($attach_inline_comments) { - $this->attachInlineComments = $attach_inline_comments; - return $this; - } - - public function addComments(array $comments) { - assert_instances_of($comments, 'PhabricatorAuditComment'); - - $commit = $this->commit; - $actor = $this->getActor(); - - $other_comments = PhabricatorAuditComment::loadComments( - $actor, - $commit->getPHID()); - - $inline_comments = array(); - if ($this->attachInlineComments) { - $inline_comments = PhabricatorAuditInlineComment::loadDraftComments( - $actor, - $commit->getPHID()); - } - - // Convert old comments into real transactions and apply them with a - // normal editor. - - $xactions = array(); - foreach ($comments as $comment) { - $xactions[] = $comment->getTransactionForSave(); - } - - foreach ($inline_comments as $inline) { - $xactions[] = id(new PhabricatorAuditTransaction()) - ->setTransactionType(PhabricatorAuditActionConstants::INLINE) - ->attachComment($inline->getTransactionComment()); - } - - $content_source = PhabricatorContentSource::newForSource( - PhabricatorContentSource::SOURCE_LEGACY, - array()); - - $editor = id(new PhabricatorAuditEditor()) - ->setActor($actor) - ->setContinueOnNoEffect(true) - ->setContinueOnMissingFields(true) - ->setContentSource($content_source) - ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) - ->applyTransactions($commit, $xactions); - } - - /** * Load the PHIDs for all objects the user has the authority to act as an * audit for. This includes themselves, and any packages they are an owner diff --git a/src/applications/audit/mail/PhabricatorAuditReplyHandler.php b/src/applications/audit/mail/PhabricatorAuditReplyHandler.php index f4b77af3f4..698be5b9ef 100644 --- a/src/applications/audit/mail/PhabricatorAuditReplyHandler.php +++ b/src/applications/audit/mail/PhabricatorAuditReplyHandler.php @@ -52,7 +52,7 @@ final class PhabricatorAuditReplyHandler extends PhabricatorMailReplyHandler { ->setCommitPHID($commit->getPHID()) ->setContent($message)); - $editor = id(new PhabricatorAuditEditor($commit)) + $editor = id(new PhabricatorAuditEditor()) ->setActor($actor) ->setContentSource($content_source) ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) diff --git a/src/applications/audit/storage/PhabricatorAuditComment.php b/src/applications/audit/storage/PhabricatorAuditComment.php deleted file mode 100644 index ac19bf9a09..0000000000 --- a/src/applications/audit/storage/PhabricatorAuditComment.php +++ /dev/null @@ -1,197 +0,0 @@ -proxy = new PhabricatorAuditTransaction(); - } - - public function __clone() { - $this->proxy = clone $this->proxy; - if ($this->proxyComment) { - $this->proxyComment = clone $this->proxyComment; - } - } - - public static function newFromModernTransaction( - PhabricatorAuditTransaction $xaction) { - - $obj = new PhabricatorAuditComment(); - $obj->proxy = $xaction; - - if ($xaction->hasComment()) { - $obj->proxyComment = $xaction->getComment(); - } - - return $obj; - } - - public static function loadComments( - PhabricatorUser $viewer, - $commit_phid) { - - $xactions = id(new PhabricatorAuditTransactionQuery()) - ->setViewer($viewer) - ->withObjectPHIDs(array($commit_phid)) - ->needComments(true) - ->execute(); - - $comments = array(); - foreach ($xactions as $xaction) { - $comments[] = self::newFromModernTransaction($xaction); - } - - return $comments; - } - - public function getPHID() { - return $this->proxy->getPHID(); - } - - public function getActorPHID() { - return $this->proxy->getAuthorPHID(); - } - - public function setActorPHID($actor_phid) { - $this->proxy->setAuthorPHID($actor_phid); - return $this; - } - - public function setTargetPHID($target_phid) { - $this->getProxyComment()->setCommitPHID($target_phid); - $this->proxy->setObjectPHID($target_phid); - return $this; - } - - public function getTargetPHID() { - return $this->proxy->getObjectPHID(); - } - - public function getContent() { - return $this->getProxyComment()->getContent(); - } - - public function setContent($content) { - $this->getProxyComment()->setContent($content); - return $this; - } - - public function setContentSource($content_source) { - $this->proxy->setContentSource($content_source); - $this->proxyComment->setContentSource($content_source); - return $this; - } - - public function getContentSource() { - return $this->proxy->getContentSource(); - } - - private function getProxyComment() { - if (!$this->proxyComment) { - $this->proxyComment = new PhabricatorAuditTransactionComment(); - } - return $this->proxyComment; - } - - public function setProxyComment(PhabricatorAuditTransactionComment $proxy) { - if ($this->proxyComment) { - throw new Exception(pht('You can not overwrite a proxy comment.')); - } - $this->proxyComment = $proxy; - return $this; - } - - public function setAction($action) { - switch ($action) { - case PhabricatorAuditActionConstants::INLINE: - case PhabricatorAuditActionConstants::ADD_CCS: - case PhabricatorTransactions::TYPE_SUBSCRIBERS: - case PhabricatorAuditActionConstants::ADD_AUDITORS: - $this->proxy->setTransactionType($action); - break; - case PhabricatorAuditActionConstants::COMMENT: - $this->proxy->setTransactionType(PhabricatorTransactions::TYPE_COMMENT); - break; - default: - $this->proxy - ->setTransactionType(PhabricatorAuditActionConstants::ACTION) - ->setNewValue($action); - break; - } - - return $this; - } - - public function getAction() { - $type = $this->proxy->getTransactionType(); - switch ($type) { - case PhabricatorTransactions::TYPE_COMMENT: - return PhabricatorAuditActionConstants::COMMENT; - case PhabricatorAuditActionConstants::INLINE: - case PhabricatorAuditActionConstants::ADD_CCS: - case PhabricatorTransactions::TYPE_SUBSCRIBERS: - case PhabricatorAuditActionConstants::ADD_AUDITORS: - return $type; - default: - return $this->proxy->getNewValue(); - } - } - - public function save() { - throw new Exception( - pht('This object can no longer be written to directly!')); - } - - public function getTransactionForSave() { - $xaction = $this->proxy; - if (strlen($this->getContent())) { - $xaction->attachComment($this->getProxyComment()); - } - - return $xaction; - } - - public function setNewValue($value) { - $this->proxy->setNewValue($value); - return $this; - } - - public function getDateCreated() { - return $this->proxy->getDateCreated(); - } - - public function getDateModified() { - return $this->proxy->getDateModified(); - } - - -/* -( PhabricatorMarkupInterface Implementation )-------------------------- */ - - - public function getMarkupFieldKey($field) { - return 'AC:'.$this->getPHID(); - } - - public function newMarkupEngine($field) { - return PhabricatorMarkupEngine::newDiffusionMarkupEngine(); - } - - public function getMarkupText($field) { - return $this->getContent(); - } - - public function didMarkupText($field, $output, PhutilMarkupEngine $engine) { - return $output; - } - - public function shouldUseMarkupCache($field) { - return (bool)$this->getPHID(); - } - -}