@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

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

+45 -1
+45 -1
src/applications/project/engine/PhabricatorProjectEditEngine.php
··· 198 198 $milestone_phid = null; 199 199 } 200 200 201 + // 202 + // Load the colors available for selection. 203 + // 204 + // Goals: 205 + // 1. Allow to pick the colors available from the option 206 + // 'projects.colors' ('getColorMap'). 207 + // 2. Allow to show what is the **current** color. 208 + // 2.1 If the current value is 'orange' but if omit 209 + // any fruit from 'projects.colors', 210 + // then show the truth: show 'Orange' (not 'Red'). 211 + // So, you can easily inspect this legacy color and replace it. 212 + // 2.2 If the current value is an internal color, like 'disabled', 213 + // which is an hardcoded color used for archived projects, 214 + // then show the truth: show 'Disabled' (not 'Red'). 215 + // 3.3. If the current color is anything else esoteric which is still 216 + // supported for rendering (available in 'getShadeMap'), 217 + // show it, and do not fallback on the first color. 218 + // In short, do not fallback on 'Red'. 219 + // 220 + // Elsewhere, if the current value is not a color, and cannot be 221 + // rendered (not in 'getShadeMap') then don't propose it. 222 + // So the UX forces you to select a "clean" one on the next edit. 223 + // 224 + // https://we.phorge.it/T16236 225 + $colors_for_select = PhabricatorProjectIconSet::getColorMap(); 226 + $color_current = $object->getColor(); 227 + if ($color_current) { 228 + $colors_supported = PHUITagView::getShadeMapCached(); 229 + if (isset($colors_supported[$color_current])) { 230 + $colors_for_select[$color_current] = $colors_supported[$color_current]; 231 + } 232 + } 233 + 201 234 $fields = array( 202 235 id(new PhabricatorHandlesEditField()) 203 236 ->setKey('parent') ··· 263 296 ->setLabel(pht('Color')) 264 297 ->setTransactionType( 265 298 PhabricatorProjectColorTransaction::TRANSACTIONTYPE) 266 - ->setOptions(PhabricatorProjectIconSet::getColorMap()) 299 + ->setOptions($colors_for_select) 267 300 ->setDescription(pht('Project tag color.')) 268 301 ->setConduitDescription(pht('Change the project tag color.')) 269 302 ->setConduitTypeDescription(pht('New project tag color.')) 303 + // When a project is archived, whatever color is set in the 304 + // storage does not really matter, since the 'getColor()' has 305 + // always been hardcoded to return a specific different color 306 + // (the 'disabled' color). 307 + // So, for archived projects, do not allow to change color. 308 + // https://we.phorge.it/T15236 309 + // 310 + // Incidentally, this means that recently archived projects will 311 + // keep their original color in their storage, so, when you 312 + // re-activate a project, its original color is successfully shown. 313 + ->setIsLocked($object->isArchived()) 270 314 ->setValue($object->getColor()), 271 315 id(new PhabricatorStringListEditField()) 272 316 ->setKey('slugs')