-
Notifications
You must be signed in to change notification settings - Fork 29
Full-Text Search for Field Configuration Screen #3540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds a case-insensitive search input and reactive signal-based filtering to the available-fields drag area in AdminEntityForm, enables prefilling a new field's label from the search, and makes AdminEntityField auto-generate a missing field ID unconditionally during initialization (no gating flag). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Form as AdminEntityForm
participant Dialog as DialogService
participant Field as AdminEntityFieldDialog
Note over Form: Search + create flow with signal-backed availableFields
User->>Form: Type search term
Form->>Form: searchFilter.valueChanges -> update searchFieldSignal -> compute filteredFields
User->>Form: Click "Create new field" (or drag placeholder)
Form->>Dialog: openFieldConfig({ label: searchText, id: null })
Dialog->>Field: instantiate with data
Field->>Field: ngOnInit sees id null -> autoGenerateId(label) -> set id
Field->>Dialog: return created field (id populated)
Dialog->>Form: add field -> Form updates availableFields signal / form model
Form->>User: Field appears in form
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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-3540.aam-digital.net/ |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
Show resolved
Hide resolved
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (1)
486-513: Consider optional UX enhancement: clearing search after operations.The filtering logic is correct and efficiently preserves placeholders while matching fields by ID or label. However, consider whether the search should be cleared after certain user actions (e.g., after successfully creating a new field or dragging a field into the form) to reset the UI to show all available fields.
Example enhancement to clear search after successful field creation:
private async dropNewField( event: CdkDragDrop<ColumnConfig[], ColumnConfig[]>, ) { if (event.container.data === this.availableFields) { return; } const newField = await this.openFieldConfig({ id: null }); if (!newField) { return; } // ... existing logic ... // Clear search to show all fields after creation this.searchFilter.setValue(""); this.emitUpdatedConfig(); }This is optional and depends on the desired UX—keeping the search active allows users to create multiple similar fields quickly.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
🧰 Additional context used
📓 Path-based instructions (14)
**/*.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/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
**/*entity*.{service,component}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use proper generics for entity types in TypeScript
Files:
src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts
**/*entity*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Test entity operations with proper data setup/teardown in unit tests
Files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts
🧠 Learnings (14)
📚 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/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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.ts : Use signals for reactive state management with `computed()` for derived state
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use `update()` or `set()` instead of `mutate()` when modifying signals in Angular
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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,directive}.ts : Implement proper permissions checking via CASL integration using `EntityAbility` and `DisableEntityOperationDirective`
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Implement OnPush change detection strategy for performance optimization
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 `input()` and `output()` functions over `Input` and `Output` decorators in Angular components
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use trackBy functions for lists in Angular templates for performance optimization
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
🧬 Code graph analysis (1)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (1)
src/app/core/common-components/entity-form/FormConfig.ts (2)
ColumnConfig(55-55)toFormFieldConfig(57-63)
⏰ 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). (4)
- GitHub Check: Seer Code Review
- GitHub Check: qa / test-unit
- GitHub Check: qa / test-e2e
- GitHub Check: build-webapp
🔇 Additional comments (8)
src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.ts (1)
64-66: LGTM: Optional flag for ID prefill behavior.The addition of the
prefillIdFromLabelflag is well-designed and appropriately optional, allowing the component to support both manual and automatic ID generation workflows.src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html (1)
149-184: LGTM: Filtered field rendering with correct placeholder handling.The loop correctly uses
filteredFields()and preserves the create-new placeholders in the filtered results. The conditional Edit button logic is maintained properly.src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts (2)
229-263: LGTM: Comprehensive test coverage for field filtering.The test correctly validates both field ID and label matching, and properly filters out non-matching fields while preserving the create-new placeholders. Good use of
fixture.detectChanges()to ensure signal updates are reflected.
265-277: LGTM: Proper validation of prefill behavior.The test correctly verifies that the search text prefills the label for new fields, and appropriately checks that the dialog receives the expected data structure with a null ID and the search term as the label.
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (4)
144-151: LGTM: Proper signal-based reactive filtering setup.The
searchFilterFormControl andsearchFieldSignalare correctly implemented usingtoSignalwith an appropriate initial value. This follows the coding guidelines for using signals for reactive state management.
240-247: LGTM: Clean prefill logic for new field creation.The prefill logic correctly checks for new field conditions and applies the trimmed search text as the label. The conditional check properly handles both the placeholder and object-based field representations.
256-256: LGTM: Consistent flag passing to dialog.The
prefillIdFromLabelflag is appropriately passed to enable automatic ID generation in the field configuration dialog.
219-219: Verify signal update triggers computed re-evaluation.Confirm that updating
availableFieldsSignalproperly triggersfilteredFieldsrecomputation in all scenarios, particularly after drag-and-drop operations that modify the available fields list.The implementation looks correct, but verify with a manual test:
- Search for a field
- Drag the found field into the form
- Confirm the field is removed from the filtered list immediately
- Clear the search
- Confirm all remaining fields appear
Based on learnings, this follows the Angular pattern for signal-based reactive state management.
src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
Show resolved
Hide resolved
…in-entity-field.component.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
Outdated
Show resolved
Hide resolved
|
Deployed to https://pr-3540.aam-digital.net/ |
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (2)
500-500: MarkavailableFieldsSignalas readonly.The signal is never reassigned—only updated via
set()—so it should be markedreadonlyper TypeScript best practices.🔎 Suggested fix
- private availableFieldsSignal = signal<ColumnConfig[]>([]); + private readonly availableFieldsSignal = signal<ColumnConfig[]>([]);As per coding guidelines, use strict type checking with TypeScript.
411-448: Critical: Signal not updated after array mutation.Both
dropNewField()anddropNewText()mutatethis.availableFieldswith.splice()(lines 433, 439, 473) but fail to updateavailableFieldsSignal. BecausefilteredFields()depends onavailableFieldsSignal, it doesn't re-evaluate, and the UI continues to show the newly created field in the available fields list.🔎 Recommended fix
After each
.splice()operation onavailableFields, update the signal:In
dropNewField()after line 433:// the schema update has added the new field to the available fields already, remove it from there this.availableFields.splice(this.availableFields.indexOf(newFieldId), 1); + this.availableFieldsSignal.set([...this.availableFields]);In
dropNewField()after line 440:const fieldIndex = this.availableFields.indexOf(newFieldId); if (fieldIndex !== -1) { this.availableFields.splice(fieldIndex, 1); + this.availableFieldsSignal.set([...this.availableFields]); }In
dropNewText()after line 473:// the schema update has added the new Text field to the available fields already, remove it from there this.availableFields.splice(this.availableFields.indexOf(newTextField), 1); + this.availableFieldsSignal.set([...this.availableFields]);Alternatively, call
this.initAvailableFields()after mutations to ensure consistency.As per coding guidelines, use
set()when modifying signals.Also applies to: 455-476
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html (1)
131-152: Add accessibility attributes for the search input and icon.The search UI has accessibility gaps:
- The input (lines 134-138) relies only on
placeholder, which is insufficient for screen readers—add anaria-labelor<mat-label>with i18n- The search icon (lines 125-129) is decorative and should be marked
aria-hidden="true"🔎 Proposed accessibility improvements
<div class="margin-top-regular"> <em i18n="Search filter label">Filter the fields</em> <mat-form-field appearance="fill" class="full-width"> <input matInput [formControl]="searchFilter" placeholder="Search fields..." i18n-placeholder="Search placeholder" + aria-label="Search fields by name or label" + i18n-aria-label="Accessible label for field search input" /> + <fa-icon + matIconPrefix + icon="search" + aria-hidden="true" + ></fa-icon> + <button matIconSuffix mat-icon-button (click)="clearSearch()" matTooltip="Clear search" i18n-matTooltip="Clear search tooltip" aria-label="Clear search" i18n-aria-label="Clear search aria label" > <fa-icon icon="times"></fa-icon> </button> </mat-form-field> </div>As per coding guidelines, follow WCAG guidelines for accessibility and use Angular Material accessibility features.
🧹 Nitpick comments (1)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (1)
306-325: LGTM: Correctly handles filtered field transfers.The logic properly resolves the index mismatch between
filteredFields()andavailableFieldsby finding the actual field in the unfiltered array before removal. The signal update (line 316) ensures the UI reflects the change.📝 Optional: Add clarifying comment
The logic is correct but non-obvious. Consider adding a comment explaining why the actual index lookup is necessary:
} else { // if transferring from filtered available fields, find the actual field in availableFields and remove it from there if (prevFieldsArray === this.filteredFields()) { + // When dragging from a filtered list, event.previousIndex corresponds to the filtered array, + // but we need to remove from the unfiltered availableFields array at its actual position const transferredField = prevFieldsArray[event.previousIndex];
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
**/*.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
**/*.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
**/*entity*.{service,component}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use proper generics for entity types in TypeScript
Files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
🧠 Learnings (10)
📚 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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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.{html,scss} : Follow WCAG guidelines for accessibility in Aam Digital
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
📚 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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
📚 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 signals for reactive state management with `computed()` for derived state
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use `update()` or `set()` instead of `mutate()` when modifying signals in Angular
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Implement OnPush change detection strategy for performance optimization
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 `input()` and `output()` functions over `Input` and `Output` decorators in Angular components
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use trackBy functions for lists in Angular templates for performance optimization
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
🧬 Code graph analysis (1)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (1)
src/app/core/common-components/entity-form/FormConfig.ts (2)
ColumnConfig(55-55)toFormFieldConfig(57-63)
⏰ 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). (4)
- GitHub Check: qa / test-unit
- GitHub Check: qa / test-e2e
- GitHub Check: build-webapp
- GitHub Check: Seer Code Review
🔇 Additional comments (4)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (4)
144-151: LGTM: Reactive search implementation.The search filter uses proper reactive patterns with
FormControlandtoSignal, correctly initialized with an empty string. This aligns with coding guidelines for signals and Reactive Forms.
240-256: LGTM: Search-driven prefill for new fields.The prefill logic correctly populates a new field's label from the search filter text when creating a field, satisfying the PR objective. The
prefillIdFromLabel: trueflag integrates withAdminEntityFieldComponentto auto-generate the field ID.
501-527: LGTM: Search filtering logic.The
filteredFieldscomputed signal correctly implements case-insensitive search over field IDs and labels, always preserving the create-new placeholders as required by the issue objectives.
529-531: LGTM: Clear search implementation.The
clearSearch()method correctly resets the search filter, satisfying the requirement to allow users to restore the complete field list.
sleidig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionality works great. Code also looks very clean already 🙂
Just a few last suggestions if we can simplify the code even further.
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts
Outdated
Show resolved
Hide resolved
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts
Outdated
Show resolved
Hide resolved
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.ts
Outdated
Show resolved
Hide resolved
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html (2)
112-119: Drag-and-drop index mismatch when filtering is active.The
cdkDropListis bound tofilteredFields()(line 116), but when an item is dragged, theevent.previousIndexcorresponds to the position in the filtered list. If thedrop()handler uses this index to access the unfilteredavailableFieldsarray, it will operate on the wrong item when a search filter is active.Ensure the
drop()handler either:
- Uses
[cdkDropListData]to pass the actual item reference rather than relying on index lookups, or- Maps the filtered index back to the correct index in the source array.
133-139: Add an accessible label to the search input.The input relies solely on the
placeholderattribute for labeling, which is insufficient for screen readers. Per WCAG guidelines, add anaria-labelor use<mat-label>to provide a proper accessible name.🔎 Proposed fix
<input matInput [formControl]="searchFilter" placeholder="Search fields" i18n-placeholder + aria-label="Search fields" + i18n-aria-label="Accessible label for field search" />As per coding guidelines, follow WCAG guidelines for accessibility.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.htmlsrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.scss
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.scsssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
**/*.component.{html,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Follow WCAG guidelines for accessibility in Aam Digital
Files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.scsssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
**/*.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
🧠 Learnings (4)
📚 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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
📚 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.{html,scss} : Follow WCAG guidelines for accessibility in Aam Digital
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
📚 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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
📚 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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
⏰ 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). (4)
- GitHub Check: Seer Code Review
- GitHub Check: qa / test-e2e
- GitHub Check: build-webapp
- GitHub Check: qa / test-unit
🔇 Additional comments (2)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.scss (1)
161-165: LGTM!The new
.muted-buttonclass correctly uses the globalcolors.$mutedvariable and has a meaningful name. The accompanying comment clarifies its purpose.src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html (1)
120-130: LGTM!The drop area hint section correctly uses i18n markers for both the title and subtitle, and the tooltip is properly translated with
i18n-matTooltip.
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.html
Show resolved
Hide resolved
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts (1)
234-286: Test doesn't verify that BOTH id AND label filtering work independently.Both test cases filter the same field (
uniqueTestFieldId):
"uniqueTest"matches the field IDuniqueTestFieldId"Special Label"matches the field labelThis test would pass even if the filter only checked ID or only checked label, not both. To properly verify the filtering logic, test with:
- A field where the search term appears only in the ID
- A different field where the search term appears only in the label
- Verify both fields are included in the filtered results
🔎 Proposed fix
it("should filter fields by field ID and label when searching", async () => { - TestEntity.schema.set("uniqueTestFieldId", { - label: "Special Label", + TestEntity.schema.set("fieldWithUniqueId", { + label: "Common Label A", dataType: "text", }); - TestEntity.schema.set("anotherFieldForTest", { - label: "Another Label", + TestEntity.schema.set("fieldWithCommonId", { + label: "Unique Label B", dataType: "text", }); try { component.config = { fieldGroups: [{ fields: ["other"] }], }; await component.ngOnChanges({ config: true as any }); - // Test filtering by field ID - component.searchFilter.setValue("uniqueTest"); + // Test filtering by field ID (matches only fieldWithUniqueId) + component.searchFilter.setValue("UniqueId"); fixture.detectChanges(); let filteredFields = component.filteredFields(); let nonPlaceholderFields = filteredFields.filter( (f) => f !== component.createNewFieldPlaceholder && f !== component.createNewTextPlaceholder, ); - expect(nonPlaceholderFields).toContain("uniqueTestFieldId"); + expect(nonPlaceholderFields).toContain("fieldWithUniqueId"); + expect(nonPlaceholderFields).not.toContain("fieldWithCommonId"); expect(nonPlaceholderFields).not.toContain("name"); - expect(nonPlaceholderFields).not.toContain("category"); - expect(nonPlaceholderFields).not.toContain("anotherFieldForTest"); - // Test filtering by label - component.searchFilter.setValue("Special Label"); + // Test filtering by label (matches only fieldWithCommonId) + component.searchFilter.setValue("Unique Label"); fixture.detectChanges(); filteredFields = component.filteredFields(); nonPlaceholderFields = filteredFields.filter( (f) => f !== component.createNewFieldPlaceholder && f !== component.createNewTextPlaceholder, ); - expect(nonPlaceholderFields).toContain("uniqueTestFieldId"); + expect(nonPlaceholderFields).toContain("fieldWithCommonId"); + expect(nonPlaceholderFields).not.toContain("fieldWithUniqueId"); expect(nonPlaceholderFields).not.toContain("name"); - expect(nonPlaceholderFields).not.toContain("category"); - expect(nonPlaceholderFields).not.toContain("anotherFieldForTest"); } finally { - TestEntity.schema.delete("uniqueTestFieldId"); - TestEntity.schema.delete("anotherFieldForTest"); + TestEntity.schema.delete("fieldWithUniqueId"); + TestEntity.schema.delete("fieldWithCommonId"); } });src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (1)
304-320: CRITICAL: Reference equality check on computed signal always fails.Line 305 checks
prevFieldsArray === this.filteredFields(), butfilteredFields()is a computed signal that returns a new array instance on every call (due to.filter()at line 529). This means the===check will always be false, even when the user drags from the filtered available fields list.Impact: When dragging a field from the filtered list into a form group, the field is not removed from
availableFieldsSignal, so it incorrectly remains visible in the available fields toolbar. This breaks the core drag-and-drop functionality.🔎 Recommended fix
Instead of comparing array references, identify the drag source by a stable identifier (e.g., the container ID). Update the drop logic to check the container ID, and when dragging from the filtered list, always update the underlying
availableFieldsSignal:if (event.previousContainer === event.container) { moveItemInArray(newFieldsArray, event.previousIndex, event.currentIndex); } else { - // if transferring from filtered available fields, find the actual field in availableFields and remove it from there - if (prevFieldsArray === this.filteredFields()) { - const transferredField = prevFieldsArray[event.previousIndex]; - this.transferArraySignalItem( - this.availableFields, - newFieldsArray, - transferredField, - event.currentIndex, - ); - } else { - transferArrayItem( - prevFieldsArray, - newFieldsArray, - event.previousIndex, - event.currentIndex, - ); - } + const transferredField = prevFieldsArray[event.previousIndex]; + // Check if dragging from the available fields toolbar (by container ID) + if (event.previousContainer.id === `available-${this.uniqueAreaId}`) { + // Transfer from signal-backed availableFields + this.transferArraySignalItem( + this.availableFields, + newFieldsArray, + transferredField, + event.currentIndex, + ); + } else { + // Transfer between form groups + transferArrayItem( + prevFieldsArray, + newFieldsArray, + event.previousIndex, + event.currentIndex, + ); + } }Note: Ensure the drag container in the template has a stable
idattribute like[id]="'available-' + uniqueAreaId"for this check to work.Run the following script to check if the template assigns a stable container ID:
#!/bin/bash # Description: Verify that the available fields drag container has a stable ID attribute rg -nP --type=html -C3 'cdkDropList.*available' src/app/core/admin/admin-entity-details/admin-entity-form/
🧹 Nitpick comments (1)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (1)
134-134: Mark the signal asreadonly.The
availableFieldssignal is never reassigned (only its value is updated via.set()and.update()), so it should be markedreadonlyto prevent accidental reassignment.🔎 Proposed fix
- availableFields = signal<ColumnConfig[]>([]); + readonly availableFields = signal<ColumnConfig[]>([]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/app/core/admin/admin-entity-details/admin-entity-field/admin-entity-field.component.ts
🧰 Additional context used
📓 Path-based instructions (11)
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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.tssrc/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
**/*.{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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts
**/*entity*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Test entity operations with proper data setup/teardown in unit tests
Files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts
**/*.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
**/*entity*.{service,component}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use proper generics for entity types in TypeScript
Files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
🧠 Learnings (13)
📚 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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 **/models/**/*.entity.ts : Define entity schemas through `DatabaseField()` annotations in model classes
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 **/*entity*.{service,component}.ts : Use proper generics for entity types in TypeScript
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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,directive}.ts : Implement proper permissions checking via CASL integration using `EntityAbility` and `DisableEntityOperationDirective`
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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.ts : Prefer Reactive Forms over Template-driven forms in Angular
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use signals for reactive state management with `computed()` for derived state
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use `update()` or `set()` instead of `mutate()` when modifying signals in Angular
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use Angular Material components for UI consistency following Material Design guidelines
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Implement OnPush change detection strategy for performance optimization
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 `input()` and `output()` functions over `Input` and `Output` decorators in Angular components
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.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 : Use trackBy functions for lists in Angular templates for performance optimization
Applied to files:
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts
🧬 Code graph analysis (2)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts (2)
src/app/core/entity/entity-field-select/entity-field-select.component.ts (1)
entityType(39-50)src/app/core/common-components/entity-form/FormConfig.ts (1)
toFormFieldConfig(57-63)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (1)
src/app/core/common-components/entity-form/FormConfig.ts (2)
ColumnConfig(55-55)toFormFieldConfig(57-63)
⏰ 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). (6)
- GitHub Check: deploy
- GitHub Check: Seer Code Review
- GitHub Check: qa / test-e2e
- GitHub Check: qa / test-unit
- GitHub Check: build-webapp
- GitHub Check: qa / test-e2e
🔇 Additional comments (9)
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.spec.ts (3)
54-61: LGTM! Mock aligns with the updated signature.The mock correctly reflects the new two-argument signature of
extendFormFieldConfigand merges schema fields with field config.
115-119: LGTM! Correct migration to signal-based access.All references correctly updated from property access (
component.availableFields) to method invocation (component.availableFields()).Also applies to: 127-127, 156-159, 170-173, 176-177, 198-199, 205-207
288-300: LGTM! Prefill behavior correctly verified.The test confirms that when creating a new field with active search text, the dialog receives a prefilled label matching the search term.
src/app/core/admin/admin-entity-details/admin-entity-form/admin-entity-form.component.ts (6)
10-10: LGTM! Correct imports for signals and reactive forms UI.The imports support the new signal-based filtering and search UI components.
Also applies to: 16-17, 20-21, 25-27, 67-70
145-152: LGTM! Clean reactive setup for search.The
searchFilterFormControl andsearchFieldSignalprovide a solid foundation for reactive filtering.
239-246: LGTM! Prefill behavior aligns with PR objectives.The logic correctly prefills the label from the search text when creating a new field, improving UX.
337-355: Helper logic is correct but currently unreachable.The
transferArraySignalItemhelper correctly updates the signal and target array, but it's currently unreachable due to the bug at line 305. Once that bug is fixed, this helper will function as intended.
521-547: LGTM! Filtering logic correctly matches both ID and label case-insensitively.The computed signal filters fields by checking if the search term (lowercased) appears in either the field ID or label (line 545), meeting the PR requirements for case-insensitive, full-text search.
549-551: LGTM! Clean method to clear the search.Simple and effective implementation for resetting the search filter.
sleidig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work 👍
|
🎉 This PR is included in version 3.68.0-master.7 🎉 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 📦🚀 |
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.
Remaining TODOs:
Summary by CodeRabbit
New Features
Tests
Style
✏️ Tip: You can customize this high-level summary in your review settings.