Skip to content

fix(visual-editor): speak EformFieldTypesEnum on the wire, map DB ids by name - #8143

Merged
renemadsen merged 3 commits into
stablefrom
fix/8141-shifted-lookup-ids
Oct 16, 2026
Merged

renemadsen merged 3 commits into
stablefrom
fix/8141-shifted-lookup-ids

Conversation

@renemadsen

Copy link
Copy Markdown
Member

SDK lookup ids are only contiguous by accident of AUTO_INCREMENT. On a
fresh install with auto_increment_increment=3 (Galera) SingleSelect is
24, FieldGroup 51, Danish 3. The earlier partial fix (b701184 ..
881b113) remapped only the Type dropdown to DB ids, so field.fieldType
became 24 while every template/ngSwitch check still compared against
EformFieldTypesEnum (SingleSelect = 8): the options editor, number/date/
pdf/picture/entity panels and the mandatory flag broke, and Text (3)
was even treated as None.

EFormVisualEditorService now converts once at the API boundary:
FieldTypes.Type -> EformFieldTypesEnum on read and enum -> FieldTypes.Id
(by name) on every write path, incl. FieldGroup children. The client's
DB-id remap, getFieldTypes() and the /field-types endpoint are removed,
so all existing enum comparisons are correct again. Contiguous installs
are unaffected (the mapping is the identity there).

Also fixed for shifted language ids:

  • SitesService falls back to Danish by code instead of LanguageId 1;
  • the date-picker locale is chosen by locale code, not languageId;
  • ShowPdf/ShowPicture file pickers find files by languageId instead of
    files[languageId - 1] (which crashed).

Integration tests seed FieldTypes/Languages with Galera-style ids; Jest
specs cover the field modal, file pickers and auth-state locale.

Fixes #8141

Test plan

  • Backend unit tests 49/49, integration ShiftedLookupIds/ShiftedLanguageIds 7/7
  • Jest 11/11, tsc clean
  • Local browser check: visual editor → Nyt felt → Rullemenu shows Options textarea, Add options and the per-language table

Known, out of scope (follow-ups)

  • updateUserLocale stores applicationLanguages ids in the auth store (separate id space from SDK languages).
  • FindFields adds ShowPdf files to PngFiles.
  • setFieldTypes lists the admin-only field types twice.
  • Version skew: a browser that still runs the old cached client sends DB ids for fieldType; a hard reload after deploy fixes it.

No seeder change and no data migration in this PR: existing installs keep their ids, the code just stops assuming them.

🤖 Generated with Claude Code

… by name

SDK lookup ids are only contiguous by accident of AUTO_INCREMENT. On a
fresh install with auto_increment_increment=3 (Galera) SingleSelect is
24, FieldGroup 51, Danish 3. The earlier partial fix (b701184 ..
881b113) remapped only the Type dropdown to DB ids, so field.fieldType
became 24 while every template/ngSwitch check still compared against
EformFieldTypesEnum (SingleSelect = 8): the options editor, number/date/
pdf/picture/entity panels and the mandatory flag broke, and Text (3)
was even treated as None.

EFormVisualEditorService now converts once at the API boundary:
FieldTypes.Type -> EformFieldTypesEnum on read and enum -> FieldTypes.Id
(by name) on every write path, incl. FieldGroup children. The client's
DB-id remap, getFieldTypes() and the /field-types endpoint are removed,
so all existing enum comparisons are correct again. Contiguous installs
are unaffected (the mapping is the identity there).

Also fixed for shifted language ids:
- SitesService falls back to Danish by code instead of LanguageId 1;
- the date-picker locale is chosen by locale code, not languageId;
- ShowPdf/ShowPicture file pickers find files by languageId instead of
  files[languageId - 1] (which crashed).

Integration tests seed FieldTypes/Languages with Galera-style ids; Jest
specs cover the field modal, file pickers and auth-state locale.

Fixes #8141

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Short locale codes emitted by existing auth helpers incorrectly fall back to English in date-locale selection.

1 open finding
What changed in this PR

Addresses #8141 by separating visual-editor field-type values from database IDs, without renumbering existing data.

Changes:

  • Maps field types by name at the API boundary and removes client-side ID remapping.
  • Resolves site languages, date locales, and file selections without positional ID assumptions.
  • Adds shifted-ID regression tests.
File Description
eFormAPI/​eFormAPI.Web/​Services/​SitesService.cs Resolves the Danish fallback by language code.
eFormAPI/​eFormAPI.Web/​Services/​EFormColumnsService.cs Removes field-type lookup retrieval.
eFormAPI/​eFormAPI.Web/​Services/​Eform/​EFormVisualEditorService.cs Maps client enum values to database IDs and back.
eFormAPI/​eFormAPI.Web/​Infrastructure/​Models/​Templates/​FieldTypeModel.cs Deletes the unused lookup model.
eFormAPI/​eFormAPI.Web/​Controllers/​Eforms/​EFormColumnsController.cs Removes the field-types endpoint.
eFormAPI/​eFormAPI.Web/​Abstractions/​Eforms/​ITemplateColumnsService.cs Removes the lookup service contract.
eFormAPI/​eFormAPI.Web.Integration.Tests/​Services/​SitesServiceShiftedLanguageIdsTests.cs Tests shifted site-language IDs.
eFormAPI/​eFormAPI.Web.Integration.Tests/​Services/​ShiftedLookupIdsSdkDatabase.cs Seeds shifted lookup IDs for tests.
eFormAPI/​eFormAPI.Web.Integration.Tests/​Services/​Eform/​EFormVisualEditorServiceShiftedLookupIdsTests.cs Tests field-type mapping across reads and writes.
eform-client/​src/​app/​modules/​eforms/​eform-visual-editor/​const/​eform-visual-editor-element-types.ts Keeps dropdown values in enum space.
eform-client/​src/​app/​modules/​eforms/​eform-visual-editor/​components/​eform-visual-editor-page/​eform-visual-editor-container/​eform-visual-editor-container.component.ts Removes database field-type loading.
eform-client/​src/​app/​modules/​eforms/​eform-visual-editor/​components/​eform-visual-editor-elements/​field/​visual-editor-field-modal/​visual-editor-field-modal.component.ts Uses enum values for field-type filtering.
eform-client/​src/​app/​modules/​eforms/​eform-visual-editor/​components/​eform-visual-editor-elements/​field/​visual-editor-field-modal/​visual-editor-field-modal.component.spec.ts Tests options rendering and mandatory controls.
eform-client/​src/​app/​modules/​eforms/​eform-visual-editor/​components/​eform-visual-editor-elements/​field/​visual-editor-additional-fields/​visual-editor-additional-field-show-picture/​visual-editor-additional-field-show-picture.component.ts Finds picture files by language ID.
eform-client/​src/​app/​modules/​eforms/​eform-visual-editor/​components/​eform-visual-editor-elements/​field/​visual-editor-additional-fields/​visual-editor-additional-field-pdf/​visual-editor-additional-field-pdf.component.ts Finds PDF files by language ID.
eform-client/​src/​app/​modules/​eforms/​eform-visual-editor/​components/​eform-visual-editor-elements/​field/​visual-editor-additional-fields/​visual-editor-additional-field-files.spec.ts Tests file pickers with shifted language IDs.
eform-client/​src/​app/​common/​store/​service/​auth-state.service.ts Selects date locales by locale code.
eform-client/​src/​app/​common/​store/​service/​auth-state.service.spec.ts Tests date locales with shifted language IDs.
eform-client/​src/​app/​common/​services/​eform/​eform.service.ts Removes the field-types API client method.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread eform-client/src/app/common/store/service/auth-state.service.ts Outdated
Codacy blocks the PR on public const fields in the integration-test
helper. They are now static read-only properties; the dependent alias
in the visual editor test becomes static readonly.

Refs #8141

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 15:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Existing auth flows store short locale codes that the new date-locale switch does not recognize, causing an English fallback.

1 open finding

🧠 Review effort: Balanced

…e date locale

updateUserLocale and updateCurrentUserLocaleAndDarkTheme store the
region-less code ('de'), while user settings store the full one
('de-DE'). dateFnsLocale switched on the full code only, so a German
user set through those helpers got English dates. The locale is now
resolved to an application locale first: exact match, else the first
application locale with the same language prefix ('de', 'de-AT' ->
'de-DE').

Raised by Copilot on #8143.

Refs #8141

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 15, 2026 15:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The boundary mappings and updated consumers are consistent, regression coverage addresses shifted IDs, and no blocking issues remain.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@renemadsen
renemadsen merged commit a49ef73 into stable Oct 16, 2026
18 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants