Merge pull request #20980 from opf/bug/68726-selected-project-phase-doesn-t-show-on-wp-form-if-it-contains-letters-from-latin-extended-set-or-cyrillic-alphabet

[#68726] Selected project phase doesnt show on wp form if it contains invalid letters

Author: ulferts

app/helpers/colors_helper.rb

@@ -68,20 +68,12 @@ def resources_scope_color_css(name, scope, inline_foreground: false)
   end
 
   # Render the highlighting color for the project phase definition.
-  # On top of the normal classes as done for other resources, we also add a class based on the
-  # base64 encoded name of the project phase definition.
-  #
-  # The class name is based on the project phase definition's name, which is guaranteed to be unique.
-  #
-  # That way the frontend does not have to load the definitions to get the color.
-  # The = signs at the end of the base64 string are replaced with _ to make it a valid class name.
-  # This needs to be kept in sync with the ProjectPhaseDisplayField#phaseIcon method in the front end.
   def project_phase_color_css
     Project::PhaseDefinition.includes(:color).find_each do |definition|
       resource_color_css("project_phase_definition", definition, inline_foreground: true)
 
       set_foreground_colors_for(
-        class_name: ".#{hl_inline_class('project_phase_definition', Base64.strict_encode64(definition.name).tr('=', '_'))}",
+        class_name: ".#{hl_inline_class('project_phase_definition', definition.id)}",
         color: definition.color
       )
     end

frontend/src/app/features/work-packages/components/wp-fast-table/builders/modes/grouped/group-header-builder.ts

@@ -35,6 +35,7 @@ import { InjectField } from 'core-app/shared/helpers/angular/inject-field.decora
 import { GroupObject } from 'core-app/features/hal/resources/wp-collection-resource';
 import { groupName } from './grouped-rows-helpers';
 import { ProjectPhaseDisplayField } from 'core-app/shared/components/fields/display/field-types/project-phase-display-field.module';
+import idFromLink from 'core-app/features/hal/helpers/id-from-link';
 
 export function groupClassNameFor(group:GroupObject) {
   return `group-${group.identifier}`;
@@ -98,13 +99,14 @@ export class GroupHeaderBuilder {
    * @private
    */
   private leadingIcon(group:GroupObject) {
-    const isProjectPhase = group.href.some((item) =>
-      // will also match project phase definitions
-      item.href !== null && item.href.includes('api/v3/project_phase'));
+    // will also match project phase definitions
+    const groupLink = group.href.find(
+      (item) =>item.href?.includes('api/v3/project_phase')
+    );
 
-    if (isProjectPhase) {
-      // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-      return ProjectPhaseDisplayField.phaseIconByName(groupName(group), false);
+    if (groupLink) {
+      const definitionId = idFromLink(groupLink.href);
+      return ProjectPhaseDisplayField.phaseIconById(definitionId, false);
     }
 
     return null;

frontend/src/app/shared/components/fields/display/field-types/project-phase-display-field.module.ts

@@ -30,6 +30,7 @@
 
 import { opPhaseIconData, toDOMString } from '@openproject/octicons-angular';
 import { DisplayField } from 'core-app/shared/components/fields/display/display-field.module';
+import { HalResource } from 'core-app/features/hal/resources/hal-resource';
 import { ProjectPhaseResource } from 'core-app/features/hal/resources/project-phase-resource';
 
 export class ProjectPhaseDisplayField extends DisplayField {
@@ -51,42 +52,40 @@ export class ProjectPhaseDisplayField extends DisplayField {
    * The icon is wrapped in a span element with the correct css class set for coloring
    * the icon in the color defined for the definition.
    *
+   * @param phaseDefinitionId The ID of the phase definition (used for CSS class)
+   * @param addPadding Whether to add right margin padding
    * @return {HTMLElement} The HTML span element containing the project phase icon.
    * @see phaseIcon
    */
-  public static phaseIconByName(phaseName?:string, addPadding = true) {
+  public static phaseIconById(phaseDefinitionId?:string, addPadding = true) {
     const icon = document.createElement('span');
 
-    if (phaseName) {
+    if (phaseDefinitionId) {
       if (addPadding) {
         icon.classList.add('mr-1');
       }
 
-      icon.setAttribute('data-test-selector', `project-phase-icon ${phaseName}`);
+      icon.setAttribute('data-test-selector', `project-phase-icon phase-definition-${phaseDefinitionId}`);
 
       icon.innerHTML = toDOMString(
         opPhaseIconData,
         'small',
         { 'aria-hidden': 'true', class: 'octicon' },
       );
 
-      // Use a base64 encoded string of the project phase name to access the definition's color.
-      // That way the frontend does not have to load the definitions to get the color.
-      // The name is guaranteed to be unique.
-      // The = signs at the end of the base64 string are replaced with _ to make it a valid class name.
-      // This needs to be kept in sync with the ColorsHelper#project_phase_color_css method in the backend.
-      icon.classList.add(`__hl_inline_project_phase_definition_${btoa(phaseName).replace(/=/g, '_')}`);
+      // Use the phase definition ID for the CSS class.
+      // This is more robust than using the name as it avoids issues with special characters.
+      icon.classList.add(`__hl_inline_project_phase_definition_${phaseDefinitionId}`);
     }
 
     return icon;
   }
 
   /**
-   * @see phaseIconByName
+   * @see phaseIconById
    */
   protected phaseIcon():HTMLElement {
-    const projectPhase = this.attribute as ProjectPhaseResource;
-
-    return ProjectPhaseDisplayField.phaseIconByName(projectPhase?.name);
+    const definition = this.resource.projectPhaseDefinition as HalResource;
+    return ProjectPhaseDisplayField.phaseIconById(definition?.id ?? undefined);
   }
 }

spec/features/work_packages/table/project_phase_field_spec.rb

@@ -227,11 +227,11 @@
 
       it "includes the group icon in the group row header" do
         within("#wp-table-rowgroup-1") do
-          expect(page).to have_test_selector("project-phase-icon #{other_project_phase.name}")
+          expect(page).to have_test_selector("project-phase-icon phase-definition-#{other_project_phase.definition_id}")
         end
 
         within("#wp-table-rowgroup-2") do
-          expect(page).to have_test_selector("project-phase-icon #{project_phase.name}")
+          expect(page).to have_test_selector("project-phase-icon phase-definition-#{project_phase.definition_id}")
         end
       end
     end