Repository navigation
fix: resolve field types and languages by name, not by numeric id - #2142
Merged
Merged
Conversation
SDK lookup ids are only contiguous by accident of AUTO_INCREMENT. On a fresh install with auto_increment_increment=3 (Galera) Picture is 15 and Danish is 3, so: - the report filtered pictures on FieldTypeId == 5 and showed/exported no images and ImagesCount 0; - the Excel planning import mapped the '|'-separated name segments to Languages Id 1..3 and threw "Sequence contains no elements". Pictures are now selected by FieldType.Type and the import pairs the name segments with da / en-US / de-DE by LanguageCode. Segments beyond the third are ignored (previously they went to whatever language had id 4+). Adds Testcontainers tests that run the real SDK seeding with --auto-increment-increment=3. Refs microting/eform-angular-frontend#8141 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The focused lookup changes have regression coverage and no identified blocking issues; tests were not executed during this review.
0 open findings
What changed in this PR
Removes assumptions about numeric SDK lookup IDs so reporting and planning imports work with noncontiguous IDs.
Changes:
- Selects pictures by field type name.
- Maps planning-name segments to language codes, ignoring segments beyond the third.
- Adds container-based regression tests with shifted lookup IDs.
| File | Description |
|---|---|
| eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn/Services/PlanningImportService/PlanningImportService.cs | Resolves translation language IDs by code. |
| eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn/Services/ItemsPlanningReportService/ItemsPlanningReportService.cs | Filters pictures by type name for images and counts. |
| eFormAPI/Plugins/ItemsPlanning.Pn/ItemsPlanning.Pn.Test/ShiftedLookupIdsTests.cs | Tests SDK seeding, reports, imports, and reimports with shifted IDs. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ests Codacy blocks the PR on the duplicated arrange block; it moves into ImportNewPlanning(). Both tests assert the same behaviour as before. Refs microting/eform-angular-frontend#8141 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SDK lookup ids are only contiguous by accident of AUTO_INCREMENT. On a
fresh install with auto_increment_increment=3 (Galera) Picture is 15 and
Danish is 3, so:
no images and ImagesCount 0;
Languages Id 1..3 and threw "Sequence contains no elements".
Pictures are now selected by FieldType.Type and the import pairs the
name segments with da / en-US / de-DE by LanguageCode. Segments beyond
the third are ignored (previously they went to whatever language had
id 4+).
Adds Testcontainers tests that run the real SDK seeding with
--auto-increment-increment=3.
Refs microting/eform-angular-frontend#8141
Test plan
Known, out of scope (follow-ups)
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