From ad057da019ecc57c73ceec3cc58eba4bfb606d5a Mon Sep 17 00:00:00 2001 From: Valerio Bozzolan Date: Fri, 5 Sep 2025 10:28:02 +0200 Subject: [PATCH] Projects: fix nonsense "Red" color after editing archived projects Summary: One edit on one archived project do not cause anymore this nonsense extra line: Foo set this project's color to Red Note that archived projects have always been designed to be shown with a special color called 'Disabled'; so, archived projects now have the Color field locked in the UX, to prevent color ping-pongs. Note that this is not a feature removal: changing colors of archived projects never worked. So now the UX is more consistent. Additionally, for archived projects, the Color field may have an entry called 'Disabled' and pre-selected (and, again, not-editable). This means the UX is finally showing the truth, and not showing another unrelated color (not showing 'Red' - not causing 'Red' to be set by mistake, etc.) Incidentally, this Color field stuck on 'Disabled' has also a good semantical meaning :P since archived projects have both a disabled color input box, and that project has really 'disabled' as color value! :P Consequent new good thing: If you recently changed the option 'projects.colors' to omit a color like 'orange', now you are still allowed to see that a project has the 'orange' value (not 'Red'). (T16236) Consequent new good thing: When a project is archived after this change, and then re-activated: the original color is always automagically restored, even if you have done many edits in the project while it was archived (so, it's not 'Red' anymore). Corner case for APIs: When the project is archived, from the API 'project.edit' you can still set another color like 'violet'. This is allowed. It just means that this new color will be shown again after you re-activate such project. This patch replaces D26290 to cover more cases. Screenshot of an active project. See the missing 'Disabled' special entry: {F6020953} Screenshot of an archived project. See the locked input stuck on 'Disabled': {F6020955} Closes T15236 Closes T16236 Test Plan: Create a project with 'Violet' color. Archive it. Edit it: Observe the color field cannot be touched while it's archived, and it shows the truth: it shows the 'Disabled' color value, instead of the 'Red' nonsense color (which was historically shown there only because it was the first color in the entry, and because 'Disabled' is not normally selectable). Save the project again. It does not become 'Red'. Incredible! Un-archive that project. The project returns as 'Violet'. Reviewers: O1 Blessed Committers, mainframe98 Reviewed By: O1 Blessed Committers, mainframe98 Subscribers: tobiaswiese, Matthew, Cigaryno Maniphest Tasks: T16236, T15236 Differential Revision: https://we.phorge.it/D26294 --- .../engine/PhabricatorProjectEditEngine.php | 46 ++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/applications/project/engine/PhabricatorProjectEditEngine.php b/src/applications/project/engine/PhabricatorProjectEditEngine.php index 51cd840d3d..942ef8f404 100644 --- a/src/applications/project/engine/PhabricatorProjectEditEngine.php +++ b/src/applications/project/engine/PhabricatorProjectEditEngine.php @@ -198,6 +198,39 @@ final class PhabricatorProjectEditEngine $milestone_phid = null; } + // + // Load the colors available for selection. + // + // Goals: + // 1. Allow to pick the colors available from the option + // 'projects.colors' ('getColorMap'). + // 2. Allow to show what is the **current** color. + // 2.1 If the current value is 'orange' but if omit + // any fruit from 'projects.colors', + // then show the truth: show 'Orange' (not 'Red'). + // So, you can easily inspect this legacy color and replace it. + // 2.2 If the current value is an internal color, like 'disabled', + // which is an hardcoded color used for archived projects, + // then show the truth: show 'Disabled' (not 'Red'). + // 3.3. If the current color is anything else esoteric which is still + // supported for rendering (available in 'getShadeMap'), + // show it, and do not fallback on the first color. + // In short, do not fallback on 'Red'. + // + // Elsewhere, if the current value is not a color, and cannot be + // rendered (not in 'getShadeMap') then don't propose it. + // So the UX forces you to select a "clean" one on the next edit. + // + // https://we.phorge.it/T16236 + $colors_for_select = PhabricatorProjectIconSet::getColorMap(); + $color_current = $object->getColor(); + if ($color_current) { + $colors_supported = PHUITagView::getShadeMapCached(); + if (isset($colors_supported[$color_current])) { + $colors_for_select[$color_current] = $colors_supported[$color_current]; + } + } + $fields = array( id(new PhabricatorHandlesEditField()) ->setKey('parent') @@ -263,10 +296,21 @@ final class PhabricatorProjectEditEngine ->setLabel(pht('Color')) ->setTransactionType( PhabricatorProjectColorTransaction::TRANSACTIONTYPE) - ->setOptions(PhabricatorProjectIconSet::getColorMap()) + ->setOptions($colors_for_select) ->setDescription(pht('Project tag color.')) ->setConduitDescription(pht('Change the project tag color.')) ->setConduitTypeDescription(pht('New project tag color.')) + // When a project is archived, whatever color is set in the + // storage does not really matter, since the 'getColor()' has + // always been hardcoded to return a specific different color + // (the 'disabled' color). + // So, for archived projects, do not allow to change color. + // https://we.phorge.it/T15236 + // + // Incidentally, this means that recently archived projects will + // keep their original color in their storage, so, when you + // re-activate a project, its original color is successfully shown. + ->setIsLocked($object->isArchived()) ->setValue($object->getColor()), id(new PhabricatorStringListEditField()) ->setKey('slugs') -- 2.51.2