feat: Infer and coerce code list value types in StudioCodeListEditor #14289

ErlingHauan · 2024-12-16T11:31:14Z

Description

Previously, when a code list's values were of type number, they would be changed to strings when edited in StudioCodeListEditor.

This PR adds functionality that checks the code list's values. If at least one value is number or boolean, it will save the other values in the same type.

In addition, test data for StudioCodeListEditor has been moved into a separate file.

Not in scope

Selecting type when creating a new code list.

Related Issue(s)

Make it possible to choose type for option lists #13977

Verification

Your code builds clean without any errors or warnings
Manual testing done (required)
Relevant automated test added (if you find this hard, leave it and we'll help out)

>> >> Two issues remain: >> 1. We get a backend error when changing one of the values in industri.json, even though the request looks fine >> 2. convertValue function needs more work if the value is a boolean

…n-codelists

…h a type

…eat/preserve-value-type-in-codelists

…n-codelists-pr-split

ErlingHauan · 2024-12-16T11:54:29Z

backend/src/Designer/Helpers/JsonConverterHelpers/OptionConverterHelper.cs

@@ -33,7 +33,7 @@ public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOp
                writer.WriteBooleanValue(b);
                break;
            default:
-                throw new InvalidOptionsFormatException($"{value} is an unsupported type for Value field. Accepted types are string, double and bool.");
+                throw new InvalidOptionsFormatException($"{value} with type {value.GetType()} is unsupported for Value field. Accepted types are string, double and bool.");


I discovered a backend bug when enabling editing of numbers - see issue #14290.
The added information in this exception makes it easier to see what is wrong.

codecov · 2024-12-16T12:03:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.57%. Comparing base (bf7e3c3) to head (d3ed8e3).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #14289   +/-   ##
=======================================
  Coverage   95.56%   95.57%           
=======================================
  Files        1837     1838    +1     
  Lines       23856    23891   +35     
  Branches     2754     2757    +3     
=======================================
+ Hits        22798    22833   +35     
  Misses        801      801           
  Partials      257      257

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

TomasEng

Dette ser absolutt gjennomtenkt ut, men jeg ville nok heller utledet typen ved å sjekke mot selve codeList-propen. Da unngår vi feil som for eksempel oppstår når man bare har ett element, som jeg nevnte i en kommentar her.

TomasEng · 2024-12-19T12:17:09Z

...dio-components/src/components/StudioCodelistEditor/test-data/codeListWithoutTextResources.ts

Jeg er usikker på om vi burde slette denne. I nyeste main er det også en kodeliste som heter codeListWithTextResources. Kanksje vi kan kalle de nye testlistene for stringCodeListWithoutTextResources osv.?

Godt forslag! Tar det inn.

TomasEng · 2024-12-19T12:21:27Z

frontend/libs/studio-components/src/components/StudioCodelistEditor/utils/valueTypeUtils.ts

+function coerceValues(codeList: CodeList, type: CodeListValueType): void {
+  codeList.forEach((codeListItem) => {
+    switch (type) {
+      case 'number':
+        codeListItem.value = tryCoerceNumber(codeListItem.value);
+        break;
+      case 'boolean':
+        codeListItem.value = coerceBoolean(codeListItem.value);
+        break;
+      case 'string':
+      case 'undefined':
+      default:
+        codeListItem.value = String(codeListItem.value);
+    }
+  });
+}


Funksjoner som brukes av React-komponenter bør være rene. Det vil si at de ikke manipulerer inndataobjektet, men returnerer et nytt objekt med modifiserte verdier. Dette gjør koden mer oversiktlig og feilsøking enklere. Når funksjonen returnerer void er det vanskeligere å se hva den gjør. Du kan lese mer om dette her: https://react.dev/learn/keeping-components-pure

Takk for innspill!
Jeg var usikker på om jeg skulle returnere void eller modifisere en kopi, men her er det tydeligvis en klar vinner.

TomasEng · 2024-12-19T12:27:36Z

...end/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditor.test.tsx

+  it('Should save all values as numbers, when a code list that contains numbers is modified', async () => {
+    const user = userEvent.setup();
+    renderCodeListEditor({ codeList: codeListWithNumbers });
+
+    const valueInput = screen.getByRole('textbox', { name: texts.itemValue(1) });
+    const newValue = '3.1416';
+    await user.type(valueInput, newValue);
+
+    expect(onChange).toHaveBeenCalledTimes(newValue.length);
+    expect(onChange).toHaveBeenLastCalledWith([
+      { ...codeListWithNumbers[0], value: Number(newValue) },
+      codeListWithNumbers[1],
+      codeListWithNumbers[2],
+    ]);
+  });


Jeg prøvde å modifisere denne testen til å teste en kodeliste med kun ett element. Da feiler den fordi verdien blir konvertert til en tekststreng.

Hvis jeg endrer verdien til '3,1416' (med komma), feiler testen også. Vi bør nok ta høyde for komma siden det er slik vi skriver desimaltall på norsk.

TomasEng · 2024-12-19T12:31:26Z

...end/libs/studio-components/src/components/StudioCodelistEditor/StudioCodeListEditor.test.tsx

+
+    expect(onChange).toHaveBeenCalledTimes(newValue.length);
+    expect(onChange).toHaveBeenLastCalledWith([
+      { ...codeListWithNumbers[0], value: Number(newValue) },


Når vi tester konvertering av typer, er det en fordel å skrive verdien rett ut. Her brukes Number() både i testen og i funksjonen som testes, så hvis noe er feil i forbindelse med nettopp Number-funksjonen, vil ikke testen oppfatte feilen.

Suggested change

{ ...codeListWithNumbers[0], value: Number(newValue) },

{ ...codeListWithNumbers[0], value: 3.1416 },

TomasEng · 2024-12-19T12:39:04Z

...tend/libs/studio-components/src/components/StudioCodelistEditor/utils/valueTypeUtils.test.ts

Samle gjerne alt i en felles describe-blokk for hele filen.

ErlingHauan added 18 commits November 21, 2024 09:51

Preserve original value type

9f04533

>> >> Two issues remain: >> 1. We get a backend error when changing one of the values in industri.json, even though the request looks fine >> 2. convertValue function needs more work if the value is a boolean

Merge remote-tracking branch 'origin' into feat/preserve-value-type-i…

1d0c887

…n-codelists

Merge remote-tracking branch 'origin' into feat/preserve-value-type-i…

2e72570

…n-codelists

Improve boolean check in coerceValue

d14c29c

Merge remote-tracking branch 'origin' into feat/preserve-value-type-i…

1b7d0ac

…n-codelists

merge main to branch

b1b4f89

add type tag to StudioCodeListEditor

1ed6f79

Add type tag to UI and set type of value if all values in a list matc…

e4c1aa1

…h a type

Merge branch 'main' of https://github.com/Altinn/altinn-studio into f…

9500733

…eat/preserve-value-type-in-codelists

Add text resources

0e83ff7

Add tooltip text resource

2149eda

Add tests

3e706cb

Merge main to branch

9e853e2

Add tests

c195e94

Create utility directory and move files

80f0dd0

Update tests

24c28c0

Merge remote-tracking branch 'origin' into feat/preserve-value-type-i…

3b1fff8

…n-codelists-pr-split

Clean up

2cda2cf

github-actions bot added solution/studio/designer Issues related to the Altinn Studio Designer solution. backend frontend labels Dec 16, 2024

ErlingHauan added team/studio-domain1 and removed backend labels Dec 16, 2024

ErlingHauan mentioned this pull request Dec 16, 2024

Type issue in backend when editing a code list with numbers #14290

Open

Fix indirect test failing

732c357

github-actions bot added area/ui-editor Area: Related to the designer tool for assembling app UI in Altinn Studio. backend labels Dec 16, 2024

ErlingHauan commented Dec 16, 2024

View reviewed changes

ErlingHauan marked this pull request as ready for review December 16, 2024 12:56

TomasEng assigned TomasEng and ErlingHauan and unassigned TomasEng Dec 17, 2024

ErlingHauan added 4 commits December 18, 2024 14:48

Change logic to check actual type of value

e123ca5

Update tests

bb305de

Fix merge conflicts

632f094

Make codeListWithDuplicatedValues more concise

d3ed8e3

ErlingHauan removed their assignment Dec 19, 2024

TomasEng self-assigned this Dec 19, 2024

TomasEng requested changes Dec 19, 2024

View reviewed changes

ErlingHauan assigned ErlingHauan and unassigned TomasEng Dec 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Infer and coerce code list value types in StudioCodeListEditor #14289

feat: Infer and coerce code list value types in StudioCodeListEditor #14289

ErlingHauan commented Dec 16, 2024 •

edited

Loading

ErlingHauan Dec 16, 2024

codecov bot commented Dec 16, 2024 •

edited

Loading

TomasEng left a comment

TomasEng Dec 19, 2024

ErlingHauan Dec 20, 2024

TomasEng Dec 19, 2024

ErlingHauan Dec 20, 2024

TomasEng Dec 19, 2024

TomasEng Dec 19, 2024

TomasEng Dec 19, 2024

	{ ...codeListWithNumbers[0], value: Number(newValue) },
	{ ...codeListWithNumbers[0], value: 3.1416 },

feat: Infer and coerce code list value types in StudioCodeListEditor #14289

Are you sure you want to change the base?

feat: Infer and coerce code list value types in StudioCodeListEditor #14289

Conversation

ErlingHauan commented Dec 16, 2024 • edited Loading

Description

Not in scope

Related Issue(s)

Verification

Choose a reason for hiding this comment

codecov bot commented Dec 16, 2024 • edited Loading

Codecov Report

TomasEng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErlingHauan commented Dec 16, 2024 •

edited

Loading

codecov bot commented Dec 16, 2024 •

edited

Loading