fix: Standardize dependency injection using inject()#98
Merged
omatheusmesmo merged 1 commit intoomatheusmesmo:developfrom Mar 13, 2026
Merged
fix: Standardize dependency injection using inject()#98omatheusmesmo merged 1 commit intoomatheusmesmo:developfrom
omatheusmesmo merged 1 commit intoomatheusmesmo:developfrom
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. |
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. |
Owner
|
@matheusandre1 check the conflicts. @coderabbitai check this PR. |
Contributor
|
✅ Actions performedReview triggered.
|
Contributor
|
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:
📝 WalkthroughWalkthroughThis PR standardizes Angular DI to field-level Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
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. |
Contributor
|
Tip For best results, initiate chat on the files or code changes. Thanks for the update! The review is already in progress. Once it completes, the findings will be posted on the PR. 🚀 |
Contributor
|
🤖 I'm sorry @coderabbitai[bot], but I was unable to process your request. Please see the logs for more details. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts (1)
30-37: IncludeisEditin the data type for type safety.The parent component passes
{ category, isEdit: true }when editing and{ isEdit: false }when creating, but the type assertion only includes{ category?: Category }. While the component doesn't currently useisEdit, including it in the type improves type safety and documents the actual data contract between parent and dialog.Suggested type update
- public data = inject(MAT_DIALOG_DATA) as { category?: Category }; + public data = inject(MAT_DIALOG_DATA) as { category?: Category; isEdit?: boolean };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts` around lines 30 - 37, Update the injected MAT_DIALOG_DATA type to include the isEdit flag so the dialog's data contract is accurate: change the assertion on data in CategoryDialogComponent (where data is injected via inject(MAT_DIALOG_DATA)) from { category?: Category } to { category?: Category; isEdit?: boolean } (or isEdit: boolean if always present) and adjust the category initialization that reads this.data.category to continue working unchanged; reference MatDialogRef, MAT_DIALOG_DATA, CategoryDialogComponent, data, and category when making the type change.frontend/src/app/shared/components/list-item/list-item.component.ts (1)
68-68: Consider adding validation for missing route parameter.
Number(null)returns0andNumber('invalid')returnsNaN, which could cause subtle bugs if the route parameter is ever missing or malformed. This is likely pre-existing behavior, but the inline initialization makes it a good opportunity to add a guard.🛡️ Optional: Add a fallback or validation
- listId = Number(this.route.snapshot.paramMap.get('listId')); + listId = Number(this.route.snapshot.paramMap.get('listId')) || 0;Or throw explicitly if the parameter is required:
listId = (() => { const id = Number(this.route.snapshot.paramMap.get('listId')); if (isNaN(id) || id <= 0) { throw new Error('listId route parameter is required'); } return id; })();🤖 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` at line 68, The inline initialization of listId using Number(this.route.snapshot.paramMap.get('listId')) can yield 0 or NaN for missing/invalid params; update the ListItemComponent to validate the route parameter (use this.route.snapshot.paramMap.get('listId')) before assigning to listId, e.g., parse it, check isNaN(...) or <=0 and then either throw a clear Error('listId route parameter is required') or set a safe fallback value, and ensure any downstream usages of listId handle the validated/throwing behavior accordingly.
🤖 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/categories-management/category-dialog/category-dialog.component.ts`:
- Around line 30-37: Update the injected MAT_DIALOG_DATA type to include the
isEdit flag so the dialog's data contract is accurate: change the assertion on
data in CategoryDialogComponent (where data is injected via
inject(MAT_DIALOG_DATA)) from { category?: Category } to { category?: Category;
isEdit?: boolean } (or isEdit: boolean if always present) and adjust the
category initialization that reads this.data.category to continue working
unchanged; reference MatDialogRef, MAT_DIALOG_DATA, CategoryDialogComponent,
data, and category when making the type change.
In `@frontend/src/app/shared/components/list-item/list-item.component.ts`:
- Line 68: The inline initialization of listId using
Number(this.route.snapshot.paramMap.get('listId')) can yield 0 or NaN for
missing/invalid params; update the ListItemComponent to validate the route
parameter (use this.route.snapshot.paramMap.get('listId')) before assigning to
listId, e.g., parse it, check isNaN(...) or <=0 and then either throw a clear
Error('listId route parameter is required') or set a safe fallback value, and
ensure any downstream usages of listId handle the validated/throwing behavior
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7d72b669-c950-43fa-be45-7acfab383144
📒 Files selected for processing (17)
frontend/src/app/list/components/categories-management/categories-management.component.tsfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.tsfrontend/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.tsfrontend/src/app/list/components/shopping-list/shopping-list.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.tsfrontend/src/app/shared/services/category.service.tsfrontend/src/app/shared/services/item.service.tsfrontend/src/app/shared/services/list-permission.service.tsfrontend/src/app/shared/services/shopping-list.service.tsfrontend/src/app/shared/services/unit.service.tsfrontend/src/app/shared/services/user.service.ts
💤 Files with no reviewable changes (6)
- frontend/src/app/shared/services/list-permission.service.ts
- frontend/src/app/shared/services/item.service.ts
- frontend/src/app/shared/services/shopping-list.service.ts
- frontend/src/app/shared/services/category.service.ts
- frontend/src/app/shared/services/user.service.ts
- frontend/src/app/shared/services/unit.service.ts
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 20680
The failure is likely due to a missing or misconfigured Google API key / Gemini credentials in the Gemini workflow. This is unrelated to CodeRabbit's review process. To investigate further, you can check:
CodeRabbit's review has already been triggered and should be running independently. 🚀 |
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
matheusandre1
force-pushed
the
fix-issue29
branch
from
51ff731 to
b0d7311
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts (1)
75-83:⚠️ Potential issue | 🔴 CriticalCritical: Duplicate DI - constructor not removed after inject() migration.
The constructor was not removed during the refactor, causing:
- Duplicate declarations of
dialogRefanddata(already declared viainject()at lines 62-63)- Duplicate initialization of
shareForm(already initialized at lines 65-68)@Injectdecorator used butInjectis not imported from@angular/coreThis will cause compilation errors due to duplicate property declarations.
🐛 Proposed fix: Remove the constructor entirely
- constructor( - public dialogRef: MatDialogRef<ListShareDialogComponent>, - `@Inject`(MAT_DIALOG_DATA) public data: { listId: number; listName: string }, - ) { - this.shareForm = this.fb.group({ - email: ['', [Validators.required, Validators.email]], - permission: [Permission.READ, Validators.required], - }); - } - ngOnInit(): void {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts` around lines 75 - 83, The constructor block should be removed to avoid duplicate DI and initialization because dialogRef and data are already provided via inject() and shareForm is already initialized; delete the entire constructor(...) { ... } in ListShareDialogComponent, remove the `@Inject` usage inside it (and any leftover Inject import if present), and ensure only the inject()-based dialogRef, data, and the existing shareForm initialization remain.frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts (1)
48-55:⚠️ Potential issue | 🔴 CriticalCritical:
categoryName()is undefined - will cause runtime error.The
onSave()method callsthis.categoryName()(line 49), but nocategoryNameproperty exists in this component. Based on the refactoring that introduced thecategoryobject (lines 38-42), this should referencethis.category.nameinstead.🐛 Proposed fix
onSave(): void { - const trimmedName = this.categoryName().trim(); + const trimmedName = this.category.name.trim(); if (!trimmedName) return; this.dialogRef.close({ - ...this.data.category, - name: trimmedName, + ...this.category, + name: trimmedName, } satisfies Category); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts` around lines 48 - 55, onSave() currently calls a non-existent method this.categoryName(), which causes a runtime error; update the method to read and trim the name from the category object (use this.category.name), validate it, and then close the dialog with the updated Category payload (preserving other fields via ...this.data.category or this.category as appropriate) so the dialogRef.close call still returns a valid Category object with the trimmed name.
🧹 Nitpick comments (11)
frontend/src/app/shared/components/item/item.component.ts (1)
34-35: Remove explicitstandalone: truedeclaration.Per coding guidelines,
standalone: trueis the default in Angular v20+ and should not be explicitly set.♻️ Proposed fix
`@Component`({ - standalone: true, selector: 'app-item',🤖 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 34 - 35, Remove the explicit "standalone: true" property from the `@Component` metadata on the ItemComponent (the Component decorator in item.component.ts); edit the `@Component`({ ... }) block to omit the standalone key while leaving all other metadata (selector, templateUrl, styleUrls, imports, etc.) intact so behavior and imports remain unchanged.frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts (1)
38-39: Remove explicitstandalone: truedeclaration.Per coding guidelines,
standalone: trueis the default in Angular v20+ and should not be explicitly set.♻️ Proposed fix
`@Component`({ selector: 'app-list-share-dialog', - standalone: true, imports: [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts` around lines 38 - 39, The `@Component` decorator in ListShareDialogComponent explicitly sets standalone: true; remove that property from the decorator (i.e., delete the standalone: true line in the `@Component` metadata for the class ListShareDialogComponent) so the component relies on the Angular v20+ default behavior; leave other metadata like selector intact.frontend/src/app/list/components/categories-management/categories-management.component.ts (1)
20-21: Remove explicitstandalone: truedeclaration.Per coding guidelines,
standalone: trueis the default in Angular v20+ and should not be explicitly set.♻️ Proposed fix
`@Component`({ selector: 'app-categories-management', - standalone: true, imports: [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/categories-management/categories-management.component.ts` around lines 20 - 21, Remove the explicit standalone: true property from the component decorator in CategoriesManagementComponent (the object containing selector: 'app-categories-management'); simply delete the standalone line so the decorator only declares necessary metadata (e.g., selector and other metadata) and relies on Angular v20+ default behavior.frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts (2)
5-5: Remove unusedsignalimport.The
signalfunction is imported but not used in this component after the refactoring.♻️ Proposed fix
import { ChangeDetectionStrategy, Component, inject, - signal, } from '@angular/core';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts` at line 5, The import of the symbol `signal` in category-dialog.component.ts is unused after the refactor; remove `signal` from the import list at the top of the CategoryDialogComponent file (where other Angular/utility imports are declared) so the file no longer imports an unused symbol and then run the linter/TypeScript check to ensure no remaining references to `signal` exist.
20-21: Remove explicitstandalone: truedeclaration.Per coding guidelines,
standalone: trueis the default in Angular v20+ and should not be explicitly set.♻️ Proposed fix
`@Component`({ selector: 'app-category-dialog', - standalone: true, imports: [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts` around lines 20 - 21, Remove the explicit standalone: true property from the `@Component` decorator on CategoryDialogComponent (the component with selector 'app-category-dialog'); open the decorator that defines selector and other metadata and delete the standalone: true line so the component relies on the Angular v20+ default instead of declaring it explicitly.frontend/src/app/shared/components/unit/unit.component.ts (1)
25-26: Remove explicitstandalone: truedeclaration.Per coding guidelines,
standalone: trueis the default in Angular v20+ and should not be explicitly set inside Angular decorators.♻️ Proposed fix
`@Component`({ - standalone: true, selector: 'app-unit',🤖 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 25 - 26, The `@Component` decorator in UnitComponent explicitly sets standalone: true which is redundant under Angular v20+; remove the standalone: true property from the `@Component`({...}) object in frontend/src/app/shared/components/unit/unit.component.ts (the decorator attached to the UnitComponent class) so the decorator no longer includes that key while preserving all other metadata.frontend/src/app/shared/components/category/category.component.ts (2)
49-51: Consider addingreadonlymodifier for consistency.In
unit.component.ts,unitFormis declared asreadonly. For consistency across similar components, consider makingcategoryFormreadonly as well since the FormGroup instance itself shouldn't be reassigned.♻️ Proposed fix
- categoryForm: FormGroup = this.fb.group({ + readonly categoryForm: FormGroup = this.fb.group({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/components/category/category.component.ts` around lines 49 - 51, Make categoryForm readonly like unitForm so the FormGroup instance cannot be reassigned: change the declaration of categoryForm (the FormGroup created with this.fb.group and validators) to include the readonly modifier, ensuring the form variable remains immutable while still allowing control value changes; update the declaration near the FormGroup creation that references categoryForm and this.fb to match the readonly pattern used for unitForm.
25-26: Remove explicitstandalone: truedeclaration.Per coding guidelines,
standalone: trueis the default in Angular v20+ and should not be explicitly set inside Angular decorators.♻️ Proposed fix
`@Component`({ - standalone: true, selector: 'app-category',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/components/category/category.component.ts` around lines 25 - 26, The `@Component` decorator on CategoryComponent currently includes an explicit standalone: true property which should be removed; edit the CategoryComponent decorator (the `@Component` declaration in category.component.ts) and delete the standalone: true entry so the decorator uses the default Angular v20+ behavior, leaving the rest of the metadata (selector, template, styles, imports, etc.) unchanged.frontend/src/app/shared/components/list-item/list-item.component.ts (1)
34-35: Remove explicitstandalone: truedeclaration.Per coding guidelines,
standalone: trueis the default in Angular v20+ and should not be explicitly set.♻️ Proposed fix
`@Component`({ - standalone: true, selector: 'app-list-item',🤖 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 34 - 35, Remove the explicit standalone: true property from the `@Component` decorator metadata in the ListItemComponent (the component declared in list-item.component.ts) — open the `@Component`(...) block and delete the standalone: true entry so the decorator relies on the Angular v20+ default; ensure the decorator object remains syntactically valid (commas/spacing) after removal.frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.ts (2)
38-43: Prefer signal-based local state for this component.This moves local form state from signals to a mutable object. Consider keeping it as a signal to stay consistent with project state-management rules.
As per coding guidelines, "Use signals for local state. Do NOT use
mutate, useupdateorset."🤖 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/list-dialog/list-dialog.component.ts` around lines 38 - 43, The local component state is assigned as a mutable object (list: ShoppingListRequestDTO = ...) instead of using an Angular signal; change this to a signal (e.g., const list = signal<ShoppingListRequestDTO>(...) or a class property list = signal<ShoppingListRequestDTO>(...)) inside ListDialogComponent so the component uses signal-based local state; initialize the signal with either a cloned this.data.list or the default object ({ name: '', idUser: 0 }), import signal from `@angular/core`, and when updating the state anywhere use list.update(...) or list.set(...) (do not use mutate).
36-43: Use signals for local state instead of mutable object.The
listfield at lines 38-43 is a plain mutable object, but Angular guidelines require using signals for local state. Signal is already imported but unused.Suggested refactor
- list: ShoppingListRequestDTO = this.data.list + list = signal<ShoppingListRequestDTO>( this.data.list - ? { ...this.data.list } + ? { ...this.data.list } : { name: '', idUser: 0, } + );Update template and
onSave()to use.set()or.update()instead of direct mutations.🤖 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/list-dialog/list-dialog.component.ts` around lines 36 - 43, The local component state uses a mutable plain object in the list property instead of an Angular signal; replace the mutable "list" object with a Signal (e.g., const list = signal<ShoppingListRequestDTO>(...)) initialized from the injected data (data = inject(MAT_DIALOG_DATA)), update all mutations to use list.set() or list.update(), and modify onSave() and the template bindings to read from the signal (list() or async pipe) and write via set/update so the component uses the imported Signal APIs rather than direct property mutations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/branch-enforcer.yml:
- Around line 71-72: Two consecutive core.setFailed(...) calls are redundant and
the first one assumes error has a message property; remove the first
core.setFailed(error.message) call and keep the safer core.setFailed(error
instanceof Error ? error.message : String(error)) to handle non-Error values
safely, locating and editing the consecutive core.setFailed calls around the
error handling block where the error variable is used.
- Around line 6-7: The workflow has duplicate pull_request_target keys in the
on: section which makes the YAML invalid; remove the redundant
pull_request_target block so only a single pull_request_target mapping remains,
and if both blocks had different types combine them into one types: array (e.g.
include opened, edited, synchronize, reopened, etc.) under the single
pull_request_target key; then validate the YAML to ensure there are no duplicate
mapping keys.
In `@frontend/src/app/shared/components/list-item/list-item.component.ts`:
- Around line 60-69: The component switched plain properties back from signals
but consumers still treat them as signals; revert these properties to signals to
match existing .set() and invocation usage: import signal and Signal from
`@angular/core` and change listItems, availableItems, isLoading, and
editingListItemId to signals (e.g., listItems: Signal<ListItemResponseDTO[]> =
signal([]); availableItems: Signal<ItemResponseDTO[]> = signal([]); isLoading:
Signal<boolean> = signal(false); editingListItemId: Signal<number | null> =
signal(null)); keep listItemForm and listId as-is; this will make existing calls
like listItems().set(...), availableItems().set(...), isLoading.set(...),
editingListItemId.set(...) and any listItems()/availableItems() invocations
valid again. Ensure types use Signal<...> where needed and update the import
list accordingly.
---
Outside diff comments:
In
`@frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts`:
- Around line 48-55: onSave() currently calls a non-existent method
this.categoryName(), which causes a runtime error; update the method to read and
trim the name from the category object (use this.category.name), validate it,
and then close the dialog with the updated Category payload (preserving other
fields via ...this.data.category or this.category as appropriate) so the
dialogRef.close call still returns a valid Category object with the trimmed
name.
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 75-83: The constructor block should be removed to avoid duplicate
DI and initialization because dialogRef and data are already provided via
inject() and shareForm is already initialized; delete the entire
constructor(...) { ... } in ListShareDialogComponent, remove the `@Inject` usage
inside it (and any leftover Inject import if present), and ensure only the
inject()-based dialogRef, data, and the existing shareForm initialization
remain.
---
Nitpick comments:
In
`@frontend/src/app/list/components/categories-management/categories-management.component.ts`:
- Around line 20-21: Remove the explicit standalone: true property from the
component decorator in CategoriesManagementComponent (the object containing
selector: 'app-categories-management'); simply delete the standalone line so the
decorator only declares necessary metadata (e.g., selector and other metadata)
and relies on Angular v20+ default behavior.
In
`@frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts`:
- Line 5: The import of the symbol `signal` in category-dialog.component.ts is
unused after the refactor; remove `signal` from the import list at the top of
the CategoryDialogComponent file (where other Angular/utility imports are
declared) so the file no longer imports an unused symbol and then run the
linter/TypeScript check to ensure no remaining references to `signal` exist.
- Around line 20-21: Remove the explicit standalone: true property from the
`@Component` decorator on CategoryDialogComponent (the component with selector
'app-category-dialog'); open the decorator that defines selector and other
metadata and delete the standalone: true line so the component relies on the
Angular v20+ default instead of declaring it explicitly.
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 38-39: The `@Component` decorator in ListShareDialogComponent
explicitly sets standalone: true; remove that property from the decorator (i.e.,
delete the standalone: true line in the `@Component` metadata for the class
ListShareDialogComponent) so the component relies on the Angular v20+ default
behavior; leave other metadata like selector intact.
In
`@frontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.ts`:
- Around line 38-43: The local component state is assigned as a mutable object
(list: ShoppingListRequestDTO = ...) instead of using an Angular signal; change
this to a signal (e.g., const list = signal<ShoppingListRequestDTO>(...) or a
class property list = signal<ShoppingListRequestDTO>(...)) inside
ListDialogComponent so the component uses signal-based local state; initialize
the signal with either a cloned this.data.list or the default object ({ name:
'', idUser: 0 }), import signal from `@angular/core`, and when updating the state
anywhere use list.update(...) or list.set(...) (do not use mutate).
- Around line 36-43: The local component state uses a mutable plain object in
the list property instead of an Angular signal; replace the mutable "list"
object with a Signal (e.g., const list = signal<ShoppingListRequestDTO>(...))
initialized from the injected data (data = inject(MAT_DIALOG_DATA)), update all
mutations to use list.set() or list.update(), and modify onSave() and the
template bindings to read from the signal (list() or async pipe) and write via
set/update so the component uses the imported Signal APIs rather than direct
property mutations.
In `@frontend/src/app/shared/components/category/category.component.ts`:
- Around line 49-51: Make categoryForm readonly like unitForm so the FormGroup
instance cannot be reassigned: change the declaration of categoryForm (the
FormGroup created with this.fb.group and validators) to include the readonly
modifier, ensuring the form variable remains immutable while still allowing
control value changes; update the declaration near the FormGroup creation that
references categoryForm and this.fb to match the readonly pattern used for
unitForm.
- Around line 25-26: The `@Component` decorator on CategoryComponent currently
includes an explicit standalone: true property which should be removed; edit the
CategoryComponent decorator (the `@Component` declaration in
category.component.ts) and delete the standalone: true entry so the decorator
uses the default Angular v20+ behavior, leaving the rest of the metadata
(selector, template, styles, imports, etc.) unchanged.
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Around line 34-35: Remove the explicit "standalone: true" property from the
`@Component` metadata on the ItemComponent (the Component decorator in
item.component.ts); edit the `@Component`({ ... }) block to omit the standalone
key while leaving all other metadata (selector, templateUrl, styleUrls, imports,
etc.) intact so behavior and imports remain unchanged.
In `@frontend/src/app/shared/components/list-item/list-item.component.ts`:
- Around line 34-35: Remove the explicit standalone: true property from the
`@Component` decorator metadata in the ListItemComponent (the component declared
in list-item.component.ts) — open the `@Component`(...) block and delete the
standalone: true entry so the decorator relies on the Angular v20+ default;
ensure the decorator object remains syntactically valid (commas/spacing) after
removal.
In `@frontend/src/app/shared/components/unit/unit.component.ts`:
- Around line 25-26: The `@Component` decorator in UnitComponent explicitly sets
standalone: true which is redundant under Angular v20+; remove the standalone:
true property from the `@Component`({...}) object in
frontend/src/app/shared/components/unit/unit.component.ts (the decorator
attached to the UnitComponent class) so the decorator no longer includes that
key while preserving all other metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 71b32881-57ed-44a1-81d3-4843a6f3427a
📒 Files selected for processing (18)
.github/workflows/branch-enforcer.ymlfrontend/src/app/list/components/categories-management/categories-management.component.tsfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.tsfrontend/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.tsfrontend/src/app/list/components/shopping-list/shopping-list.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.tsfrontend/src/app/shared/components/unit/unit.component.htmlfrontend/src/app/shared/components/unit/unit.component.tsfrontend/src/app/shared/services/auth.service.tsfrontend/src/app/shared/services/category.service.tsfrontend/src/app/shared/services/list-permission.service.tsfrontend/src/app/shared/services/unit.service.ts
💤 Files with no reviewable changes (3)
- frontend/src/app/shared/services/unit.service.ts
- frontend/src/app/shared/services/category.service.ts
- frontend/src/app/shared/services/list-permission.service.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/src/app/shared/services/auth.service.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/app/list/components/shopping-list/shopping-list.component.ts
- frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts
- frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts
matheusandre1
force-pushed
the
fix-issue29
branch
from
b0d7311 to
5fbe85e
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
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/list/components/list-share-dialog/list-share-dialog.component.ts (1)
77-85:⚠️ Potential issue | 🔴 CriticalRemove the constructor to complete the
inject()migration.The constructor was not removed after adding
inject()at lines 64-65, causing:
- Missing import:
@Injectdecorator is used but not imported from@angular/core- Duplicate declarations:
dialogRefanddataare declared both as class fields (lines 64-65) and constructor parameters- Duplicate initialization:
shareFormis initialized inline (lines 67-70) and in the constructorThis will fail TypeScript compilation. As per coding guidelines: "Use the
inject()function instead of constructor injection." The linked issue#29also requires removing constructors that exist solely for DI.🐛 Proposed fix: Remove the entire constructor
readonly isLoading = signal(false); - constructor( - public dialogRef: MatDialogRef<ListShareDialogComponent>, - `@Inject`(MAT_DIALOG_DATA) public data: { listId: number; listName: string }, - ) { - this.shareForm = this.fb.group({ - email: ['', [Validators.required, Validators.email]], - permission: [Permission.READ, Validators.required], - }); - } - ngOnInit(): void {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts` around lines 77 - 85, Remove the entire constructor block from ListShareDialogComponent so DI is done exclusively via inject(): delete the constructor that takes MatDialogRef and `@Inject`(MAT_DIALOG_DATA) data and the inline initialization of shareForm inside it; keep the class-level inject() usages for dialogRef and data (MAT_DIALOG_DATA/MatDialogRef) and the existing inline shareForm initialization (which uses fb), and remove any leftover `@Inject` import or duplicate declarations to avoid duplicate initialization and missing-import errors.frontend/src/app/shared/components/item/item.component.ts (2)
1-8:⚠️ Potential issue | 🔴 CriticalRemove duplicated
injectimport to fix parse failure.The
injectfunction is imported twice from@angular/core(lines 5 and 7), creating an invalid import declaration that breaks compilation.Proposed fix
import { ChangeDetectionStrategy, Component, OnInit, inject, signal, - inject, } from '@angular/core';🤖 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 1 - 8, The import statement in item.component.ts includes a duplicated symbol `inject` from `@angular/core`, causing a parse/compile failure; open the import declaration that currently lists ChangeDetectionStrategy, Component, OnInit, inject, signal, inject and remove the duplicate `inject` entry so each symbol is imported only once (leave ChangeDetectionStrategy, Component, OnInit, signal, inject in the list).
55-86:⚠️ Potential issue | 🔴 CriticalRemove constructor; class has duplicate service declarations that will not compile.
Lines 55–58 declare services via
inject(), but the constructor at lines 75–85 redeclares the identical names as parameter properties (private itemService,private categoryService,private unitService,private fb). This creates duplicate class members, which is a TypeScript compilation error. Additionally,itemFormis already initialized in the field declaration (lines 61–65), making the constructor body redundant.Proposed fix
private confirmDialog = inject(ConfirmDialogService); private feedback = inject(FeedbackService); - - constructor( - private itemService: ItemService, - private categoryService: CategoryService, - private unitService: UnitService, - private fb: FormBuilder, - ) { - this.itemForm = this.fb.group({ - name: ['', [Validators.required, Validators.minLength(2)]], - idCategory: ['', Validators.required], - idUnit: ['', Validators.required], - }); - } ngOnInit(): void {Per Angular guidelines, use the
inject()function instead of constructor injection.🤖 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 55 - 86, The class currently declares services via inject() (itemService, categoryService, unitService, fb) and then re-declares them as constructor parameters, causing duplicate members and a compile error; remove the entire constructor (including its parameter list and body) so the class relies solely on the inject() fields, and ensure the existing itemForm field (which already uses this.fb) remains as the single FormGroup initialization (delete the redundant itemForm initialization inside the removed constructor if present).frontend/src/app/shared/components/category/category.component.ts (1)
44-65:⚠️ Potential issue | 🔴 CriticalRemove duplicate DI declarations and add missing MatSnackBar import.
The class has conflicting member declarations:
- Lines 44–45 declare
categoryServiceandfbviainject()- Lines 59–60 redeclare the same members as constructor parameters, violating the coding guideline to use
inject()instead of constructor injection- Line 46 uses
inject(MatSnackBar)but MatSnackBar is not imported, causing a compile error- The form is initialized twice: inline (lines 51–52) and again in the constructor (lines 62–63)
Remove the constructor and constructor-based DI declarations. Add the MatSnackBar import from
@angular/material/snack-bar.Proposed fix
+import { MatSnackBarModule } from '@angular/material/snack-bar'; `@Component`({ standalone: true, selector: 'app-category',- constructor( - private categoryService: CategoryService, - private fb: FormBuilder, - ) { - this.categoryForm = this.fb.group({ - name: ['', [Validators.required, Validators.minLength(2)]], - }); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/components/category/category.component.ts` around lines 44 - 65, Remove the redundant constructor and its parameters (CategoryService, FormBuilder) and keep the class-level inject() usages for categoryService, fb, snackBar, confirmDialog, and feedback; delete the duplicate categoryForm initialization in the constructor so the inline initialization (categoryForm = this.fb.group(...)) is the sole initializer; add an import for MatSnackBar from '@angular/material/snack-bar' so the existing private snackBar = inject(MatSnackBar) compiles; ensure editingCategoryId, categories, and isLoading remain unchanged.
♻️ Duplicate comments (2)
.github/workflows/branch-enforcer.yml (2)
71-72:⚠️ Potential issue | 🟠 MajorDrop the first
core.setFailed(...)call in the catch block.Line 71 is redundant and less safe; keep only the guarded call on Line 72.
🐛 Proposed fix
} catch (error) { console.error("❌ Error processing branch enforcement:", error); - core.setFailed(error.message); core.setFailed(error instanceof Error ? error.message : String(error)); }#!/bin/bash set -euo pipefail FILE=".github/workflows/branch-enforcer.yml" echo "Inspecting setFailed calls in catch block:" rg -n -C3 'catch \(error\)|core\.setFailed\(' "$FILE"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/branch-enforcer.yml around lines 71 - 72, Remove the redundant unguarded core.setFailed(error.message) call in the catch block and keep only the guarded call core.setFailed(error instanceof Error ? error.message : String(error)); i.e., locate the catch block that calls core.setFailed twice and delete the first/core.setFailed(error.message) invocation so only the safe, type-checked core.setFailed(...) remains.
6-7:⚠️ Potential issue | 🔴 CriticalRemove the duplicated
pull_request_targetmapping.Line 6 duplicates the key defined at Line 4, making the
on:mapping invalid YAML and breaking workflow parsing.🐛 Proposed fix
on: pull_request_target: types: [opened, edited, synchronize, reopened] - pull_request_target: - types: [opened, edited, synchronize, reopened]#!/bin/bash set -euo pipefail FILE=".github/workflows/branch-enforcer.yml" echo "Checking duplicate pull_request_target entries inside 'on:'" awk ' /^on:\s*$/ {in_on=1; next} in_on && /^[^[:space:]]/ {in_on=0} in_on && /^[[:space:]]+pull_request_target:\s*$/ {print NR ":" $0} ' "$FILE" if command -v actionlint >/dev/null 2>&1; then actionlint "$FILE" else echo "actionlint not installed; skipped." fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/branch-enforcer.yml around lines 6 - 7, There is a duplicated pull_request_target mapping inside the on: workflow map causing invalid YAML; remove the redundant pull_request_target block (or merge its types into the existing pull_request_target mapping) so only a single pull_request_target key exists under on:, ensuring the combined types list (opened, edited, synchronize, reopened) remains present in the remaining pull_request_target entry.
🧹 Nitpick comments (2)
frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts (1)
56-61: Prefer a strongly typed non-nullable reactive form here.
FormGroup+listForm.value.nameweakens type safety. UsenonNullable.group(or explicit typed controls) sonameis always astringand compile-time checks stay strict.Suggested refactor
- readonly listForm: FormGroup = this.fb.group({ + readonly listForm = this.fb.nonNullable.group({ name: [ this.data.list?.listName ?? '', [Validators.required, Validators.minLength(3)], ], });- const name = (this.listForm.value.name ?? '').trim(); + const name = this.listForm.controls.name.value.trim();As per coding guidelines, "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/list/components/shopping-list-dialog/shopping-list-dialog.component.ts` around lines 56 - 61, The reactive form listForm currently typed as FormGroup allows nullable control values; change its creation to use the non-nullable form builders so the name control is always a string (e.g., use this.fb.nonNullable.group or explicit typed controls) and update the listForm declaration from FormGroup to the appropriate typed form group type so accesses like listForm.value.name are statically string-typed; update any code using listForm.value to match the new typed API (e.g., remove unnecessary null checks) and ensure validators remain unchanged on the name control.frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts (1)
55-59: Consider handling subscription cleanup and errors.The
loadItems()subscription is not unsubscribed and lacks error handling. While this may be acceptable for a short-lived dialog, adding error handling would improve resilience.♻️ Optional: Add error handling
loadItems(): void { - this.itemService.getAllItems().subscribe((items) => { - this.items.set(items); + this.itemService.getAllItems().subscribe({ + next: (items) => this.items.set(items), + error: (err) => console.error('Failed to load items', err), }); }🤖 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` around lines 55 - 59, The loadItems() subscription currently neither handles errors nor cleans up; update loadItems() to pipe the observable from itemService.getAllItems() through takeUntil(this.destroy$) (add a private destroy$ = new Subject<void>() to the component), subscribe with an object callback like { next: items => this.items.set(items), error: err => /* log or surface error (e.g., console.error or process via a logger) */ }, and implement ngOnDestroy() to call this.destroy$.next() and this.destroy$.complete() so the subscription is properly torn down; this changes references in loadItems(), itemService.getAllItems(), this.items.set, and adds the destroy$ Subject and ngOnDestroy lifecycle hook.
🤖 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/list/components/categories-management/categories-management.component.ts`:
- Around line 36-46: Replace constructor injection for ConfirmDialogService and
FeedbackService with the inject() pattern and remove the unused MatSnackBar
injection and field: add private confirmDialog = inject(ConfirmDialogService)
and private feedback = inject(FeedbackService) to the class properties, delete
the constructor entirely (or remove those params), and remove the snackBar
property and its MatSnackBar import to eliminate the unused symbol.
In
`@frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.ts`:
- Around line 1-6: Remove the unused signal import from the Angular core import
list to satisfy the `@typescript-eslint/no-unused-vars` rule; update the import
statement that currently includes ChangeDetectionStrategy, Component, inject,
signal to only import the used symbols (e.g., ChangeDetectionStrategy,
Component, inject) so the CategoryDialogComponent file no longer imports the
unused symbol "signal".
- Around line 38-42: The onSave method currently calls a non-existent
categoryName() function causing a build error—update onSave to read the category
name from the component property (use this.category.name) and ensure it handles
empty/whitespace names the same way it intended; also remove the unused import
of signal from the file imports. Locate the CategoryDialogComponent, update the
onSave implementation to reference this.category.name instead of
this.categoryName(), and delete the unused signal import statement.
In
`@frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts`:
- Around line 40-49: The signals selectedItemId and quantity are not initialized
from the injected data after removing the constructor; update
ListItemDialogComponent to restore the edit-mode initialization in ngOnInit:
check if this.data.listItem exists and then set the selectedItemId signal to the
existing item id (e.g., call selectedItemId.set(this.data.listItem.itemId)) and
set the quantity signal to the existing quantity
(quantity.set(this.data.listItem.quantity)); ensure this logic runs early in
ngOnInit so the dialog shows the current values when editing.
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 64-70: The file now uses inject() to initialize dialogRef, data,
and inline shareForm, but the old constructor (the class constructor that also
declares dialogRef, data, and shareForm) remains and causes duplicate property
declarations; remove the entire constructor block so there are no duplicate
declarations and the class relies solely on the injected properties (dialogRef,
data) and the inline FormGroup (shareForm) defined at class scope.
In
`@frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts`:
- Around line 74-75: The submit handler currently ignores whitespace-only input:
when computing name from this.listForm.value.name you must mark the name control
touched and set a validation error before returning so the UI/ARIA shows
feedback; for example, get the FormControl via this.listForm.get('name') (or
this.listForm.controls.name), call control.markAsTouched(), and call
control.setErrors({ requiredTrim: true }) (or add a trim-aware validator
function and attach it to the 'name' control) then return so the form displays
the accessible error message; ensure you update any existing validators on the
'name' control (e.g. in ShoppingListDialogComponent form initialization) if
opting for the validator approach.
In `@frontend/src/app/shared/components/category/category.component.ts`:
- Line 46: The file references MatSnackBar via the private snackBar =
inject(MatSnackBar) field but MatSnackBar is not imported and the snackBar field
is unused; remove the unused injection line (private snackBar =
inject(MatSnackBar)) from the CategoryComponent and also remove any leftover
import for MatSnackBar from `@angular/material/snack-bar` if present so the
component compiles cleanly.
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Line 59: Import MatSnackBar from '@angular/material/snack-bar' and add
MatSnackBarModule to the standalone component's imports array so the injected
MatSnackBar (snackBar field) is available; locate the snackBar =
inject(MatSnackBar) line in item.component.ts, add the MatSnackBar import at the
top, and add MatSnackBarModule to the `@Component` imports list (or remove the
snackBar field entirely if it is unused).
---
Outside diff comments:
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 77-85: Remove the entire constructor block from
ListShareDialogComponent so DI is done exclusively via inject(): delete the
constructor that takes MatDialogRef and `@Inject`(MAT_DIALOG_DATA) data and the
inline initialization of shareForm inside it; keep the class-level inject()
usages for dialogRef and data (MAT_DIALOG_DATA/MatDialogRef) and the existing
inline shareForm initialization (which uses fb), and remove any leftover `@Inject`
import or duplicate declarations to avoid duplicate initialization and
missing-import errors.
In `@frontend/src/app/shared/components/category/category.component.ts`:
- Around line 44-65: Remove the redundant constructor and its parameters
(CategoryService, FormBuilder) and keep the class-level inject() usages for
categoryService, fb, snackBar, confirmDialog, and feedback; delete the duplicate
categoryForm initialization in the constructor so the inline initialization
(categoryForm = this.fb.group(...)) is the sole initializer; add an import for
MatSnackBar from '@angular/material/snack-bar' so the existing private snackBar
= inject(MatSnackBar) compiles; ensure editingCategoryId, categories, and
isLoading remain unchanged.
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Around line 1-8: The import statement in item.component.ts includes a
duplicated symbol `inject` from `@angular/core`, causing a parse/compile
failure; open the import declaration that currently lists
ChangeDetectionStrategy, Component, OnInit, inject, signal, inject and remove
the duplicate `inject` entry so each symbol is imported only once (leave
ChangeDetectionStrategy, Component, OnInit, signal, inject in the list).
- Around line 55-86: The class currently declares services via inject()
(itemService, categoryService, unitService, fb) and then re-declares them as
constructor parameters, causing duplicate members and a compile error; remove
the entire constructor (including its parameter list and body) so the class
relies solely on the inject() fields, and ensure the existing itemForm field
(which already uses this.fb) remains as the single FormGroup initialization
(delete the redundant itemForm initialization inside the removed constructor if
present).
---
Duplicate comments:
In @.github/workflows/branch-enforcer.yml:
- Around line 71-72: Remove the redundant unguarded
core.setFailed(error.message) call in the catch block and keep only the guarded
call core.setFailed(error instanceof Error ? error.message : String(error));
i.e., locate the catch block that calls core.setFailed twice and delete the
first/core.setFailed(error.message) invocation so only the safe, type-checked
core.setFailed(...) remains.
- Around line 6-7: There is a duplicated pull_request_target mapping inside the
on: workflow map causing invalid YAML; remove the redundant pull_request_target
block (or merge its types into the existing pull_request_target mapping) so only
a single pull_request_target key exists under on:, ensuring the combined types
list (opened, edited, synchronize, reopened) remains present in the remaining
pull_request_target entry.
---
Nitpick comments:
In
`@frontend/src/app/list/components/list-details/list-item-dialog/list-item-dialog.component.ts`:
- Around line 55-59: The loadItems() subscription currently neither handles
errors nor cleans up; update loadItems() to pipe the observable from
itemService.getAllItems() through takeUntil(this.destroy$) (add a private
destroy$ = new Subject<void>() to the component), subscribe with an object
callback like { next: items => this.items.set(items), error: err => /* log or
surface error (e.g., console.error or process via a logger) */ }, and implement
ngOnDestroy() to call this.destroy$.next() and this.destroy$.complete() so the
subscription is properly torn down; this changes references in loadItems(),
itemService.getAllItems(), this.items.set, and adds the destroy$ Subject and
ngOnDestroy lifecycle hook.
In
`@frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts`:
- Around line 56-61: The reactive form listForm currently typed as FormGroup
allows nullable control values; change its creation to use the non-nullable form
builders so the name control is always a string (e.g., use
this.fb.nonNullable.group or explicit typed controls) and update the listForm
declaration from FormGroup to the appropriate typed form group type so accesses
like listForm.value.name are statically string-typed; update any code using
listForm.value to match the new typed API (e.g., remove unnecessary null checks)
and ensure validators remain unchanged on the name control.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9417d0ef-a912-46e4-a4cf-dcc0d6031d90
📒 Files selected for processing (17)
.github/workflows/branch-enforcer.ymlfrontend/src/app/list/components/categories-management/categories-management.component.tsfrontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.tsfrontend/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.tsfrontend/src/app/list/components/shopping-list/shopping-list.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.htmlfrontend/src/app/shared/components/unit/unit.component.tsfrontend/src/app/shared/services/auth.service.tsfrontend/src/app/shared/services/category.service.tsfrontend/src/app/shared/services/list-permission.service.tsfrontend/src/app/shared/services/unit.service.ts
💤 Files with no reviewable changes (3)
- frontend/src/app/shared/services/unit.service.ts
- frontend/src/app/shared/services/list-permission.service.ts
- frontend/src/app/shared/services/category.service.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- frontend/src/app/shared/components/unit/unit.component.ts
- frontend/src/app/list/components/shopping-list/shopping-list.component.ts
- frontend/src/app/shared/components/unit/unit.component.html
- frontend/src/app/shared/services/auth.service.ts
- frontend/src/app/shared/components/list-item/list-item.component.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/list/components/shopping-list/list-dialog/list-dialog.component.ts`:
- Around line 30-38: ListDialogComponent currently injects MAT_DIALOG_DATA
unsafely and accesses this.data.list without guarding; change the injection of
MAT_DIALOG_DATA to be optional (use inject(MAT_DIALOG_DATA, { optional: true }))
and update the typed field (data) to accept undefined, then initialize list in
the constructor or inline with a null-safe check (e.g., if this.data?.list use
that else use the default { name: '', idUser: 0 }) so the component does not
crash when opened without data; reference symbols: ListDialogComponent,
MAT_DIALOG_DATA, data, list, dialogRef.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 191f8048-987e-4fb8-965a-9ea0b459fa41
📒 Files selected for processing (2)
frontend/src/app/list/components/categories-management/category-dialog/category-dialog.component.tsfrontend/src/app/list/components/shopping-list/list-dialog/list-dialog.component.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/shared/components/unit/unit.component.ts`:
- Line 25: Remove the unused MatSnackBar import and its constructor injection in
the UnitComponent: delete the import statement for MatSnackBar and remove the
MatSnackBar parameter from the UnitComponent constructor (and any corresponding
visibility modifier) since FeedbackService handles notifications; ensure
constructor parameter ordering remains valid after removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 634c942d-d218-4624-b86d-48b3bab165af
📒 Files selected for processing (1)
frontend/src/app/shared/components/unit/unit.component.ts
omatheusmesmo
requested changes
Mar 13, 2026
Owner
omatheusmesmo
left a comment
omatheusmesmo
left a comment
There was a problem hiding this comment.
Hi @matheusandre1! Thanks for working on the dependency injection standardization. It's a great step toward modernizing our codebase.
After reviewing the changes, I found a few points that need attention:
- Partial Migration: In some components like
CategoriesManagementComponent, the migration is only partial (some services useinject()while others remain in the constructor). To fully address #29, we should move all dependencies toinject(). - Redundancy & Duplication: In
ItemComponent, the dependencies are declared usinginject()but are still being injected and initialized in the constructor as well. Also, theinjectfunction is imported twice in this file. - Compilation Errors: Some components (like
ItemComponentandUnitComponent) useinject(MatSnackBar)but are missing the corresponding import forMatSnackBar, which will cause build failures. - Constructor Cleanup: For components like
ListItemComponent, we can remove the constructor entirely by initializing properties directly or moving logic tongOnInit.
Could you please address these inconsistencies and ensure the project builds correctly? Thank you!
matheusandre1
force-pushed
the
fix-issue29
branch
from
a571806 to
247e3e2
Compare
omatheusmesmo
approved these changes
Mar 13, 2026
Owner
omatheusmesmo
left a comment
omatheusmesmo
left a comment
There was a problem hiding this comment.
Great job @matheusandre1 , well done! ☕
This was referenced Mar 13, 2026
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.
Closes: #29
Summary by CodeRabbit
Refactor
Bug Fixes
Chores