1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 06:42:42 +01:00

Do not add actor as subscriber when removing a comment

Summary:
When an admin removes a comment (e.g. spam), the admin gets subscribed to the task. This is usually unwanted as the removal action does not imply that the admin is interested in receiving future notifications about the task, in contrast to e.g. adding a comment to a discussion in the task.

Any transaction of a comment (add, edit, remove) is a `"core:comment"` action. The code calls `applyImplicitCC()` which calls `shouldImplyCC()` which returns the bool `$xaction->isCommentTransaction()`. Expand this bool to `$xaction->isCommentTransaction() && !($xaction->getComment()->getIsRemoved())`.

Closes T15899

Test Plan:
* As an admin, go to a task which has comments and to which you are not subscribed
* Click the dropdown for the comment, select Remove comment
* See that you did not get subscribed to the task

Reviewers: O1 Blessed Committers, valerio.bozzolan

Reviewed By: O1 Blessed Committers, valerio.bozzolan

Subscribers: tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Maniphest Tasks: T15899

Differential Revision: https://we.phorge.it/D25760
This commit is contained in:
Andre Klapper 2024-08-02 00:17:25 +02:00
parent 4da3b096b0
commit 93c9afd2f3
3 changed files with 20 additions and 2 deletions

View file

@ -3299,7 +3299,10 @@ abstract class PhabricatorApplicationTransactionEditor
/** /**
* When a user interacts with an object, we might want to add them to CC. * Adds the actor as a subscriber to the object with which they interact
* @param PhabricatorLiskDAO $object on which the action is performed
* @param array $xactions Transactions to apply
* @return array Transactions to apply
*/ */
final public function applyImplicitCC( final public function applyImplicitCC(
PhabricatorLiskDAO $object, PhabricatorLiskDAO $object,
@ -3381,11 +3384,18 @@ abstract class PhabricatorApplicationTransactionEditor
return $xactions; return $xactions;
} }
/**
* Whether the action implies the actor should be subscribed on the object
* @param PhabricatorLiskDAO $object on which the action is performed
* @param PhabricatorApplicationTransaction $xaction Transaction to apply
* @return bool True if the actor should be subscribed on the object
*/
protected function shouldImplyCC( protected function shouldImplyCC(
PhabricatorLiskDAO $object, PhabricatorLiskDAO $object,
PhabricatorApplicationTransaction $xaction) { PhabricatorApplicationTransaction $xaction) {
return $xaction->isCommentTransaction(); return ($xaction->isCommentTransaction() &&
!($xaction->getComment()->getIsRemoved()));
} }

View file

@ -1580,6 +1580,10 @@ abstract class PhabricatorApplicationTransaction
return 100; return 100;
} }
/**
* Whether the transaction concerns a comment (e.g. add, edit, remove)
* @return bool True if the transaction concerns a comment
*/
public function isCommentTransaction() { public function isCommentTransaction() {
if ($this->hasComment()) { if ($this->hasComment()) {
return true; return true;

View file

@ -74,6 +74,10 @@ abstract class PhabricatorApplicationTransactionComment
return PhabricatorContentSource::newFromSerialized($this->contentSource); return PhabricatorContentSource::newFromSerialized($this->contentSource);
} }
/**
* Whether the transaction removes the comment
* @return bool True if the transaction removes the comment
*/
public function getIsRemoved() { public function getIsRemoved() {
return ($this->getIsDeleted() == 2); return ($this->getIsDeleted() == 2);
} }