revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00
Code Issues Releases Wiki Activity

Prevent users from removing task titles with "Bulk Edit"

Browse source 
Summary:
See downstream <https://phabricator.wikimedia.org/T209449>.

The "Bulk Edit" flow works with `setContinueOnMissingFields(true)`, so `newRequiredError()` errors are ignored. This allows you to apply a transaction which changes the title to `""` (the empty string) without actually hitting any errors which the workflow respects.

(Normally, `setContinueOnMissingFields(...)` workflows only edit properties that can't be missing, like the status of an object, so this is an unusual flow.)

Instead, validate more narrowly:

  - Transactions which would remove the title get an "invalid" error, which is respected even under "setContinueOnMissingFields()".
  - Then, we try to raise a "missing/required" error if everything otherwise looks okay.

Test Plan:
  - Edited a task title normally.
  - Edited a task to remove the title (got an error).
  - Created a task with no title (disallowed: got an error).
  - Bulk edited a task to remove its title.
    - Before change: allowed.
    - After change: disallowed.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20339
This commit is contained in:
epriestley 2019-03-28 08:53:07 -07:00
parent 6bb9d3ac67
commit e69b349b1b
1 changed files with 21 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
src/applications/maniphest/xaction/ManiphestTaskTitleTransaction.php
View file

@ -64,9 +64,27 @@ final class ManiphestTaskTitleTransaction
public function validateTransactions($object, array $xactions) {
$errors = array();
if ($this->isEmptyTextTransaction($object->getTitle(), $xactions)) {
$errors[] = $this->newRequiredError(
pht('Tasks must have a title.'));
// If the user is acting via "Bulk Edit" or another workflow which
// continues on missing fields, they may be applying a transaction which
// removes the task title. Mark these transactions as invalid first,
// then flag the missing field error if we don't find any more specific
// problems.
foreach ($xactions as $xaction) {
$new = $xaction->getNewValue();
if (!strlen($new)) {
$errors[] = $this->newInvalidError(
pht('Tasks must have a title.'),
$xaction);
continue;
}
}
if (!$errors) {
if ($this->isEmptyTextTransaction($object->getTitle(), $xactions)) {
$errors[] = $this->newRequiredError(
pht('Tasks must have a title.'));
}
}
return $errors;