fix: prevent invalid dropdown options to be added (#2493)

* fix: prevent invalid dropdown options to be added

fixes #2491

Author: sleidig

src/app/core/basic-datatypes/configurable-enum/configurable-enum.spec.ts

@@ -0,0 +1,63 @@
+import { ConfigurableEnumValue } from "./configurable-enum.interface";
+import { ConfigurableEnum } from "./configurable-enum";
+
+describe("ConfigurableEnum", () => {
+  let sampleValues: ConfigurableEnumValue[];
+  let testEnum: ConfigurableEnum;
+
+  beforeEach(async () => {
+    sampleValues = [
+      { id: "1", label: "one" },
+      { id: "2", label: "two" },
+    ];
+    testEnum = new ConfigurableEnum("test-enum");
+    testEnum.values = JSON.parse(JSON.stringify(sampleValues));
+  });
+
+  it("should create", () => {
+    expect(testEnum).toBeTruthy();
+  });
+
+  it("should add option from value object", () => {
+    const newOption: ConfigurableEnumValue = {
+      id: "3",
+      label: "three",
+      color: "red",
+    };
+    const returnedOption = testEnum.addOption(newOption);
+    expect(returnedOption).toEqual(newOption);
+    expect(testEnum.values).toContain(newOption);
+    expect(testEnum.values.length).toBe(sampleValues.length + 1);
+  });
+
+  it("should add option from string", () => {
+    const newOption: string = "three";
+    const returnedOption = testEnum.addOption(newOption);
+    expect(returnedOption.label).toEqual(newOption);
+    expect(testEnum.values).toContain(
+      jasmine.objectContaining({ id: "THREE", label: "three" }),
+    );
+    expect(testEnum.values.length).toBe(sampleValues.length + 1);
+  });
+
+  it("should not add option for empty values", () => {
+    testEnum.addOption("");
+    testEnum.addOption(undefined);
+    expect(testEnum.values).toEqual(sampleValues);
+  });
+
+  it("should not add option for duplicate label", () => {
+    expect(() => testEnum.addOption(sampleValues[0].label)).toThrowError();
+    expect(testEnum.values).toEqual(sampleValues);
+  });
+
+  it("should adapt generated id if duplicate", () => {
+    const newOption: string = "1"; // already exists as id in sampleValues (but with different label)
+    const returnedOption = testEnum.addOption(newOption);
+
+    expect(returnedOption.label).toEqual(newOption);
+    expect(returnedOption.id).toEqual("1_");
+    expect(testEnum.values).toContain(returnedOption);
+    expect(testEnum.values.length).toBe(sampleValues.length + 1);
+  });
+});

src/app/core/basic-datatypes/configurable-enum/configurable-enum.ts

@@ -2,12 +2,73 @@ import { Entity } from "../../entity/model/entity";
 import { DatabaseEntity } from "../../entity/database-entity.decorator";
 import { ConfigurableEnumValue } from "./configurable-enum.interface";
 import { DatabaseField } from "../../entity/database-field.decorator";
+import { Logging } from "../../logging/logging.service";
 
 @DatabaseEntity("ConfigurableEnum")
 export class ConfigurableEnum extends Entity {
   @DatabaseField() values: ConfigurableEnumValue[] = [];
+
   constructor(id?: string, values: ConfigurableEnumValue[] = []) {
     super(id);
     this.values = values;
   }
+
+  /**
+   * Add a new valid option to the enum values, if it is not a duplicate or invalid.
+   * Returns the newly added option upon success.
+   * @param newOptionInput String or option object to be added
+   */
+  addOption(
+    newOptionInput: ConfigurableEnumValue | string,
+  ): ConfigurableEnumValue | undefined {
+    const option: ConfigurableEnumValue =
+      typeof newOptionInput === "string"
+        ? this.convertStringToOption(newOptionInput)
+        : newOptionInput;
+
+    if (!option || !(option?.id && option?.label)) {
+      Logging.debug(
+        "Trying to add invalid enum option",
+        newOptionInput,
+        option,
+      );
+      return;
+    }
+
+    // check for duplicates
+    if (this.values.some((v) => v.label === option.label)) {
+      throw new DuplicateEnumOptionException(newOptionInput);
+    }
+    if (this.values.some((v) => v.id === option.id)) {
+      option.id = option.id + "_";
+    }
+
+    this.values.push(option);
+    return option;
+  }
+
+  private convertStringToOption(
+    newOption: string,
+  ): ConfigurableEnumValue | undefined {
+    newOption = newOption.trim();
+    if (newOption.length === 0) {
+      return;
+    }
+
+    return {
+      id: newOption.toUpperCase(),
+      label: newOption,
+    };
+  }
+}
+
+/**
+ * Error thrown when trying to add an option that already exists in the enum values.
+ */
+export class DuplicateEnumOptionException extends Error {
+  constructor(newOptionInput) {
+    super("Enum Option already exists");
+
+    this["newOptionInput"] = newOptionInput;
+  }
 }

src/app/core/basic-datatypes/configurable-enum/configure-enum-popup/configure-enum-popup.component.ts

@@ -23,6 +23,7 @@ import { MatButtonModule } from "@angular/material/button";
 import { ConfirmationDialogService } from "../../../common-components/confirmation-dialog/confirmation-dialog.service";
 import { EntityRegistry } from "../../../entity/database-entity.decorator";
 import { Entity } from "../../../entity/model/entity";
+import { OkButton } from "../../../common-components/confirmation-dialog/confirmation-dialog/confirmation-dialog.component";
 
 @Component({
   selector: "app-configure-enum-popup",
@@ -126,11 +127,18 @@ export class ConfigureEnumPopupComponent {
     );
   }
 
-  createNewOption() {
-    this.enumEntity.values.push({
-      id: this.newOptionInput,
-      label: this.newOptionInput,
-    });
+  async createNewOption() {
+    try {
+      this.enumEntity.addOption(this.newOptionInput);
+    } catch (error) {
+      await this.confirmationService.getConfirmation(
+        $localize`Failed to create new option`,
+        $localize`Couldn't create this new option. Please check if the value already exists.`,
+        OkButton,
+      );
+      return;
+    }
+
     this.newOptionInput = "";
   }
 }

src/app/core/basic-datatypes/configurable-enum/enum-dropdown/enum-dropdown.component.spec.ts

@@ -97,10 +97,10 @@ describe("EnumDropdownComponent", () => {
     const res = await component.createNewOption("second");
 
     expect(confirmationSpy).toHaveBeenCalled();
-    expect(res).toEqual({ id: "second", label: "second" });
+    expect(res).toEqual({ id: "SECOND", label: "second" });
     expect(enumEntity.values).toEqual([
       { id: "1", label: "first" },
-      { id: "second", label: "second" },
+      { id: "SECOND", label: "second" },
     ]);
     expect(saveSpy).toHaveBeenCalledWith(enumEntity);
   });

src/app/core/basic-datatypes/configurable-enum/enum-dropdown/enum-dropdown.component.ts

@@ -15,6 +15,7 @@ import { FontAwesomeModule } from "@fortawesome/angular-fontawesome";
 import { ErrorHintComponent } from "../../../common-components/error-hint/error-hint.component";
 import { MatButtonModule } from "@angular/material/button";
 import { ConfirmationDialogService } from "../../../common-components/confirmation-dialog/confirmation-dialog.service";
+import { OkButton } from "../../../common-components/confirmation-dialog/confirmation-dialog/confirmation-dialog.component";
 
 @Component({
   selector: "app-enum-dropdown",
@@ -80,18 +81,35 @@ export class EnumDropdownComponent implements OnChanges {
   }
 
   private async addNewOption(name: string) {
+    const prevValues = JSON.stringify(this.enumEntity.values);
+    let addedOption: ConfigurableEnumValue;
+
+    try {
+      addedOption = this.enumEntity.addOption(name);
+    } catch (error) {
+      await this.confirmation.getConfirmation(
+        $localize`Failed to create new option`,
+        $localize`Couldn't create this new option. Please check if the value already exists.`,
+        OkButton,
+      );
+      return undefined;
+    }
+
+    if (!addedOption) {
+      return undefined;
+    }
+
     const userConfirmed = await this.confirmation.getConfirmation(
       $localize`Create new option`,
-      $localize`Do you want to create the new option "${name}"?`,
+      $localize`Do you want to create the new option "${addedOption.label}"?`,
     );
     if (!userConfirmed) {
+      this.enumEntity.values = JSON.parse(prevValues);
       return undefined;
     }
 
-    const option = { id: name, label: name };
-    this.enumEntity.values.push(option);
     await this.entityMapper.save(this.enumEntity);
-    return option;
+    return addedOption;
   }
 
   openSettings(event: Event) {