From 903d71e67d84e04337d6dc4d9febcd71eb6f9c53 Mon Sep 17 00:00:00 2001 From: Valerio Bozzolan Date: Mon, 25 Sep 2023 08:06:09 +0200 Subject: [PATCH] Workboard: Milestone Name easily editable (instead of surfing 3 pages) Summary: After this change, a new input field "Milestone Name" appears in the "Edit" menu of a Milestone: | Before | After | |-----------|-----------| | {F314008} | {F314005} | So you can quickly change the name of your Milestones, from a Workboard. Before this change, from a Workboard, this was the way to rename a Milestone: 1. click on the Milestone name (yes, that is a link) 2. click on Manage 3. click on Edit Details 4. rename 5. Save 6. Manually visit again the Project's Workboard After this change, from a Workboard, you just need to: 1. click on Milestone > Edit 2. click on Edit Column 3. rename 4. Save Example usage: {F314015} This does not change the level of permissions needed: if you have not enough permissions to see or edit a Milestone, you cannot access this feature indeed. In short, this is just a frontend change, keeping current policies as-is. Closes T15143 Test Plan: Create a Project or use an existing editable one. Create a Milestone called "Test Milestone". You can create Milestones visiting the Project's menu {nav icon=sitemap,name=Subprojects > icon=plus,name=Create next milestone} Visit the Project's Workboard. Find the column "Test Milestone". Click the Edit button on a Milestone, and: - try to save another name: it must work - try to save an empty name: nice error message shown - try to save both the score points and the name: it must work - try to save "FOO" as Points: you still see the error message Also: - do the same for the Backlog column: it still works (name still allowed to be empty) - do the same for a "normal" Column (not the Backlog): it still work (name still __not__ allowed to be empty) Reviewers: O1 Blessed Committers, Cigaryno, 20after4, waldyrious Reviewed By: O1 Blessed Committers, Cigaryno, 20after4, waldyrious Subscribers: waldyrious, brennen, aklapper, 20after4, speck, tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T15143 Differential Revision: https://we.phorge.it/D25066 --- ...PhabricatorProjectColumnEditController.php | 59 ++++++++++++++++++- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/src/applications/project/controller/PhabricatorProjectColumnEditController.php b/src/applications/project/controller/PhabricatorProjectColumnEditController.php index 9ddb2b7d8a..bf579b274f 100644 --- a/src/applications/project/controller/PhabricatorProjectColumnEditController.php +++ b/src/applications/project/controller/PhabricatorProjectColumnEditController.php @@ -45,16 +45,32 @@ final class PhabricatorProjectColumnEditController $e_name = null; $e_limit = null; + $e_milestone_name = null; $v_limit = $column->getPointLimit(); $v_name = $column->getName(); + $proxy = $column->getProxy(); + + // Is this a normal Column? Example: when true, this is not a Milestone. + $is_column = !$proxy; + + // Is this a Milestone? Example: when true, this is not a normal Column. + $is_milestone = $proxy && $proxy->isMilestone(); + + // Milestone name, eventually coming from the proxed object. + $v_milestone_name = null; + if ($is_milestone) { + $v_milestone_name = $proxy->getName(); + } + $validation_exception = null; $view_uri = $project->getWorkboardURI(); if ($request->isFormPost()) { $v_name = $request->getStr('name'); $v_limit = $request->getStr('limit'); + $v_milestone_name = $request->getStr('milestone.name'); if ($is_new) { $column->setProjectPHID($project->getPHID()); @@ -74,14 +90,22 @@ final class PhabricatorProjectColumnEditController } $xactions = array(); + $xactions_milestone = array(); $type_name = PhabricatorProjectColumnNameTransaction::TRANSACTIONTYPE; $type_limit = PhabricatorProjectColumnLimitTransaction::TRANSACTIONTYPE; + $type_project_name = PhabricatorProjectNameTransaction::TRANSACTIONTYPE; - if (!$column->getProxy()) { + if ($is_column) { + // Transaction for Column name. $xactions[] = id(new PhabricatorProjectColumnTransaction()) ->setTransactionType($type_name) ->setNewValue($v_name); + } else if ($is_milestone) { + // Transaction for Milestone name (that internally is a Project Name). + $xactions_milestone[] = id(new PhabricatorProjectTransaction()) + ->setTransactionType($type_project_name) + ->setNewValue($v_milestone_name); } $xactions[] = id(new PhabricatorProjectColumnTransaction()) @@ -94,24 +118,53 @@ final class PhabricatorProjectColumnEditController ->setContinueOnNoEffect(true) ->setContentSourceFromRequest($request) ->applyTransactions($column, $xactions); - return id(new AphrontRedirectResponse())->setURI($view_uri); } catch (PhabricatorApplicationTransactionValidationException $ex) { + // Error messages related to the Column (like invalid Name, etc.) $e_name = $ex->getShortMessage($type_name); $e_limit = $ex->getShortMessage($type_limit); $validation_exception = $ex; } + + // Save Milestone-related stuff but only if there were no prior problems + // and only if we have changes. + if (!$validation_exception && $xactions_milestone) { + try { + $editor_milestone = id(new PhabricatorProjectTransactionEditor()) + ->setActor($viewer) + ->setContinueOnNoEffect(true) + ->setContentSourceFromRequest($request) + ->applyTransactions($proxy, $xactions_milestone); + } catch (PhabricatorApplicationTransactionValidationException $ex) { + // Error messages related to the Milestone (like invalid Name, etc.) + $e_milestone_name = $ex->getShortMessage($type_project_name); + $validation_exception = $ex; + } + } + + // Refresh the page only if there are no errors to show. + if (!$validation_exception) { + return id(new AphrontRedirectResponse())->setURI($view_uri); + } } $form = id(new AphrontFormView()) ->setUser($request->getUser()); - if (!$column->getProxy()) { + // Show the most appropriate input field for the name. + if ($is_column) { $form->appendChild( id(new AphrontFormTextControl()) ->setValue($v_name) ->setLabel(pht('Name')) ->setName('name') ->setError($e_name)); + } else if ($is_milestone) { + $form->appendChild( + id(new AphrontFormTextControl()) + ->setValue($v_milestone_name) + ->setLabel(pht('Milestone Name')) + ->setName('milestone.name') + ->setError($e_milestone_name)); } $form->appendChild(