feat: multi-value import #3552
feat: multi-value import #3552
Conversation
|
Warning Rate limit exceeded@sleidig has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 7 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds configurable multi-value import support: new Additional Settings UI for multiValueSeparator, splitting/deduplication helpers and array-aware import mapping in ImportService, propagation of additionalSettings through import flow and mapping dialogs, discrete-import UI updated to use flattened split values, and datatype import mapping simplified to single-value returns. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ImportUI as ImportComponent
participant ColumnCfg as DiscreteImportConfigComponent
participant Service as ImportService
participant Registry as EntityRegistry
participant Datatype as DefaultDatatype
User->>ImportUI: set multiValueSeparator (Additional Settings)
ImportUI->>ImportUI: store additionalSettings
User->>ImportUI: open column mapping
ImportUI->>ColumnCfg: pass additionalSettings + values
ColumnCfg->>ColumnCfg: splitAndFlattenValues(values, separator)
ColumnCfg-->>User: render flattened mapping table
User->>ImportUI: start import
ImportUI->>Service: transformRawDataToEntities(rawData, importSettings)
Service->>Registry: get schema for field
Registry-->>Service: schema (field.isArray?)
alt field.isArray
Service->>Service: splitArrayValue(cellValue, separator / JSON)
loop each split item
Service->>Datatype: importMapFunction(item)
Datatype-->>Service: mapped object
end
Service->>Service: deduplicate mapped objects
else
Service->>Datatype: importMapFunction(cellValue)
Datatype-->>Service: mapped object
end
Service-->>ImportUI: transformed entities
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Deployed to https://pr-3552.aam-digital.net/ |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
There was a problem hiding this comment.
Pull request overview
This PR adds support for importing comma-separated (or custom-separated) values for multi-select dropdown fields and other array-type fields. Users can now configure the separator character used to split multi-value columns during the import process.
Key changes:
- New
ImportAdditionalSettingsComponentallows users to configure the multi-value separator (default: comma, also supports semicolon or custom separators) - Modified
ImportServiceto handle array field parsing by splitting values based on the configured separator, with support for JSON arrays - Updated discrete import configuration to split and flatten values for proper mapping UI
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
import.service.ts |
Core logic for splitting and parsing array values with configurable separator, including new splitArrayValue function |
import.service.spec.ts |
Comprehensive test coverage for array field parsing with different separators and edge cases |
import-additional-settings.component.ts |
New component for configuring import settings like multi-value separator |
import-additional-settings.component.spec.ts |
Unit tests for the new settings component |
import-additional-settings.component.html |
UI for separator configuration with autocomplete and help text |
import-additional-settings.component.scss |
Styling for settings panel |
import.component.ts & .html |
Integration of additional settings component into import workflow |
import-review-data.component.ts |
Pass additional settings through to import processing |
import-metadata.ts |
Extended to include additionalSettings in import configuration |
import-column-mapping/*.ts |
Updated components to pass additional settings through mapping pipeline |
mapping-dialog-data.ts |
Extended interface to include additional settings |
discrete-import-config.component.ts |
Modified to split multi-value data for proper value mapping |
discrete-import-config.component.html & .scss |
UI improvements for better layout of mapping table |
default.datatype.ts |
Simplified importMapFunction return type - array handling now in ImportService |
configurable-enum.datatype.spec.ts |
Removed obsolete test for array handling that's now done elsewhere |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...p/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scss
Show resolved
Hide resolved
...app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/import/import-additional-settings/import-additional-settings.component.html
Outdated
Show resolved
Hide resolved
...app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In
@src/app/core/import/import-additional-settings/import-additional-settings.component.ts:
- Around line 1-51: Add a new spec file
import-additional-settings.component.spec.ts that creates an
ImportAdditionalSettingsComponent test host (or uses standalone TestBed) and
covers: component instantiation, default multiValueSeparator returns "," when
settings is empty, updating settings via the multiValueSeparator setter updates
the settings model (verify settings() or the underlying model update behavior),
separatorOptions contains the expected [",",";"], and createCustomSeparator
returns the passed string; reference the ImportAdditionalSettingsComponent
class, its multiValueSeparator getter/setter, settings model, separatorOptions
array, and createCustomSeparator method in the tests, and follow existing
/*.{component,service}.spec.ts patterns for setup and teardown.
In @src/app/core/import/import.service.ts:
- Around line 235-254: Change the parameter type from any to unknown on
splitArrayValue and add explicit type guards: if val is null or undefined return
an empty array; if val is an array (Array.isArray(val)) coerce/validate its
items to strings and return string[]; if val is a string keep the existing
JSON-parse-first logic but ensure parsed result is an array of strings
(map/validate elements) and on fallback split by separator and trim; for any
other non-string primitive coerce via String(val) into a single-element string
array. Ensure all branches return string[] and avoid wrapping null/undefined.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.ts (1)
35-35: Remove explicitstandalone: truedeclaration.Per coding guidelines, standalone components are the default behavior and
standalone: trueshould not be explicitly set.🔎 Proposed fix
selector: "app-edit-import-column-mapping", templateUrl: "./edit-import-column-mapping.component.html", styleUrls: ["./edit-import-column-mapping.component.scss"], - standalone: true, imports: [Based on coding guidelines for **/*.component.ts.
🧹 Nitpick comments (6)
src/app/core/import/import-additional-settings/import-additional-settings.component.scss (1)
1-3: Consider using global padding classes.The custom
.settings-contentclass applies simple vertical padding. Per coding guidelines, global style classes fromsrc/styles/globals/(e.g.,padding-vertical-small) could be used instead if available, reducing custom CSS.Based on coding guidelines.
src/app/core/import/import-additional-settings/import-additional-settings.component.html (1)
6-6: Consider replacing inline style with a CSS class.The inline
style="float: right"could be replaced with a global style class or a component-specific class for better maintainability.🔎 Suggested alternatives
Option 1: Use flexbox on the parent (line 3-12):
- <mat-panel-title> + <mat-panel-title class="flex-row"> <span i18n>Additional Settings [optional]</span> <app-help-button - style="float: right" + style="margin-left: auto"Option 2: Add a CSS class in the component's SCSS file:
<app-help-button - style="float: right" + class="float-right"Based on coding guidelines, prefer global style classes over inline styles.
src/app/core/import/import-column-mapping/import-column-mapping.component.ts (1)
20-20: Consider migrating toinput()function.The new
additionalSettingsinput follows the existing pattern but uses the@Input()decorator. As per coding guidelines, prefer theinput()function over@Input()decorators.🔎 Proposed refactor to use input() function
- @Input() additionalSettings: ImportAdditionalSettings; + additionalSettings = input<ImportAdditionalSettings>();Note: This would also apply to the other
@Input()decorators in this component (lines 43-44) and the@Output()decorator (line 46) for full consistency with the guidelines.Based on coding guidelines for **/*.component.ts.
Also applies to: 45-45
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.ts (1)
80-83: Consider migrating toinput()function.The new
additionalSettingsinput uses the@Input()decorator. As per coding guidelines, prefer theinput()function over@Input()decorators.🔎 Proposed refactor to use input() function
- /** - * Additional settings for import processing - */ - @Input() additionalSettings: ImportAdditionalSettings; + /** + * Additional settings for import processing + */ + additionalSettings = input<ImportAdditionalSettings>();Note: Accessing the value would require calling it as a function:
this.additionalSettings()instead ofthis.additionalSettings(see line 133).Based on coding guidelines for **/*.component.ts.
src/app/core/import/import-review-data/import-review-data.component.ts (1)
70-70: Consider migrating toinput()function.The new
additionalSettingsinput uses the@Input()decorator. As per coding guidelines, prefer theinput()function over@Input()decorators.🔎 Proposed refactor to use input() function
- @Input() additionalSettings: ImportAdditionalSettings; + additionalSettings = input<ImportAdditionalSettings>();Note: Accessing the value would require calling it as a function:
this.additionalSettings()instead ofthis.additionalSettings(see line 101).Based on coding guidelines for **/*.component.ts.
src/app/core/import/import-additional-settings/import-additional-settings.component.ts (1)
40-46: Consider simplifying the getter/setter pattern with signals.The current getter/setter approach works but could be more idiomatic with signals. Since
settingsis already a signal model, you could either:
- Use
computed()for a derived read-only signal, or- Access
settings().multiValueSeparatordirectly in the templateOption 1: Using computed() for a read-only derived value
- get multiValueSeparator(): string { - return this.settings()?.multiValueSeparator ?? ","; - } - - set multiValueSeparator(value: string) { - this.settings.update((s) => ({ ...s, multiValueSeparator: value })); - } + multiValueSeparator = computed(() => this.settings()?.multiValueSeparator ?? ",");Note: This would require adjusting the template if it uses two-way binding.
Option 2: Direct access in template
Access the value directly in the template as
settings().multiValueSeparator ?? ","and update via a method or the existing setter.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/app/core/basic-datatypes/configurable-enum/configurable-enum-datatype/configurable-enum.datatype.spec.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.htmlsrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scsssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/entity/default-datatype/default.datatype.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.htmlsrc/app/core/import/import-additional-settings/import-additional-settings.component.scsssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.spec.tssrc/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.htmlsrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/import/import-column-mapping/mapping-dialog-data.tssrc/app/core/import/import-metadata.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import.service.spec.tssrc/app/core/import/import.service.tssrc/app/core/import/import/import.component.htmlsrc/app/core/import/import/import.component.ts
💤 Files with no reviewable changes (1)
- src/app/core/basic-datatypes/configurable-enum/configurable-enum-datatype/configurable-enum.datatype.spec.ts
🧰 Additional context used
📓 Path-based instructions (15)
**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.ts: Use standalone components (default behavior, do NOT setstandalone: true)
Preferinput()andoutput()functions over@Inputand@Outputdecorators in Angular components
Use signals for reactive state management withcomputed()for derived state
SetchangeDetection: ChangeDetectionStrategy.OnPushfor all components
Prefer inline templates for small components in Angular
Prefer Reactive Forms over Template-driven forms in Angular
UseNgOptimizedImagefor static images instead of base64 encoded images
Use Angular Material components for UI consistency following Material Design guidelines
Implement OnPush change detection strategy for performance optimization
Use trackBy functions for lists in Angular templates for performance optimization
Files:
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.ts
**/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use
inject()function instead of constructor injection for dependencies
Files:
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.tssrc/app/core/import/import.service.ts
**/*.{component,service}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,service}.ts: Useupdate()orset()instead ofmutate()when modifying signals in Angular
Create interfaces for configuration objects and let component classes implement them
Files:
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.tssrc/app/core/import/import.service.ts
**/*.{component,directive}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,directive}.ts: Do NOT use@HostBindingand@HostListenerdecorators; put host bindings inside thehostobject of@Componentor@Directivedecorator
Implement proper permissions checking via CASL integration usingEntityAbilityandDisableEntityOperationDirective
Files:
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use strict type checking with TypeScript and avoidanytype; useunknownwhen type is uncertain
Prefer type inference in TypeScript when obvious
Files:
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/entity/default-datatype/default.datatype.tssrc/app/core/import/import-metadata.tssrc/app/core/import/import-column-mapping/mapping-dialog-data.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import.service.spec.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.tssrc/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.spec.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.tssrc/app/core/import/import.service.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js}: Use ESLint for linting (npm run lint)
Use Prettier for code formatting
Files:
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/entity/default-datatype/default.datatype.tssrc/app/core/import/import-metadata.tssrc/app/core/import/import-column-mapping/mapping-dialog-data.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import.service.spec.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.tssrc/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.spec.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.tssrc/app/core/import/import.service.ts
**/*.component.{ts,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.{ts,html}: All user-facing strings must be translatable using Angular i18n markers or$localizein Aam Digital
Follow existing translation key patterns in Aam Digital
Use Angular Material accessibility features in components
Files:
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.htmlsrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.htmlsrc/app/core/import/import/import.component.htmlsrc/app/core/import/import-additional-settings/import-additional-settings.component.htmlsrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.ts
**/*.component.scss
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.scss: Use SCSS for styling with global variables and mixins fromsrc/styles/
If custom styles are necessary, create a class with meaningful name in component-specific SCSS file instead of inline styles
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scsssrc/app/core/import/import-additional-settings/import-additional-settings.component.scss
**/*.component.{scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use global style classes from
src/styles/globals/(e.g.,flex-row,margin-regular) instead of creating new custom styles
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scsssrc/app/core/import/import-additional-settings/import-additional-settings.component.scsssrc/app/core/import/import-column-mapping/import-column-mapping.component.htmlsrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.htmlsrc/app/core/import/import/import.component.htmlsrc/app/core/import/import-additional-settings/import-additional-settings.component.html
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scsssrc/app/core/import/import-additional-settings/import-additional-settings.component.scsssrc/app/core/import/import-column-mapping/import-column-mapping.component.htmlsrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.htmlsrc/app/core/import/import/import.component.htmlsrc/app/core/import/import-additional-settings/import-additional-settings.component.html
**/*.{component,service}.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Write unit tests for all new components and services using Jasmine/Karma
Files:
src/app/core/import/import.service.spec.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.tssrc/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.spec.ts
**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.ts: Mock dependencies properly using Angular testing utilities in unit tests
Use established testing patterns from the Aam Digital project in unit tests
Files:
src/app/core/import/import.service.spec.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.tssrc/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.spec.ts
**/*.component.html
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.html: Use native control flow (@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch)
Use class bindings instead ofngClassdirective in Angular templates
Use style bindings instead ofngStyledirective in Angular templates
Keep templates simple and avoid complex logic in Angular templates
Use async pipe for observables in Angular templates instead of manual subscriptions
Files:
src/app/core/import/import-column-mapping/import-column-mapping.component.htmlsrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.htmlsrc/app/core/import/import/import.component.htmlsrc/app/core/import/import-additional-settings/import-additional-settings.component.html
**/*.service.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.service.ts: UseprovidedIn: 'root'for singleton services in Angular
Implement proper error handling and logging in Angular services
Files:
src/app/core/import/import.service.ts
src/app/core/**/*.{service,module}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Core system modules and services belong in
src/app/core/
Files:
src/app/core/import/import.service.ts
🧠 Learnings (20)
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scsssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{scss,html} : Use global style classes from `src/styles/globals/` (e.g., `flex-row`, `margin-regular`) instead of creating new custom styles
Applied to files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
Applied to files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scsssrc/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import-column-mapping/import-column-mapping.component.tssrc/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.scss : If custom styles are necessary, create a class with meaningful name in component-specific SCSS file instead of inline styles
Applied to files:
src/app/core/import/import-additional-settings/import-additional-settings.component.scss
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/models/**/*.entity.ts : Define entity schemas through `DatabaseField()` annotations in model classes
Applied to files:
src/app/core/entity/default-datatype/default.datatype.tssrc/app/core/import/import-metadata.tssrc/app/core/import/import.service.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*datatype*.{service,component}.ts : Implement specific datatypes extending the `DefaultDatatype` class with 'edit' and 'display' components in Aam Digital
Applied to files:
src/app/core/entity/default-datatype/default.datatype.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*entity*.{service,component}.ts : Use proper generics for entity types in TypeScript
Applied to files:
src/app/core/entity/default-datatype/default.datatype.tssrc/app/core/import/import.service.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use standalone components (default behavior, do NOT set `standalone: true`)
Applied to files:
src/app/core/import/import-additional-settings/import-additional-settings.component.tssrc/app/core/import/import/import.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : All user-facing strings must be translatable using Angular i18n markers or `$localize` in Aam Digital
Applied to files:
src/app/core/import/import-additional-settings/import-additional-settings.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.{component,service}.ts : Create interfaces for configuration objects and let component classes implement them
Applied to files:
src/app/core/import/import-additional-settings/import-additional-settings.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Follow existing translation key patterns in Aam Digital
Applied to files:
src/app/core/import/import-additional-settings/import-additional-settings.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*entity*.spec.ts : Test entity operations with proper data setup/teardown in unit tests
Applied to files:
src/app/core/import/import.service.spec.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.tssrc/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.spec.ts : Use established testing patterns from the Aam Digital project in unit tests
Applied to files:
src/app/core/import/import.service.spec.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.{component,service}.spec.ts : Write unit tests for all new components and services using Jasmine/Karma
Applied to files:
src/app/core/import/import.service.spec.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.spec.ts : Mock dependencies properly using Angular testing utilities in unit tests
Applied to files:
src/app/core/import/import.service.spec.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to src/app/core/demo-data/**/*.ts : Provide demo data generators for new entities using `faker-js/faker` following existing demo data patterns
Applied to files:
src/app/core/import/import.service.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*entity*.service.ts : Use entity services for data access and caching in Aam Digital
Applied to files:
src/app/core/import/import.service.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer Reactive Forms over Template-driven forms in Angular
Applied to files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to src/app/core/demo-data/**/*.ts : Follow existing demo data patterns in `core/demo-data/` when creating test data
Applied to files:
src/app/core/import/import-additional-settings/import-additional-settings.component.spec.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to src/app/core/common-components/**/*.component.ts : Shared components belong in `src/app/core/common-components/`
Applied to files:
src/app/core/import/import-review-data/import-review-data.component.tssrc/app/core/import/import/import.component.ts
🧬 Code graph analysis (9)
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.ts (1)
src/app/core/import/import-additional-settings/import-additional-settings.component.ts (1)
ImportAdditionalSettings(14-16)
src/app/core/import/import-metadata.ts (1)
src/app/core/import/import-additional-settings/import-additional-settings.component.ts (1)
ImportAdditionalSettings(14-16)
src/app/core/import/import-column-mapping/mapping-dialog-data.ts (3)
src/app/core/import/column-mapping.ts (1)
ColumnMapping(4-22)src/app/core/entity/model/entity.ts (1)
EntityConstructor(43-46)src/app/core/import/import-additional-settings/import-additional-settings.component.ts (1)
ImportAdditionalSettings(14-16)
src/app/core/import/import-additional-settings/import-additional-settings.component.ts (3)
src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.ts (1)
Component(31-164)src/app/core/import/import-column-mapping/import-column-mapping.component.ts (1)
Component(26-80)src/app/core/import/import-review-data/import-review-data.component.ts (1)
Component(38-150)
src/app/core/import/import.service.spec.ts (4)
src/app/core/entity/model/entity.ts (1)
Entity(68-490)src/app/core/entity/database-field.decorator.ts (1)
DatabaseField(12-32)src/app/core/entity/database-entity.decorator.ts (1)
EntityRegistry(5-24)src/app/core/import/column-mapping.ts (1)
ColumnMapping(4-22)
src/app/core/import/import-column-mapping/import-column-mapping.component.ts (1)
src/app/core/import/import-additional-settings/import-additional-settings.component.ts (1)
ImportAdditionalSettings(14-16)
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts (1)
src/app/core/import/import.service.ts (1)
splitArrayValue(235-254)
src/app/core/import/import-review-data/import-review-data.component.ts (1)
src/app/core/import/import-additional-settings/import-additional-settings.component.ts (1)
ImportAdditionalSettings(14-16)
src/app/core/import/import.service.ts (1)
src/app/core/import/import-processing-context.ts (1)
ImportProcessingContext(9-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-image
- GitHub Check: qa / test-e2e
🔇 Additional comments (26)
src/app/core/import/import-additional-settings/import-additional-settings.component.spec.ts (1)
6-45: Well-structured unit tests.The test suite appropriately covers the component's core functionality:
- Component creation
- Default multi-value separator behavior
- Settings update reactivity
- Custom separator creation
The tests follow established Angular testing patterns with proper setup and focused assertions.
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.scss (1)
1-17: Layout improvements support multi-value display.The styling changes improve the dialog layout for multi-value imports:
- New
.first-columnclass provides appropriate width and spacing for the imported values column- Removed
max-widthconstraints allow the table to utilize available space for better readabilityThe component-specific layout adjustments are reasonable for this dialog's requirements.
src/app/core/import/import-column-mapping/import-column-mapping.component.html (1)
23-23: TheadditionalSettingsproperty binding is correctly configured. Both the parent component (ImportColumnMappingComponent) and child component (EditImportColumnMappingComponent) have the property properly declared as@Input() additionalSettings: ImportAdditionalSettings.src/app/core/entity/default-datatype/default.datatype.ts (1)
113-120: Array handling correctly moved upstream and integrated into ImportService.The breaking signature change from
Promise<EntityType | EntityType[]>toPromise<EntityType>has been properly implemented. Array splitting now occurs inImportService.importMapValue()before callingimportMapFunction, which correctly:
- Calls
splitArrayValue()whenschema.isArrayis true- Loops through individual values, calling
importMapFunctiononce per value- Accumulates results in the final array
All callers have been updated, and array field imports continue to work as verified by existing tests.
src/app/core/import/import-column-mapping/mapping-dialog-data.ts (1)
3-3: LGTM: Clean interface extension for additional settings.The addition of the optional
additionalSettingsproperty toMappingDialogDatais well-typed and backward compatible, enabling the import flow to pass multi-value separator configuration through to mapping dialogs.Also applies to: 9-9
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.html (2)
2-7: LGTM: Helpful user guidance added.The hint box provides clear upfront explanation of the mapping process, improving user experience. The text is properly internationalized.
11-11: The "first-column" CSS class is defined in the component's SCSS file and correctly replaces inline styles with a meaningful class name.src/app/core/import/import/import.component.ts (1)
23-23: LGTM: Proper component registration.The
ImportAdditionalSettingsComponentis correctly imported and registered in the standalone component's imports array, enabling its use in the template. This follows Angular standalone component patterns correctly.Also applies to: 42-42
src/app/core/import/import-metadata.ts (1)
6-6: LGTM: Well-documented interface extension.The addition of the optional
additionalSettingsproperty toImportSettingsis properly typed and includes clear JSDoc documentation. The placement at the end of the interface is appropriate and maintains backward compatibility.Also applies to: 76-78
src/app/core/import/import-additional-settings/import-additional-settings.component.html (2)
1-13: LGTM: Well-structured expansion panel with helpful guidance.The expansion panel is appropriately disabled until an entity type is selected, and the help button provides clear context to users. Internationalization is properly implemented.
15-29: LGTM: Clear form field implementation.The multi-value separator field is well-implemented with autocomplete functionality and helpful guidance. The internationalization is properly applied.
Note: The template uses
[(ngModel)]for two-way binding. While this works, the coding guidelines prefer Reactive Forms for most scenarios. However, for a simple settings panel like this, the template-driven approach is acceptable.src/app/core/import/import-column-mapping/edit-import-column-mapping/edit-import-column-mapping.component.spec.ts (1)
66-66: LGTM! Test correctly verifies the new data flow.The test properly sets
additionalSettingsand verifies it's passed through to the dialog data, ensuring the new functionality works as expected.Also applies to: 83-83
src/app/core/import/import-additional-settings/import-additional-settings.component.ts (4)
14-16: LGTM! Clean interface definition.The
ImportAdditionalSettingsinterface is well-defined with an optionalmultiValueSeparatorproperty, providing clear typing for the additional import settings.
21-34: LGTM! Component configuration follows best practices.The component properly uses:
- OnPush change detection strategy for performance
- Angular Material components for UI consistency
- Appropriate module imports
36-38: LGTM! Proper use of signal-based inputs.The component correctly uses
input()andmodel()functions instead of decorators, following the coding guidelines for modern Angular patterns.Based on coding guidelines for **/*.component.ts.
50-50: Remove this comment; the function is necessary.
createCustomSeparatoris thecreateOptioncallback passed toBasicAutocompleteComponent(line 21) that enables users to create custom separators beyond the predefined options. The current implementation (returning input unchanged) is valid and tested. It can accept additional validation logic in the future without changing the function's signature.Likely an incorrect or invalid review comment.
src/app/core/import/import.service.ts (2)
181-213: Array handling implementation looks solid.The logic correctly:
- Splits values using the configurable separator
- Maps each item individually with
isArray: falseto ensure datatypes handle single values- Filters out empty/null/undefined mapped results
- Deduplicates values using
SetThis approach properly delegates single-value mapping to existing datatype logic.
216-222: LGTM! Empty array validation is appropriate.Treating empty arrays as undefined is consistent with the existing validation logic for other falsy values and prevents saving meaningless empty array fields.
src/app/core/import/import.service.spec.ts (3)
212-254: LGTM! Comprehensive test coverage for array parsing.The test thoroughly validates:
- JSON array parsing
- Comma-separated value splitting
- Single value handling
- Empty string handling (returns undefined)
- Invalid JSON fallback (treated as single string)
Proper mocking and spy verification ensure the datatype's
importMapFunctionis invoked correctly.Based on learnings, this follows established testing patterns for the Aam Digital project.
256-299: LGTM! Validates configurable separator functionality.The test effectively demonstrates that the
multiValueSeparatorsetting works correctly by testing the same raw data with different separators and verifying different parsing results. This ensures the separator configuration flows through the import pipeline properly.
301-328: LGTM! Verifies deduplication behavior.The test confirms that duplicate values are properly filtered from both comma-separated and JSON array inputs, preventing data bloat in multi-select fields.
src/app/core/import/import/import.component.html (2)
80-83: LGTM! Additional settings properly integrated into import flow.The new
app-import-additional-settingscomponent is correctly positioned in STEP 2 after entity type selection, with appropriate bindings for entity context and two-way data flow.
132-132: LGTM! Settings correctly propagated to downstream components.The
additionalSettingsare properly passed to both the column mapping and review data components, ensuring the multi-value separator configuration is available throughout the import pipeline.Also applies to: 162-162
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts (3)
17-17: LGTM! Proper code reuse and component imports.Importing
splitArrayValuefrom the import service promotes code reuse and consistency. The addition ofHintBoxComponentaligns with UI enhancements for the discrete import configuration dialog.Also applies to: 24-24, 41-41
66-87: LGTM! Proper value splitting and deduplication.The method correctly:
- Uses the configured separator with a sensible default
- Leverages
splitArrayValuefor consistent parsing- Filters out null/empty values before and after splitting
- Deduplicates using
SetThis ensures the discrete import mapping dialog displays unique values from potentially multi-value cells.
60-63: LGTM! Good refactoring for separation of concerns.Extracting the splitting logic into
splitAndFlattenValuesand passing the result togetFormValuesmakes the data flow explicit and improves testability. The signature change clearly indicates thatgetFormValuesoperates on pre-processed values.Also applies to: 89-89
src/app/core/import/import-additional-settings/import-additional-settings.component.ts
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In
@src/app/core/import/import-additional-settings/import-additional-settings.component.html:
- Around line 23-27: The i18n attribute is placed incorrectly on the component
element; change the bare i18n on the app-help-button to target the text input by
replacing i18n with i18n-text so the text attribute is marked for translation
(update the app-help-button instance where text="Which character..." to use
i18n-text instead of i18n).
🧹 Nitpick comments (1)
src/app/core/import/import-additional-settings/import-additional-settings.component.html (1)
5-11: Replace inline style with a class binding.Line 6 uses an inline
styleattribute, which violates the coding guidelines. Based on coding guidelines, prefer using global style classes or style bindings instead of inline styles.🔎 Suggested fix using a style class
If a global utility class exists for float-right, use it:
<app-help-button - style="float: right" + class="float-right" text="Configure additional settings for the import process.Alternatively, use style binding:
<app-help-button - style="float: right" + [style.float]="'right'" text="Configure additional settings for the import process.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.htmlsrc/app/core/import/import.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/core/import/import.service.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.component.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.ts: Use standalone components (default behavior, do NOT setstandalone: true)
Preferinput()andoutput()functions over@Inputand@Outputdecorators in Angular components
Use signals for reactive state management withcomputed()for derived state
SetchangeDetection: ChangeDetectionStrategy.OnPushfor all components
Prefer inline templates for small components in Angular
Prefer Reactive Forms over Template-driven forms in Angular
UseNgOptimizedImagefor static images instead of base64 encoded images
Use Angular Material components for UI consistency following Material Design guidelines
Implement OnPush change detection strategy for performance optimization
Use trackBy functions for lists in Angular templates for performance optimization
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
**/*.{component,service,directive,guard,interceptor,resolver,pipe}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use
inject()function instead of constructor injection for dependencies
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
**/*.{component,service}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,service}.ts: Useupdate()orset()instead ofmutate()when modifying signals in Angular
Create interfaces for configuration objects and let component classes implement them
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
**/*.{component,directive}.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{component,directive}.ts: Do NOT use@HostBindingand@HostListenerdecorators; put host bindings inside thehostobject of@Componentor@Directivedecorator
Implement proper permissions checking via CASL integration usingEntityAbilityandDisableEntityOperationDirective
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.ts: Use strict type checking with TypeScript and avoidanytype; useunknownwhen type is uncertain
Prefer type inference in TypeScript when obvious
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
src/**/*.{ts,js}
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.{ts,js}: Use ESLint for linting (npm run lint)
Use Prettier for code formatting
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
**/*.component.{ts,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.{ts,html}: All user-facing strings must be translatable using Angular i18n markers or$localizein Aam Digital
Follow existing translation key patterns in Aam Digital
Use Angular Material accessibility features in components
Files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.tssrc/app/core/import/import-additional-settings/import-additional-settings.component.html
**/*.component.html
📄 CodeRabbit inference engine (AGENTS.md)
**/*.component.html: Use native control flow (@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch)
Use class bindings instead ofngClassdirective in Angular templates
Use style bindings instead ofngStyledirective in Angular templates
Keep templates simple and avoid complex logic in Angular templates
Use async pipe for observables in Angular templates instead of manual subscriptions
Files:
src/app/core/import/import-additional-settings/import-additional-settings.component.html
**/*.component.{scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use global style classes from
src/styles/globals/(e.g.,flex-row,margin-regular) instead of creating new custom styles
Files:
src/app/core/import/import-additional-settings/import-additional-settings.component.html
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/import/import-additional-settings/import-additional-settings.component.html
🧠 Learnings (3)
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.ts : Prefer Reactive Forms over Template-driven forms in Angular
Applied to files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
📚 Learning: 2025-11-26T16:20:52.119Z
Learnt from: CR
Repo: Aam-Digital/ndb-core PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T16:20:52.119Z
Learning: Applies to **/*.component.{ts,html} : Use Angular Material accessibility features in components
Applied to files:
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
🧬 Code graph analysis (1)
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts (1)
src/app/core/import/import.service.ts (1)
splitArrayValue(235-255)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: qa / test-unit
- GitHub Check: qa / test-e2e
- GitHub Check: build-webapp
🔇 Additional comments (7)
src/app/core/import/import-additional-settings/import-additional-settings.component.html (1)
1-30: Overall structure looks good.The template properly uses Angular Material components, provides accessibility features (labels, help buttons, disabled state), and implements the multi-value separator setting as intended. The help text on line 24 appropriately describes the feature for array-type fields with multi-select as an example.
src/app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts (6)
17-17: LGTM: Imports are correct and necessary.The imports of
splitArrayValueandHintBoxComponentsupport the new multi-value splitting functionality and UI updates.Also applies to: 24-24
41-41: LGTM: Component import follows standalone pattern.Adding
HintBoxComponentto the imports array is correct for standalone components.
60-63: LGTM: ngOnInit correctly integrates multi-value splitting.The initialization logic properly computes split values before passing them to form construction, maintaining clean separation of concerns.
69-87: LGTM: splitAndFlattenValues correctly implements multi-value splitting.The implementation properly handles:
- Configurable separator with sensible default
- Null and empty value filtering at both input and part levels
- Type safety via
String(part)conversion (necessary sincesplitArrayValuereturns[val]for non-string inputs)- Efficient deduplication using Set
The JSDoc clearly documents the method's purpose.
89-89: LGTM: Method signature update is appropriate.Adding the
valuesparameter enables the method to work with pre-split values. Since this is a private method called only from within the component, the signature change doesn't create breaking changes.
101-101: LGTM: Iteration correctly uses pre-split values.Iterating the
valuesparameter instead ofthis.data.valuesensures form controls are created for the deduplicated, split values as intended.
src/app/core/import/import-additional-settings/import-additional-settings.component.html
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
...app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
...app/core/basic-datatypes/discrete/discrete-import-config/discrete-import-config.component.ts
Outdated
Show resolved
Hide resolved
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
sadaf895
left a comment
sadaf895
left a comment
There was a problem hiding this comment.
@sleidig The PR is technically sound with good architecture and test coverage. Just one edge case in my mind that the current implementation doesn't handle CSV standard quoting conventions. If a field value is ["value, with comma"] and the separator is , (comma) it will be split incorrectly. Isn't it?
What do we define as correct splitting there? 😅 ... because I don't see a very simple way out, I'll give myself the excuse saying "it's extremely rare that you have a perfectly correct JSON string but don't want it to be interpreted as JSON". Let's keep it aside for now. The clean solution would probably be to add another option to the settings UI that allows to turn JSON parsing on/off. |
|
🎉 This PR is included in version 3.68.0-master.8 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 3.68.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Remaining TODOs
PR Checklist
Before you request a manual PR review from someone, please go through all of this list, check the points and mark them checked here.
This helps us reduce the number of review cycles and use the time of our core reviewers more effectively.
ng lint) passesng test) passes⏪ if issues are commented from testing or code review, make changes. Please then re-check every item of the checklist here again.
Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.