1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-28 16:30:59 +01:00

Allow users to remove their own comments, and administrators to remove any comment

Summary:
Fixes T4909. Adds a "remove" link next to the edit link, which permanently hides a comment. Addresses two use cases:

  - Allowing administrators to clean up spam.
  - Allowing users to try to put the genie back in the bottle if they post passwords or sensitive links, etc.

The user who removed the comment is named in the removal text to enforce some level of administrative accountability.

No data is deleted, but there's currently no method to restore these comments. We'll see if we need one.

This is cheating a little bit by storing "removed" as "2" in the isDeleted field. This doesn't seem tooooo bad for now.

Test Plan:
  - Removed some of my comments.
  - As an administrator, removed other users' comments.
  - Failed to view history of a removed comment.
  - Failed to edit a removed comment.
  - Failed to remove a removed comment.
  - Verified feed doesn't show the old comment after comment removal.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: qgil, chad, epriestley

Maniphest Tasks: T4909

Differential Revision: https://secure.phabricator.com/D8945
This commit is contained in:
epriestley 2014-05-05 10:55:32 -07:00
parent 6bced2170e
commit 58f66fea80
11 changed files with 181 additions and 19 deletions

View file

@ -1164,6 +1164,7 @@ phutil_register_library_map(array(
'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',
@ -3647,6 +3648,7 @@ phutil_register_library_map(array(
4 => 'PhabricatorFlaggableInterface',
5 => 'PhrequentTrackableInterface',
6 => 'PhabricatorCustomFieldInterface',
7 => 'PhabricatorDestructableInterface',
),
'ManiphestTaskDescriptionPreviewController' => 'ManiphestController',
'ManiphestTaskDetailController' => 'ManiphestController',
@ -3934,6 +3936,7 @@ phutil_register_library_map(array(
'PhabricatorApplicationTransactionCommentEditor' => 'PhabricatorEditor',
'PhabricatorApplicationTransactionCommentHistoryController' => 'PhabricatorApplicationTransactionController',
'PhabricatorApplicationTransactionCommentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorApplicationTransactionCommentRemoveController' => 'PhabricatorApplicationTransactionController',
'PhabricatorApplicationTransactionCommentView' => 'AphrontView',
'PhabricatorApplicationTransactionController' => 'PhabricatorController',
'PhabricatorApplicationTransactionDetailController' => 'PhabricatorApplicationTransactionController',

View file

@ -15,6 +15,8 @@ final class PhabricatorApplicationTransactions extends PhabricatorApplication {
'/transactions/' => array(
'edit/(?<phid>[^/]+)/'
=> 'PhabricatorApplicationTransactionCommentEditController',
'remove/(?<phid>[^/]+)/'
=> 'PhabricatorApplicationTransactionCommentRemoveController',
'history/(?<phid>[^/]+)/'
=> 'PhabricatorApplicationTransactionCommentHistoryController',
'detail/(?<phid>[^/]+)/'

View file

@ -28,6 +28,11 @@ final class PhabricatorApplicationTransactionCommentEditController
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)
@ -75,7 +80,7 @@ final class PhabricatorApplicationTransactionCommentEditController
->setValue($xaction->getComment()->getContent())));
$dialog
->addSubmitButton(pht('Edit Comment'))
->addSubmitButton(pht('Save Changes'))
->addCancelButton($obj_handle->getURI());
return id(new AphrontDialogResponse())->setDialog($dialog);

View file

@ -31,6 +31,11 @@ final class PhabricatorApplicationTransactionCommentHistoryController
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())

View file

@ -0,0 +1,82 @@
<?php
final class PhabricatorApplicationTransactionCommentRemoveController
extends PhabricatorApplicationTransactionController {
private $phid;
public function willProcessRequest(array $data) {
$this->phid = $data['phid'];
}
public function processRequest() {
$request = $this->getRequest();
$viewer = $request->getUser();
$xaction = id(new PhabricatorObjectQuery())
->withPHIDs(array($this->phid))
->setViewer($viewer)
->executeOne();
if (!$xaction) {
return new Aphront404Response();
}
if (!$xaction->getComment()) {
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($viewer)
->withPHIDs(array($obj_phid))
->executeOne();
if ($request->isDialogFormPost()) {
$comment = $xaction->getApplicationTransactionCommentObject()
->setContent('')
->setIsRemoved(true);
$editor = id(new PhabricatorApplicationTransactionCommentEditor())
->setActor($viewer)
->setContentSource(PhabricatorContentSource::newFromRequest($request))
->applyEdit($xaction, $comment);
if ($request->isAjax()) {
return id(new PhabricatorApplicationTransactionResponse())
->setViewer($viewer)
->setTransactions(array($xaction))
->setAnchorOffset($request->getStr('anchor'));
} else {
return id(new AphrontReloadResponse())->setURI($obj_handle->getURI());
}
}
$form = id(new AphrontFormView())
->setUser($viewer);
$dialog = $this->newDialog()
->setTitle(pht('Remove Comment'));
$dialog
->addHiddenInput('anchor', $request->getStr('anchor'))
->appendParagraph(
pht(
"Removing a comment prevents anyone (including you) from reading ".
"it. Removing a comment also hides the comment's edit history ".
"and prevents it from being edited."))
->appendParagraph(
pht('Really remove this comment?'));
$dialog
->addSubmitButton(pht('Remove Comment'))
->addCancelButton($obj_handle->getURI());
return $dialog;
}
}

View file

@ -95,10 +95,15 @@ final class PhabricatorApplicationTransactionCommentEditor
$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);
}
}

View file

@ -248,6 +248,10 @@ abstract class PhabricatorApplicationTransaction
break;
}
if ($this->getComment()) {
$phids[] = array($this->getComment()->getAuthorPHID());
}
return array_mergev($phids);
}

View file

@ -59,6 +59,19 @@ abstract class PhabricatorApplicationTransactionComment
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 )----------------------------------------- */

View file

@ -220,7 +220,18 @@ class PhabricatorApplicationTransactionView extends AphrontView {
$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 @@ class PhabricatorApplicationTransactionView extends AphrontView {
$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 @@ class PhabricatorApplicationTransactionView extends AphrontView {
$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);
}
}
}
}

View file

@ -14,6 +14,7 @@ final class PHUITimelineEventView extends AphrontView {
private $anchor;
private $isEditable;
private $isEdited;
private $isRemovable;
private $transactionPHID;
private $isPreview;
private $eventGroup = array();
@ -66,6 +67,15 @@ final class PHUITimelineEventView extends AphrontView {
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;
@ -367,6 +377,16 @@ final class PHUITimelineEventView extends AphrontView {
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) {

View file

@ -96,20 +96,24 @@ JX.behavior('phabricator-transaction-list', function(config) {
}
}
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'],