diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1162,6 +1162,7 @@ 'PhabricatorApplicationTransactionCommentEditor' => 'applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php', 'PhabricatorApplicationTransactionCommentHistoryController' => 'applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php', 'PhabricatorApplicationTransactionCommentQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php', + 'PhabricatorApplicationTransactionCommentRemoveController' => 'applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php', 'PhabricatorApplicationTransactionCommentView' => 'applications/transactions/view/PhabricatorApplicationTransactionCommentView.php', 'PhabricatorApplicationTransactionController' => 'applications/transactions/controller/PhabricatorApplicationTransactionController.php', 'PhabricatorApplicationTransactionDetailController' => 'applications/transactions/controller/PhabricatorApplicationTransactionDetailController.php', @@ -3644,6 +3645,7 @@ 4 => 'PhabricatorFlaggableInterface', 5 => 'PhrequentTrackableInterface', 6 => 'PhabricatorCustomFieldInterface', + 7 => 'PhabricatorDestructableInterface', ), 'ManiphestTaskDescriptionPreviewController' => 'ManiphestController', 'ManiphestTaskDetailController' => 'ManiphestController', @@ -3930,6 +3932,7 @@ 'PhabricatorApplicationTransactionCommentEditor' => 'PhabricatorEditor', 'PhabricatorApplicationTransactionCommentHistoryController' => 'PhabricatorApplicationTransactionController', 'PhabricatorApplicationTransactionCommentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorApplicationTransactionCommentRemoveController' => 'PhabricatorApplicationTransactionController', 'PhabricatorApplicationTransactionCommentView' => 'AphrontView', 'PhabricatorApplicationTransactionController' => 'PhabricatorController', 'PhabricatorApplicationTransactionDetailController' => 'PhabricatorApplicationTransactionController', diff --git a/src/applications/transactions/application/PhabricatorApplicationTransactions.php b/src/applications/transactions/application/PhabricatorApplicationTransactions.php --- a/src/applications/transactions/application/PhabricatorApplicationTransactions.php +++ b/src/applications/transactions/application/PhabricatorApplicationTransactions.php @@ -15,6 +15,8 @@ '/transactions/' => array( 'edit/(?[^/]+)/' => 'PhabricatorApplicationTransactionCommentEditController', + 'remove/(?[^/]+)/' + => 'PhabricatorApplicationTransactionCommentRemoveController', 'history/(?[^/]+)/' => 'PhabricatorApplicationTransactionCommentHistoryController', 'detail/(?[^/]+)/' diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php @@ -28,6 +28,11 @@ return new Aphront404Response(); } + if ($xaction->getComment()->getIsRemoved()) { + // You can't edit history of a transaction with a removed comment. + return new Aphront400Response(); + } + $obj_phid = $xaction->getObjectPHID(); $obj_handle = id(new PhabricatorHandleQuery()) ->setViewer($user) diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php @@ -31,6 +31,11 @@ return new Aphront404Response(); } + if ($xaction->getComment()->getIsRemoved()) { + // You can't view history of a transaction with a removed comment. + return new Aphront400Response(); + } + $comments = id(new PhabricatorApplicationTransactionCommentQuery()) ->setViewer($user) ->setTemplate($xaction->getApplicationTransactionCommentObject()) diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php copy from src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php copy to src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php @@ -1,6 +1,6 @@ getRequest(); - $user = $request->getUser(); + $viewer = $request->getUser(); $xaction = id(new PhabricatorObjectQuery()) ->withPHIDs(array($this->phid)) - ->setViewer($user) + ->setViewer($viewer) ->executeOne(); - if (!$xaction) { return new Aphront404Response(); } if (!$xaction->getComment()) { - // You can't currently edit a transaction which doesn't have a comment. - // Some day you may be able to edit the visibility. return new Aphront404Response(); } + if ($xaction->getComment()->getIsRemoved()) { + // You can't remove an already-removed comment. + return new Aphront400Response(); + } + $obj_phid = $xaction->getObjectPHID(); $obj_handle = id(new PhabricatorHandleQuery()) - ->setViewer($user) + ->setViewer($viewer) ->withPHIDs(array($obj_phid)) ->executeOne(); if ($request->isDialogFormPost()) { - $text = $request->getStr('text'); - - $comment = $xaction->getApplicationTransactionCommentObject(); - $comment->setContent($text); - if (!strlen($text)) { - $comment->setIsDeleted(true); - } + $comment = $xaction->getApplicationTransactionCommentObject() + ->setContent('') + ->setIsRemoved(true); $editor = id(new PhabricatorApplicationTransactionCommentEditor()) - ->setActor($user) + ->setActor($viewer) ->setContentSource(PhabricatorContentSource::newFromRequest($request)) ->applyEdit($xaction, $comment); if ($request->isAjax()) { return id(new PhabricatorApplicationTransactionResponse()) - ->setViewer($user) + ->setViewer($viewer) ->setTransactions(array($xaction)) ->setAnchorOffset($request->getStr('anchor')); } else { @@ -58,25 +56,22 @@ } } - $dialog = id(new AphrontDialogView()) - ->setUser($user) - ->setTitle(pht('Edit Comment')); + $form = id(new AphrontFormView()) + ->setUser($viewer); + + $dialog = $this->newDialog() + ->setTitle(pht('Remove Comment')); $dialog ->addHiddenInput('anchor', $request->getStr('anchor')) - ->appendChild( - id(new PHUIFormLayoutView()) - ->setFullWidth(true) - ->appendChild( - id(new PhabricatorRemarkupControl()) - ->setName('text') - ->setValue($xaction->getComment()->getContent()))); + ->appendParagraph( + pht('Really remove this comment?')); $dialog - ->addSubmitButton(pht('Edit Comment')) + ->addSubmitButton(pht('Remove Comment')) ->addCancelButton($obj_handle->getURI()); - return id(new AphrontDialogResponse())->setDialog($dialog); + return $dialog; } } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php @@ -95,10 +95,15 @@ $xaction, PhabricatorPolicyCapability::CAN_VIEW); - PhabricatorPolicyFilter::requireCapability( - $actor, - $xaction, - PhabricatorPolicyCapability::CAN_EDIT); + if ($comment->getIsRemoved() && $actor->getIsAdmin()) { + // NOTE: Administrators can remove comments by any user, and don't need + // to pass the edit check. + } else { + PhabricatorPolicyFilter::requireCapability( + $actor, + $xaction, + PhabricatorPolicyCapability::CAN_EDIT); + } } 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 @@ -248,6 +248,10 @@ break; } + if ($this->getComment()) { + $phids[] = array($this->getComment()->getAuthorPHID()); + } + return array_mergev($phids); } diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php --- a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php +++ b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php @@ -59,6 +59,19 @@ return PhabricatorContentSource::newFromSerialized($this->contentSource); } + public function getIsRemoved() { + return ($this->getIsDeleted() == 2); + } + + public function setIsRemoved($removed) { + if ($removed) { + $this->setIsDeleted(2); + } else { + $this->setIsDeleted(0); + } + return $this; + } + /* -( PhabricatorMarkupInterface )----------------------------------------- */ diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php --- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php +++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php @@ -220,7 +220,18 @@ $comment = $xaction->getComment(); if ($comment) { - if ($comment->getIsDeleted()) { + if ($comment->getIsRemoved()) { + return javelin_tag( + 'span', + array( + 'class' => 'comment-deleted', + 'sigil' => 'transaction-comment', + 'meta' => array('phid' => $comment->getTransactionPHID()), + ), + pht( + 'This comment was removed by %s.', + $xaction->getHandle($comment->getAuthorPHID())->renderLink())); + } else if ($comment->getIsDeleted()) { return javelin_tag( 'span', array( @@ -357,8 +368,11 @@ $has_deleted_comment = $xaction->getComment() && $xaction->getComment()->getIsDeleted(); + $has_removed_comment = $xaction->getComment() && + $xaction->getComment()->getIsRemoved(); + if ($this->getShowEditActions() && !$this->isPreview) { - if ($xaction->getCommentVersion() > 1) { + if ($xaction->getCommentVersion() > 1 && !$has_removed_comment) { $event->setIsEdited(true); } @@ -369,9 +383,14 @@ $viewer, $xaction, $can_edit); - if ($has_edit_capability) { + if ($has_edit_capability && !$has_removed_comment) { $event->setIsEditable(true); } + if ($has_edit_capability || $viewer->getIsAdmin()) { + if (!$has_removed_comment) { + $event->setIsRemovable(true); + } + } } } diff --git a/src/view/phui/PHUITimelineEventView.php b/src/view/phui/PHUITimelineEventView.php --- a/src/view/phui/PHUITimelineEventView.php +++ b/src/view/phui/PHUITimelineEventView.php @@ -14,6 +14,7 @@ private $anchor; private $isEditable; private $isEdited; + private $isRemovable; private $transactionPHID; private $isPreview; private $eventGroup = array(); @@ -66,6 +67,15 @@ return $this->isEditable; } + public function setIsRemovable($is_removable) { + $this->isRemovable = $is_removable; + return $this; + } + + public function getIsRemovable() { + return $this->isRemovable; + } + public function setDateCreated($date_created) { $this->dateCreated = $date_created; return $this; @@ -360,6 +370,16 @@ pht('Edit')); } + if ($this->getIsRemovable()) { + $extra[] = javelin_tag( + 'a', + array( + 'href' => '/transactions/remove/'.$xaction_phid.'/', + 'sigil' => 'workflow transaction-remove', + ), + pht('Remove')); + } + if ($is_first_extra) { $source = $this->getContentSource(); if ($source) { diff --git a/webroot/rsrc/js/application/transactions/behavior-transaction-list.js b/webroot/rsrc/js/application/transactions/behavior-transaction-list.js --- a/webroot/rsrc/js/application/transactions/behavior-transaction-list.js +++ b/webroot/rsrc/js/application/transactions/behavior-transaction-list.js @@ -96,20 +96,24 @@ } } - JX.DOM.listen(list, 'click', 'transaction-edit', function(e) { - if (!e.isNormalClick()) { - return; - } + JX.DOM.listen( + list, + 'click', + ['transaction-edit', 'transaction-remove'], + function(e) { + if (!e.isNormalClick()) { + return; + } - var transaction = e.getNode('transaction'); + var transaction = e.getNode('transaction'); - JX.Workflow.newFromLink(e.getTarget()) - .setData({anchor: e.getNodeData('transaction').anchor}) - .setHandler(JX.bind(null, edittransaction, transaction)) - .start(); + JX.Workflow.newFromLink(e.getTarget()) + .setData({anchor: e.getNodeData('transaction').anchor}) + .setHandler(JX.bind(null, edittransaction, transaction)) + .start(); - e.kill(); - }); + e.kill(); + }); JX.Stratcom.listen( ['submit', 'didSyntheticSubmit'],