HomePhabricator

Prevent editing and deleting comments in locked conversations

Description

Prevent editing and deleting comments in locked conversations

Summary:
Ref T13289. This tightens up a couple of corner cases around locked threads.

Locking is primarily motivated by two use cases: stopping nonproductive conversations on open source installs (similar to GitHub's feature); and freezing object state for audit/record-keeping purposes.

Currently, you can edit or remove comments on a locked thread, but neither use case is well-served by allowing this. Require "CAN_INTERACT" to edit or remove a comment.

Administrators can still remove comments from a locked thread to serve "lock a flamewar, then clean it up", since "Remove Comment" on a comment you don't own is fairly unambiguously an administrative action.

Test Plan:

  • On a locked task, tried to edit and remove my comments as a non-administrator. Saw appropriate disabled UI state and error dialogs (actions were disallowed).
  • On a locked task, tried to remove another user's comments as an administrator. This works.
  • On a normal task, edited comments normally.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13289

Differential Revision: https://secure.phabricator.com/D20551

Details

Provenance
epriestleyAuthored on May 23 2019, 5:55 PM
epriestleyPushed on May 24 2019, 2:04 AM
Reviewer
amckinley
Differential Revision
D20551: Prevent editing and deleting comments in locked conversations
Parents
rPf838ad182753: Fix two straggling pagination issues in Drydock
Branches
Unknown
Tags
Unknown
Tasks
T13289: Plans: 2019 Week 21-23 Bonus Content
Build Status
Buildable 22908
Build 31427: Run Core Tests

Event Timeline

Apparently commits don't support CAN_INTERACT capability?

What are you seeing in particular?

Commits don't have an explicit CAN_INTERACT, but objects with no explicit CAN_INTERACT should generally fall back to CAN_VIEW behavior -- my expectation/intention here is that commits aren't affected.

(Test: editing this comment.)

@epriestley: Try removing a comment, it throws an exception

/src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php
39

exception:

Testing for capability "interact" on an object ("PhabricatorRepositoryCommit") which does not support that capability.

Thanks! D20648 should fix that -- it slipped through in D20558.