From cd19ddf111dc95a927488e9ae249b4911c88b08b Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 19 Jun 2017 12:37:00 -0700 Subject: [PATCH] Improve validation errors for changing task priorities Summary: Ref T12124. Currently, Conduit provides a fairly rough error message if you provide an invalid priority. Instead, provide a more tailored message. Also, block `!!unknown!!` except from web edits. Test Plan: Before: {F5007964} After: {F5007965} Also, changed a priority to `999` in the database, edited it with the normal web UI form, it let me make the edit without being forced to adjust the priority. Reviewers: amckinley, chad Reviewed By: amckinley Maniphest Tasks: T12124 Differential Revision: https://secure.phabricator.com/D18135 --- .../ManiphestTaskPriorityTransaction.php | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/applications/maniphest/xaction/ManiphestTaskPriorityTransaction.php b/src/applications/maniphest/xaction/ManiphestTaskPriorityTransaction.php index 4e505668d1..2ed7c4e15b 100644 --- a/src/applications/maniphest/xaction/ManiphestTaskPriorityTransaction.php +++ b/src/applications/maniphest/xaction/ManiphestTaskPriorityTransaction.php @@ -129,4 +129,50 @@ final class ManiphestTaskPriorityTransaction } } + public function validateTransactions($object, array $xactions) { + $errors = array(); + + $content_source = $this->getEditor()->getContentSource(); + $is_web = ($content_source instanceof PhabricatorWebContentSource); + + foreach ($xactions as $xaction) { + $value = $xaction->getNewValue(); + + // If this is a legitimate keyword like "low" or "high", this transaction + // is fine and apply normally. + $keyword = ManiphestTaskPriority::getTaskPriorityFromKeyword($value); + if ($keyword !== null) { + continue; + } + + // If this is the magic "don't change things" value for editing tasks + // with an obsolete priority constant in the database, let it through if + // this is a web edit. + if ($value === ManiphestTaskPriority::UNKNOWN_PRIORITY_KEYWORD) { + if ($is_web) { + continue; + } + } + + $keyword_list = array(); + foreach (ManiphestTaskPriority::getTaskPriorityMap() as $pri => $name) { + $keyword = ManiphestTaskPriority::getKeywordForTaskPriority($pri); + if ($keyword === null) { + continue; + } + $keyword_list[] = $keyword; + } + + $errors[] = $this->newInvalidError( + pht( + 'Task priority "%s" is not a valid task priority. Use a priority '. + 'keyword to choose a task priority: %s.', + $value, + implode(', ', $keyword_list)), + $xaction); + } + + return $errors; + } + }