fix: Typed Reactive Forms#94
Open
matheusandre1 wants to merge 13 commits intoomatheusmesmo:developfrom
Open
Conversation
Contributor
|
🤖 Attention @matheusandre1! Only the repository owner can target the 'main' branch directly. I've automatically updated your PR to target 'develop' instead. Thank you! 🚀 |
Contributor
|
🤖 Hi @matheusandre1, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
Contributor
|
🤖 I'm sorry @matheusandre1, but I was unable to process your request. Please see the logs for more details. |
omatheusmesmo
requested changes
Mar 8, 2026
Owner
omatheusmesmo
left a comment
omatheusmesmo
left a comment
There was a problem hiding this comment.
Hello @matheusandre1 ! I have performed a comprehensive review of the changes in PR #94. The implementation significantly modernizes the application's form management and state handling.
✅ Key Improvements
- Strict Type Safety: Most forms (
Login,Signup,CategoryDialog,ItemDialog,ShoppingListDialog,UnitDialog, andListShareDialog) have been successfully migrated to strongly typedFormGroupandFormControlstructures. - Type Integrity: The use of
this.fb.nonNullable.groupis an excellent choice. It ensures that form controls do not revert tonullduring resets, maintaining strict type consistency. - Modern Angular Patterns:
- Dependency Injection: Consistent adoption of the
inject()function over traditional constructor injection. - Signals: Effective use of
signalsfor reactive state management (e.g.,existingCategories,units). - Change Detection: Application of
ChangeDetectionStrategy.OnPushfor better performance in new components.
- Dependency Injection: Consistent adoption of the
- Robust Validation: The addition of the
duplicateNameValidatorimproves data integrity and enhances the user experience by providing real-time feedback for duplicate entries.
⚠️ Required Changes (Blocking 100% Compliance)
1. Incomplete Migration - ListItemDialogComponent
The file frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts is still using FormsModule and ngModel. To fulfill the "100% Typed Reactive Forms" requirement, this component must be refactored to use ReactiveFormsModule with typed controls.
2. Dead Code & Redundancy Cleanup
The following directories contain old, untyped versions of components that are no longer imported anywhere in the project and should be removed:
- Path:
frontend/src/app/list/components/categories-management/category-dialog/- Reason: Replaced by the new typed version in
frontend/src/app/list/components/category-dialog/.
- Reason: Replaced by the new typed version in
- Path:
frontend/src/app/list/components/shopping-list/list-dialog/- Reason: Replaced by the new typed
ShoppingListDialogComponent.
- Reason: Replaced by the new typed
Summary: This PR is a major step forward in code quality. Once the ListItemDialogComponent is migrated and the redundant code is purged, it will be fully compliant with the project goals. Great work! 🚀
You also need to resolve some conflicts to merge.
Contributor
|
🤖 Hi @matheusandre1, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
Contributor
|
🤖 Hi @matheusandre1, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
Contributor
|
🤖 I'm sorry @matheusandre1, but I was unable to process your request. Please see the logs for more details. |
Contributor
|
🤖 Hi @matheusandre1, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
Contributor
|
🤖 I'm sorry @matheusandre1, but I was unable to process your request. Please see the logs for more details. |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR strengthens Angular form typing and migrates several templates to reactive forms, removes two dialog components, adds a new GitHub Actions "gemini-dispatch" workflow, and applies assorted documentation, license, and CI formatting updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Event as GitHub Event
participant Dispatcher as Gemini Dispatch Job
participant Auth as Token Service
participant GitHub as GitHub API
participant Orchestrator as Downstream Orchestration
Event->>Dispatcher: webhook / comment / PR payload
Dispatcher->>Dispatcher: parse command & extract context
alt requires auth
Dispatcher->>Auth: mint identity token (if APP_ID)
Auth-->>Dispatcher: token
end
Dispatcher->>GitHub: post acknowledgement comment
Dispatcher->>Orchestrator: set outputs (command, request, context)
Orchestrator->>Orchestrator: route to review/triage/invoke/fallthrough jobs
Orchestrator->>GitHub: (optional) post results or failure comment
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
Owner
|
@coderabbitai check this PR. |
|
✅ Actions performedReview triggered.
|
Contributor
|
🤖 Hi @coderabbitai[bot], I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
|
Tip For best results, initiate chat on the files or code changes.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/app/list/components/units-management/units-management.component.ts (1)
57-60: Remove the debugconsole.logcalls before merge.These logs will fire on every units refresh and clutter the browser console with response payloads during normal operation. If you still need diagnostics here, keep them behind a dev-only logger instead.
Proposed cleanup
tap((response) => { - console.log('UnitsManagementComponent - Units loaded:', response); - if (!response || response.length === 0) { - console.log('UnitsManagementComponent - No units found'); - } this.units.set(response || []); }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/units-management/units-management.component.ts` around lines 57 - 60, Remove the debug console.log calls in UnitsManagementComponent that print the response and "No units found" during the units refresh; locate the logs inside the method that handles the units API response (the block that currently contains console.log('UnitsManagementComponent - Units loaded:', response) and console.log('UnitsManagementComponent - No units found')) and either delete them or replace them with a dev-only logger/verbose flag check so normal production runs do not emit the payload to the browser console.frontend/src/app/list/components/category-dialog/category-dialog.component.ts (1)
106-115: Consider using direct control access for better type safety.With typed forms, you can access controls directly via
this.categoryForm.controls.nameinstead ofthis.categoryForm.get('name'). This eliminates optional chaining and provides full type inference.♻️ Suggested improvement
updateNameValidator(): void { const names = this.existingCategories().map((c) => c.name); const originalName = this.isEdit() && this.data.category ? this.data.category.name : undefined; - this.categoryForm - .get('name') - ?.addValidators(duplicateNameValidator(names, originalName)); - this.categoryForm.get('name')?.updateValueAndValidity(); + this.categoryForm.controls.name.addValidators( + duplicateNameValidator(names, originalName) + ); + this.categoryForm.controls.name.updateValueAndValidity(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/category-dialog/category-dialog.component.ts` around lines 106 - 115, In updateNameValidator(), replace the optional get('name') calls with a typed direct access to the control via this.categoryForm.controls.name (or this.categoryForm.controls['name']) so you avoid optional chaining and gain type inference; update the two spots where .get('name') is used to reference the control variable (e.g., const nameCtrl = this.categoryForm.controls.name) and then call nameCtrl.addValidators(duplicateNameValidator(names, originalName)) and nameCtrl.updateValueAndValidity(); ensure categoryForm is typed so controls.name has the correct FormControl type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@frontend/src/app/list/components/category-dialog/category-dialog.component.ts`:
- Around line 106-115: In updateNameValidator(), replace the optional
get('name') calls with a typed direct access to the control via
this.categoryForm.controls.name (or this.categoryForm.controls['name']) so you
avoid optional chaining and gain type inference; update the two spots where
.get('name') is used to reference the control variable (e.g., const nameCtrl =
this.categoryForm.controls.name) and then call
nameCtrl.addValidators(duplicateNameValidator(names, originalName)) and
nameCtrl.updateValueAndValidity(); ensure categoryForm is typed so controls.name
has the correct FormControl type.
In
`@frontend/src/app/list/components/units-management/units-management.component.ts`:
- Around line 57-60: Remove the debug console.log calls in
UnitsManagementComponent that print the response and "No units found" during the
units refresh; locate the logs inside the method that handles the units API
response (the block that currently contains
console.log('UnitsManagementComponent - Units loaded:', response) and
console.log('UnitsManagementComponent - No units found')) and either delete them
or replace them with a dev-only logger/verbose flag check so normal production
runs do not emit the payload to the browser console.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d0a68c2-7589-4737-9270-fabf1bb96a0d
📒 Files selected for processing (21)
frontend/src/app/auth/login/login.component.tsfrontend/src/app/auth/signup/signup.component.tsfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.htmlfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.scssfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.tsfrontend/src/app/list/components/category-dialog/category-dialog.component.tsfrontend/src/app/list/components/items-management/item-dialog/item-dialog.component.tsfrontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.htmlfrontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.tsfrontend/src/app/list/components/list-share-dialog/list-share-dialog.component.tsfrontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.tsfrontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.htmlfrontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.scssfrontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.tsfrontend/src/app/list/components/units-management/unit-dialog/unit-dialog.component.tsfrontend/src/app/list/components/units-management/units-management.component.tsfrontend/src/app/shared/components/category/category.component.tsfrontend/src/app/shared/components/item/item.component.tsfrontend/src/app/shared/components/list-item/list-item.component.htmlfrontend/src/app/shared/components/list-item/list-item.component.tsfrontend/src/app/shared/components/unit/unit.component.ts
💤 Files with no reviewable changes (6)
- frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.ts
- frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.html
- frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts
- frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.scss
- frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.html
- frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.scss
Contributor
|
🤖 I'm sorry @coderabbitai[bot], but I was unable to process your request. Please see the logs for more details. |
|
✅ Actions performedReview triggered.
|
omatheusmesmo
force-pushed
the
develop
branch
2 times, most recently
from
4ad5927 to
27ebf42
Compare
github-actions
bot
force-pushed
the
develop
branch
from
d2d1107 to
d613db3
Compare
# Conflicts: # frontend/src/app/auth/login/login.component.ts # frontend/src/app/auth/signup/signup.component.ts # frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts # frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts # frontend/src/app/shared/components/category/category.component.ts # frontend/src/app/shared/components/item/item.component.ts # frontend/src/app/shared/components/list-item/list-item.component.ts # frontend/src/app/shared/components/unit/unit.component.ts
# Conflicts: # frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.html # frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts # frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.html # frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts # frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.html # frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.ts
… for main # Conflicts: # .github/workflows/branch-enforcer.yml # .github/workflows/gemini-dispatch.yml
# Conflicts: # .github/workflows/gemini-dispatch.yml
# Conflicts: # frontend/src/app/list/components/items-management/items-management.component.ts # frontend/src/app/list/components/list-details/list-details.component.ts # frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.html # frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts # frontend/src/app/list/components/units-management/units-management.component.ts # frontend/src/app/shared/services/auth.service.ts
# Conflicts: # frontend/src/app/list/components/items-management/items-management.component.ts # frontend/src/app/list/components/list-details/list-details.component.ts # frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts # frontend/src/app/list/components/units-management/units-management.component.ts
matheusandre1
force-pushed
the
fix-issue42
branch
from
b49a572 to
f7846b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/shared/components/list-item/list-item.component.ts (1)
101-109:⚠️ Potential issue | 🟠 MajorUse extracted raw values when creating
listItemDataThe
quantityvariable extracted at line 101 is unused (triggers ESLint unused variable warning), and the DTO unnecessarily rebuilds values fromthis.listItemForm.valueinstead of using the already-extracted values fromgetRawValue(). ExtractunitPricealongsideitemIdandquantity, then use these variables directly in the DTO to ensure consistency and stronger nullability handling.Suggested fix
- const { itemId, quantity } = this.listItemForm.getRawValue(); + const { itemId, quantity, unitPrice } = this.listItemForm.getRawValue(); if (itemId === null) return; const listItemData: ListItemRequestDTO = { listId: this.listId, - itemId: this.listItemForm.value.itemId, - quantity: this.listItemForm.value.quantity, - unitPrice: this.listItemForm.value.unitPrice, + itemId, + quantity, + unitPrice, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/components/list-item/list-item.component.ts` around lines 101 - 109, The extracted values from this.listItemForm.getRawValue() are not being used when building listItemData, leaving quantity unused and causing inconsistencies; update the extraction to const { itemId, quantity, unitPrice } = this.listItemForm.getRawValue() and then construct the ListItemRequestDTO (listItemData) using these variables (itemId, quantity, unitPrice) along with this.listId to eliminate the unused-variable warning and ensure consistent/null-safe values from getRawValue() rather than re-reading this.listItemForm.value.
🧹 Nitpick comments (5)
.github/workflows/greetings.yml (1)
12-16: Consider pinning to a specific action version for improved security.The indentation fix looks correct. However,
actions/first-interaction@v1uses a floating major version tag. For better supply chain security and reproducibility, consider pinning to a full commit SHA or a specific semantic version tag (e.g.,v1.1.4).🔒 Suggested improvement
- - uses: actions/first-interaction@v1 + - uses: actions/first-interaction@v1.1.4 # Or pin to full SHAAs per coding guidelines: "Use specific action versions (commit SHA or tagged versions)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/greetings.yml around lines 12 - 16, The workflow uses the floating tag actions/first-interaction@v1; replace that with a pinned version (either a specific semver tag like actions/first-interaction@v1.1.4 or the full commit SHA) in the step that references actions/first-interaction to improve supply-chain security and reproducibility, updating the `uses:` value accordingly and keeping the existing `with:` inputs unchanged..github/workflows/gemini-dispatch.yml (1)
10-11: Security-sensitive trigger:pull_request_targetwith write permissions.Using
pull_request_targetruns the workflow in the context of the base branch with elevated permissions. While authorization checks on lines 56-58 restrict command invocation to trusted users (OWNER/MEMBER/COLLABORATOR), ensure that:
- The
REQUESTvariable (line 88) containing user-provided content is never directly interpolated into shell commands- The reusable workflows (
gemini-review.yml, etc.) also sanitize any user-controlled inputThe current implementation appears safe since user input is processed via
actions/github-scriptrather than shell interpolation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/gemini-dispatch.yml around lines 10 - 11, The workflow uses pull_request_target and a REQUEST variable that may contain user-provided content; ensure REQUEST is never directly interpolated into any shell/runner step and instead is passed as a safe input to actions/github-script or to reusable workflows (e.g., gemini-review.yml) where it is sanitized or JSON.stringified before use; review all reusable workflows referenced (gemini-review.yml, etc.) to validate they treat inputs from REQUEST as data (not shell commands), perform explicit escaping/validation in their entrypoint functions, and reduce permissions to least-privilege where possible (or consider switching the trigger from pull_request_target to pull_request) so untrusted PR content cannot lead to command execution.frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.html (1)
8-8: Use native@forinstead of*ngForLine 8 should follow the project’s native control-flow standard.
Suggested fix
- <mat-option *ngFor="let item of items" [value]="item.id"> - {{ item.name }} ({{ item.unit.symbol }}) - </mat-option> + `@for` (item of items; track item.id) { + <mat-option [value]="item.id"> + {{ item.name }} ({{ item.unit.symbol }}) + </mat-option> + }As per coding guidelines, "Use native control flow (
@if,@for,@switch) instead of*ngIf,*ngFor."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.html` at line 8, Replace the template's structural directive "*ngFor" with the project's native control-flow "@for" in the list-item-dialog template: change the mat-option line that iterates over items (currently using *ngFor="let item of items") to use `@for` so the loop binds the same iterator (item) and collection (items) and continues to set the option value from item.id; ensure the attribute name is updated on the mat-option element in list-item-dialog.component.html and that any template expressions referencing item or item.id remain unchanged.frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts (2)
45-45: Prefer signal state foritemsThis regresses local state from signal-based to plain property; keep it reactive and consistent.
Suggested fix
- items: ItemResponseDTO[] = []; + readonly items = signal<ItemResponseDTO[]>([]); ... - this.items = items; + this.items.set(items);As per coding guidelines, "Use signals for local state. Do NOT use
mutate, useupdateorset."Also applies to: 76-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts` at line 45, Replace the plain property items: ItemResponseDTO[] = [] with a signal-based state and update all usages to use the signal API; specifically, declare items as a signal (e.g., items = signal<ItemResponseDTO[]>([])) in the ListItemDialogComponent, and replace any direct array mutations or reads (e.g., pushes, assignments, or property accesses referenced around methods in this component) with items.update(prev => ...) or items.set(...) and read the value via items() rather than mutating the array directly—ensure no use of mutate and update all references accordingly.
4-4: Useinject()instead of constructor injectionThe constructor now handles all DI; project standard is
inject()-based dependency wiring.Suggested fix
import { ChangeDetectionStrategy, Component, - Inject, + inject, OnInit, } from '@angular/core'; ... - constructor( - public dialogRef: MatDialogRef<ListItemDialogComponent>, - `@Inject`(MAT_DIALOG_DATA) - public data: { listItem?: ListItemResponseDTO; listId: number }, - private itemService: ItemService, - private fb: FormBuilder, - ) { + readonly dialogRef = inject(MatDialogRef<ListItemDialogComponent>); + readonly data = inject(MAT_DIALOG_DATA) as { + listItem?: ListItemResponseDTO; + listId: number; + }; + private readonly itemService = inject(ItemService); + private readonly fb = inject(FormBuilder); + + constructor() { const isEdit = !!data.listItem;As per coding guidelines, "Use the
inject()function instead of constructor injection."Also applies to: 51-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts` at line 4, ListItemDialogComponent currently uses constructor injection; replace that with Angular's inject() pattern by removing the constructor and creating class-level readonly properties initialized via inject() for each dependency previously injected (e.g., MatDialogRef, MAT_DIALOG_DATA, FormBuilder, ChangeDetectorRef, Store — whatever symbols appear in the component's constructor). Ensure you import inject from `@angular/core`, assign the exact tokens/types used in the constructor to readonly fields on the class, and update any usages to reference those fields; run the request_verification after changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/README.md`:
- Around line 16-18: The three TOC anchor fragments for "Shopping Lists",
"Shopping List Items", and "Shopping List User Permissions" are invalid; update
their link targets so they match the actual Markdown heading IDs used in the
document (use the exact heading text lowercased, spaces replaced with hyphens,
and strip any parentheses/slashes). Locate the TOC entries that show "[Shopping
Lists (/shopplists)]", "[Shopping List Items (/shopplist/{shopplistId}/items)]"
and "[Shopping List User Permissions (/shopplist/{shopplistId}/permissions)]"
and replace their fragment parts with the correct anchors generated from the
headings (e.g., shopping-lists, shopping-list-items,
shopping-list-user-permissions) so in-page navigation works.
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 79-80: The class currently declares two members named isLoading
(readonly isLoading = signal(false); and isLoading = false;) which conflicts;
remove the plain boolean declaration and keep the signal-based member (readonly
isLoading = signal(false)); then update any usages of isLoading in
ListShareDialogComponent to use the signal APIs (call isLoading() or read
isLoading()/.value per your signal convention) instead of treating it as a
boolean so references compile cleanly.
- Line 11: Remove the unused FormControl import: delete the FormControl import
from the import list at the top of list-share-dialog.component.ts since the
component uses FormBuilder.group() and the returned FormGroup (and its controls)
directly; ensure no other references to FormControl exist in the file (if found,
replace them to use the FormGroup/FormControl accessed via the form instance)
and run a build/linter to confirm the unused import is gone.
In
`@frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts`:
- Around line 13-15: The import list in shopping-list-dialog.component.ts
includes an unused FormControl; remove FormControl from the import statement
(leave FormBuilder and FormGroup intact) so the ShoppingListDialogComponent no
longer imports an unused symbol and the lint/type checks will pass.
- Around line 83-86: Remove the redundant redeclaration that overwrites the
already-validated/trimmed name: do not use "const { name } =
this.listForm.getRawValue()" — instead populate listData.name from the
previously trimmed/validated value (the value you trimmed/validated earlier from
this.listForm) so ShoppingListRequestDTO.listData uses the trimmed name; update
the code that builds listData to reference the existing trimmed variable or
re-read and trim via this.listForm.get('name')?.value.trim() rather than
destructuring getRawValue().
In `@frontend/src/app/shared/components/category/category.component.ts`:
- Line 11: The import FormControl is unused in CategoryComponent; either remove
FormControl from the import list at the top of category.component.ts or consume
it by typing/instantiating a control (e.g., add a typed FormControl field or use
new FormControl in your form group declaration) so the import is used; update
the class (CategoryComponent) form declaration accordingly to eliminate the
unused-import error.
- Around line 79-80: The categoryForm is declared as an untyped FormGroup which
makes getRawValue() return an implicitly typed object and loses the compile-time
guarantee for name; change the categoryForm declaration to FormGroup<{ name:
FormControl<string> }> and build it with this.fb.nonNullable.group(...) (and use
this.fb.nonNullable.control(...) for the name control) so getRawValue() returns
a properly typed object and you can safely destructure name into the Category
payload used by the Category type.
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Line 11: The import list in item.component.ts includes an unused symbol
FormControl; remove FormControl from the import statement so the file no longer
imports an identifier that isn't referenced (update the import that currently
lists FormControl alongside other Angular forms symbols used by ItemComponent,
e.g., remove "FormControl" from that import and run lint/TS compile to confirm
no other references remain).
In `@frontend/src/app/shared/components/list-item/list-item.component.ts`:
- Line 11: The import for FormControl is unused in list-item.component (remove
the unused symbol to satisfy lint); locate the import statement that includes
FormControl and delete FormControl from the import list (or remove the entire
import line if it only contains FormControl) so the file no longer imports the
unused FormControl symbol.
In `@frontend/src/app/shared/components/unit/unit.component.ts`:
- Around line 82-83: The payload uses raw values from unitForm which can contain
leading/trailing or whitespace-only strings; before creating unitData in the
component (where you call this.unitForm.getRawValue()), trim and normalize the
fields: const { name: rawName, symbol: rawSymbol } =
this.unitForm.getRawValue(); const name = (rawName || '').trim(); const symbol =
(rawSymbol || '').trim(); then build const unitData: Unit = { name, symbol } and
handle/validate empty strings (reject or show validation) so whitespace-only
values are not persisted.
- Line 11: The import statement includes an unused symbol FormControl which
triggers a no-unused-vars lint/build error; remove FormControl from the import
list (the import that currently includes FormControl) in
frontend/src/app/shared/components/unit/unit.component.ts or, if FormControl was
intended to be used, replace its usage with the appropriate reference in the
UnitComponent so the symbol is actually used; most likely simply delete
FormControl from the import to resolve the lint failure.
---
Outside diff comments:
In `@frontend/src/app/shared/components/list-item/list-item.component.ts`:
- Around line 101-109: The extracted values from this.listItemForm.getRawValue()
are not being used when building listItemData, leaving quantity unused and
causing inconsistencies; update the extraction to const { itemId, quantity,
unitPrice } = this.listItemForm.getRawValue() and then construct the
ListItemRequestDTO (listItemData) using these variables (itemId, quantity,
unitPrice) along with this.listId to eliminate the unused-variable warning and
ensure consistent/null-safe values from getRawValue() rather than re-reading
this.listItemForm.value.
---
Nitpick comments:
In @.github/workflows/gemini-dispatch.yml:
- Around line 10-11: The workflow uses pull_request_target and a REQUEST
variable that may contain user-provided content; ensure REQUEST is never
directly interpolated into any shell/runner step and instead is passed as a safe
input to actions/github-script or to reusable workflows (e.g.,
gemini-review.yml) where it is sanitized or JSON.stringified before use; review
all reusable workflows referenced (gemini-review.yml, etc.) to validate they
treat inputs from REQUEST as data (not shell commands), perform explicit
escaping/validation in their entrypoint functions, and reduce permissions to
least-privilege where possible (or consider switching the trigger from
pull_request_target to pull_request) so untrusted PR content cannot lead to
command execution.
In @.github/workflows/greetings.yml:
- Around line 12-16: The workflow uses the floating tag
actions/first-interaction@v1; replace that with a pinned version (either a
specific semver tag like actions/first-interaction@v1.1.4 or the full commit
SHA) in the step that references actions/first-interaction to improve
supply-chain security and reproducibility, updating the `uses:` value
accordingly and keeping the existing `with:` inputs unchanged.
In
`@frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.html`:
- Line 8: Replace the template's structural directive "*ngFor" with the
project's native control-flow "@for" in the list-item-dialog template: change
the mat-option line that iterates over items (currently using *ngFor="let item
of items") to use `@for` so the loop binds the same iterator (item) and collection
(items) and continues to set the option value from item.id; ensure the attribute
name is updated on the mat-option element in list-item-dialog.component.html and
that any template expressions referencing item or item.id remain unchanged.
In
`@frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts`:
- Line 45: Replace the plain property items: ItemResponseDTO[] = [] with a
signal-based state and update all usages to use the signal API; specifically,
declare items as a signal (e.g., items = signal<ItemResponseDTO[]>([])) in the
ListItemDialogComponent, and replace any direct array mutations or reads (e.g.,
pushes, assignments, or property accesses referenced around methods in this
component) with items.update(prev => ...) or items.set(...) and read the value
via items() rather than mutating the array directly—ensure no use of mutate and
update all references accordingly.
- Line 4: ListItemDialogComponent currently uses constructor injection; replace
that with Angular's inject() pattern by removing the constructor and creating
class-level readonly properties initialized via inject() for each dependency
previously injected (e.g., MatDialogRef, MAT_DIALOG_DATA, FormBuilder,
ChangeDetectorRef, Store — whatever symbols appear in the component's
constructor). Ensure you import inject from `@angular/core`, assign the exact
tokens/types used in the constructor to readonly fields on the class, and update
any usages to reference those fields; run the request_verification after
changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6b4878fa-2e43-4426-b4a7-b79c9e9bd58b
📒 Files selected for processing (34)
.github/workflows/backend.yml.github/workflows/frontend.yml.github/workflows/gemini-dispatch.yml.github/workflows/greetings.yml.github/workflows/root.yml.github/workflows/sync-develop.ymlCONTRIBUTING.mdLICENSE.mdREADME.mdbackend/LICENSE.mdbackend/README.mdbackend/bruno/README.mdbackend/bruno/bruno.jsonbackend/docker-compose.ymldocker-compose.ymlfrontend/src/app/auth/login/login.component.tsfrontend/src/app/auth/signup/signup.component.tsfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.htmlfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.scssfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.tsfrontend/src/app/list/components/category-dialog/category-dialog.component.tsfrontend/src/app/list/components/items-management/item-dialog/item-dialog.component.tsfrontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.htmlfrontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.tsfrontend/src/app/list/components/list-share-dialog/list-share-dialog.component.tsfrontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.tsfrontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.htmlfrontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.scssfrontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.tsfrontend/src/app/list/components/units-management/unit-dialog/unit-dialog.component.tsfrontend/src/app/shared/components/category/category.component.tsfrontend/src/app/shared/components/item/item.component.tsfrontend/src/app/shared/components/list-item/list-item.component.tsfrontend/src/app/shared/components/unit/unit.component.ts
💤 Files with no reviewable changes (7)
- frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.html
- frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.scss
- frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.ts
- frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts
- backend/bruno/README.md
- frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.scss
- frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.html
✅ Files skipped from review due to trivial changes (8)
- backend/bruno/bruno.json
- README.md
- .github/workflows/root.yml
- .github/workflows/backend.yml
- .github/workflows/frontend.yml
- docker-compose.yml
- backend/docker-compose.yml
- .github/workflows/sync-develop.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/app/auth/login/login.component.ts
- frontend/src/app/list/components/category-dialog/category-dialog.component.ts
- frontend/src/app/auth/signup/signup.component.ts
- frontend/src/app/list/components/units-management/unit-dialog/unit-dialog.component.ts
frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts
Outdated
Show resolved
Hide resolved
frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts
Outdated
Show resolved
Hide resolved
frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts
Show resolved
Hide resolved
Comment on lines
83
to
86
| const { name } = this.listForm.getRawValue(); | ||
| const listData: ShoppingListRequestDTO = { | ||
| name, | ||
| idUser: userId, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify duplicate declarations and inspect the onSubmit block
sed -n '64,90p' frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts
rg -nP 'const\s+name\s*=|const\s+\{\s*name\s*\}\s*=' frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.tsRepository: omatheusmesmo/ShoppMate
Length of output: 951
Remove the name redeclaration to use the validated, trimmed value in the request.
Line 83 redeclares name in the same scope and overwrites the trimmed value from line 75. This causes listData.name to include leading/trailing whitespace despite the validation logic on line 75 trimming it.
🔧 Proposed fix
- const { name } = this.listForm.getRawValue();
const listData: ShoppingListRequestDTO = {
name,
idUser: userId,
};🧰 Tools
🪛 Biome (2.4.6)
[error] 83-83: Shouldn't redeclare 'name'. Consider to delete it or rename it.
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts`
around lines 83 - 86, Remove the redundant redeclaration that overwrites the
already-validated/trimmed name: do not use "const { name } =
this.listForm.getRawValue()" — instead populate listData.name from the
previously trimmed/validated value (the value you trimmed/validated earlier from
this.listForm) so ShoppingListRequestDTO.listData uses the trimmed name; update
the code that builds listData to reference the existing trimmed variable or
re-read and trim via this.listForm.get('name')?.value.trim() rather than
destructuring getRawValue().
frontend/src/app/shared/components/list-item/list-item.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/app/shared/components/item/item.component.ts (1)
59-63: Form lacks strong typing, undermining the PR's "Typed Reactive Forms" objective.The
itemFormis declared asFormGroupwithout type parameters, sogetRawValue()returnsany. This loses compile-time type safety when buildingItemRequestDTO.To align with the PR's goal and other components that adopted strongly-typed forms, consider typing the form:
♻️ Suggested strongly-typed form
+import { FormControl } from '@angular/forms'; - itemForm: FormGroup = this.fb.group({ - name: ['', [Validators.required, Validators.minLength(2)]], - idCategory: ['', Validators.required], - idUnit: ['', Validators.required], - }); + itemForm = this.fb.group({ + name: ['', [Validators.required, Validators.minLength(2)]], + idCategory: [null as number | null, Validators.required], + idUnit: [null as number | null, Validators.required], + });This enables TypeScript to infer the form type, and
getRawValue()will return properly typed values. Initializing withnullinstead of''also better represents "no selection" for numeric fields.As per coding guidelines: "Use strict type checking" and "Avoid the
anytype; useunknownwhen type is uncertain."Also applies to: 100-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/components/item/item.component.ts` around lines 59 - 63, The form is currently untyped (itemForm: FormGroup) which causes getRawValue() to return any; define a typed form value interface (e.g., ItemFormValue with name: string, idCategory: number|null, idUnit: number|null), change the declaration to a generic FormGroup or typed controls (e.g., itemForm: FormGroup<ItemFormValue> or FormGroup<{ name: FormControl<string>; idCategory: FormControl<number|null>; idUnit: FormControl<number|null> }>), initialize numeric/select controls with null instead of '' in the fb.group call, and update any usage (particularly where you build ItemRequestDTO from getRawValue()) so the compiler infers correct types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Around line 59-63: The form is currently untyped (itemForm: FormGroup) which
causes getRawValue() to return any; define a typed form value interface (e.g.,
ItemFormValue with name: string, idCategory: number|null, idUnit: number|null),
change the declaration to a generic FormGroup or typed controls (e.g., itemForm:
FormGroup<ItemFormValue> or FormGroup<{ name: FormControl<string>; idCategory:
FormControl<number|null>; idUnit: FormControl<number|null> }>), initialize
numeric/select controls with null instead of '' in the fb.group call, and update
any usage (particularly where you build ItemRequestDTO from getRawValue()) so
the compiler infers correct types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abf23cc6-daab-4916-93ec-8db22514c66f
📒 Files selected for processing (5)
frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.tsfrontend/src/app/shared/components/category/category.component.tsfrontend/src/app/shared/components/item/item.component.tsfrontend/src/app/shared/components/list-item/list-item.component.tsfrontend/src/app/shared/components/unit/unit.component.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/src/app/shared/components/category/category.component.ts
- frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts
- frontend/src/app/shared/components/unit/unit.component.ts
- frontend/src/app/shared/components/list-item/list-item.component.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/app/shared/components/unit/unit.component.ts (1)
81-85: Use a non-nullable typed form model before trimming raw values.
getRawValue()here is read from a bareFormGroup, sonameandsymbolare weakly typed at extraction time. Preferthis.fb.nonNullable.group(...)(and remove explicit: FormGroup) so.trim()is type-safe and resilient.♻️ Suggested refactor
import { FormBuilder, - FormGroup, Validators, ReactiveFormsModule, } from '@angular/forms'; @@ - readonly unitForm: FormGroup = this.fb.group({ + readonly unitForm = this.fb.nonNullable.group({ name: ['', [Validators.required, Validators.minLength(2)]], symbol: ['', [Validators.required]], });This aligns with the coding guideline "Use strict type checking."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/components/unit/unit.component.ts` around lines 81 - 85, The form currently reads weakly-typed values via this.unitForm.getRawValue() before trimming, which makes name/symbol possibly nullable; change the form creation to use the FormBuilder nonNullable API (this.fb.nonNullable.group(...)) and remove any explicit : FormGroup typing so unitForm is non-nullable; then keep the existing extraction (const { name, symbol } = this.unitForm.getRawValue()) and trims safely, and ensure the Unit creation (const unitData: Unit = { name: normalizedName, symbol: normalizedSymbol }) can rely on non-null types.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/README.md`:
- Around line 16-18: The three TOC entries for "Shopping Lists (/lists)",
"Shopping List Items (/lists/{listId}/items)", and "Shopping List User
Permissions (/lists/{listId}/permissions)" have backticks around the anchor
fragments which prevents them from becoming working links; remove the backticks
so each entry uses normal Markdown link syntax like [Shopping Lists
(/lists)](`#shopping-lists-lists`) (similarly update the other two) to restore
in-page navigation.
---
Nitpick comments:
In `@frontend/src/app/shared/components/unit/unit.component.ts`:
- Around line 81-85: The form currently reads weakly-typed values via
this.unitForm.getRawValue() before trimming, which makes name/symbol possibly
nullable; change the form creation to use the FormBuilder nonNullable API
(this.fb.nonNullable.group(...)) and remove any explicit : FormGroup typing so
unitForm is non-nullable; then keep the existing extraction (const { name,
symbol } = this.unitForm.getRawValue()) and trims safely, and ensure the Unit
creation (const unitData: Unit = { name: normalizedName, symbol:
normalizedSymbol }) can rely on non-null types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: db91e93d-1b62-469a-9cfa-d7cbe3a600ea
📒 Files selected for processing (2)
backend/README.mdfrontend/src/app/shared/components/unit/unit.component.ts
Comment on lines
+16
to
+18
| - [Shopping Lists (/lists)](`#shopping-lists-lists`) | ||
| - [Shopping List Items (/lists/{listId}/items)](`#shopping-list-items-listslistiditems`) | ||
| - [Shopping List User Permissions (/lists/{listId}/permissions)](`#shopping-list-user-permissions-listslistidpermissions`) |
There was a problem hiding this comment.
Fix Markdown link syntax - remove backticks from anchor fragments.
These three TOC links wrap their anchor fragments in backticks, which renders them as literal text instead of working hyperlinks. This breaks in-page navigation.
🔗 Proposed fix
- - [Shopping Lists (/lists)](`#shopping-lists-lists`)
- - [Shopping List Items (/lists/{listId}/items)](`#shopping-list-items-listslistiditems`)
- - [Shopping List User Permissions (/lists/{listId}/permissions)](`#shopping-list-user-permissions-listslistidpermissions`)
+ - [Shopping Lists (/lists)](`#shopping-lists-lists`)
+ - [Shopping List Items (/lists/{listId}/items)](`#shopping-list-items-listslistiditems`)
+ - [Shopping List User Permissions (/lists/{listId}/permissions)](`#shopping-list-user-permissions-listslistidpermissions`)📝 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
| - [Shopping Lists (/lists)](`#shopping-lists-lists`) | |
| - [Shopping List Items (/lists/{listId}/items)](`#shopping-list-items-listslistiditems`) | |
| - [Shopping List User Permissions (/lists/{listId}/permissions)](`#shopping-list-user-permissions-listslistidpermissions`) | |
| - [Shopping Lists (/lists)](`#shopping-lists-lists`) | |
| - [Shopping List Items (/lists/{listId}/items)](`#shopping-list-items-listslistiditems`) | |
| - [Shopping List User Permissions (/lists/{listId}/permissions)](`#shopping-list-user-permissions-listslistidpermissions`) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/README.md` around lines 16 - 18, The three TOC entries for "Shopping
Lists (/lists)", "Shopping List Items (/lists/{listId}/items)", and "Shopping
List User Permissions (/lists/{listId}/permissions)" have backticks around the
anchor fragments which prevents them from becoming working links; remove the
backticks so each entry uses normal Markdown link syntax like [Shopping Lists
(/lists)](`#shopping-lists-lists`) (similarly update the other two) to restore
in-page navigation.
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
2 participants
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.
This is Pr Closes : #42
Summary by CodeRabbit
Documentation
Refactor
UI Changes