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 @@ -31,6 +31,25 @@ $done_uri = $obj_handle->getURI(); + // If an object is locked, you can't edit comments on it. Two reasons to + // lock threads are to calm contentious issues and to freeze state for + // auditing, and editing comments serves neither goal. + + $object = $xaction->getObject(); + $can_interact = PhabricatorPolicyFilter::hasCapability( + $viewer, + $object, + PhabricatorPolicyCapability::CAN_INTERACT); + if (!$can_interact) { + return $this->newDialog() + ->setTitle(pht('Conversation Locked')) + ->appendParagraph( + pht( + 'You can not edit this comment because the conversation is '. + 'locked.')) + ->addCancelButton($done_uri); + } + if ($request->isFormOrHisecPost()) { $text = $request->getStr('text'); diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentRemoveController.php @@ -32,6 +32,26 @@ $done_uri = $obj_handle->getURI(); + // We allow administrative removal of comments even if an object is locked, + // so you can lock a flamewar and then go clean it up. Locked threads may + // not otherwise be edited, and non-administrators can not remove comments + // from locked threads. + + $object = $xaction->getObject(); + $can_interact = PhabricatorPolicyFilter::hasCapability( + $viewer, + $object, + PhabricatorPolicyCapability::CAN_INTERACT); + if (!$can_interact && !$viewer->getIsAdmin()) { + return $this->newDialog() + ->setTitle(pht('Conversation Locked')) + ->appendParagraph( + pht( + 'You can not remove this comment because the conversation is '. + 'locked.')) + ->addCancelButton($done_uri); + } + if ($request->isFormOrHisecPost()) { $comment = $xaction->getApplicationTransactionCommentObject() ->setContent('') 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 @@ -189,6 +189,10 @@ $actor, $xaction, PhabricatorPolicyCapability::CAN_EDIT); + PhabricatorPolicyFilter::requireCapability( + $actor, + $xaction->getObject(), + PhabricatorPolicyCapability::CAN_INTERACT); } } 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 @@ -505,12 +505,19 @@ if ($has_edit_capability && !$has_removed_comment) { $event->setIsEditable(true); } + if ($has_edit_capability || $viewer->getIsAdmin()) { if (!$has_removed_comment) { $event->setIsRemovable(true); } } } + + $can_interact = PhabricatorPolicyFilter::hasCapability( + $viewer, + $xaction->getObject(), + PhabricatorPolicyCapability::CAN_INTERACT); + $event->setCanInteract($can_interact); } $comment = $this->renderTransactionContent($xaction); 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 @@ -32,6 +32,7 @@ private $isSilent; private $isMFA; private $isLockOverride; + private $canInteract; public function setAuthorPHID($author_phid) { $this->authorPHID = $author_phid; @@ -114,6 +115,15 @@ return $this->isEditable; } + public function setCanInteract($can_interact) { + $this->canInteract = $can_interact; + return $this; + } + + public function getCanInteract() { + return $this->canInteract; + } + public function setIsRemovable($is_removable) { $this->isRemovable = $is_removable; return $this; @@ -650,6 +660,10 @@ private function getMenuItems($anchor) { $xaction_phid = $this->getTransactionPHID(); + $can_interact = $this->getCanInteract(); + $viewer = $this->getViewer(); + $is_admin = $viewer->getIsAdmin(); + $items = array(); if ($this->getIsEditable()) { @@ -658,6 +672,7 @@ ->setHref('/transactions/edit/'.$xaction_phid.'/') ->setName(pht('Edit Comment')) ->addSigil('transaction-edit') + ->setDisabled(!$can_interact) ->setMetadata( array( 'anchor' => $anchor, @@ -727,17 +742,23 @@ $items[] = id(new PhabricatorActionView()) ->setType(PhabricatorActionView::TYPE_DIVIDER); - $items[] = id(new PhabricatorActionView()) + $remove_item = id(new PhabricatorActionView()) ->setIcon('fa-trash-o') ->setHref('/transactions/remove/'.$xaction_phid.'/') ->setName(pht('Remove Comment')) - ->setColor(PhabricatorActionView::RED) ->addSigil('transaction-remove') ->setMetadata( array( 'anchor' => $anchor, )); + if (!$is_admin && !$can_interact) { + $remove_item->setDisabled(!$is_admin && !$can_interact); + } else { + $remove_item->setColor(PhabricatorActionView::RED); + } + + $items[] = $remove_item; } return $items;