Email, phone, date filter improvements, migrate filters to signals#1698
Open
Email, phone, date filter improvements, migrate filters to signals#1698
Conversation
Introduce dedicated Email and Phone filter field components for table filtering. Add onComparatorChange output to base filter/edit fields so filter components can dynamically update the comparator. Register Email widget type in backend enum and frontend display/edit/filter type maps. Also move @types packages to devDependencies and add yarn resolutions for vulnerable transitive dependencies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix dynamic component loading for widget-type filters in all three filter contexts (main dialog, saved-filters dialog, saved-filters panel). Widget data was passed as an object but guards checked .length which is undefined on objects, so setWidgets() was never called and widget filter components never loaded. - Add built-in comparator selectors to Email and Phone filter components (domain equal, equal, contains, starts with, etc.) so each component manages its own filter logic independently. - Move comparator emission from ngOnInit to ngAfterViewInit so it fires after ndc-dynamic subscribes to outputs. - Show field name label for nonComparable filter fields. - Add Email to widget defaultParams so it can be configured. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nt state Remove dead setWidgets() from saved-filters-dialog and panel, no-op cast.subscribe() calls, spurious @Injectable() decorators, debug console.log statements, commented-out code, and redundant textValue state in email/phone filters. Cache country and timezone lists at module level. Replace O(n) isWidget array scan with O(1) object lookup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…e layout Migrate BaseFilterFieldComponent and all 22 concrete filter components from legacy @input()/@output() to Angular signals (input(), output(), computed()). Update all template bindings to use signal calls. Fix custom filter layout so column name and dynamic component render on the same line in both filter dialogs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds backend enum member Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Dialog
participant Widget
participant Parent
User->>Dialog: open dialog / add filter
Dialog->>Widget: render widget with inputs (label(), key(), value)
User->>Widget: change comparator or value
Widget->>Parent: emit onComparatorChange(comparator, field)
Parent->>Parent: updateComparatorFromComponent(comparator, field)
Parent->>Parent: tableRowFieldsComparator[field] = comparator
Widget->>Parent: emit onFieldChange(value, field)
Parent->>Parent: apply/update filter value/state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves filtering UX for email/phone fields and migrates filter-field component inputs/outputs to Angular signal-based APIs so dynamic filter UIs can react to comparator changes emitted by filter widgets.
Changes:
- Added
Emailwidget support across backend enum + frontend display/edit/filter widget registries. - Migrated
BaseFilterFieldComponentto signalinput()/output()and updated multiple filter field templates/components/tests accordingly. - Introduced new
EmailFilterComponentandPhoneFilterComponentthat can emit comparator changes (e.g., domain/country modes) to saved-filter and filters dialogs/panels.
Reviewed changes
Copilot reviewed 59 out of 59 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/app/consts/table-display-types.ts | Registers Email display widget mapping. |
| frontend/src/app/consts/record-edit-types.ts | Registers Email edit widget mapping. |
| frontend/src/app/consts/filter-types.ts | Registers new Phone and Email filter widgets. |
| frontend/src/app/components/ui-components/record-edit-fields/base-row-field/base-row-field.component.ts | Adds comparator-change output for edit-field base component. |
| frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.ts | Refactors timezone list building and allow-null handling. |
| frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.spec.ts | Updates tests for signal inputs. |
| frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/time/time.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/time-interval/time-interval.component.ts | Removes now-unneeded super.ngOnInit() call. |
| frontend/src/app/components/ui-components/filter-fields/time-interval/time-interval.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/text/text.component.ts | Updates autofocus handling to signal input. |
| frontend/src/app/components/ui-components/filter-fields/text/text.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/static-text/static-text.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/select/select.component.ts | Updates widget/structure access to signal inputs. |
| frontend/src/app/components/ui-components/filter-fields/select/select.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/point/point.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/phone/phone.component.ts | Adds phone filter with comparator modes + country autocomplete. |
| frontend/src/app/components/ui-components/filter-fields/phone/phone.component.html | Phone filter UI (comparator + country autocomplete/value input). |
| frontend/src/app/components/ui-components/filter-fields/phone/phone.component.css | Styling for phone filter UI. |
| frontend/src/app/components/ui-components/filter-fields/password/password.component.ts | Updates autofocus handling; removes super init. |
| frontend/src/app/components/ui-components/filter-fields/password/password.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/number/number.component.ts | Updates autofocus handling to signal input. |
| frontend/src/app/components/ui-components/filter-fields/number/number.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/long-text/long-text.component.ts | Updates widgetStructure access to signal input; updates autofocus handling. |
| frontend/src/app/components/ui-components/filter-fields/long-text/long-text.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/json-editor/json-editor.component.ts | Updates label usage to signal input; removes super init. |
| frontend/src/app/components/ui-components/filter-fields/json-editor/json-editor.component.spec.ts | Updates tests for signal inputs/computed label. |
| frontend/src/app/components/ui-components/filter-fields/json-editor/json-editor.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/id/id.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts | Updates relations access to signal input. |
| frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.spec.ts | Updates tests for signal inputs and relations access. |
| frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/email/email.component.ts | Adds email filter with domain mode + comparator emission. |
| frontend/src/app/components/ui-components/filter-fields/email/email.component.spec.ts | Adds tests for email filter behavior and comparator emission. |
| frontend/src/app/components/ui-components/filter-fields/email/email.component.html | Email filter UI (comparator + domain/value input). |
| frontend/src/app/components/ui-components/filter-fields/email/email.component.css | Styling for email filter UI. |
| frontend/src/app/components/ui-components/filter-fields/date/date.component.ts | Removes super init; updates autofocus handling. |
| frontend/src/app/components/ui-components/filter-fields/date/date.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts | Migrates output to output(); updates autofocus handling. |
| frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/country/country.component.ts | Refactors country list building; uses signal inputs for allow-null. |
| frontend/src/app/components/ui-components/filter-fields/country/country.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.ts | Uses signal inputs; removes debug logging. |
| frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.spec.ts | Updates tests for signal inputs. |
| frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/binary-data-caption/binary-data-caption.component.html | Updates template bindings to signal calls. |
| frontend/src/app/components/ui-components/filter-fields/base-filter-field/base-filter-field.component.ts | Converts inputs/outputs to signals; adds onComparatorChange. |
| frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts | Enables signal-based dynamic IO; merges widget registries; handles comparator updates. |
| frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.html | Wires onComparatorChange from dynamic filter widgets; button type fixes. |
| frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.ts | Enables signal-based dynamic IO; merges widget registries; stores comparator from widgets. |
| frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.html | Wires onComparatorChange from dynamic filter widgets; button type fixes. |
| frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.css | Layout adjustments to show column name with filter control. |
| frontend/src/app/components/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.ts | Adds “Email” widget settings placeholder. |
| frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.ts | Enables signal-based dynamic IO; merges widget registries; stores comparator from widgets. |
| frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.spec.ts | Adds tests for comparator propagation and widget-object format. |
| frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.html | Wires onComparatorChange from dynamic filter widgets; button type fixes. |
| frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.css | Layout adjustments to show column name with filter control. |
| frontend/package.json | Moves type/tooling deps to devDependencies; expands Yarn resolutions set. |
| backend/src/enums/widget-type.enum.ts | Adds Email widget type to backend enum. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+37
to
+42
| const BASE_TIMEZONES: { value: string; label: string }[] = Intl.supportedValuesOf('timeZone') | ||
| .map((tz) => ({ | ||
| value: tz, | ||
| label: `${tz} (UTC${getTimezoneOffset(tz)})`, | ||
| })) | ||
| .sort((a, b) => a.value.localeCompare(b.value)); |
There was a problem hiding this comment.
BASE_TIMEZONES is computed at module load using Intl.supportedValuesOf('timeZone'). If supportedValuesOf is missing (older browsers, some test/SSR environments), the import will throw before the component is created. Consider computing the list lazily in ngOnInit (or a memoized helper) with a feature-check/fallback, and update the timezone option type to allow the { value: null } entry explicitly (e.g., string | null).
Comment on lines
+19
to
+25
| <mat-autocomplete #auto="matAutocomplete" [displayWith]="displayFn"> | ||
| @for (country of filteredCountries | async; track country.code) { | ||
| <mat-option [value]="country" (onSelectionChange)="country && onCountrySelected(country)"> | ||
| <span class="country-flag">{{country.flag}}</span> | ||
| <span class="country-name">{{country.name}}</span> | ||
| <span class="country-dial-code">({{country.dialCode}})</span> | ||
| </mat-option> |
There was a problem hiding this comment.
Using (onSelectionChange) on each mat-option will fire on both selection and deselection, so onCountrySelected() can run multiple times and emit comparator/value unexpectedly. Prefer handling country selection via the autocomplete's (optionSelected) event (or gate on $event.isUserInput && $event.source.selected).
| @@ -1,5 +1,5 @@ | |||
| import { CommonModule } from '@angular/common'; | |||
| import { Component, EventEmitter, Input, OnDestroy, OnInit, Output } from '@angular/core'; | |||
| import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, SimpleChanges } from '@angular/core'; | |||
There was a problem hiding this comment.
OnChanges is imported but the class doesn’t implement OnChanges (and the symbol isn’t otherwise referenced). Either add implements OnChanges to match the ngOnChanges hook or drop OnChanges from the import to avoid dead imports.
Suggested change
| import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, SimpleChanges } from '@angular/core'; | |
| import { Component, EventEmitter, Input, OnDestroy, OnInit, Output, SimpleChanges } from '@angular/core'; |
Comment on lines
+203
to
+204
| // EmailFilterComponent emits 'eq' (default mode) in ngAfterViewInit, | ||
| // which updateComparatorFromComponent receives |
There was a problem hiding this comment.
The comment says EmailFilterComponent emits 'eq' in ngAfterViewInit, but the implementation only emits when filterMode !== 'eq'. Updating/removing the comment would prevent misleading future readers of the test.
Suggested change
| // EmailFilterComponent emits 'eq' (default mode) in ngAfterViewInit, | |
| // which updateComparatorFromComponent receives | |
| // Rendering the EmailFilterComponent should not change the comparator; | |
| // it remains the 'eq' value that addFilter already set. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts (1)
158-174:⚠️ Potential issue | 🟡 MinorAdd null guard for
relsbefore accessing its properties.The
relsvariable is captured fromthis.relations()but is not null-checked before accessingrels.referenced_table_nameandrels.referenced_column_name. Ifrelations()returnsundefined, this will cause a runtime error.🛡️ Suggested fix
async fetchSuggestions(): Promise<void> { const currentRow = this.suggestions()?.find( (suggestion) => suggestion.displayString === this.currentDisplayedString, ); const rels = this.relations(); + if (!rels) return; if (currentRow !== undefined) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts` around lines 158 - 174, The code calls this.relations() into rels and immediately accesses rels.referenced_table_name and rels.referenced_column_name inside the fetchTable call; add a null/undefined guard for rels (from relations()) before any property access—e.g., if rels is falsy, handle by early returning or setting a safe fallback and avoid calling this._tables.fetchTable; ensure the earlier block that sets this.currentFieldValue and emits onFieldChange also accounts for rels being undefined so no property access occurs; update references to rels.referenced_table_name and rels.referenced_column_name in the fetchTable invocation accordingly.frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts (1)
52-52:⚠️ Potential issue | 🟡 MinorClass should implement
OnChangesinterface.The class imports
OnChangesandSimpleChanges(line 2), and implementsngOnChanges(line 243), but the class declaration only implementsOnInit, OnDestroy. AddOnChangesto the implements clause for proper interface compliance.🔧 Proposed fix
-export class SavedFiltersPanelComponent implements OnInit, OnDestroy { +export class SavedFiltersPanelComponent implements OnInit, OnDestroy, OnChanges {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts` at line 52, The SavedFiltersPanelComponent class currently implements OnInit and OnDestroy but also defines ngOnChanges and imports OnChanges and SimpleChanges; update the class declaration to also implement OnChanges so the class signature matches its lifecycle method (modify the implements clause on SavedFiltersPanelComponent to include OnChanges and keep OnInit and OnDestroy intact), ensuring TypeScript interface compliance for ngOnChanges.frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.ts (1)
54-54:⚠️ Potential issue | 🟡 MinorType mismatch:
nullvalue in array typed as{ value: string; label: string }[].Line 66 adds
{ value: null, label: '' }to an array typed as{ value: string; label: string }[]. This is a type inconsistency that TypeScript may flag depending on strict null checks.🛠️ Proposed fix to allow null values
- public timezones: { value: string; label: string }[] = []; + public timezones: { value: string | null; label: string }[] = [];Also update
BASE_TIMEZONEStype annotation:-const BASE_TIMEZONES: { value: string; label: string }[] = Intl.supportedValuesOf('timeZone') +const BASE_TIMEZONES: { value: string | null; label: string }[] = Intl.supportedValuesOf('timeZone')Also applies to: 66-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.ts` at line 54, The timezones array and BASE_TIMEZONES contain an element with value null but are typed as { value: string; label: string }[], causing a type mismatch; update the type to allow null (e.g., { value: string | null; label: string }[]) or change the inserted value to an empty string so it matches the existing type. Locate the public timezones declaration and the BASE_TIMEZONES definition and make the types consistent (either broaden value to string | null on both, or remove/null usage and ensure all value fields are strings). Ensure any usages of timezones or BASE_TIMEZONES (e.g., in selection components) handle the updated nullable type if you choose string | null.frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.ts (1)
226-228:⚠️ Potential issue | 🟡 Minor
updateComparatordoesn't updatetableRowFieldsComparatorfor the field.This method handles UI-driven comparator changes but doesn't update
tableRowFieldsComparator[fieldName]likeupdateComparatorFromComponentdoes. This could lead to inconsistent state if both methods are expected to keep comparator state in sync.🔧 Proposed fix to sync comparator state
updateComparator(event, fieldName: string) { + this.tableRowFieldsComparator[fieldName] = event; if (event === 'empty') this.tableRowFieldsShown[fieldName] = ''; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.ts` around lines 226 - 228, The updateComparator method only updates tableRowFieldsShown and fails to sync tableRowFieldsComparator for the given field, causing inconsistent comparator state; modify updateComparator (the function named updateComparator) so that when handling the 'empty' event it also sets tableRowFieldsComparator[fieldName] = '' (and mirror any other comparator value assignments the same way updateComparatorFromComponent does) to keep comparator state in sync with tableRowFieldsShown for the specified fieldName.
🧹 Nitpick comments (20)
frontend/src/app/components/ui-components/filter-fields/time-interval/time-interval.component.html (1)
2-2: Use@ifinstead of*ngIffor new code.The template uses
*ngIfstructural directive, but the coding guidelines require using Angular's built-in control flow syntax in all new code.♻️ Suggested fix
- <span class="label">{{normalizedLabel()}} <span *ngIf="required()">*</span></span> + <span class="label">{{normalizedLabel()}} `@if` (required()) {<span>*</span>}</span>As per coding guidelines: "Use built-in control flow syntax (
@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch) in all new code"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/time-interval/time-interval.component.html` at line 2, Replace the structural directive usage: the inner span currently uses "*ngIf" to conditionally render the required asterisk; update the template to use the built-in control-flow syntax instead (use "@if" with the same predicate). Concretely, remove the "*ngIf" on the span that contains the asterisk and change it to use `@if`="required()" while keeping the surrounding binding to normalizedLabel() unchanged; no other logic changes are needed in normalizedLabel() or required().frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.html (1)
1-1: Use@if/@elseinstead of*ngIfwith template reference.The template uses
*ngIfwithelsetemplate reference, but coding guidelines require built-in control flow syntax in new code.♻️ Suggested refactor
-<div *ngIf="isRadiogroup; else checkboxElement" class="radio-line"> +@if (isRadiogroup) { +<div class="radio-line"> <label id="">{{normalizedLabel()}}</label> ... </div> - -<ng-template `#checkboxElement`> +} `@else` { <div class="radio-line"> ... </div> -</ng-template> +}As per coding guidelines: "Use built-in control flow syntax (
@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch) in all new code"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.html` at line 1, Replace the structural directive usage in the template by changing the "*ngIf" with the built-in control-flow syntax: use `@if` on the element that currently has "*ngIf='isRadiogroup; else checkboxElement'" and convert the referenced else block (currently named "checkboxElement") to use the corresponding `@else` form (e.g., an <ng-template> or element with `@else`) so the same conditional rendering behavior remains; update any template reference names if needed to match the new `@else` usage and ensure the condition still checks the isRadiogroup symbol.frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.html (1)
25-29: Guardrelations()before composing router links.These links assume
relations()is always present. A guarded alias avoids brittle dereferences and duplicate calls.💡 Suggested refactor
- <mat-hint>Improve search performance by configuring <em>Foreign key search fields</em> - <a routerLink="/dashboard/{{connectionID}}/{{relations().referenced_table_name}}/settings" target="_blank" class="hint-link">here</a>. - </mat-hint> + `@if` (relations(); as rel) { + <mat-hint>Improve search performance by configuring <em>Foreign key search fields</em> + <a routerLink="/dashboard/{{connectionID}}/{{rel.referenced_table_name}}/settings" target="_blank" class="hint-link">here</a>. + </mat-hint> + } @@ - <a routerLink="/dashboard/{{connectionID}}/{{relations().referenced_table_name}}/entry" + `@if` (relations(); as rel) { + <a routerLink="/dashboard/{{connectionID}}/{{rel.referenced_table_name}}/entry" [queryParams]="currentFieldQueryParams" target="_blank" class="foreign-key__link"> @@ - </a> + </a> + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.html` around lines 25 - 29, The routerLink anchors assume relations() always returns a value and call it twice; wrap these link elements in a guard (e.g., *ngIf="relations() as rel") or otherwise assign relations() to a local/template variable and then use rel.referenced_table_name when composing the routerLink values (and keep using connectionID and currentFieldQueryParams as before) so you avoid brittle dereferences and duplicate calls to relations().frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts (1)
19-19: Avoidanytype in output signal.The
output<any>()should use a specific type. Based on theonDateChangeandonTimeChangemethods, astring(datetime ISO format) is emitted.♻️ Suggested fix
- override readonly onFieldChange = output<any>(); + override readonly onFieldChange = output<string>();As per coding guidelines: "Avoid any types - use specific types or generics instead".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts` at line 19, The output signal onFieldChange is typed as output<any>() but based on the onDateChange and onTimeChange methods it always emits a datetime ISO string; change the generic from any to string (i.e., output<string>()) and update any related consumers/handlers to expect a string to eliminate the any type and ensure type safety for onFieldChange.frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.html (2)
41-44: Minor formatting inconsistency inndcDynamicOutputsbinding.Line 43 has inconsistent indentation compared to the surrounding code - the
onComparatorChangeline appears to be missing leading whitespace alignment.🔧 Suggested fix for formatting
[ndcDynamicOutputs]="{ onFieldChange: { handler: updateField, args: ['$event', value.key] }, - onComparatorChange: { handler: updateComparatorFromComponent, args: ['$event', value.key] } + onComparatorChange: { handler: updateComparatorFromComponent, args: ['$event', value.key] } }"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.html` around lines 41 - 44, Align the indentation inside the ndcDynamicOutputs binding so both entries line up consistently; adjust the whitespace for the onComparatorChange line in the template containing ndcDynamicOutputs, keeping the same keys and handlers (onFieldChange -> updateField(args: ['$event', value.key]) and onComparatorChange -> updateComparatorFromComponent(args: ['$event', value.key])) to match the surrounding lines and maintain consistent formatting.
57-61: Same indentation inconsistency in thedefaultTableFieldtemplate.Line 60 has the same indentation issue as line 43.
🔧 Suggested fix for formatting
[ndcDynamicOutputs]="{ onFieldChange: { handler: updateField, args: ['$event', value.key] }, - onComparatorChange: { handler: updateComparatorFromComponent, args: ['$event', value.key] } + onComparatorChange: { handler: updateComparatorFromComponent, args: ['$event', value.key] } }"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.html` around lines 57 - 61, Fix the indentation inconsistency in the defaultTableField template by aligning the ndcDynamicOutputs block to the same indentation as the surrounding attributes; adjust the lines containing "onFieldChange: { handler: updateField, args: ['$event', value.key] }" and "onComparatorChange: { handler: updateComparatorFromComponent, args: ['$event', value.key] }" so they match the attribute indentation style used elsewhere (same as line 43), ensuring consistent formatting for the updateField and updateComparatorFromComponent handlers and their args.frontend/src/app/components/ui-components/filter-fields/select/select.component.ts (1)
14-15: Consider migratingvalueinput to signal-based API.The
@Input() valueproperty uses the legacy decorator pattern while other inputs in the base class have been migrated to signals. For consistency with the signal migration effort, consider usinginput<string>()instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/select/select.component.ts` around lines 14 - 15, The SelectFilterComponent currently declares a legacy `@Input`() value: string;—migrate this to the signal-based API by replacing the decorator-backed property with a signal using input<string>() on the component (mirror how other inputs on BaseFilterFieldComponent were migrated); update any internal uses to read from the signal (e.g., value() or value.get()) and ensure any bindings from parent templates still bind to the input signal so SelectFilterComponent, BaseFilterFieldComponent, and consumers remain consistent.frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.spec.ts (1)
183-206: Test verifies comparator stability but assertions are redundant.The test asserts
tableRowFieldsComparator['FirstName']equals'eq'both before (line 197) and after (line 205) change detection. If the intent is to verify the comparator doesn't change unexpectedly during render, consider adding a comment clarifying this, or testing that a different comparator value is properly received.💡 Consider clarifying the test intent
// Default comparator is 'eq' from addFilter expect(component.tableRowFieldsComparator['FirstName']).toEqual('eq'); // Trigger change detection so ndc-dynamic renders EmailFilterComponent fixture.detectChanges(); await fixture.whenStable(); - // EmailFilterComponent emits 'eq' (default mode) in ngAfterViewInit, - // which updateComparatorFromComponent receives + // Verify comparator remains stable after EmailFilterComponent renders + // and emits its default 'eq' value via onComparatorChange expect(component.tableRowFieldsComparator['FirstName']).toEqual('eq');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.spec.ts` around lines 183 - 206, The test redundantly asserts component.tableRowFieldsComparator['FirstName'] === 'eq' both before and after rendering; either remove the pre-render assertion or change the post-render assertion to confirm a different expected outcome (e.g., simulate EmailFilterComponent emitting a different comparator) and/or add a clear comment about verifying stability. Update the spec around component.setWidgets, component.addFilter, fixture.detectChanges/whenStable and the EmailFilterComponent/updateComparatorFromComponent interaction so the intent is explicit: either assert only once that the default is 'eq' before render, or simulate/emulate an emitted comparator and assert the post-render value changes accordingly, documenting the purpose in a brief comment.frontend/src/app/components/ui-components/filter-fields/email/email.component.spec.ts (2)
40-49: Same concern: preferfixture.detectChanges()over manual lifecycle calls.Setting
component.valuebefore detection and then calling lifecycle hooks manually may not reflect how Angular initializes components. Consider setting the input viafixture.componentRef.setInput('value', '@example.com')followed byfixture.detectChanges().♻️ Suggested approach
it('should emit endswith comparator when initialized with `@domain` value', () => { - component.value = '@example.com'; + fixture.componentRef.setInput('value', '@example.com'); const emitted: string[] = []; component.onComparatorChange.subscribe((v: string) => emitted.push(v)); - component.ngOnInit(); - component.ngAfterViewInit(); + fixture.detectChanges(); expect(emitted).toEqual(['endswith']); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/email/email.component.spec.ts` around lines 40 - 49, Replace the manual lifecycle calls and direct input assignment with Angular TestBed fixture APIs: set the input using fixture.componentRef.setInput('value', '@example.com') and then call fixture.detectChanges() instead of manually invoking component.ngOnInit() and component.ngAfterViewInit(); this will ensure the component (the email component under test) initializes naturally and emits via onComparatorChange as expected.
30-38: Consider usingfixture.detectChanges()before lifecycle hooks.The test manually calls
ngOnInit()andngAfterViewInit()without first triggering change detection. Angular'sTestBednormally calls these duringfixture.detectChanges(). Calling them manually may lead to inconsistent component state.♻️ Suggested approach
it('should not emit comparator in default eq mode', () => { const emitted: string[] = []; component.onComparatorChange.subscribe((v: string) => emitted.push(v)); - component.ngOnInit(); - component.ngAfterViewInit(); + fixture.detectChanges(); expect(emitted).toEqual([]); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/email/email.component.spec.ts` around lines 30 - 38, The test is manually invoking ngOnInit() and ngAfterViewInit() which can produce inconsistent state; instead, keep the onComparatorChange subscription setup but call fixture.detectChanges() to run Angular's lifecycle and change detection (and remove the explicit component.ngOnInit() / component.ngAfterViewInit() calls), then assert that the emitted array is empty; this ensures the component's bindings and view initialization are performed the same way TestBed expects.frontend/src/app/components/ui-components/filter-fields/select/select.component.html (1)
7-10: Migrate*ngForto built-in@forcontrol flow syntax withtrack.The template uses the legacy
*ngFordirective which should be updated to Angular's built-in control flow syntax with atrackfunction for better performance.♻️ Suggested fix
- <mat-option *ngFor="let option of options" - [value]="option.value"> - {{option.label}} - </mat-option> + `@for` (option of options; track option.value) { + <mat-option [value]="option.value"> + {{option.label}} + </mat-option> + }As per coding guidelines: "Use built-in control flow syntax (
@if,@for,@switch) instead of structural directives" and "Usetrackfunction in@forloops for better performance".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/select/select.component.html` around lines 7 - 10, Replace the legacy *ngFor on the <mat-option> loop with the built-in control-flow `@for` syntax and add a track function for stable identities: change the loop to use `@for`="let option of options; trackBy: trackOption" (or `@for`="let option of options; track: option.value" if you prefer inline) and implement a trackOption function in the corresponding component that returns option.value (or option.id) so bindings [value]="option.value" and interpolation {{option.label}} remain unchanged; update any references to "options" or "option" in the template accordingly.frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.ts (1)
16-16: Add type annotation to@Input() value.The
valueinput lacks a type annotation, resulting in an implicitanytype. This violates the coding guidelines requiring explicit type annotations.♻️ Suggested fix
- `@Input`() value; + `@Input`() value: boolean | string | number | null;As per coding guidelines: "Always add type annotations to function parameters and return types in TypeScript" and "Avoid any types - use specific types or generics instead".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.ts` at line 16, The `@Input`() value property in the BooleanComponent lacks a type annotation (currently implicit any); update the declaration of value in the BooleanComponent class to an explicit type (for example boolean, boolean | null, or boolean | undefined as appropriate for your component's logic) so the input is strongly typed and aligns with the project's TypeScript guidelines.frontend/src/app/components/ui-components/filter-fields/id/id.component.html (1)
9-9: Migrate*ngIfto built-in@ifcontrol flow syntax.The template uses the legacy
*ngIfdirective which should be updated to Angular's built-in control flow syntax per coding guidelines.♻️ Suggested fix
- <mat-error *ngIf="idField.errors?.pattern">Value doesn't match pattern.</mat-error> + `@if` (idField.errors?.pattern) { + <mat-error>Value doesn't match pattern.</mat-error> + }As per coding guidelines: "Use built-in control flow syntax (
@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch) in all new code".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/id/id.component.html` at line 9, The template uses the legacy structural directive "*ngIf" on the mat-error; replace it with Angular's built-in control-flow syntax by changing the conditional to the `@if` form (e.g. use `@if`="idField.errors?.pattern" on the mat-error or an enclosing ng-container) so the mat-error only renders when idField.errors?.pattern is truthy; update the id component template where "idField" and the mat-error element are defined to remove "*ngIf" and use "@if" instead.frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts (1)
45-45: Add type annotation to@Input() value.The
valueinput lacks a type annotation, resulting in an implicitanytype.♻️ Suggested fix
- `@Input`() value; + `@Input`() value: unknown;As per coding guidelines: "Always add type annotations to function parameters and return types in TypeScript" and "Avoid any types".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts` at line 45, The `@Input`() property named "value" in the ForeignKeyComponent is missing a TypeScript type and is implicitly any; update the declaration of the Input property (the field annotated with `@Input`() value in foreign-key.component.ts / the ForeignKeyComponent class) to an explicit type that matches expected foreign-key values (for example string | number | null or a specific FK interface), or create and use a shared type alias/DTO if one exists, and adjust any usages/assignments in the component/template to satisfy that type.frontend/src/app/components/ui-components/filter-fields/base-filter-field/base-filter-field.component.ts (1)
26-26: Consider removing emptyngOnInitor adding a comment.The empty
ngOnInitis a remnant from the previous implementation. It can be removed unless intentionally kept for subclass compatibility.♻️ Option 1: Remove if not needed
- ngOnInit(): void {}Also remove
OnInitfrom the class declaration and imports if not needed.♻️ Option 2: Add comment if intentionally kept
- ngOnInit(): void {} + // Intentionally empty - kept for subclass compatibility + ngOnInit(): void {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/base-filter-field/base-filter-field.component.ts` at line 26, The empty lifecycle hook ngOnInit in BaseFilterFieldComponent is unused; either remove the ngOnInit method and also remove the OnInit import and the implements OnInit from the class declaration, or if it's intentionally left for subclass compatibility add a brief comment above ngOnInit explaining why it is intentionally empty (e.g., "Intentionally empty: extension point for subclasses"). Reference: BaseFilterFieldComponent, ngOnInit, OnInit.frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.ts (1)
52-52: Consider migrating@Input() valueto signal-basedinput()for consistency.The base class
BaseFilterFieldComponenthas been migrated to use Angular signals (input()/output()), but this component still uses the legacy@Input()decorator. For consistency with the PR's migration effort, consider updating this to useinput<string>().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.ts` at line 52, Replace the legacy `@Input`() value property with the signal-based input() in TimezoneComponent to match BaseFilterFieldComponent: remove the `@Input`() decorator and create a reactive signal via input<string>() (importing input from `@angular/core`) and update all usages of the value property inside TimezoneComponent (and its template) to read from the new signal (e.g., value()) or subscribe as needed; ensure type is string and keep the property name "value" so BaseFilterFieldComponent's integrations continue to work.frontend/src/app/components/ui-components/filter-fields/phone/phone.component.ts (2)
97-102: Private method should use underscore prefix per coding guidelines.The method
setupAutocompleteis effectively private (only called fromngOnInit). Per coding guidelines, private members should be prefixed with underscore.♻️ Proposed fix
- private setupAutocomplete(): void { + private _setupAutocomplete(): void { this.filteredCountries = this.countryControl.valueChanges.pipe( startWith(''), map((value) => this._filter(typeof value === 'string' ? value : value?.name || '')), ); }And update the call in
ngOnInit:ngOnInit(): void { - this.setupAutocomplete(); + this._setupAutocomplete();As per coding guidelines: "Prefix private members with underscore (e.g.,
_privateMethod,_http)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/phone/phone.component.ts` around lines 97 - 102, The private method setupAutocomplete should be renamed to _setupAutocomplete and its internal references kept intact; update the call site in ngOnInit to call _setupAutocomplete instead, leaving the observable logic using filteredCountries, countryControl, and _filter unchanged so behavior is preserved while following the private-member underscore convention.
37-37: Consider migrating@Input() valueto signal-basedinput()for consistency.Similar to other filter components, this uses the legacy
@Input()decorator while the base class has been migrated to signals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/phone/phone.component.ts` at line 37, The PhoneComponent currently declares a legacy input property "@Input() value: string"; replace it with a signal-based input by removing that decorator/property and adding a signal input using Angular's input API (e.g. import { input } from '@angular/core' and add public readonly value = input<string>();) within the PhoneComponent class so it matches the base class signal usage; update any internal reads/assignments to use the signal API (read via value() in TS or template and set via value.set(...) if needed) and remove any other direct assignments to the old property.frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts (1)
288-288: Emptynextcallback in subscribe is acceptable but consider removing if not needed.The empty callback
next: () => {}serves as a placeholder. If no action is needed on success, you could use the shorthand form or add a comment explaining the intentional no-op.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts` at line 288, In the subscribe call inside SavedFiltersPanelComponent (saved-filters-panel.component.ts) there is an empty observer property `next: () => {}`; remove that no-op `next` from the observer object (or replace the observer-style call with the shorthand subscribe(...) form if you only handle error/complete), or if the empty callback is intentional add a brief comment like `// intentional no-op` next to `next` to make intent explicit; target the subscribe invocation in this component to apply the change.frontend/src/app/components/ui-components/filter-fields/email/email.component.ts (1)
16-17: Consider migrating@Input() valueto signal-basedinput()for consistency with base class.The base class now uses Angular signals. For full consistency with the PR's migration effort, consider using
input<string>()instead of@Input().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/email/email.component.ts` around lines 16 - 17, Replace the plain `@Input`() value: string property with a signal-based input to match the base class migration: create a WritableSignal/Signal for the value (e.g., value = signal<string>('')) and expose it via the signal-style input API used by the base class (use the input<string>() helper or the base-class-provided input integration), and update any usage in this EmailComponent to read/write the signal instead of the raw string; keep the existing inputElement ViewChild intact. Ensure imports are updated to bring in signal/input helper and that the component forwards incoming input updates into the signal so the component and base class behave consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/components/ui-components/filter-fields/country/country.component.ts`:
- Around line 62-64: Update the displayFn signature to accept a properly typed
parameter instead of any: use CountryOption | string | null for the parameter
and adjust the function body to handle the three cases (when it's a
CountryOption return its label, when it's a string return the string, and when
it's null return an empty string); update references to displayFn in the
component/template if needed to satisfy the new type.
- Around line 57-60: The selection handler onCountrySelected currently assigns
null into this.value (declared as string) when the special "allow_null" option
is chosen; pick one consistent contract: either make the `@Input`() value type
nullable or coerce the null option to an empty string. To keep the existing
string contract change onCountrySelected to map a selectedCountry.value === null
to '' before assigning and emitting (referencing onCountrySelected,
CountryOption and the value property), or alternatively update the value
declaration and any downstream consumers to accept string | null and propagate
that type change throughout.
In
`@frontend/src/app/components/ui-components/filter-fields/email/email.component.ts`:
- Around line 41-55: onFilterModeChange currently sets this.value to
`@${this.domainValue}` when entering 'domain' but does not remove the '@' prefix
when switching out, causing stale domain-formatted values to be emitted; update
onFilterModeChange to normalize/clear the value when leaving 'domain' (e.g., if
previous mode was 'domain' and new mode !== 'domain', remove a leading '@' or
clear value before emitting), while keeping existing behavior for entering
'domain' (use this.domainValue) and still emitting the correct comparator via
this.onComparatorChange and the normalized value via this.onFieldChange;
reference: onFilterModeChange, filterMode, domainValue, value,
onComparatorChange, onFieldChange.
In
`@frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts`:
- Around line 215-220: getModifiedRow currently calls this.relations() into rels
and uses rels.autocomplete_columns without checking rels; add a null/undefined
guard so the function returns the original row (or a shallow copy) if rels is
falsy or rels.autocomplete_columns is falsy/empty. Specifically, in
getModifiedRow (and where rels.autocomplete_columns is read and passed to
splice/indexOf), first verify rels && Array.isArray(rels.autocomplete_columns)
before using it and only perform the identityColumn removal when the array
exists; this prevents runtime errors when this.relations() returns undefined.
In
`@frontend/src/app/components/ui-components/filter-fields/phone/phone.component.css`:
- Around line 32-35: The mat-option rules in phone.component.css won't reach
overlay options because they're rendered in the CDK overlay; update the template
to add panelClass attributes to the mat-select and mat-autocomplete elements
(e.g. "phone-select-panel" and "phone-autocomplete-panel") and move the
mat-option CSS into a global stylesheet using selectors like
".phone-select-panel mat-option" and ".phone-autocomplete-panel mat-option" to
apply display:flex and align-items:center; as an alternative fallback, you can
keep the rules in the component by using ::ng-deep before the mat-option
selector.
In
`@frontend/src/app/components/ui-components/filter-fields/point/point.component.html`:
- Around line 5-10: The inputs only respect readonly() for their disabled state;
update the [disabled] bindings on the point inputs (the input binding(s) inside
the point component template, e.g., the inputs bound with [(ngModel)]="value.x"
and [(ngModel)]="value.y") to also consider disabled() by changing
[disabled]="readonly()" to [disabled]="readonly() || disabled()" so they become
disabled when either flag is true.
---
Outside diff comments:
In
`@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.ts`:
- Around line 226-228: The updateComparator method only updates
tableRowFieldsShown and fails to sync tableRowFieldsComparator for the given
field, causing inconsistent comparator state; modify updateComparator (the
function named updateComparator) so that when handling the 'empty' event it also
sets tableRowFieldsComparator[fieldName] = '' (and mirror any other comparator
value assignments the same way updateComparatorFromComponent does) to keep
comparator state in sync with tableRowFieldsShown for the specified fieldName.
In
`@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts`:
- Line 52: The SavedFiltersPanelComponent class currently implements OnInit and
OnDestroy but also defines ngOnChanges and imports OnChanges and SimpleChanges;
update the class declaration to also implement OnChanges so the class signature
matches its lifecycle method (modify the implements clause on
SavedFiltersPanelComponent to include OnChanges and keep OnInit and OnDestroy
intact), ensuring TypeScript interface compliance for ngOnChanges.
In
`@frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts`:
- Around line 158-174: The code calls this.relations() into rels and immediately
accesses rels.referenced_table_name and rels.referenced_column_name inside the
fetchTable call; add a null/undefined guard for rels (from relations()) before
any property access—e.g., if rels is falsy, handle by early returning or setting
a safe fallback and avoid calling this._tables.fetchTable; ensure the earlier
block that sets this.currentFieldValue and emits onFieldChange also accounts for
rels being undefined so no property access occurs; update references to
rels.referenced_table_name and rels.referenced_column_name in the fetchTable
invocation accordingly.
In
`@frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.ts`:
- Line 54: The timezones array and BASE_TIMEZONES contain an element with value
null but are typed as { value: string; label: string }[], causing a type
mismatch; update the type to allow null (e.g., { value: string | null; label:
string }[]) or change the inserted value to an empty string so it matches the
existing type. Locate the public timezones declaration and the BASE_TIMEZONES
definition and make the types consistent (either broaden value to string | null
on both, or remove/null usage and ensure all value fields are strings). Ensure
any usages of timezones or BASE_TIMEZONES (e.g., in selection components) handle
the updated nullable type if you choose string | null.
---
Nitpick comments:
In
`@frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.html`:
- Around line 41-44: Align the indentation inside the ndcDynamicOutputs binding
so both entries line up consistently; adjust the whitespace for the
onComparatorChange line in the template containing ndcDynamicOutputs, keeping
the same keys and handlers (onFieldChange -> updateField(args: ['$event',
value.key]) and onComparatorChange -> updateComparatorFromComponent(args:
['$event', value.key])) to match the surrounding lines and maintain consistent
formatting.
- Around line 57-61: Fix the indentation inconsistency in the defaultTableField
template by aligning the ndcDynamicOutputs block to the same indentation as the
surrounding attributes; adjust the lines containing "onFieldChange: { handler:
updateField, args: ['$event', value.key] }" and "onComparatorChange: { handler:
updateComparatorFromComponent, args: ['$event', value.key] }" so they match the
attribute indentation style used elsewhere (same as line 43), ensuring
consistent formatting for the updateField and updateComparatorFromComponent
handlers and their args.
In
`@frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.spec.ts`:
- Around line 183-206: The test redundantly asserts
component.tableRowFieldsComparator['FirstName'] === 'eq' both before and after
rendering; either remove the pre-render assertion or change the post-render
assertion to confirm a different expected outcome (e.g., simulate
EmailFilterComponent emitting a different comparator) and/or add a clear comment
about verifying stability. Update the spec around component.setWidgets,
component.addFilter, fixture.detectChanges/whenStable and the
EmailFilterComponent/updateComparatorFromComponent interaction so the intent is
explicit: either assert only once that the default is 'eq' before render, or
simulate/emulate an emitted comparator and assert the post-render value changes
accordingly, documenting the purpose in a brief comment.
In
`@frontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.ts`:
- Line 288: In the subscribe call inside SavedFiltersPanelComponent
(saved-filters-panel.component.ts) there is an empty observer property `next: ()
=> {}`; remove that no-op `next` from the observer object (or replace the
observer-style call with the shorthand subscribe(...) form if you only handle
error/complete), or if the empty callback is intentional add a brief comment
like `// intentional no-op` next to `next` to make intent explicit; target the
subscribe invocation in this component to apply the change.
In
`@frontend/src/app/components/ui-components/filter-fields/base-filter-field/base-filter-field.component.ts`:
- Line 26: The empty lifecycle hook ngOnInit in BaseFilterFieldComponent is
unused; either remove the ngOnInit method and also remove the OnInit import and
the implements OnInit from the class declaration, or if it's intentionally left
for subclass compatibility add a brief comment above ngOnInit explaining why it
is intentionally empty (e.g., "Intentionally empty: extension point for
subclasses"). Reference: BaseFilterFieldComponent, ngOnInit, OnInit.
In
`@frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.html`:
- Line 1: Replace the structural directive usage in the template by changing the
"*ngIf" with the built-in control-flow syntax: use `@if` on the element that
currently has "*ngIf='isRadiogroup; else checkboxElement'" and convert the
referenced else block (currently named "checkboxElement") to use the
corresponding `@else` form (e.g., an <ng-template> or element with `@else`) so the
same conditional rendering behavior remains; update any template reference names
if needed to match the new `@else` usage and ensure the condition still checks the
isRadiogroup symbol.
In
`@frontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.ts`:
- Line 16: The `@Input`() value property in the BooleanComponent lacks a type
annotation (currently implicit any); update the declaration of value in the
BooleanComponent class to an explicit type (for example boolean, boolean | null,
or boolean | undefined as appropriate for your component's logic) so the input
is strongly typed and aligns with the project's TypeScript guidelines.
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts`:
- Line 19: The output signal onFieldChange is typed as output<any>() but based
on the onDateChange and onTimeChange methods it always emits a datetime ISO
string; change the generic from any to string (i.e., output<string>()) and
update any related consumers/handlers to expect a string to eliminate the any
type and ensure type safety for onFieldChange.
In
`@frontend/src/app/components/ui-components/filter-fields/email/email.component.spec.ts`:
- Around line 40-49: Replace the manual lifecycle calls and direct input
assignment with Angular TestBed fixture APIs: set the input using
fixture.componentRef.setInput('value', '@example.com') and then call
fixture.detectChanges() instead of manually invoking component.ngOnInit() and
component.ngAfterViewInit(); this will ensure the component (the email component
under test) initializes naturally and emits via onComparatorChange as expected.
- Around line 30-38: The test is manually invoking ngOnInit() and
ngAfterViewInit() which can produce inconsistent state; instead, keep the
onComparatorChange subscription setup but call fixture.detectChanges() to run
Angular's lifecycle and change detection (and remove the explicit
component.ngOnInit() / component.ngAfterViewInit() calls), then assert that the
emitted array is empty; this ensures the component's bindings and view
initialization are performed the same way TestBed expects.
In
`@frontend/src/app/components/ui-components/filter-fields/email/email.component.ts`:
- Around line 16-17: Replace the plain `@Input`() value: string property with a
signal-based input to match the base class migration: create a
WritableSignal/Signal for the value (e.g., value = signal<string>('')) and
expose it via the signal-style input API used by the base class (use the
input<string>() helper or the base-class-provided input integration), and update
any usage in this EmailComponent to read/write the signal instead of the raw
string; keep the existing inputElement ViewChild intact. Ensure imports are
updated to bring in signal/input helper and that the component forwards incoming
input updates into the signal so the component and base class behave
consistently.
In
`@frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.html`:
- Around line 25-29: The routerLink anchors assume relations() always returns a
value and call it twice; wrap these link elements in a guard (e.g.,
*ngIf="relations() as rel") or otherwise assign relations() to a local/template
variable and then use rel.referenced_table_name when composing the routerLink
values (and keep using connectionID and currentFieldQueryParams as before) so
you avoid brittle dereferences and duplicate calls to relations().
In
`@frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts`:
- Line 45: The `@Input`() property named "value" in the ForeignKeyComponent is
missing a TypeScript type and is implicitly any; update the declaration of the
Input property (the field annotated with `@Input`() value in
foreign-key.component.ts / the ForeignKeyComponent class) to an explicit type
that matches expected foreign-key values (for example string | number | null or
a specific FK interface), or create and use a shared type alias/DTO if one
exists, and adjust any usages/assignments in the component/template to satisfy
that type.
In
`@frontend/src/app/components/ui-components/filter-fields/id/id.component.html`:
- Line 9: The template uses the legacy structural directive "*ngIf" on the
mat-error; replace it with Angular's built-in control-flow syntax by changing
the conditional to the `@if` form (e.g. use `@if`="idField.errors?.pattern" on the
mat-error or an enclosing ng-container) so the mat-error only renders when
idField.errors?.pattern is truthy; update the id component template where
"idField" and the mat-error element are defined to remove "*ngIf" and use "@if"
instead.
In
`@frontend/src/app/components/ui-components/filter-fields/phone/phone.component.ts`:
- Around line 97-102: The private method setupAutocomplete should be renamed to
_setupAutocomplete and its internal references kept intact; update the call site
in ngOnInit to call _setupAutocomplete instead, leaving the observable logic
using filteredCountries, countryControl, and _filter unchanged so behavior is
preserved while following the private-member underscore convention.
- Line 37: The PhoneComponent currently declares a legacy input property
"@Input() value: string"; replace it with a signal-based input by removing that
decorator/property and adding a signal input using Angular's input API (e.g.
import { input } from '@angular/core' and add public readonly value =
input<string>();) within the PhoneComponent class so it matches the base class
signal usage; update any internal reads/assignments to use the signal API (read
via value() in TS or template and set via value.set(...) if needed) and remove
any other direct assignments to the old property.
In
`@frontend/src/app/components/ui-components/filter-fields/select/select.component.html`:
- Around line 7-10: Replace the legacy *ngFor on the <mat-option> loop with the
built-in control-flow `@for` syntax and add a track function for stable
identities: change the loop to use `@for`="let option of options; trackBy:
trackOption" (or `@for`="let option of options; track: option.value" if you prefer
inline) and implement a trackOption function in the corresponding component that
returns option.value (or option.id) so bindings [value]="option.value" and
interpolation {{option.label}} remain unchanged; update any references to
"options" or "option" in the template accordingly.
In
`@frontend/src/app/components/ui-components/filter-fields/select/select.component.ts`:
- Around line 14-15: The SelectFilterComponent currently declares a legacy
`@Input`() value: string;—migrate this to the signal-based API by replacing the
decorator-backed property with a signal using input<string>() on the component
(mirror how other inputs on BaseFilterFieldComponent were migrated); update any
internal uses to read from the signal (e.g., value() or value.get()) and ensure
any bindings from parent templates still bind to the input signal so
SelectFilterComponent, BaseFilterFieldComponent, and consumers remain
consistent.
In
`@frontend/src/app/components/ui-components/filter-fields/time-interval/time-interval.component.html`:
- Line 2: Replace the structural directive usage: the inner span currently uses
"*ngIf" to conditionally render the required asterisk; update the template to
use the built-in control-flow syntax instead (use "@if" with the same
predicate). Concretely, remove the "*ngIf" on the span that contains the
asterisk and change it to use `@if`="required()" while keeping the surrounding
binding to normalizedLabel() unchanged; no other logic changes are needed in
normalizedLabel() or required().
In
`@frontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.ts`:
- Line 52: Replace the legacy `@Input`() value property with the signal-based
input() in TimezoneComponent to match BaseFilterFieldComponent: remove the
`@Input`() decorator and create a reactive signal via input<string>() (importing
input from `@angular/core`) and update all usages of the value property inside
TimezoneComponent (and its template) to read from the new signal (e.g., value())
or subscribe as needed; ensure type is string and keep the property name "value"
so BaseFilterFieldComponent's integrations continue to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d56f7634-31b2-4f4c-aa8c-b200d6bc694f
📒 Files selected for processing (59)
backend/src/enums/widget-type.enum.tsfrontend/package.jsonfrontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.cssfrontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.htmlfrontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.spec.tsfrontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.tsfrontend/src/app/components/dashboard/db-table-view/db-table-widgets/db-table-widgets.component.tsfrontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.cssfrontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.htmlfrontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.tsfrontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.htmlfrontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-panel.component.tsfrontend/src/app/components/ui-components/filter-fields/base-filter-field/base-filter-field.component.tsfrontend/src/app/components/ui-components/filter-fields/binary-data-caption/binary-data-caption.component.htmlfrontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.htmlfrontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.spec.tsfrontend/src/app/components/ui-components/filter-fields/boolean/boolean.component.tsfrontend/src/app/components/ui-components/filter-fields/country/country.component.htmlfrontend/src/app/components/ui-components/filter-fields/country/country.component.tsfrontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.htmlfrontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.tsfrontend/src/app/components/ui-components/filter-fields/date/date.component.htmlfrontend/src/app/components/ui-components/filter-fields/date/date.component.tsfrontend/src/app/components/ui-components/filter-fields/email/email.component.cssfrontend/src/app/components/ui-components/filter-fields/email/email.component.htmlfrontend/src/app/components/ui-components/filter-fields/email/email.component.spec.tsfrontend/src/app/components/ui-components/filter-fields/email/email.component.tsfrontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.htmlfrontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.spec.tsfrontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.tsfrontend/src/app/components/ui-components/filter-fields/id/id.component.htmlfrontend/src/app/components/ui-components/filter-fields/json-editor/json-editor.component.htmlfrontend/src/app/components/ui-components/filter-fields/json-editor/json-editor.component.spec.tsfrontend/src/app/components/ui-components/filter-fields/json-editor/json-editor.component.tsfrontend/src/app/components/ui-components/filter-fields/long-text/long-text.component.htmlfrontend/src/app/components/ui-components/filter-fields/long-text/long-text.component.tsfrontend/src/app/components/ui-components/filter-fields/number/number.component.htmlfrontend/src/app/components/ui-components/filter-fields/number/number.component.tsfrontend/src/app/components/ui-components/filter-fields/password/password.component.htmlfrontend/src/app/components/ui-components/filter-fields/password/password.component.tsfrontend/src/app/components/ui-components/filter-fields/phone/phone.component.cssfrontend/src/app/components/ui-components/filter-fields/phone/phone.component.htmlfrontend/src/app/components/ui-components/filter-fields/phone/phone.component.tsfrontend/src/app/components/ui-components/filter-fields/point/point.component.htmlfrontend/src/app/components/ui-components/filter-fields/select/select.component.htmlfrontend/src/app/components/ui-components/filter-fields/select/select.component.tsfrontend/src/app/components/ui-components/filter-fields/static-text/static-text.component.htmlfrontend/src/app/components/ui-components/filter-fields/text/text.component.htmlfrontend/src/app/components/ui-components/filter-fields/text/text.component.tsfrontend/src/app/components/ui-components/filter-fields/time-interval/time-interval.component.htmlfrontend/src/app/components/ui-components/filter-fields/time-interval/time-interval.component.tsfrontend/src/app/components/ui-components/filter-fields/time/time.component.htmlfrontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.htmlfrontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.spec.tsfrontend/src/app/components/ui-components/filter-fields/timezone/timezone.component.tsfrontend/src/app/components/ui-components/record-edit-fields/base-row-field/base-row-field.component.tsfrontend/src/app/consts/filter-types.tsfrontend/src/app/consts/record-edit-types.tsfrontend/src/app/consts/table-display-types.ts
💤 Files with no reviewable changes (1)
- frontend/src/app/components/ui-components/filter-fields/time-interval/time-interval.component.ts
Comment on lines
+57
to
+60
| onCountrySelected(selectedCountry: CountryOption): void { | ||
| this.value = selectedCountry.value; | ||
| this.onFieldChange.emit(this.value); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify value/emitter contracts and strict nullability settings.
rg -n -C3 '@Input\(\)\s+value|onFieldChange|class BaseFilterFieldComponent' frontend/src/app/components/ui-components/filter-fields
rg -n -C3 'onCountrySelected\(|onFieldChange\.emit|optionSelected' frontend/src/app/components/ui-components/filter-fields/country
fd -i 'tsconfig*.json' | xargs -I{} sh -c 'echo "--- {}"; rg -n "\"strict\"|\"strictNullChecks\"" "{}"'Repository: rocket-admin/rocketadmin
Length of output: 49065
🏁 Script executed:
# Find CountryOption type definition
rg -n 'type CountryOption|interface CountryOption' frontend/
# Search for imports in country component
head -40 frontend/src/app/components/ui-components/filter-fields/country/country.component.ts
# Find allowNull usage
rg -n 'allowNull' frontend/src/app/components/ui-components/filter-fields/country/country.component.tsRepository: rocket-admin/rocketadmin
Length of output: 1784
🏁 Script executed:
# Check the full country component to see allowNull usage
sed -n '40,80p' frontend/src/app/components/ui-components/filter-fields/country/country.component.ts
# Check BaseFilterFieldComponent definition for allowNull
grep -n 'allowNull' frontend/src/app/components/ui-components/filter-fields/base-filter-field/base-filter-field.component.ts
# Check if there's initialization code that handles null
sed -n '45,56p' frontend/src/app/components/ui-components/filter-fields/country/country.component.tsRepository: rocket-admin/rocketadmin
Length of output: 1407
🏁 Script executed:
# Check how other filter components handle allowNull cases
grep -A 5 "allow_null" frontend/src/app/components/ui-components/filter-fields/*/[^b]*.component.ts | head -40
# Verify strict mode is enabled
cat frontend/tsconfig.json | grep -A 5 '"strict"'Repository: rocket-admin/rocketadmin
Length of output: 2825
🏁 Script executed:
# Check tsconfig strict setting
cat frontend/tsconfig.jsonRepository: rocket-admin/rocketadmin
Length of output: 600
🏁 Script executed:
# Verify the complete flow - check select component's Input definition too
rg -n '@Input\(\) value' frontend/src/app/components/ui-components/filter-fields/select/select.component.ts
# Check if any type narrowing or coercion happens elsewhere in country component
wc -l frontend/src/app/components/ui-components/filter-fields/country/country.component.ts
tail -20 frontend/src/app/components/ui-components/filter-fields/country/country.component.tsRepository: rocket-admin/rocketadmin
Length of output: 654
🏁 Script executed:
# Check how select component handles the same scenario - type of value
sed -n '14,30p' frontend/src/app/components/ui-components/filter-fields/select/select.component.ts
# Verify the BaseFilterFieldComponent output type
grep -A 2 'readonly onFieldChange' frontend/src/app/components/ui-components/filter-fields/base-filter-field/base-filter-field.component.ts
# Check if other components that allow null actually type the `@Input` as nullable
rg -B 3 '@Input\(\) value.*string' frontend/src/app/components/ui-components/filter-fields/*/[^b]*.component.ts | grep -A 3 'allow_null'Repository: rocket-admin/rocketadmin
Length of output: 619
🏁 Script executed:
# Double-check the actual tsconfig strict setting
grep -E '"strict"|"strictNullChecks"' frontend/tsconfig.jsonRepository: rocket-admin/rocketadmin
Length of output: 50
🏁 Script executed:
# Check if there's any other config file with strict settings
find frontend -name 'tsconfig*.json' -o -name 'angular.json' | xargs grep -l 'strict' 2>/dev/nullRepository: rocket-admin/rocketadmin
Length of output: 90
Nullability contract is inconsistent in selection flow.
When allow_null is enabled (line 48), the component adds { value: null, label: '', flag: '' } to the options list. Selecting this option causes onCountrySelected to assign null to this.value (declared as string at line 32) and emit it. This breaks the @Input() value: string contract and can cause downstream consumers to receive unexpected null values.
Suggested fix
onCountrySelected(selectedCountry: CountryOption): void {
const nextValue: string | null = selectedCountry.value;
this.value = (nextValue ?? '') as string;
this.onFieldChange.emit(nextValue);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onCountrySelected(selectedCountry: CountryOption): void { | |
| this.value = selectedCountry.value; | |
| this.onFieldChange.emit(this.value); | |
| } | |
| onCountrySelected(selectedCountry: CountryOption): void { | |
| const nextValue: string | null = selectedCountry.value; | |
| this.value = (nextValue ?? '') as string; | |
| this.onFieldChange.emit(nextValue); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/country/country.component.ts`
around lines 57 - 60, The selection handler onCountrySelected currently assigns
null into this.value (declared as string) when the special "allow_null" option
is chosen; pick one consistent contract: either make the `@Input`() value type
nullable or coerce the null option to an empty string. To keep the existing
string contract change onCountrySelected to map a selectedCountry.value === null
to '' before assigning and emitting (referencing onCountrySelected,
CountryOption and the value property), or alternatively update the value
declaration and any downstream consumers to accept string | null and propagate
that type change throughout.
Comment on lines
+62
to
+64
| displayFn(country: any): string { | ||
| return country ? country.label : ''; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/src/app/components/ui-components/filter-fields/country/country.component.ts | head -100Repository: rocket-admin/rocketadmin
Length of output: 3451
Replace any type with CountryOption | string | null in displayFn parameter.
The parameter uses any at line 62, which weakens type safety. Since countryControl can hold either CountryOption (when an option is selected) or string (while filtering), the method should type the parameter accordingly and handle both cases.
Suggested fix
- displayFn(country: any): string {
- return country ? country.label : '';
- }
+ displayFn(country: CountryOption | string | null): string {
+ if (!country) return '';
+ return typeof country === 'string' ? country : country.label;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/country/country.component.ts`
around lines 62 - 64, Update the displayFn signature to accept a properly typed
parameter instead of any: use CountryOption | string | null for the parameter
and adjust the function body to handle the three cases (when it's a
CountryOption return its label, when it's a string return the string, and when
it's null return an empty string); update references to displayFn in the
component/template if needed to satisfy the new type.
Comment on lines
+41
to
+55
| onFilterModeChange(mode: string): void { | ||
| this.filterMode = mode; | ||
|
|
||
| if (mode === 'domain') { | ||
| this.value = this.domainValue ? `@${this.domainValue}` : ''; | ||
| this.onComparatorChange.emit('endswith'); | ||
| } else if (mode === 'empty') { | ||
| this.value = ''; | ||
| this.onComparatorChange.emit('empty'); | ||
| } else { | ||
| this.onComparatorChange.emit(mode); | ||
| } | ||
|
|
||
| this.onFieldChange.emit(this.value); | ||
| } |
There was a problem hiding this comment.
onFilterModeChange doesn't preserve existing value when switching modes.
When switching from 'domain' mode to 'eq' mode (line 50-51), the existing value is emitted as-is, which may still contain the @ prefix from domain mode. Consider clearing or transforming the value when switching away from domain mode.
🔧 Proposed fix
onFilterModeChange(mode: string): void {
this.filterMode = mode;
if (mode === 'domain') {
this.value = this.domainValue ? `@${this.domainValue}` : '';
this.onComparatorChange.emit('endswith');
} else if (mode === 'empty') {
this.value = '';
this.onComparatorChange.emit('empty');
} else {
+ // Clear domain-prefixed value when switching to eq mode
+ if (this.value?.startsWith('@')) {
+ this.value = '';
+ }
this.onComparatorChange.emit(mode);
}
this.onFieldChange.emit(this.value);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onFilterModeChange(mode: string): void { | |
| this.filterMode = mode; | |
| if (mode === 'domain') { | |
| this.value = this.domainValue ? `@${this.domainValue}` : ''; | |
| this.onComparatorChange.emit('endswith'); | |
| } else if (mode === 'empty') { | |
| this.value = ''; | |
| this.onComparatorChange.emit('empty'); | |
| } else { | |
| this.onComparatorChange.emit(mode); | |
| } | |
| this.onFieldChange.emit(this.value); | |
| } | |
| onFilterModeChange(mode: string): void { | |
| this.filterMode = mode; | |
| if (mode === 'domain') { | |
| this.value = this.domainValue ? `@${this.domainValue}` : ''; | |
| this.onComparatorChange.emit('endswith'); | |
| } else if (mode === 'empty') { | |
| this.value = ''; | |
| this.onComparatorChange.emit('empty'); | |
| } else { | |
| // Clear domain-prefixed value when switching to eq mode | |
| if (this.value?.startsWith('@')) { | |
| this.value = ''; | |
| } | |
| this.onComparatorChange.emit(mode); | |
| } | |
| this.onFieldChange.emit(this.value); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/email/email.component.ts`
around lines 41 - 55, onFilterModeChange currently sets this.value to
`@${this.domainValue}` when entering 'domain' but does not remove the '@' prefix
when switching out, causing stale domain-formatted values to be emitted; update
onFilterModeChange to normalize/clear the value when leaving 'domain' (e.g., if
previous mode was 'domain' and new mode !== 'domain', remove a leading '@' or
clear value before emitting), while keeping existing behavior for entering
'domain' (use this.domainValue) and still emitting the correct comparator via
this.onComparatorChange and the normalized value via this.onFieldChange;
reference: onFilterModeChange, filterMode, domainValue, value,
onComparatorChange, onFieldChange.
Comment on lines
215
to
+220
| getModifiedRow(row: Record<string, unknown>): Record<string, unknown> { | ||
| const rels = this.relations(); | ||
| let modifiedRow: Record<string, unknown>; | ||
| if (this.relations.autocomplete_columns && this.relations.autocomplete_columns.length > 0) { | ||
| let autocompleteColumns = [...this.relations.autocomplete_columns]; | ||
| if (this.identityColumn) | ||
| autocompleteColumns.splice(this.relations.autocomplete_columns.indexOf(this.identityColumn), 1); | ||
| if (rels.autocomplete_columns && rels.autocomplete_columns.length > 0) { | ||
| let autocompleteColumns = [...rels.autocomplete_columns]; | ||
| if (this.identityColumn) autocompleteColumns.splice(rels.autocomplete_columns.indexOf(this.identityColumn), 1); |
There was a problem hiding this comment.
Add null guard for rels in getModifiedRow.
Similar to fetchSuggestions, this method accesses rels.autocomplete_columns without verifying that rels is defined, risking a runtime error if this.relations() returns undefined.
🛡️ Suggested fix
getModifiedRow(row: Record<string, unknown>): Record<string, unknown> {
const rels = this.relations();
+ if (!rels) return row;
let modifiedRow: Record<string, unknown>;
if (rels.autocomplete_columns && rels.autocomplete_columns.length > 0) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| getModifiedRow(row: Record<string, unknown>): Record<string, unknown> { | |
| const rels = this.relations(); | |
| let modifiedRow: Record<string, unknown>; | |
| if (this.relations.autocomplete_columns && this.relations.autocomplete_columns.length > 0) { | |
| let autocompleteColumns = [...this.relations.autocomplete_columns]; | |
| if (this.identityColumn) | |
| autocompleteColumns.splice(this.relations.autocomplete_columns.indexOf(this.identityColumn), 1); | |
| if (rels.autocomplete_columns && rels.autocomplete_columns.length > 0) { | |
| let autocompleteColumns = [...rels.autocomplete_columns]; | |
| if (this.identityColumn) autocompleteColumns.splice(rels.autocomplete_columns.indexOf(this.identityColumn), 1); | |
| getModifiedRow(row: Record<string, unknown>): Record<string, unknown> { | |
| const rels = this.relations(); | |
| if (!rels) return row; | |
| let modifiedRow: Record<string, unknown>; | |
| if (rels.autocomplete_columns && rels.autocomplete_columns.length > 0) { | |
| let autocompleteColumns = [...rels.autocomplete_columns]; | |
| if (this.identityColumn) autocompleteColumns.splice(rels.autocomplete_columns.indexOf(this.identityColumn), 1); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/foreign-key/foreign-key.component.ts`
around lines 215 - 220, getModifiedRow currently calls this.relations() into
rels and uses rels.autocomplete_columns without checking rels; add a
null/undefined guard so the function returns the original row (or a shallow
copy) if rels is falsy or rels.autocomplete_columns is falsy/empty.
Specifically, in getModifiedRow (and where rels.autocomplete_columns is read and
passed to splice/indexOf), first verify rels &&
Array.isArray(rels.autocomplete_columns) before using it and only perform the
identityColumn removal when the array exists; this prevents runtime errors when
this.relations() returns undefined.
Comment on lines
+32
to
+35
| mat-option { | ||
| display: flex; | ||
| align-items: center; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'phone.component.ts' frontend/src/app/components/ui-components/filter-fields
fd -i 'phone.component.html' frontend/src/app/components/ui-components/filter-fields
rg -n -C3 'encapsulation|ViewEncapsulation|mat-autocomplete|mat-option|panelClass' \
frontend/src/app/components/ui-components/filter-fields/phone/phone.component.ts \
frontend/src/app/components/ui-components/filter-fields/phone/phone.component.html \
frontend/src/app/components/ui-components/filter-fields/phone/phone.component.cssRepository: rocket-admin/rocketadmin
Length of output: 4577
🏁 Script executed:
cat -n frontend/src/app/components/ui-components/filter-fields/phone/phone.component.tsRepository: rocket-admin/rocketadmin
Length of output: 4366
mat-option styling will not apply from component-scoped CSS.
The component uses default emulated encapsulation and neither mat-select nor mat-autocomplete has a panelClass defined. This means the mat-option selector at lines 32–35 will not reach the options rendered in the CDK overlay. Use panelClass on both the mat-select (line 3) and mat-autocomplete (line 19) to apply styles from a global stylesheet, or use ::ng-deep as a fallback.
Suggested approach: use panelClass
In the template:
-<mat-select [(ngModel)]="filterMode" (ngModelChange)="onFilterModeChange($event)">
+<mat-select [(ngModel)]="filterMode" (ngModelChange)="onFilterModeChange($event)" panelClass="phone-select-panel">-<mat-autocomplete `#auto`="matAutocomplete" [displayWith]="displayFn">
+<mat-autocomplete `#auto`="matAutocomplete" [displayWith]="displayFn" panelClass="phone-autocomplete-panel">Then add to global styles:
.phone-select-panel mat-option { display: flex; align-items: center; }
.phone-autocomplete-panel mat-option { display: flex; align-items: center; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/phone/phone.component.css`
around lines 32 - 35, The mat-option rules in phone.component.css won't reach
overlay options because they're rendered in the CDK overlay; update the template
to add panelClass attributes to the mat-select and mat-autocomplete elements
(e.g. "phone-select-panel" and "phone-autocomplete-panel") and move the
mat-option CSS into a global stylesheet using selectors like
".phone-select-panel mat-option" and ".phone-autocomplete-panel mat-option" to
apply display:flex and align-items:center; as an alternative fallback, you can
keep the rules in the component by using ::ng-deep before the mat-option
selector.
Comment on lines
+5
to
+10
| [required]="required()" [disabled]="readonly()"> | ||
| </mat-form-field> | ||
| <mat-form-field *ngIf="value" appearance="outline"> | ||
| <mat-label>{{normalizedLabel}} Y coordinate</mat-label> | ||
| <input type="number" matInput [(ngModel)]="value.y" name="{{label}}-{{key}}-y" | ||
| [required]="required" [disabled]="readonly"> | ||
| <mat-label>{{normalizedLabel()}} Y coordinate</mat-label> | ||
| <input type="number" matInput [(ngModel)]="value.y" name="{{label()}}-{{key()}}-y" | ||
| [required]="required()" [disabled]="readonly()"> |
There was a problem hiding this comment.
Respect disabled() in point inputs.
Both inputs currently disable only on readonly(), which drops the explicit disabled() signal and can leave fields interactive when parent state marks them disabled.
💡 Proposed fix
- <input type="number" matInput [(ngModel)]="value.x" name="{{label()}}-{{key()}}-x"
- [required]="required()" [disabled]="readonly()">
+ <input type="number" matInput [(ngModel)]="value.x" name="{{label()}}-{{key()}}-x"
+ [required]="required()" [disabled]="disabled() || readonly()" [readonly]="readonly()">
@@
- <input type="number" matInput [(ngModel)]="value.y" name="{{label()}}-{{key()}}-y"
- [required]="required()" [disabled]="readonly()">
+ <input type="number" matInput [(ngModel)]="value.y" name="{{label()}}-{{key()}}-y"
+ [required]="required()" [disabled]="disabled() || readonly()" [readonly]="readonly()">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/point/point.component.html`
around lines 5 - 10, The inputs only respect readonly() for their disabled
state; update the [disabled] bindings on the point inputs (the input binding(s)
inside the point component template, e.g., the inputs bound with
[(ngModel)]="value.x" and [(ngModel)]="value.y") to also consider disabled() by
changing [disabled]="readonly()" to [disabled]="readonly() || disabled()" so
they become disabled when either flag is true.
Enhance DateTimeFilterComponent to be a self-contained "smart" filter with preset intervals (last hour/day/week/month/year) and custom comparator selection (equal/after/before/on or after/on or before), following the pattern established by email and phone filter components. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts (2)
26-26: Mark as readonly.The
_presetModesarray is never modified after initialization. Mark it asreadonlyto signal immutability.Proposed fix
- private _presetModes = ['last_hour', 'last_day', 'last_week', 'last_month', 'last_year']; + private readonly _presetModes = ['last_hour', 'last_day', 'last_week', 'last_month', 'last_year'];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts` at line 26, The _presetModes array is never modified, so update its declaration in DateTimeComponent to be immutable by adding readonly: change "private _presetModes = ['last_hour', 'last_day', 'last_week', 'last_month', 'last_year'];" to "private readonly _presetModes: readonly string[] = ['last_hour', 'last_day', 'last_week', 'last_month', 'last_year'];" (or use ReadonlyArray<string>) so the compiler and readers know it is immutable.
22-24: Consider using signals for component state.Per coding guidelines, component state should use
signal()instead of plain properties. However, since these are used with[(ngModel)]two-way binding in the template, plain properties may be acceptable here. If you want full signal adoption, consider usingmodel()for two-way bindable signals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts` around lines 22 - 24, Replace the plain component properties filterMode, date and time with Angular signals: import signal (or model for two-way binding) from `@angular/core` and declare filterMode = signal('last_day') and date/time as signals; if you need to keep [(ngModel)] in the template use model() for two-way bindable signals so the template can remain [(ngModel)]="date" / "time", otherwise update template bindings to read from and write to the signals (use the signal API to get/set values) and update any internal usages of filterMode/date/time to use the signal get/set methods.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.spec.ts`:
- Line 123: The test assigns undefined to component.time which is typed as
string; update the test to set component.time = '' (empty string) or change the
component's property type to string | undefined; locate the spec referencing
component.time in date-time.component.spec.ts and either replace the undefined
assignment with an empty string or update the DateTimeComponent's time property
type declaration to string | undefined and adjust other tests accordingly.
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts`:
- Around line 87-112: The _computePresetValue function returns a string using
format(targetDate, "yyyy-MM-dd'T'HH:mm:ss'Z'") which prints local time with a
literal Z; replace that call with targetDate.toISOString() so the returned
timestamp is the correct UTC ISO string (ensure the replacement occurs in the
_computePresetValue method where format(...) is used).
---
Nitpick comments:
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts`:
- Line 26: The _presetModes array is never modified, so update its declaration
in DateTimeComponent to be immutable by adding readonly: change "private
_presetModes = ['last_hour', 'last_day', 'last_week', 'last_month',
'last_year'];" to "private readonly _presetModes: readonly string[] =
['last_hour', 'last_day', 'last_week', 'last_month', 'last_year'];" (or use
ReadonlyArray<string>) so the compiler and readers know it is immutable.
- Around line 22-24: Replace the plain component properties filterMode, date and
time with Angular signals: import signal (or model for two-way binding) from
`@angular/core` and declare filterMode = signal('last_day') and date/time as
signals; if you need to keep [(ngModel)] in the template use model() for two-way
bindable signals so the template can remain [(ngModel)]="date" / "time",
otherwise update template bindings to read from and write to the signals (use
the signal API to get/set values) and update any internal usages of
filterMode/date/time to use the signal get/set methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 27cef1ad-94a0-47cb-93e6-cb81e9376069
📒 Files selected for processing (4)
frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.cssfrontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.htmlfrontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.spec.tsfrontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.css
| it('should default time to 00:00 on date change if time is not set', () => { | ||
| component.filterMode = 'eq'; | ||
| component.date = '2021-08-26'; | ||
| component.time = undefined; |
There was a problem hiding this comment.
Type inconsistency: assigning undefined to string property.
component.time is typed as string, but this test assigns undefined. This may cause TypeScript errors or unexpected behavior. Consider using an empty string or adjusting the type to string | undefined.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.spec.ts`
at line 123, The test assigns undefined to component.time which is typed as
string; update the test to set component.time = '' (empty string) or change the
component's property type to string | undefined; locate the spec referencing
component.time in date-time.component.spec.ts and either replace the undefined
assignment with an empty string or update the DateTimeComponent's time property
type declaration to string | undefined and adjust other tests accordingly.
frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts
Show resolved
Hide resolved
- Use toISOString() instead of format() with literal 'Z' suffix to produce correct UTC timestamps for preset intervals - Default time to '00:00:00' (with seconds) for format consistency - Remove redundant output() override inherited from base class Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts (1)
17-17: Consider migrating to signal-based inputs and state.Per coding guidelines,
signal()should be used for component state and the base class already uses signal-basedinput(). The current implementation uses legacy@Input()and plain properties:
@Input() value→ should bereadonly value = input<string>()filterMode,date,time→ should usesignal()isPresetMode()→ could become acomputed()signalThis would complete the signal migration mentioned in the PR objectives. Based on learnings: "Use
signal()for component state instead of plain properties".Proposed refactor sketch
-import { AfterViewInit, Component, ElementRef, Input, OnInit, ViewChild } from '@angular/core'; +import { AfterViewInit, Component, computed, ElementRef, input, OnInit, signal, ViewChild } from '@angular/core'; export class DateTimeFilterComponent extends BaseFilterFieldComponent implements OnInit, AfterViewInit { - `@Input`() value: string; + readonly value = input<string>(); `@ViewChild`('inputElement') inputElement: ElementRef<HTMLInputElement>; - public filterMode: string = 'last_day'; - public date: string; - public time: string; + public filterMode = signal<string>('last_day'); + public date = signal<string>(''); + public time = signal<string>(''); + + readonly isPresetMode = computed(() => this._presetModes.includes(this.filterMode()));Also applies to: 20-22
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts` at line 17, Replace the legacy `@Input`() value and plain properties with signals: remove the `@Input`() value declaration and instead declare readonly value = input<string>() (leveraging the base class input()), convert filterMode, date, and time into signal() state variables, and change isPresetMode() into a computed() signal that derives from those signals; update any references to these symbols (value, filterMode, date, time, isPresetMode) throughout the component to read from the signals/computed rather than from plain properties or method calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts`:
- Line 61: The default time string is inconsistent: update the fallback used in
the date-time component so both occurrences use the same format—change the const
time = this.time || '00:00' to match the other use of '00:00:00' (or vice versa)
and make the same change at the other site; ensure you also update any related
parsing/formatting code that references this.time, the local time variable, or
helper methods like combineDateTime/parseTime so they all expect the same HH:mm
or HH:mm:ss format.
- Line 62: The code in DateTimeComponent is incorrectly appending 'Z' to
local-formatted parts (the `${this.date}T${time}Z` construction in the datetime
assembly) which produces wrong UTC timestamps; update the assembly logic in the
methods that build datetime strings (where `datetime` is constructed) to produce
true UTC values instead of local values — either reconstruct the Date from the
local date/time parts and use its UTC getters (e.g., build a Date and use
toISOString() or
getUTCFullYear/getUTCMonth/getUTCDate/getUTCHours/getUTCMinutes/getUTCSeconds)
or otherwise convert the formatted local parts back to UTC before appending 'Z';
ensure consistency with the existing `_computePresetValue()` which uses
`toISOString()` so the assembled datetime matches that UTC behavior.
---
Nitpick comments:
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts`:
- Line 17: Replace the legacy `@Input`() value and plain properties with signals:
remove the `@Input`() value declaration and instead declare readonly value =
input<string>() (leveraging the base class input()), convert filterMode, date,
and time into signal() state variables, and change isPresetMode() into a
computed() signal that derives from those signals; update any references to
these symbols (value, filterMode, date, time, isPresetMode) throughout the
component to read from the signals/computed rather than from plain properties or
method calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cd3e713d-4c73-4a3e-8a49-66cbfa3d3fe5
📒 Files selected for processing (2)
frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.spec.tsfrontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.spec.ts
| } else { | ||
| this.onComparatorChange.emit(mode); | ||
| if (this.date) { | ||
| const time = this.time || '00:00'; |
There was a problem hiding this comment.
Inconsistent default time format.
Line 61 uses '00:00' (HH:mm) while line 69 uses '00:00:00' (HH:mm:ss). This inconsistency could cause issues depending on how the backend parses these values.
Proposed fix
} else {
this.onComparatorChange.emit(mode);
if (this.date) {
- const time = this.time || '00:00';
+ const time = this.time || '00:00:00';
const datetime = `${this.date}T${time}Z`;
this.onFieldChange.emit(datetime);
}Also applies to: 69-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts`
at line 61, The default time string is inconsistent: update the fallback used in
the date-time component so both occurrences use the same format—change the const
time = this.time || '00:00' to match the other use of '00:00:00' (or vice versa)
and make the same change at the other site; ensure you also update any related
parsing/formatting code that references this.time, the local time variable, or
helper methods like combineDateTime/parseTime so they all expect the same HH:mm
or HH:mm:ss format.
| this.onComparatorChange.emit(mode); | ||
| if (this.date) { | ||
| const time = this.time || '00:00'; | ||
| const datetime = `${this.date}T${time}Z`; |
There was a problem hiding this comment.
Timezone inconsistency when constructing datetime strings.
Lines 29-30 use format() which outputs date/time in the local timezone. Then lines 62, 70, and 76 append 'Z' (indicating UTC) to these local values, creating incorrect UTC timestamps.
For example, if a user in UTC+5 loads a value 2024-01-15T10:30:00Z:
format()produces local time15:30:00- Reconstruction produces
2024-01-15T15:30:00Z— shifted by 5 hours
This is inconsistent with _computePresetValue() at line 109, which correctly uses toISOString() for proper UTC output.
Consider either:
- Store the original UTC date/time parts, or
- Convert local date/time back to UTC before appending 'Z'
Proposed fix using UTC methods
ngOnInit(): void {
if (this.value) {
const datetime = new Date(this.value);
- this.date = format(datetime, 'yyyy-MM-dd');
- this.time = format(datetime, 'HH:mm:ss');
+ // Store as UTC to maintain consistency with 'Z' suffix
+ this.date = datetime.toISOString().slice(0, 10);
+ this.time = datetime.toISOString().slice(11, 19);
this.filterMode = 'gte';
}
}Also applies to: 70-71, 76-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.ts`
at line 62, The code in DateTimeComponent is incorrectly appending 'Z' to
local-formatted parts (the `${this.date}T${time}Z` construction in the datetime
assembly) which produces wrong UTC timestamps; update the assembly logic in the
methods that build datetime strings (where `datetime` is constructed) to produce
true UTC values instead of local values — either reconstruct the Date from the
local date/time parts and use its UTC getters (e.g., build a Date and use
toISOString() or
getUTCFullYear/getUTCMonth/getUTCDate/getUTCHours/getUTCMinutes/getUTCSeconds)
or otherwise convert the formatted local parts back to UTC before appending 'Z';
ensure consistency with the existing `_computePresetValue()` which uses
`toISOString()` so the assembled datetime matches that UTC behavior.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
gugu
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/app/components/ui-components/filter-fields/FILTER_WIDGETS.md`:
- Around line 45-51: The markdown code fence in FILTER_WIDGETS.md lacks a
language identifier which prevents proper syntax highlighting; update the fenced
block that documents getInputType(field) and getComparatorType(type) to use a
language tag (e.g., add "```typescript" at the start of the block) so the block
beginning with "getInputType(field) → reads ComponentClass.type (static
property)" and the subsequent getComparatorType mapping render with TypeScript
highlighting.
- Line 256: Clarify that this gotcha specifically applies to date-fns (used in
the codebase) and not all date libraries: explain that date-fns's format()
recognizes a timezone token 'Z' (which renders the numeric offset or 'Z' for
UTC) while the escaped form "'Z'" emits a literal "Z" character (and using that
literal can produce a fake UTC marker when the underlying date is local), and
instruct to prefer Date.prototype.toISOString() for genuine UTC output; update
the doc to include a short plain-English example describing that format(date,
"'Z'") on a local-time Date yields a local timestamp with a literal Z suffix
(misleading), whereas calling date.toISOString() returns a true UTC ISO string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 834c7b5d-b1aa-41e6-944b-77c1b80193ce
📒 Files selected for processing (1)
frontend/src/app/components/ui-components/filter-fields/FILTER_WIDGETS.md
Comment on lines
+45
to
+51
| ``` | ||
| getInputType(field) → reads ComponentClass.type (static property) | ||
| getComparatorType(type): | ||
| 'text' → dialog shows text comparators | ||
| 'number'/'datetime' → dialog shows number comparators | ||
| anything else → 'nonComparable' → dialog shows nothing, component manages itself | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to the code fence.
The code block is missing a language specifier, which affects markdown rendering and syntax highlighting.
📝 Proposed fix
-```
+```typescript
getInputType(field) → reads ComponentClass.type (static property)
getComparatorType(type):
'text' → dialog shows text comparators
'number'/'datetime' → dialog shows number comparators
anything else → 'nonComparable' → dialog shows nothing, component manages itself
</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/app/components/ui-components/filter-fields/FILTER_WIDGETS.md`
around lines 45 - 51, The markdown code fence in FILTER_WIDGETS.md lacks a
language identifier which prevents proper syntax highlighting; update the fenced
block that documents getInputType(field) and getComparatorType(type) to use a
language tag (e.g., add "```typescript" at the start of the block) so the block
beginning with "getInputType(field) → reads ComponentClass.type (static
property)" and the subsequent getComparatorType mapping render with TypeScript
highlighting.
| - **Do NOT override `onFieldChange` or `onComparatorChange`** — the base class outputs work fine. Overriding creates a shadowed emitter. | ||
| - **Comparator mapping**: internal filter modes can differ from backend comparators. Email's `'domain'` mode emits `'endswith'`; phone's `'country'` mode emits `'startswith'`. | ||
| - **URL restoration**: when restoring from URL, you can't distinguish preset modes from custom ones — default to showing as a custom comparator. | ||
| - **Timezone**: for datetime values, use `toISOString()` for genuine UTC. Never use `format()` with literal `'Z'` suffix — it outputs local time with a fake UTC marker. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which date formatting libraries are in use
echo "=== Checking package.json for date libraries ==="
if [ -f "frontend/package.json" ]; then
cat frontend/package.json | jq '.dependencies + .devDependencies | to_entries | .[] | select(.key | test("moment|date-fns|dayjs|luxon"))'
else
echo "package.json not found"
fi
echo ""
echo "=== Searching for format() calls with 'Z' in filter components ==="
rg -n "\.format\([^)]*['\"].*Z.*['\"]" frontend/src/app/components/ui-components/filter-fields/ --type ts -C 2Repository: rocket-admin/rocketadmin
Length of output: 290
🏁 Script executed:
# Read the FILTER_WIDGETS.md file to see context around line 256
head -n 280 frontend/src/app/components/ui-components/filter-fields/FILTER_WIDGETS.md | tail -n 40Repository: rocket-admin/rocketadmin
Length of output: 1876
🏁 Script executed:
# Check if the documentation mentions which date library to use elsewhere
rg -n "date-fns|format|toISOString" frontend/src/app/components/ui-components/filter-fields/FILTER_WIDGETS.md -B 2 -A 2Repository: rocket-admin/rocketadmin
Length of output: 733
🌐 Web query:
date-fns format function 'Z' behavior version 4.1.0 literal Z UTC
💡 Result:
In date-fns version 4.1.0, the format function's 'Z' token is part of the ISO-8601 timezone tokens (X, XX, XXX, etc.). It does not output a literal 'Z'; instead, it formats the timezone offset of the date. For a standard JavaScript Date object (which represents a point in time with local timezone interpretation for formatting), format(date, 'Z') outputs the local timezone offset in ISO-8601 basic format without separator (e.g., '+0000' for UTC, '-0800' for PST). For UTC dates (offset 0), it outputs 'Z'. To output a literal 'Z', escape it with single quotes: format(date, "'Z'"). This behavior follows Unicode Technical Standard #35 for timezone tokens, where 'Z' in X tokens specifically denotes zero offset (UTC/Zulu time). In v4.1.0, with TZ support added, passing a TZDate with UTC timezone or using {in: tz('UTC')} will ensure offset 0 and thus 'Z' output for 'Z' token. Examples from docs: X tokens output "-08, +0530, Z" where Z appears for zero offset.
Citations:
- 1: https://cdn.jsdelivr.net/npm/date-fns@4.0.0/format.d.ts
- 2: https://fossies.org/linux/date-fns/src/format/index.ts
- 3: https://github.com/date-fns/date-fns/blob/dd663983/CHANGELOG.md
- 4: Using Z in the format pattern (using time zone) throws an error date-fns/date-fns#1473
Clarify which date library and format pattern this gotcha applies to.
The warning about format() with literal 'Z' is valid for date-fns (which your codebase uses), but the documentation should be more explicit. In date-fns 4.1.0, the 'Z' token outputs timezone offset (and 'Z' itself for UTC), while the escaped form format(date, "'Z'") outputs a literal Z character—which is the problematic pattern this gotcha warns against. Specify:
- That this applies to date-fns (not generic date libraries)
- The difference between the
'Z'token and escaped"'Z'"form - A concrete example showing the incorrect output and how
toISOString()fixes it
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/app/components/ui-components/filter-fields/FILTER_WIDGETS.md` at
line 256, Clarify that this gotcha specifically applies to date-fns (used in the
codebase) and not all date libraries: explain that date-fns's format()
recognizes a timezone token 'Z' (which renders the numeric offset or 'Z' for
UTC) while the escaped form "'Z'" emits a literal "Z" character (and using that
literal can produce a fake UTC marker when the underlying date is local), and
instruct to prefer Date.prototype.toISOString() for genuine UTC output; update
the doc to include a short plain-English example describing that format(date,
"'Z'") on a local-time Date yields a local timestamp with a literal Z suffix
(misleading), whereas calling date.toISOString() returns a true UTC ISO string.
Rename "equal" to "on" for date-time filter comparator, remove leftover top margin on saved-filters column name, and fix indentation in the filters dialog template. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.html`:
- Line 3: The comparator <mat-select> bound to filterMode and using
onFilterModeChange lacks a name attribute and a [disabled] binding, causing
inconsistent form state compared to the date/time inputs; add a name (e.g.,
name="filterMode") and bind its disabled state to the same property used by the
inputs (e.g., [disabled]="isDisabled" or the component's existing control/flag)
so the <mat-select> uses the same form management and enabled/disabled behavior
as the sibling date/time inputs.
- Line 24: The template is using a literal attribute name instead of Angular
binding for data-testid; replace attr.data-testid="record-{{label()}}-date" with
a proper attribute binding such as [attr.data-testid]="'record-' + label() +
'-date'" or [attr.data-testid]="`record-${label()}-date`" in the
date-time.component template so the data-testid is evaluated using the label()
function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a7110701-74f6-405b-8fc1-68d9f1ba2bbe
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.htmlfrontend/src/app/components/dashboard/db-table-view/saved-filters-panel/saved-filters-dialog/saved-filters-dialog.component.cssfrontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.html
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/components/dashboard/db-table-view/db-table-filters-dialog/db-table-filters-dialog.component.html
| [(ngModel)]="date" (change)="onDateChange()"> | ||
| <div class="filter-row"> | ||
| <mat-form-field class="comparator-field" appearance="outline"> | ||
| <mat-select [(ngModel)]="filterMode" (ngModelChange)="onFilterModeChange($event)"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find mat-select tags using ngModel that may be missing `name`/standalone.
python - <<'PY'
import re, pathlib
for p in pathlib.Path("frontend").rglob("*.component.html"):
text = p.read_text(encoding="utf-8")
for m in re.finditer(r"<mat-select\b[^>]*\[\(ngModel\)\][^>]*>", text, flags=re.I|re.S):
tag = m.group(0)
if " name=" not in tag and "[ngModelOptions]" not in tag:
line = text.count("\n", 0, m.start()) + 1
print(f"{p}:{line}: {tag.strip()}")
PYRepository: rocket-admin/rocketadmin
Length of output: 1261
🏁 Script executed:
#!/bin/bash
# Check the full context of the date-time component HTML
cat -n frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.htmlRepository: rocket-admin/rocketadmin
Length of output: 2194
🏁 Script executed:
#!/bin/bash
# Check the date-time component class to understand the disabled() method and form context
cat -n frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.tsRepository: rocket-admin/rocketadmin
Length of output: 4047
🏁 Script executed:
#!/bin/bash
# Check if these components are used within form elements
rg -A 3 -B 3 "date-time\.component" frontend --type html | head -50Repository: rocket-admin/rocketadmin
Length of output: 50
🏁 Script executed:
#!/bin/bash
# Check Angular Material mat-select documentation references and ngModel usage patterns
rg "mat-select.*ngModel" frontend --type html -A 2 | head -100Repository: rocket-admin/rocketadmin
Length of output: 13788
🏁 Script executed:
#!/bin/bash
# Find BaseFilterFieldComponent to verify disabled() method
find frontend -name "base-filter-field.component.ts" -exec cat -n {} \;Repository: rocket-admin/rocketadmin
Length of output: 1282
🏁 Script executed:
#!/bin/bash
# Search for usage of these filter field components in parent forms/dialogs
rg "app-filter-date-time|app-filter-email|app-filter-phone" frontend --type html -B 5 | head -80Repository: rocket-admin/rocketadmin
Length of output: 50
Add name attribute and disabled binding to the comparator select for consistency.
The mat-select lacks a name attribute and [disabled] binding, while the sibling date/time inputs have both. This creates inconsistency in how form state is managed across the component.
🔧 Proposed fix
- <mat-select [(ngModel)]="filterMode" (ngModelChange)="onFilterModeChange($event)">
+ <mat-select
+ [(ngModel)]="filterMode"
+ (ngModelChange)="onFilterModeChange($event)"
+ name="{{label()}}-{{key()}}-mode"
+ [disabled]="disabled()">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <mat-select [(ngModel)]="filterMode" (ngModelChange)="onFilterModeChange($event)"> | |
| <mat-select | |
| [(ngModel)]="filterMode" | |
| (ngModelChange)="onFilterModeChange($event)" | |
| name="{{label()}}-{{key()}}-mode" | |
| [disabled]="disabled()"> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.html`
at line 3, The comparator <mat-select> bound to filterMode and using
onFilterModeChange lacks a name attribute and a [disabled] binding, causing
inconsistent form state compared to the date/time inputs; add a name (e.g.,
name="filterMode") and bind its disabled state to the same property used by the
inputs (e.g., [disabled]="isDisabled" or the component's existing control/flag)
so the <mat-select> uses the same form management and enabled/disabled behavior
as the sibling date/time inputs.
| <input type="date" matInput name="{{label()}}-{{key()}}-date" | ||
| #inputElement | ||
| [required]="required()" [disabled]="disabled()" [readonly]="readonly()" | ||
| attr.data-testid="record-{{label()}}-date" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Detect literal `attr.data-testid` usage in templates.
rg -nP '\battr\.data-testid\s*=' frontend --iglob '*.component.html'Repository: rocket-admin/rocketadmin
Length of output: 11937
🏁 Script executed:
cat -n frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.html | sed -n '20,35p'Repository: rocket-admin/rocketadmin
Length of output: 1073
Fix data-testid binding syntax; current attributes are not bound.
attr.data-testid="..." is treated as a literal HTML attribute name. Use Angular attribute binding syntax to set data-testid correctly.
🔧 Proposed fix
- attr.data-testid="record-{{label()}}-date"
+ [attr.data-testid]="'record-' + label() + '-date'"- attr.data-testid="record-{{label()}}-time"
+ [attr.data-testid]="'record-' + label() + '-time'"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/ui-components/filter-fields/date-time/date-time.component.html`
at line 24, The template is using a literal attribute name instead of Angular
binding for data-testid; replace attr.data-testid="record-{{label()}}-date" with
a proper attribute binding such as [attr.data-testid]="'record-' + label() +
'-date'" or [attr.data-testid]="`record-${label()}-date`" in the
date-time.component template so the data-testid is evaluated using the label()
function.
Why: simplify the dialog template, align all filter rows on a consistent grid regardless of widget type, surface widget display names, and prevent users from re-adding the same column twice. - Add DefaultFilterComponent that wraps simple value editors with their comparator dropdown, mirroring the smart-filter pattern (Email/Phone) - Dialog now resolves one component per row via getFilterComponent / getFilterInputs, eliminating the comparable / nonComparable branching - Show widget.name as the field label in both the "Add filter by" dropdown and the active filter rows - Disable already-filtered fields in the autocomplete options - Replace flex layout with a 3-column grid + subgrid so every filter row renders on a single line with name / value-area / delete aligned
Boolean filter: emit onComparatorChange, rename Unknown → Null, only show the Null option when the column allows nulls, and restore null/empty/'' values correctly on URL load. Select filter: convert to mat-select multiple, emit eq+scalar for a single pick, in+array for multi-pick, and undefined when cleared so the filter is dropped. Restore scalar, null, and array values on URL load. Both use the existing smart-filter pattern so the dialog manages no comparator dropdown for them. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit