draft UX: Improve operation feedback using SnackBars#100
Merged
omatheusmesmo merged 7 commits intoomatheusmesmo:developfrom Mar 13, 2026
Merged
draft UX: Improve operation feedback using SnackBars#100omatheusmesmo merged 7 commits intoomatheusmesmo:developfrom
omatheusmesmo merged 7 commits 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. |
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
|
@coderabbitai check this PR. @matheusandre1 , please check the conflicts. |
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 replaces native browser Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Comp as Component
participant CDS as ConfirmDialogService
participant MD as MatDialog
participant CDC as ConfirmDialogComponent
participant API as API Service
participant FB as FeedbackService
User->>Comp: Trigger delete action
Comp->>CDS: open({title, message})
CDS->>MD: open(ConfirmDialogComponent)
MD->>CDC: Display dialog
CDC-->>User: Show dialog UI
User->>CDC: Click confirm
CDC-->>MD: Close with true
MD-->>CDS: Dialog closed
CDS-->>Comp: Observable<true>
Comp->>API: Delete resource
alt Success
API-->>Comp: Success response
Comp->>FB: success(message)
FB-->>User: Show success snackbar
else Error
API-->>Comp: Error response
Comp->>FB: error(message)
FB-->>User: Show error snackbar
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ 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 |
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.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
O Pull Request implementa uma melhoria significativa na experiência do usuário ao substituir os diálogos de confirmação nativos do navegador por componentes do Angular Material e centralizar o feedback de operações via SnackBars. A arquitetura proposta com os serviços ConfirmDialogService e FeedbackService é sólida e promove o reuso de código.
🔍 General Feedback
- Consistência de Linguagem: Notei que algumas mensagens foram alteradas para português, o que parece ser o padrão do projeto, mas houve algumas regressões ortográficas (faltando acentos em "excluída", "validação", "unidade", etc.) em vários componentes.
- Limpeza de Código: Foram adicionados arquivos vazios (SCSS) que podem ser removidos.
- Prática Recomendada: Em futuras iterações, considere utilizar o operador
take(1)oufirst()em subscrições de diálogos, embora oafterClosed()do Material complete automaticamente, é uma prática defensiva comum em Angular.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/items-management/item-dialog/item-dialog.component.ts (1)
97-105:⚠️ Potential issue | 🟡 MinorBlock save when duplicate-name validation cannot be initialized.
If
getAllItems()fails here, the dialog only shows feedback and keeps the form submittable withoutduplicateNameValidator, so duplicate item names can slip through. Set a blocking form/control error or otherwise prevent save until this bootstrap call succeeds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/items-management/item-dialog/item-dialog.component.ts` around lines 97 - 105, The loadItems error path currently only shows feedback but leaves the form submittable; update the error handler in loadItems (which calls this.itemService.getAllItems() and then this.updateNameValidator()) to block saves until duplicate-name validation is available by either disabling the dialog form (e.g., call this.form.disable()) or setting a control-level error on the name control (e.g., this.nameControl.setErrors({validationUnavailable: true})), and ensure any submit logic that checks this.form.valid or this.nameControl.hasError('validationUnavailable') prevents saving; keep references to existingItems and updateNameValidator unchanged so success still initializes duplicateNameValidator.frontend/src/app/list/components/units-management/unit-dialog/unit-dialog.component.ts (1)
81-89:⚠️ Potential issue | 🟡 MinorDo not keep the dialog saveable after unit-name validation bootstrap fails.
When
getAllUnits()errors, the form still allows submission without the duplicate-name validator being installed. Please surface the error and block save until the validator can be initialized again.🤖 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/unit-dialog/unit-dialog.component.ts` around lines 81 - 89, The loadUnits() error handler currently logs an error but leaves the dialog saveable; modify the error branch in loadUnits() (which calls unitService.getAllUnits(), existingUnits.set(), updateNameValidator()) to both surface the error and block saves until validation can be reinitialized — e.g., after feedback.error('...') set the component state that prevents submission (disable the FormGroup or set a saveEnabled/saveBlocked flag used by the save button and onSave logic) so the dialog cannot be submitted until updateNameValidator() successfully runs again.
🧹 Nitpick comments (9)
frontend/src/app/shared/services/feedback.service.ts (1)
10-21: Factor the repeated snackbar setup into one helper.Both methods repeat the same config, which makes future UX tweaks easier to miss in one branch.
♻️ Proposed refactor
export class FeedbackService { private readonly snackBar = inject(MatSnackBar); + private open(message: string, panelClass: string): void { + this.snackBar.open(message, 'Fechar', { + duration: 3000, + panelClass: [panelClass], + }); + } + success(message: string): void { - this.snackBar.open(message, 'Fechar', { - duration: 3000, - panelClass: ['success-snackbar'], - }); + this.open(message, 'success-snackbar'); } error(message: string): void { - this.snackBar.open(message, 'Fechar', { - duration: 3000, - panelClass: ['error-snackbar'], - }); + this.open(message, 'error-snackbar'); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/services/feedback.service.ts` around lines 10 - 21, Both success and error duplicate the same MatSnackBar call; create a single private helper (e.g., showSnackbar or openToast) that accepts the message and the panel class (or a type enum) and centralizes snackBar.open(...) with the shared config (duration: 3000, action 'Fechar', and panelClass) and then have success(message: string) and error(message: string) call that helper with the appropriate panelClass (['success-snackbar'] or ['error-snackbar']). This consolidates the config and keeps only the unique parameter per public method.frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts (1)
56-56: OnPush compatibility note.Same recommendation as other components: consider adding
ChangeDetectorRefto properly handleisLoadingstate changes withChangeDetectionStrategy.OnPush.Also applies to: 62-63, 70-70
🤖 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` at line 56, The component uses ChangeDetectionStrategy.OnPush but doesn't inject ChangeDetectorRef, so updates to isLoading may not trigger view refresh; inject ChangeDetectorRef into the ListShareDialogComponent constructor and call markForCheck() (or detectChanges() where appropriate) after any assignment that changes isLoading (and similar state updates referenced at lines 62-63 and 70) to ensure the template updates correctly with OnPush.frontend/src/app/shared/components/item/item.component.ts (1)
50-50: Consider addingChangeDetectorReffor OnPush compatibility.This component uses
ChangeDetectionStrategy.OnPushbut mutatesisLoadingdirectly without triggering change detection. Unlikeunits-management.component.tswhich correctly injectsChangeDetectorRef, this component may have stale UI states (e.g., spinner not hiding after load completes).♻️ Suggested fix
import { ChangeDetectionStrategy, + ChangeDetectorRef, Component, OnInit, inject, } from '@angular/core';private confirmDialog = inject(ConfirmDialogService); private feedback = inject(FeedbackService); + private cdr = inject(ChangeDetectorRef);Then call
this.cdr.detectChanges()after settingisLoading = falsein the subscribe callbacks.Also applies to: 56-56, 60-61
🤖 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` at line 50, This component uses ChangeDetectionStrategy.OnPush but mutates isLoading directly; inject Angular's ChangeDetectorRef into the component constructor (e.g., add a private cdr: ChangeDetectorRef parameter) and import it, then call this.cdr.detectChanges() immediately after setting this.isLoading = false in the relevant subscribe callbacks (success, error, or finalize) so the template updates; reference the isLoading property and the subscribe callbacks where loading is toggled in item.component.ts when making the change.frontend/src/app/list/components/shopping-list/shopping-list.component.ts (1)
29-29: Same OnPush +isLoadingconsideration.Consider adding
ChangeDetectorReffor proper change detection withChangeDetectionStrategy.OnPush.Also applies to: 33-33, 35-40
🤖 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/shopping-list.component.ts` at line 29, The component uses ChangeDetectionStrategy.OnPush but doesn't trigger detection when isLoading changes; inject ChangeDetectorRef into ShoppingListComponent (constructor) and call markForCheck() (or detectChanges()) after any updates to the isLoading flag or other async state changes (e.g., in methods that set isLoading or in subscriptions/async callbacks) so the template is updated; look for the isLoading property and places where it is toggled and add cdr.markForCheck() there.frontend/src/app/shared/components/list-item/list-item.component.ts (1)
51-51: Same OnPush +isLoadingissue asitem.component.ts.Consider injecting
ChangeDetectorRefand callingdetectChanges()after state mutations to ensure the UI updates correctly withChangeDetectionStrategy.OnPush.Also applies to: 56-56, 61-62
🤖 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 51, ListItemComponent is set to ChangeDetectionStrategy.OnPush but state flags like isLoading are mutated without triggering change detection; inject ChangeDetectorRef into the component (e.g., add private cdr: ChangeDetectorRef to the constructor) and call this.cdr.detectChanges() (or markForCheck() where appropriate) immediately after any mutation of isLoading or other view-bound state in methods that perform async work so the template updates; apply the same change to other affected spots referenced in the comment (the other blocks around the OnPush setting at the same component and similar methods).frontend/src/app/list/components/categories-management/categories-management.component.ts (1)
26-26: OnPush compatibility note applies here as well.This component uses constructor injection (which is fine), but like the others, it would benefit from
ChangeDetectorRefto handleisLoadingstate changes withChangeDetectionStrategy.OnPush.Also applies to: 30-30, 32-37
🤖 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` at line 26, The component uses ChangeDetectionStrategy.OnPush but doesn't trigger checks when isLoading updates; inject ChangeDetectorRef into the constructor (e.g., private cdr: ChangeDetectorRef) in categories-management.component (with ChangeDetectionStrategy.OnPush) and call this.cdr.markForCheck() or this.cdr.detectChanges() immediately after any assignment to isLoading (and similar async state updates) so the template refreshes; apply the same pattern to other usages referenced (lines ~30 and ~32-37) where async state changes occur.frontend/src/app/shared/components/unit/unit.component.ts (1)
120-140: Same nested subscription pattern as CategoryComponent.Apply the same flattening approach with
switchMapandtakeUntilDestroyed()for consistent cleanup across components.♻️ Proposed refactor
+import { filter, switchMap } from 'rxjs'; +import { DestroyRef, takeUntilDestroyed } from '@angular/core';+ private destroyRef = inject(DestroyRef); + deleteUnit(id: number): void { - this.confirmDialog - .open({ - title: 'Excluir unidade', - message: 'Tem certeza que deseja excluir esta unidade?', - confirmText: 'Excluir', - }) - .subscribe((confirmed) => { - if (!confirmed) return; - - this.unitService.deleteUnit(id).subscribe({ - next: () => { - this.feedback.success('Unidade excluida com sucesso'); - this.loadUnits(); - }, - error: () => { - this.feedback.error('Erro ao excluir unidade'); - }, - }); - }); + this.confirmDialog + .open({ + title: 'Excluir unidade', + message: 'Tem certeza que deseja excluir esta unidade?', + confirmText: 'Excluir', + }) + .pipe( + takeUntilDestroyed(this.destroyRef), + filter((confirmed) => confirmed), + switchMap(() => this.unitService.deleteUnit(id)) + ) + .subscribe({ + next: () => { + this.feedback.success('Unidade excluída com sucesso'); + this.loadUnits(); + }, + error: () => { + this.feedback.error('Erro ao excluir unidade'); + }, + }); }🤖 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 120 - 140, Refactor deleteUnit to avoid nested subscriptions: call this.confirmDialog.open(...) and pipe it with switchMap to only proceed when confirmed, then switchMap to this.unitService.deleteUnit(id), and subscribe with takeUntilDestroyed() for automatic cleanup; update success/error handling to run in the final subscribe (or tap) and call this.loadUnits() on success, referencing the deleteUnit method, confirmDialog.open, unitService.deleteUnit, loadUnits, switchMap, and takeUntilDestroyed symbols so the nested subscribe is replaced by a single flattened observable chain.frontend/src/app/shared/components/category/category.component.ts (2)
117-137: Nested subscription creates potential memory leak risk.If the component is destroyed while waiting for the user to confirm, both the outer and inner subscriptions continue. Consider flattening with
switchMaporconcatMap, and usingtakeUntilDestroyed()for automatic cleanup.♻️ Proposed refactor using RxJS operators
+import { filter, switchMap } from 'rxjs'; +import { DestroyRef, takeUntilDestroyed } from '@angular/core';+ private destroyRef = inject(DestroyRef); + deleteCategory(id: number): void { - this.confirmDialog - .open({ - title: 'Excluir categoria', - message: 'Tem certeza que deseja excluir esta categoria?', - confirmText: 'Excluir', - }) - .subscribe((confirmed) => { - if (!confirmed) return; - - this.categoryService.deleteCategory(id).subscribe({ - next: () => { - this.feedback.success('Categoria excluida com sucesso'); - this.loadCategories(); - }, - error: () => { - this.feedback.error('Erro ao excluir categoria'); - }, - }); - }); + this.confirmDialog + .open({ + title: 'Excluir categoria', + message: 'Tem certeza que deseja excluir esta categoria?', + confirmText: 'Excluir', + }) + .pipe( + takeUntilDestroyed(this.destroyRef), + filter((confirmed) => confirmed), + switchMap(() => this.categoryService.deleteCategory(id)) + ) + .subscribe({ + next: () => { + this.feedback.success('Categoria excluída com sucesso'); + this.loadCategories(); + }, + error: () => { + this.feedback.error('Erro ao excluir categoria'); + }, + }); }🤖 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 117 - 137, The deleteCategory method currently nests subscriptions (confirmDialog.open(...).subscribe(...) and categoryService.deleteCategory(...).subscribe(...)) which can leak if the component is destroyed; refactor deleteCategory to flatten the flow by piping confirmDialog.open(...) into switchMap or concatMap to call categoryService.deleteCategory(id) and subscribe once, and add automatic teardown with takeUntilDestroyed() (or pipe takeUntilDestroyed() on the stream) so both the confirmation and delete observables are cancelled when the component is destroyed; keep existing success/error handling (feedback.success/feedback.error and loadCategories()) inside the single subscription or via tap/catchError in the pipeline.
48-58: Mixed dependency injection styles are used.The component uses
inject()forConfirmDialogServiceandFeedbackService(lines 48-49) while using constructor injection forCategoryServiceandFormBuilder(lines 52-53). Both approaches are valid in Angular 14+, but consistency improves readability.🤖 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 48 - 58, The component mixes inject() and constructor injection (confirmDialog, feedback vs CategoryService, fb/FormBuilder) which is inconsistent; pick one style (prefer constructor injection for readability) and refactor so all dependencies are injected the same way: add ConfirmDialogService and FeedbackService as private constructor parameters (e.g., private confirmDialog: ConfirmDialogService, private feedback: FeedbackService), remove the top-level inject(...) calls, and ensure usages of confirmDialog, feedback, CategoryService, fb/FormBuilder and the existing categoryForm initialization remain correct.
🤖 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/category/category.component.ts`:
- Line 129: The success message in CategoryComponent uses a misspelled
Portuguese word; update the call to this.feedback.success in
category.component.ts to use the correct accented form "Categoria excluída com
sucesso" (replace "excluida" with "excluída") so the displayed message is
orthographically correct.
In `@frontend/src/app/shared/components/unit/unit.component.ts`:
- Line 132: The success message in UnitComponent currently uses a missing accent
("excluida"); update the string passed to this.feedback.success in the
UnitComponent (method containing this.feedback.success call) to use the correct
Portuguese spelling "Unidade excluída com sucesso" so it matches the corrected
form used in CategoryComponent and other messages.
In `@frontend/src/app/shared/services/confirm-dialog.service.ts`:
- Around line 15-23: SignupComponent and LoginComponent still inject MatSnackBar
directly instead of using the centralized FeedbackService; refactor them to
remove MatSnackBar from their constructors and inject FeedbackService (replace
references to MatSnackBar with FeedbackService). Update calls like
snackBar.open(message, action, config) in SignupComponent (previous calls around
the signup success/error flows) and LoginComponent (previous login error flow)
to the appropriate FeedbackService method (e.g., show/notify or the service’s
public API) passing the same message, action and any important config. Ensure
constructor parameter names and imports are updated (replace MatSnackBar
injection with FeedbackService) and run/adjust unit tests if they reference
MatSnackBar directly.
---
Outside diff comments:
In
`@frontend/src/app/list/components/items-management/item-dialog/item-dialog.component.ts`:
- Around line 97-105: The loadItems error path currently only shows feedback but
leaves the form submittable; update the error handler in loadItems (which calls
this.itemService.getAllItems() and then this.updateNameValidator()) to block
saves until duplicate-name validation is available by either disabling the
dialog form (e.g., call this.form.disable()) or setting a control-level error on
the name control (e.g., this.nameControl.setErrors({validationUnavailable:
true})), and ensure any submit logic that checks this.form.valid or
this.nameControl.hasError('validationUnavailable') prevents saving; keep
references to existingItems and updateNameValidator unchanged so success still
initializes duplicateNameValidator.
In
`@frontend/src/app/list/components/units-management/unit-dialog/unit-dialog.component.ts`:
- Around line 81-89: The loadUnits() error handler currently logs an error but
leaves the dialog saveable; modify the error branch in loadUnits() (which calls
unitService.getAllUnits(), existingUnits.set(), updateNameValidator()) to both
surface the error and block saves until validation can be reinitialized — e.g.,
after feedback.error('...') set the component state that prevents submission
(disable the FormGroup or set a saveEnabled/saveBlocked flag used by the save
button and onSave logic) so the dialog cannot be submitted until
updateNameValidator() successfully runs again.
---
Nitpick comments:
In
`@frontend/src/app/list/components/categories-management/categories-management.component.ts`:
- Line 26: The component uses ChangeDetectionStrategy.OnPush but doesn't trigger
checks when isLoading updates; inject ChangeDetectorRef into the constructor
(e.g., private cdr: ChangeDetectorRef) in categories-management.component (with
ChangeDetectionStrategy.OnPush) and call this.cdr.markForCheck() or
this.cdr.detectChanges() immediately after any assignment to isLoading (and
similar async state updates) so the template refreshes; apply the same pattern
to other usages referenced (lines ~30 and ~32-37) where async state changes
occur.
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Line 56: The component uses ChangeDetectionStrategy.OnPush but doesn't inject
ChangeDetectorRef, so updates to isLoading may not trigger view refresh; inject
ChangeDetectorRef into the ListShareDialogComponent constructor and call
markForCheck() (or detectChanges() where appropriate) after any assignment that
changes isLoading (and similar state updates referenced at lines 62-63 and 70)
to ensure the template updates correctly with OnPush.
In `@frontend/src/app/list/components/shopping-list/shopping-list.component.ts`:
- Line 29: The component uses ChangeDetectionStrategy.OnPush but doesn't trigger
detection when isLoading changes; inject ChangeDetectorRef into
ShoppingListComponent (constructor) and call markForCheck() (or detectChanges())
after any updates to the isLoading flag or other async state changes (e.g., in
methods that set isLoading or in subscriptions/async callbacks) so the template
is updated; look for the isLoading property and places where it is toggled and
add cdr.markForCheck() there.
In `@frontend/src/app/shared/components/category/category.component.ts`:
- Around line 117-137: The deleteCategory method currently nests subscriptions
(confirmDialog.open(...).subscribe(...) and
categoryService.deleteCategory(...).subscribe(...)) which can leak if the
component is destroyed; refactor deleteCategory to flatten the flow by piping
confirmDialog.open(...) into switchMap or concatMap to call
categoryService.deleteCategory(id) and subscribe once, and add automatic
teardown with takeUntilDestroyed() (or pipe takeUntilDestroyed() on the stream)
so both the confirmation and delete observables are cancelled when the component
is destroyed; keep existing success/error handling
(feedback.success/feedback.error and loadCategories()) inside the single
subscription or via tap/catchError in the pipeline.
- Around line 48-58: The component mixes inject() and constructor injection
(confirmDialog, feedback vs CategoryService, fb/FormBuilder) which is
inconsistent; pick one style (prefer constructor injection for readability) and
refactor so all dependencies are injected the same way: add ConfirmDialogService
and FeedbackService as private constructor parameters (e.g., private
confirmDialog: ConfirmDialogService, private feedback: FeedbackService), remove
the top-level inject(...) calls, and ensure usages of confirmDialog, feedback,
CategoryService, fb/FormBuilder and the existing categoryForm initialization
remain correct.
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Line 50: This component uses ChangeDetectionStrategy.OnPush but mutates
isLoading directly; inject Angular's ChangeDetectorRef into the component
constructor (e.g., add a private cdr: ChangeDetectorRef parameter) and import
it, then call this.cdr.detectChanges() immediately after setting this.isLoading
= false in the relevant subscribe callbacks (success, error, or finalize) so the
template updates; reference the isLoading property and the subscribe callbacks
where loading is toggled in item.component.ts when making the change.
In `@frontend/src/app/shared/components/list-item/list-item.component.ts`:
- Line 51: ListItemComponent is set to ChangeDetectionStrategy.OnPush but state
flags like isLoading are mutated without triggering change detection; inject
ChangeDetectorRef into the component (e.g., add private cdr: ChangeDetectorRef
to the constructor) and call this.cdr.detectChanges() (or markForCheck() where
appropriate) immediately after any mutation of isLoading or other view-bound
state in methods that perform async work so the template updates; apply the same
change to other affected spots referenced in the comment (the other blocks
around the OnPush setting at the same component and similar methods).
In `@frontend/src/app/shared/components/unit/unit.component.ts`:
- Around line 120-140: Refactor deleteUnit to avoid nested subscriptions: call
this.confirmDialog.open(...) and pipe it with switchMap to only proceed when
confirmed, then switchMap to this.unitService.deleteUnit(id), and subscribe with
takeUntilDestroyed() for automatic cleanup; update success/error handling to run
in the final subscribe (or tap) and call this.loadUnits() on success,
referencing the deleteUnit method, confirmDialog.open, unitService.deleteUnit,
loadUnits, switchMap, and takeUntilDestroyed symbols so the nested subscribe is
replaced by a single flattened observable chain.
In `@frontend/src/app/shared/services/feedback.service.ts`:
- Around line 10-21: Both success and error duplicate the same MatSnackBar call;
create a single private helper (e.g., showSnackbar or openToast) that accepts
the message and the panel class (or a type enum) and centralizes
snackBar.open(...) with the shared config (duration: 3000, action 'Fechar', and
panelClass) and then have success(message: string) and error(message: string)
call that helper with the appropriate panelClass (['success-snackbar'] or
['error-snackbar']). This consolidates the config and keeps only the unique
parameter per public method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8356b381-cf0d-4992-84a9-b6fe9d49a953
📒 Files selected for processing (20)
frontend/src/app/list/components/categories-management/categories-management.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/items-management/items-management.component.tsfrontend/src/app/list/components/list-details/list-details.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/shopping-list.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/confirm-dialog/confirm-dialog.component.htmlfrontend/src/app/shared/components/confirm-dialog/confirm-dialog.component.scssfrontend/src/app/shared/components/confirm-dialog/confirm-dialog.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/confirm-dialog.service.tsfrontend/src/app/shared/services/feedback.service.tsfrontend/src/styles.scss
Comment on lines
+15
to
+23
| open(data: ConfirmDialogData): Observable<boolean> { | ||
| return this.dialog | ||
| .open(ConfirmDialogComponent, { | ||
| width: '420px', | ||
| data, | ||
| }) | ||
| .afterClosed() | ||
| .pipe(map((confirmed) => confirmed === true)); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\nNative confirm() usages (expected: no matches):\n'
rg -nP '\b(?:window\.)?confirm\s*\(' frontend/src || true
printf '\nDirect MatSnackBar usages outside FeedbackService (expected: no matches):\n'
rg -nP 'MatSnackBar|\bsnackBar\.open\s*\(' frontend/src/app \
-g '!frontend/src/app/shared/services/feedback.service.ts' || trueRepository: omatheusmesmo/ShoppMate
Length of output: 2955
Complete the feedback service migration in auth components.
The ConfirmDialogService is correctly implemented, but the migration to centralized feedback is incomplete. No native confirm() calls remain, but two components still inject MatSnackBar directly instead of using FeedbackService:
frontend/src/app/auth/signup/signup.component.ts(lines 66, 73)frontend/src/app/auth/login/login.component.ts(line 68)
These need to be refactored to use the shared FeedbackService before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/app/shared/services/confirm-dialog.service.ts` around lines 15 -
23, SignupComponent and LoginComponent still inject MatSnackBar directly instead
of using the centralized FeedbackService; refactor them to remove MatSnackBar
from their constructors and inject FeedbackService (replace references to
MatSnackBar with FeedbackService). Update calls like snackBar.open(message,
action, config) in SignupComponent (previous calls around the signup
success/error flows) and LoginComponent (previous login error flow) to the
appropriate FeedbackService method (e.g., show/notify or the service’s public
API) passing the same message, action and any important config. Ensure
constructor parameter names and imports are updated (replace MatSnackBar
injection with FeedbackService) and run/adjust unit tests if they reference
MatSnackBar directly.
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 @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.
|
Contributor
|
🤖 I'm sorry @coderabbitai[bot], but I was unable to process your request. Please see the logs for more details. |
Contributor
|
Here's what I'd suggest:
Would you like me to trigger a new review now, or would you prefer to wait until the conflicts are resolved? ✅ Actions performedReview triggered.
|
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
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
frontend/src/app/auth/signup/signup.component.ts (1)
78-82:⚠️ Potential issue | 🟠 MajorPreserve error styling on the signup failure path.
This call drops the
error-snackbarstyling, so signup failures will look different from the other error flows.FeedbackService.error()infrontend/src/app/shared/services/feedback.service.tsalready defines the error presentation, and this path currently bypasses it. That breaks the PR’s consistency goal.Suggested fix
this.feedback.notify( 'Falha ao realizar cadastro. Tente novamente.', 'Fechar', - { duration: 5000 }, + { + duration: 5000, + panelClass: ['error-snackbar'], + }, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/auth/signup/signup.component.ts` around lines 78 - 82, The signup failure path is calling this.feedback.notify(...) which bypasses the standardized error styling; replace that call with the FeedbackService error method (e.g., this.feedback.error(...)) used elsewhere so the error-snackbar styling and consistent presentation are preserved. Update the call in signup.component.ts (the block handling signup failure) to invoke the same signature/parameters as FeedbackService.error and remove the direct notify usage so styling/behavior matches other error flows.frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts (1)
118-129:⚠️ Potential issue | 🟠 MajorRemove the non-null assertion on
user.id.The
Userinterface definesidas optional, sogetAllUsers()can return users without anid. The non-null assertionuser.id!bypasses the type system and could send an invalididUserpayload. Add a guard to check ifuser.idis defined before callingaddListPermission.Suggested fix
const user = this.users().find((u) => u.email === email); if (!user) { this.feedback.error('Usuario nao encontrado com este e-mail'); return; } + + if (user.id == null) { + this.feedback.error('Usuario selecionado sem identificador valido'); + return; + } const request = { idList: this.data.listId, - idUser: user.id!, + idUser: user.id, permission: this.shareForm.value.permission, };🤖 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 118 - 129, The code uses a non-null assertion on user.id when building the request in the ListShareDialog component; instead, guard that id is present before calling addListPermission: after finding user via users(), check if user.id is defined, show the existing feedback.error (or a new message) and return if missing, then build the request using user.id (no !) and call addListPermission; update any references in the submit/share handler (e.g., the method that constructs request and calls addListPermission) to perform this guard so an undefined id is never sent to the API.frontend/src/app/shared/components/category/category.component.ts (2)
82-100:⚠️ Potential issue | 🟡 MinorCapture the edit state before the request starts.
editingCategoryId()is re-read inside the asyncnexthandler. If the form state changes before the response returns, the success message can announce the wrong operation even though the API call was correct.🛠️ Suggested fix
const categoryData: Category = { name: this.categoryForm.value.name, }; - if (this.editingCategoryId() !== null) { - categoryData.id = this.editingCategoryId()!; - } - - const operation = - this.editingCategoryId() !== null - ? this.categoryService.updateCategory(categoryData) - : this.categoryService.addCategory(categoryData); + const editingCategoryId = this.editingCategoryId(); + const isEdit = editingCategoryId !== null; + + if (isEdit) { + categoryData.id = editingCategoryId; + } + + const operation = isEdit + ? this.categoryService.updateCategory(categoryData) + : this.categoryService.addCategory(categoryData); operation.subscribe({ next: () => { this.feedback.success( - this.editingCategoryId() !== null + isEdit ? 'Categoria atualizada com sucesso' : 'Categoria criada com sucesso', );🤖 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 82 - 100, The success message re-evaluates editingCategoryId() inside the async subscribe next handler which can be different if the form changed before the response returns; capture the edit state once before making the request (e.g. const isEditing = this.editingCategoryId() !== null), use that to set categoryData.id if needed and to decide which service method to call (operation) and then use isEditing inside the subscribe next callback to pick the correct success text; update references to editingCategoryId(), categoryData, operation, and the next handler accordingly.
65-76:⚠️ Potential issue | 🟠 MajorSeparate mutation feedback from the follow-up refresh.
loadCategories()is reused after create/update/delete and always emitsErro ao carregar categorias. If the refetch fails after a successful mutation, users can get both a success message and an error message for the same action, and the list may remain stale. A silent refresh or a refresh-specific error message would keep the UX consistent.Also applies to: 95-103, 129-131
🤖 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 65 - 76, loadCategories() currently always shows feedback.error on any fetch failure which causes confusing UX when it’s used as a follow-up refresh after a successful create/update/delete; change loadCategories signature to accept an optional flag (e.g., suppressError?: boolean or silent?: boolean) and use that flag to avoid showing the global error message on silent refreshes, only log or show a refresh-specific message when not silent; update all callers (including the create/update/delete follow-ups referenced at the other occurrences) to call loadCategories(true) for background refreshes and leave default behavior for direct user-initiated loads, and ensure isLoading.set(false) is still executed in both success and error paths.frontend/src/app/list/components/categories-management/categories-management.component.ts (1)
48-59:⚠️ Potential issue | 🟠 MajorSeparate delete feedback from refresh feedback.
loadCategories()always showsErro ao carregar categorias. Because the delete success path immediately calls it, a transient refetch failure can produce both a success message and an error message for the same delete action, and the stale category will stay visible until a later refresh succeeds. A silent refresh path, or updating the local signal before refetching, would keep the feedback consistent.💡 One possible direction
- loadCategories(): void { + loadCategories(showErrorFeedback = true): void { this.isLoading.set(true); this.categoryService.getAllCategories().subscribe({ next: (categories) => { this.categories.set(categories); this.isLoading.set(false); }, error: () => { - this.feedback.error('Erro ao carregar categorias'); + if (showErrorFeedback) { + this.feedback.error('Erro ao carregar categorias'); + } this.isLoading.set(false); }, }); } @@ next: () => { this.feedback.success('Categoria excluída com sucesso'); - this.loadCategories(); + this.categories.update((categories) => + categories.filter((category) => category.id !== id), + ); + this.loadCategories(false); },Also applies to: 99-101
🤖 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 48 - 59, loadCategories() currently always surfaces an error via feedback.error when the refetch fails, which causes conflicting UI messages after a delete that calls loadCategories(); modify the refresh path so delete success does not trigger user-facing error feedback: either (a) update the local signal this.categories (or remove the deleted item from this.categories) immediately in the delete success handler and then call a silent refresh that suppresses feedback.error from categoryService.getAllCategories() (e.g., add a boolean silent flag to loadCategories() that skips feedback on error), or (b) have loadCategories() detect a silent refresh parameter and only set this.isLoading/feedback on non-silent calls; reference loadCategories(), categoryService.getAllCategories(), and feedback.error to implement the silent-refresh or immediate local update approach.
🧹 Nitpick comments (6)
frontend/src/app/shared/components/unit/unit.component.ts (2)
49-55: Consider unifying dependency injection style.The component mixes
inject()function (lines 49-50) with constructor injection (lines 53-54). While both work correctly in Angular 19, using one consistent pattern improves readability.♻️ Optional: Migrate all dependencies to inject()
- private confirmDialog = inject(ConfirmDialogService); - private feedback = inject(FeedbackService); + private readonly unitService = inject(UnitService); + private readonly fb = inject(FormBuilder); + private readonly confirmDialog = inject(ConfirmDialogService); + private readonly feedback = inject(FeedbackService); - constructor( - private unitService: UnitService, - private fb: FormBuilder, - ) { + constructor() { this.unitForm = this.fb.group({ name: ['', [Validators.required, Validators.minLength(2)]], symbol: ['', [Validators.required]], }); }🤖 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 49 - 55, The component mixes inject() calls for confirmDialog and feedback with constructor-based injection for UnitService and FormBuilder; pick one style and apply it consistently (either move confirmDialog and feedback into the constructor or convert unitService and fb to use inject()). Locate the class where confirmDialog and feedback are declared and the constructor that accepts UnitService and FormBuilder (e.g., the UnitComponent class) and refactor so all dependencies use the same approach, updating property declarations accordingly and removing any now-unused constructor parameters or inject() calls.
121-141: Consider flattening nested subscriptions with RxJS operators.The nested
subscribecalls (dialog result → delete operation) can be refactored usingswitchMapandfilterfor cleaner, more idiomatic RxJS code. This avoids the callback pyramid and improves readability.♻️ Proposed refactor using RxJS operators
Add to imports:
import { filter, switchMap } from 'rxjs';deleteUnit(id: number): void { this.confirmDialog .open({ title: 'Excluir unidade', message: 'Tem certeza que deseja excluir esta unidade?', confirmText: 'Excluir', }) - .subscribe((confirmed) => { - if (!confirmed) return; - - this.unitService.deleteUnit(id).subscribe({ - next: () => { - this.feedback.success('Unidade excluída com sucesso'); - this.loadUnits(); - }, - error: () => { - this.feedback.error('Erro ao excluir unidade'); - }, - }); - }); + .pipe( + filter((confirmed) => confirmed), + switchMap(() => this.unitService.deleteUnit(id)) + ) + .subscribe({ + next: () => { + this.feedback.success('Unidade excluída com sucesso'); + this.loadUnits(); + }, + error: () => { + this.feedback.error('Erro ao excluir unidade'); + }, + }); }🤖 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 121 - 141, Refactor the nested subscribes in deleteUnit to use RxJS operators: have confirmDialog.open(...) return an observable piped through filter(Boolean) to only continue when confirmed, then switchMap to unitService.deleteUnit(id) and subscribe to handle next/error (calling feedback.success, loadUnits, feedback.error). Import filter and switchMap from 'rxjs' and remove the inner subscribe; keep function name deleteUnit and calls to confirmDialog.open, unitService.deleteUnit, loadUnits, feedback.success, and feedback.error unchanged.frontend/src/app/list/components/items-management/items-management.component.ts (1)
135-153: Consider using RxJS operators to flatten nested subscriptions.The current pattern subscribes inside another subscription. While both observables complete (no memory leak), using RxJS operators improves readability and follows reactive best practices.
♻️ Proposed refactor using filter and switchMap
+import { filter, switchMap } from 'rxjs';deleteItem(item: ItemResponseDTO): void { this.confirmDialog .open({ title: 'Excluir item', message: `Tem certeza que deseja excluir o item "${item.name}"?`, confirmText: 'Excluir', }) - .subscribe((confirmed) => { - if (!confirmed) return; - - this.itemService.deleteItem(item.id).subscribe({ - next: () => { - this.loadData(); - this.feedback.success('Item excluído com sucesso'); - }, - error: () => { - this.feedback.error('Erro ao excluir item'); - }, - }); + .pipe( + filter((confirmed) => confirmed), + switchMap(() => this.itemService.deleteItem(item.id)) + ) + .subscribe({ + next: () => { + this.loadData(); + this.feedback.success('Item excluído com sucesso'); + }, + error: () => { + this.feedback.error('Erro ao excluir item'); + }, }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/items-management/items-management.component.ts` around lines 135 - 153, The nested subscription should be flattened: instead of subscribing inside confirmDialog.open(...).subscribe(...), pipe the confirmDialog.open(...) observable through operators (e.g., filter to keep only confirmed results and switchMap to call itemService.deleteItem(item.id)), then subscribe once to handle next (call loadData and feedback.success) and error (feedback.error); reference confirmDialog.open, itemService.deleteItem, loadData, and feedback.success/error when making the change.frontend/src/app/shared/components/item/item.component.ts (2)
139-159: Consider flattening nested subscriptions with RxJS operators.The nested subscription pattern (subscribing inside a subscribe callback) can be refactored using RxJS operators for better readability and maintainability. While there's no memory leak here since both observables complete naturally, the flattened approach is more idiomatic RxJS.
♻️ Proposed refactor using filter and switchMap
+import { filter, switchMap } from 'rxjs';deleteItem(id: number): void { this.confirmDialog .open({ title: 'Excluir item', message: 'Tem certeza que deseja excluir este item?', confirmText: 'Excluir', }) - .subscribe((confirmed) => { - if (!confirmed) return; - - this.itemService.deleteItem(id).subscribe({ - next: () => { - this.feedback.success('Item excluído com sucesso'); - this.loadInitialData(); - }, - error: () => { - this.feedback.error('Erro ao excluir item'); - }, - }); + .pipe( + filter((confirmed) => confirmed), + switchMap(() => this.itemService.deleteItem(id)), + ) + .subscribe({ + next: () => { + this.feedback.success('Item excluído com sucesso'); + this.loadInitialData(); + }, + error: () => { + this.feedback.error('Erro ao excluir 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 139 - 159, The deleteItem method currently nests a subscription inside the confirmDialog.open callback; refactor to flatten the stream by returning confirmDialog.open(...) and piping it through RxJS operators (e.g., filter to pass only confirmed values and switchMap/concatMap to call itemService.deleteItem(id)), then subscribe once to handle next/error and call feedback.success/feedback.error and loadInitialData from that single subscription; update references: deleteItem, confirmDialog.open, itemService.deleteItem, loadInitialData, feedback.success, feedback.error.
61-75: Consider using consistent dependency injection pattern.The component mixes
inject()function (lines 61-62) with constructor injection (lines 65-68). Both patterns work correctly, but using a consistent approach improves readability. Theinject()pattern is the modern Angular style.♻️ Optional: Migrate all dependencies to inject()
- private confirmDialog = inject(ConfirmDialogService); - private feedback = inject(FeedbackService); - - constructor( - private itemService: ItemService, - private categoryService: CategoryService, - private unitService: UnitService, - private fb: FormBuilder, - ) { + private readonly confirmDialog = inject(ConfirmDialogService); + private readonly feedback = inject(FeedbackService); + private readonly itemService = inject(ItemService); + private readonly categoryService = inject(CategoryService); + private readonly unitService = inject(UnitService); + private readonly fb = inject(FormBuilder); + + constructor() { this.itemForm = this.fb.group({ name: ['', [Validators.required, Validators.minLength(2)]], idCategory: ['', Validators.required], idUnit: ['', Validators.required], }); }🤖 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 61 - 75, The component currently mixes inject() for confirmDialog/feedback with constructor injection for ItemService, CategoryService, UnitService and FormBuilder; make the injection style consistent by replacing the constructor parameters with inject() calls: add private itemService = inject(ItemService), private categoryService = inject(CategoryService), private unitService = inject(UnitService), and private fb = inject(FormBuilder), remove those parameters from the constructor, and keep/remove the empty constructor body; then ensure itemForm (this.itemForm) is initialized using the injected fb (either inline or in the constructor body) so references to itemService, categoryService, unitService, fb, confirmDialog and feedback all use the same inject() pattern.frontend/src/app/auth/login/login.component.ts (1)
73-80: Use the shared error helper instead of re-specifying snackbar config here.This still hard-codes
panelClassand duration in the component, so any future change to error feedback has to be updated in multiple places again. Prefer routing this throughFeedbackService.error(...)so the auth screens stay aligned with the centralized feedback API.Possible direction
this.feedback.error( error.message || 'Falha ao fazer login. Verifique suas credenciais.', );If you need the 5s duration specifically, consider extending
FeedbackService.error()to accept optional overrides and keep that policy centralized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/auth/login/login.component.ts` around lines 73 - 80, Replace the direct call to this.feedback.notify(...) in LoginComponent with the shared error helper to centralize snackbar styling and duration; specifically, swap the notify invocation in login.component.ts to this.feedback.error(error.message || 'Falha ao fazer login. Verifique suas credenciais.'); if you must preserve a 5s override, update FeedbackService.error to accept an optional options parameter (e.g., duration/panelClass) and call it from LoginComponent with that override instead of hard-coding panelClass/duration locally.
🤖 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/items-management/items-management.component.ts`:
- Line 147: In ItemsManagementComponent locate the this.feedback.success call
that currently logs "Item excluido com sucesso" and fix the typo by updating the
message to include the accent (use "Item excluído com sucesso"); ensure you
update the single string passed to this.feedback.success so the success
notification displays the correct Portuguese spelling.
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Line 72: Remove the unused class field declaration "snackBar: any;" from the
ListShareDialogComponent (the leftover field in list-share-dialog.component.ts)
to satisfy `@typescript-eslint/no-explicit-any` and cleanup after the
FeedbackService migration; locate the declaration inside the component class and
delete that single line so no other references are affected.
In `@frontend/src/app/list/components/shopping-list/shopping-list.component.ts`:
- Around line 101-104: The success message is shown before the UI state is
updated, so remove the deleted item from the component's local state first
(e.g., update the shopping lists array maintained by ShoppingListComponent),
then call feedback.success('Lista excluida com sucesso') and finally invoke
loadLists() for a full refresh; locate the HTTP call in
shoppingListService.deleteShoppingList and the subscriber in
ShoppingListComponent (the delete handler that currently calls loadLists()),
update the local collection (e.g., this.shoppingLists =
this.shoppingLists.filter(...) or splice by id) on next, and handle errors in
error: to restore state or show a failure message if needed.
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Line 151: The success message string passed to this.feedback.success contains
a typo: "Item excluido com sucesso" should use the accented "í". Update the
literal in the call to this.feedback.success (in ItemComponent /
item.component.ts) to "Item excluído com sucesso" so the Portuguese message is
correct.
---
Outside diff comments:
In `@frontend/src/app/auth/signup/signup.component.ts`:
- Around line 78-82: The signup failure path is calling
this.feedback.notify(...) which bypasses the standardized error styling; replace
that call with the FeedbackService error method (e.g., this.feedback.error(...))
used elsewhere so the error-snackbar styling and consistent presentation are
preserved. Update the call in signup.component.ts (the block handling signup
failure) to invoke the same signature/parameters as FeedbackService.error and
remove the direct notify usage so styling/behavior matches other error flows.
In
`@frontend/src/app/list/components/categories-management/categories-management.component.ts`:
- Around line 48-59: loadCategories() currently always surfaces an error via
feedback.error when the refetch fails, which causes conflicting UI messages
after a delete that calls loadCategories(); modify the refresh path so delete
success does not trigger user-facing error feedback: either (a) update the local
signal this.categories (or remove the deleted item from this.categories)
immediately in the delete success handler and then call a silent refresh that
suppresses feedback.error from categoryService.getAllCategories() (e.g., add a
boolean silent flag to loadCategories() that skips feedback on error), or (b)
have loadCategories() detect a silent refresh parameter and only set
this.isLoading/feedback on non-silent calls; reference loadCategories(),
categoryService.getAllCategories(), and feedback.error to implement the
silent-refresh or immediate local update approach.
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 118-129: The code uses a non-null assertion on user.id when
building the request in the ListShareDialog component; instead, guard that id is
present before calling addListPermission: after finding user via users(), check
if user.id is defined, show the existing feedback.error (or a new message) and
return if missing, then build the request using user.id (no !) and call
addListPermission; update any references in the submit/share handler (e.g., the
method that constructs request and calls addListPermission) to perform this
guard so an undefined id is never sent to the API.
In `@frontend/src/app/shared/components/category/category.component.ts`:
- Around line 82-100: The success message re-evaluates editingCategoryId()
inside the async subscribe next handler which can be different if the form
changed before the response returns; capture the edit state once before making
the request (e.g. const isEditing = this.editingCategoryId() !== null), use that
to set categoryData.id if needed and to decide which service method to call
(operation) and then use isEditing inside the subscribe next callback to pick
the correct success text; update references to editingCategoryId(),
categoryData, operation, and the next handler accordingly.
- Around line 65-76: loadCategories() currently always shows feedback.error on
any fetch failure which causes confusing UX when it’s used as a follow-up
refresh after a successful create/update/delete; change loadCategories signature
to accept an optional flag (e.g., suppressError?: boolean or silent?: boolean)
and use that flag to avoid showing the global error message on silent refreshes,
only log or show a refresh-specific message when not silent; update all callers
(including the create/update/delete follow-ups referenced at the other
occurrences) to call loadCategories(true) for background refreshes and leave
default behavior for direct user-initiated loads, and ensure
isLoading.set(false) is still executed in both success and error paths.
---
Nitpick comments:
In `@frontend/src/app/auth/login/login.component.ts`:
- Around line 73-80: Replace the direct call to this.feedback.notify(...) in
LoginComponent with the shared error helper to centralize snackbar styling and
duration; specifically, swap the notify invocation in login.component.ts to
this.feedback.error(error.message || 'Falha ao fazer login. Verifique suas
credenciais.'); if you must preserve a 5s override, update FeedbackService.error
to accept an optional options parameter (e.g., duration/panelClass) and call it
from LoginComponent with that override instead of hard-coding
panelClass/duration locally.
In
`@frontend/src/app/list/components/items-management/items-management.component.ts`:
- Around line 135-153: The nested subscription should be flattened: instead of
subscribing inside confirmDialog.open(...).subscribe(...), pipe the
confirmDialog.open(...) observable through operators (e.g., filter to keep only
confirmed results and switchMap to call itemService.deleteItem(item.id)), then
subscribe once to handle next (call loadData and feedback.success) and error
(feedback.error); reference confirmDialog.open, itemService.deleteItem,
loadData, and feedback.success/error when making the change.
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Around line 139-159: The deleteItem method currently nests a subscription
inside the confirmDialog.open callback; refactor to flatten the stream by
returning confirmDialog.open(...) and piping it through RxJS operators (e.g.,
filter to pass only confirmed values and switchMap/concatMap to call
itemService.deleteItem(id)), then subscribe once to handle next/error and call
feedback.success/feedback.error and loadInitialData from that single
subscription; update references: deleteItem, confirmDialog.open,
itemService.deleteItem, loadInitialData, feedback.success, feedback.error.
- Around line 61-75: The component currently mixes inject() for
confirmDialog/feedback with constructor injection for ItemService,
CategoryService, UnitService and FormBuilder; make the injection style
consistent by replacing the constructor parameters with inject() calls: add
private itemService = inject(ItemService), private categoryService =
inject(CategoryService), private unitService = inject(UnitService), and private
fb = inject(FormBuilder), remove those parameters from the constructor, and
keep/remove the empty constructor body; then ensure itemForm (this.itemForm) is
initialized using the injected fb (either inline or in the constructor body) so
references to itemService, categoryService, unitService, fb, confirmDialog and
feedback all use the same inject() pattern.
In `@frontend/src/app/shared/components/unit/unit.component.ts`:
- Around line 49-55: The component mixes inject() calls for confirmDialog and
feedback with constructor-based injection for UnitService and FormBuilder; pick
one style and apply it consistently (either move confirmDialog and feedback into
the constructor or convert unitService and fb to use inject()). Locate the class
where confirmDialog and feedback are declared and the constructor that accepts
UnitService and FormBuilder (e.g., the UnitComponent class) and refactor so all
dependencies use the same approach, updating property declarations accordingly
and removing any now-unused constructor parameters or inject() calls.
- Around line 121-141: Refactor the nested subscribes in deleteUnit to use RxJS
operators: have confirmDialog.open(...) return an observable piped through
filter(Boolean) to only continue when confirmed, then switchMap to
unitService.deleteUnit(id) and subscribe to handle next/error (calling
feedback.success, loadUnits, feedback.error). Import filter and switchMap from
'rxjs' and remove the inner subscribe; keep function name deleteUnit and calls
to confirmDialog.open, unitService.deleteUnit, loadUnits, feedback.success, and
feedback.error unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b6043d3b-8c33-4c52-bc1a-2fe5a2c1d1d0
📒 Files selected for processing (15)
frontend/src/app/auth/login/login.component.tsfrontend/src/app/auth/signup/signup.component.tsfrontend/src/app/list/components/categories-management/categories-management.component.tsfrontend/src/app/list/components/category-dialog/category-dialog.component.tsfrontend/src/app/list/components/items-management/items-management.component.tsfrontend/src/app/list/components/list-details/list-details.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/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.tsfrontend/src/app/shared/services/feedback.service.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/src/app/list/components/list-details/list-details.component.ts
- frontend/src/app/list/components/category-dialog/category-dialog.component.ts
- frontend/src/app/shared/components/list-item/list-item.component.ts
- frontend/src/app/list/components/units-management/units-management.component.ts
- frontend/src/app/shared/services/feedback.service.ts
- frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts
Contributor
|
🤖 I'm sorry @matheusandre1, but I was unable to process your request. Please see the logs for more details. |
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.
|
Contributor
|
🤖 I'm sorry @coderabbitai[bot], but I was unable to process your request. Please see the logs for more details. |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Here's the recommended path forward:
No further action is needed on my end until the conflicts are cleared. 🙌 |
Contributor
There was a problem hiding this comment.
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/list/components/list-share-dialog/list-share-dialog.component.ts (1)
104-121:⚠️ Potential issue | 🟠 MajorDon't treat an empty user cache as "user not found".
If
getAllUsers()is still loading, or failed earlier,this.users()is empty and every valid email falls into the "Usuario nao encontrado" branch. GateonShare()on a loaded user list, or show a distinct "users unavailable" message instead of a false negative.🤖 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 104 - 121, The current onShare() treats an empty this.users() as "user not found" which produces false negatives when loadUsers() is still loading or previously failed; modify loadUsers() to maintain a usersLoaded flag (or similar) and set it true in the next callback and false in error, and update onShare() to first check usersLoaded (or this.users().length) and show a distinct "users unavailable" feedback (or disable the share action) when the list isn't loaded instead of treating it as not found; ensure references to loadUsers(), userService.getAllUsers(), this.users, onShare(), shareForm are updated accordingly.
🧹 Nitpick comments (1)
frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts (1)
145-165: Flatten the confirm/delete flow instead of nestingsubscribes.This works, but the nested subscription makes the async path harder to follow and extend. Composing the confirmation result into the delete request keeps the method linear and easier to maintain.
♻️ Suggested refactor
+import { EMPTY, switchMap } from 'rxjs'; + removePermission(permissionId: number): void { this.confirmDialog .open({ title: 'Remover permissao', message: 'Tem certeza que deseja remover esta permissao?', confirmText: 'Remover', }) - .subscribe((confirmed) => { - if (!confirmed) return; - - this.listPermissionService - .deleteListPermission(this.data.listId, permissionId) - .subscribe({ - next: () => { - this.feedback.success('Permissao removida com sucesso'); - this.loadPermissions(); - }, - error: () => { - this.feedback.error('Erro ao remover permissao'); - }, - }); - }); + .pipe( + switchMap((confirmed) => + confirmed + ? this.listPermissionService.deleteListPermission( + this.data.listId, + permissionId, + ) + : EMPTY, + ), + ) + .subscribe({ + next: () => { + this.feedback.success('Permissao removida com sucesso'); + this.loadPermissions(); + }, + error: () => { + this.feedback.error('Erro ao remover permissao'); + }, + }); }🤖 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 145 - 165, The confirm/delete flow nests subscriptions; refactor confirmDialog.open(...) and the deleteListPermission(...) call into a single observable chain: call this.confirmDialog.open({...}).pipe(filter(confirmed => !!confirmed), switchMap(() => this.listPermissionService.deleteListPermission(this.data.listId, permissionId))).subscribe({ next: () => { this.feedback.success('Permissao removida com sucesso'); this.loadPermissions(); }, error: () => { this.feedback.error('Erro ao remover permissao'); } }); this keeps the logic linear while still using confirmDialog.open, listPermissionService.deleteListPermission, loadPermissions, and feedback.success/error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 104-121: The current onShare() treats an empty this.users() as
"user not found" which produces false negatives when loadUsers() is still
loading or previously failed; modify loadUsers() to maintain a usersLoaded flag
(or similar) and set it true in the next callback and false in error, and update
onShare() to first check usersLoaded (or this.users().length) and show a
distinct "users unavailable" feedback (or disable the share action) when the
list isn't loaded instead of treating it as not found; ensure references to
loadUsers(), userService.getAllUsers(), this.users, onShare(), shareForm are
updated accordingly.
---
Nitpick comments:
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 145-165: The confirm/delete flow nests subscriptions; refactor
confirmDialog.open(...) and the deleteListPermission(...) call into a single
observable chain: call this.confirmDialog.open({...}).pipe(filter(confirmed =>
!!confirmed), switchMap(() =>
this.listPermissionService.deleteListPermission(this.data.listId,
permissionId))).subscribe({ next: () => { this.feedback.success('Permissao
removida com sucesso'); this.loadPermissions(); }, error: () => {
this.feedback.error('Erro ao remover permissao'); } }); this keeps the logic
linear while still using confirmDialog.open,
listPermissionService.deleteListPermission, loadPermissions, and
feedback.success/error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 881d3bcd-642d-416a-b991-029591eb99a5
📒 Files selected for processing (1)
frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts
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 @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.
|
Contributor
|
🤖 I'm sorry @coderabbitai[bot], but I was unable to process your request. Please see the logs for more details. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
frontend/src/app/shared/components/item/item.component.ts (2)
61-68: Minor: Inconsistent dependency injection style.The component mixes
inject()function (lines 61-62) with constructor-based DI (lines 65-68). Both work correctly, but consider using one style consistently for better maintainability.♻️ Option: Use inject() for all services
- private confirmDialog = inject(ConfirmDialogService); - private feedback = inject(FeedbackService); - - constructor( - private itemService: ItemService, - private categoryService: CategoryService, - private unitService: UnitService, - private fb: FormBuilder, - ) { + private confirmDialog = inject(ConfirmDialogService); + private feedback = inject(FeedbackService); + private itemService = inject(ItemService); + private categoryService = inject(CategoryService); + private unitService = inject(UnitService); + private fb = inject(FormBuilder); + + constructor() {🤖 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 61 - 68, The component mixes inject() calls for ConfirmDialogService and FeedbackService with constructor-based DI for ItemService, CategoryService, UnitService and FormBuilder; make the injection style consistent by converting the constructor-injected services to use inject() as well (replace usages of the constructor and its parameters ItemService, CategoryService, UnitService, FormBuilder with private fields assigned via inject(ItemService), inject(CategoryService), inject(UnitService), inject(FormBuilder)), or alternatively convert the two inject() calls into constructor parameters—ensure all references to ConfirmDialogService, FeedbackService, ItemService, CategoryService, UnitService, and FormBuilder are updated accordingly and remove the now-unused constructor or inject variables.
139-159: Delete confirmation flow meets PR objectives.The replacement of native
confirm()withConfirmDialogServicesatisfies the acceptance criteria. The typo fix from the previous review ("excluído") is correctly applied.Consider flattening the nested subscriptions using
switchMapfor a more idiomatic RxJS approach:♻️ Optional: Flatten nested subscriptions with switchMap
+import { filter, switchMap } from 'rxjs';deleteItem(id: number): void { this.confirmDialog .open({ title: 'Excluir item', message: 'Tem certeza que deseja excluir este item?', confirmText: 'Excluir', }) - .subscribe((confirmed) => { - if (!confirmed) return; - - this.itemService.deleteItem(id).subscribe({ - next: () => { - this.feedback.success('Item excluído com sucesso'); - this.loadInitialData(); - }, - error: () => { - this.feedback.error('Erro ao excluir item'); - }, - }); + .pipe( + filter((confirmed) => confirmed), + switchMap(() => this.itemService.deleteItem(id)) + ) + .subscribe({ + next: () => { + this.feedback.success('Item excluído com sucesso'); + this.loadInitialData(); + }, + error: () => { + this.feedback.error('Erro ao excluir 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 139 - 159, The nested subscription in deleteItem should be flattened: replace the inner subscribe pattern by piping the confirmDialog.open() Observable into switchMap and then into itemService.deleteItem(id), handling success and error in the final subscription (or in tap/catchError) so you avoid subscribing inside a subscribe; update the deleteItem method (referencing deleteItem, confirmDialog.open, itemService.deleteItem, loadInitialData, feedback.success and feedback.error) to use switchMap (and optionally catchError) to call loadInitialData on success and show feedback messages on success/error.frontend/src/app/list/components/items-management/items-management.component.ts (1)
92-102: Extract the repeated post-mutation feedback path.These three handlers now duplicate the same
loadData()+feedbackpattern. A small helper would keep CRUD feedback behavior consistent and reduce drift as more actions are added.♻️ Suggested refactor
+ private handleMutationSuccess(message: string): void { + this.loadData(); + this.feedback.success(message); + } + + private handleMutationError(message: string): void { + this.feedback.error(message); + } + openAddItemDialog(): void { const dialogRef = this.dialog.open(ItemDialogComponent, { width: '400px', data: { item: null }, }); dialogRef.afterClosed().subscribe((result) => { if (result) { this.itemService.addItem(result).subscribe({ next: () => { - this.loadData(); - this.feedback.success('Item adicionado com sucesso'); + this.handleMutationSuccess('Item adicionado com sucesso'); }, error: () => { - this.feedback.error('Erro ao adicionar item'); + this.handleMutationError('Erro ao adicionar item'); }, }); } }); }this.itemService.updateItem(item.id, result).subscribe({ next: () => { - this.loadData(); - this.feedback.success('Item atualizado com sucesso'); + this.handleMutationSuccess('Item atualizado com sucesso'); }, error: () => { - this.feedback.error('Erro ao atualizar item'); + this.handleMutationError('Erro ao atualizar item'); }, });this.itemService.deleteItem(item.id).subscribe({ next: () => { - this.loadData(); - this.feedback.success('Item excluído com sucesso'); + this.handleMutationSuccess('Item excluído com sucesso'); }, error: () => { - this.feedback.error('Erro ao excluir item'); + this.handleMutationError('Erro ao excluir item'); }, });Also applies to: 119-129, 144-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/list/components/items-management/items-management.component.ts` around lines 92 - 102, The code repeats the same post-mutation sequence (calling this.loadData() and showing success/error via this.feedback) in multiple places (e.g., the dialogRef.afterClosed() callback that calls this.itemService.addItem). Introduce a small helper on the component (e.g., applyMutation(observable, successMsg, errorMsg) or handleMutationResult) that subscribes to the passed Observable, calls this.loadData() on next and invokes this.feedback.success(successMsg) or this.feedback.error(errorMsg) on error; then replace the inline subscribe blocks in addItem, editItem, deleteItem flows (references: itemService.addItem, itemService.updateItem, itemService.deleteItem and dialogRef.afterClosed callbacks) with calls to that helper to centralize the pattern.
🤖 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/items-management/items-management.component.ts`:
- Around line 92-102: The code repeats the same post-mutation sequence (calling
this.loadData() and showing success/error via this.feedback) in multiple places
(e.g., the dialogRef.afterClosed() callback that calls
this.itemService.addItem). Introduce a small helper on the component (e.g.,
applyMutation(observable, successMsg, errorMsg) or handleMutationResult) that
subscribes to the passed Observable, calls this.loadData() on next and invokes
this.feedback.success(successMsg) or this.feedback.error(errorMsg) on error;
then replace the inline subscribe blocks in addItem, editItem, deleteItem flows
(references: itemService.addItem, itemService.updateItem, itemService.deleteItem
and dialogRef.afterClosed callbacks) with calls to that helper to centralize the
pattern.
In `@frontend/src/app/shared/components/item/item.component.ts`:
- Around line 61-68: The component mixes inject() calls for ConfirmDialogService
and FeedbackService with constructor-based DI for ItemService, CategoryService,
UnitService and FormBuilder; make the injection style consistent by converting
the constructor-injected services to use inject() as well (replace usages of the
constructor and its parameters ItemService, CategoryService, UnitService,
FormBuilder with private fields assigned via inject(ItemService),
inject(CategoryService), inject(UnitService), inject(FormBuilder)), or
alternatively convert the two inject() calls into constructor parameters—ensure
all references to ConfirmDialogService, FeedbackService, ItemService,
CategoryService, UnitService, and FormBuilder are updated accordingly and remove
the now-unused constructor or inject variables.
- Around line 139-159: The nested subscription in deleteItem should be
flattened: replace the inner subscribe pattern by piping the
confirmDialog.open() Observable into switchMap and then into
itemService.deleteItem(id), handling success and error in the final subscription
(or in tap/catchError) so you avoid subscribing inside a subscribe; update the
deleteItem method (referencing deleteItem, confirmDialog.open,
itemService.deleteItem, loadInitialData, feedback.success and feedback.error) to
use switchMap (and optionally catchError) to call loadInitialData on success and
show feedback messages on success/error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd63de5b-3c79-4614-81d2-7618f30f0864
📒 Files selected for processing (3)
frontend/src/app/list/components/items-management/items-management.component.tsfrontend/src/app/list/components/shopping-list/shopping-list.component.tsfrontend/src/app/shared/components/item/item.component.ts
Contributor
|
Tip For best results, initiate chat on the files or code changes.
Root cause: The PR is still in draft status with unresolved merge conflicts on the To unblock the review,
Once the conflicts are cleared, a fresh review can be triggered. No further action is needed on my end until then. 🙌 |
github-actions
Bot
force-pushed
the
develop
branch
from
d2d1107 to
d613db3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
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/list/components/list-details/list-details.component.ts (1)
95-102:⚠️ Potential issue | 🟠 MajorAdd success feedback for item status updates.
This update path only reports errors; successful status toggles are silent, while other CRUD actions now show success messages. That breaks the PR goal of consistent operation feedback.
💡 Suggested patch
this.listItemService .updateListItem(this.listId, item.idListItem, updatedItem) .subscribe({ - next: () => this.loadData(), + next: () => { + this.loadData(); + this.feedback.success('Status do item atualizado com sucesso'); + }, error: () => { this.feedback.error('Erro ao atualizar status do item'); }, });🤖 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-details.component.ts` around lines 95 - 102, The updateListItem call in list-details.component.ts currently only shows an error on failure; add a success feedback call in the success branch so users get confirmation when a status toggle succeeds. Modify the subscribe handlers on listItemService.updateListItem(this.listId, item.idListItem, updatedItem) to call this.feedback.success('Item atualizado com sucesso') (or the appropriate success message) inside the next handler before or after invoking this.loadData(), and keep the existing error handler this.feedback.error('Erro ao atualizar status do item').
♻️ Duplicate comments (4)
frontend/src/app/list/components/shopping-list/shopping-list.component.ts (1)
101-105:⚠️ Potential issue | 🟠 MajorUpdate local state before showing delete success feedback.
Success is shown immediately after triggering reload; users can momentarily see stale UI with a success toast.
🐛 Suggested fix
this.shoppingListService.deleteShoppingList(id).subscribe({ next: () => { - this.loadLists(); - this.feedback.success('Lista excluida com sucesso'); + this.shoppingLists.update((lists) => + lists.filter((list) => list.idList !== id), + ); + this.feedback.success('Lista excluída com sucesso'); + this.loadLists(); },🤖 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/shopping-list.component.ts` around lines 101 - 105, When handling deleteShoppingList(id) in the subscribe next handler, remove the deleted item from the local collection (this.shoppingLists) before calling feedback.success so the UI reflects the deletion immediately; then call loadLists() or refresh if needed after updating local state. Update the code around shoppingListService.deleteShoppingList, next: (), feedback.success and loadLists to perform the optimistic local removal (filter out the id) first, then show feedback.success('Lista excluida com sucesso') and finally trigger loadLists() only if a full refresh is required.frontend/src/app/shared/components/unit/unit.component.ts (1)
133-133:⚠️ Potential issue | 🟡 MinorCorrect Portuguese spelling in success toast.
Use the accented form “excluída”.
✏️ Suggested fix
- this.feedback.success('Unidade excluida com sucesso'); + this.feedback.success('Unidade excluída com sucesso');🤖 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` at line 133, Update the success toast message in the UnitComponent where this.feedback.success(...) is called to use the correct Portuguese accent: replace the string "Unidade excluida com sucesso" with "Unidade excluída com sucesso" (locate the call to this.feedback.success in unit.component.ts).frontend/src/app/shared/components/category/category.component.ts (1)
130-130:⚠️ Potential issue | 🟡 MinorFix accent in delete success message.
Use “excluída” instead of “excluida”.
✏️ Suggested fix
- this.feedback.success('Categoria excluida com sucesso'); + this.feedback.success('Categoria excluída com sucesso');🤖 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` at line 130, The success message string in CategoryComponent (where this.feedback.success('Categoria excluida com sucesso') is called) has a missing accent; update the literal to use the correct Portuguese spelling "Categoria excluída com sucesso" so the UI shows the accented 'í' in the delete confirmation message.frontend/src/app/list/components/category-dialog/category-dialog.component.ts (1)
93-93:⚠️ Potential issue | 🟡 MinorCorrect Portuguese orthography in error message.
This still shows “validacao” instead of “validação”.
✏️ Suggested fix
- this.feedback.error('Erro ao carregar categorias para validacao'); + this.feedback.error('Erro ao carregar categorias para validação');🤖 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` at line 93, Fix the Portuguese spelling in the error message inside CategoryDialogComponent (the line calling this.feedback.error). Replace "Erro ao carregar categorias para validacao" with the correctly accented "Erro ao carregar categorias para validação" so the user-facing message uses proper orthography.
🧹 Nitpick comments (1)
frontend/src/app/shared/components/confirm-dialog/confirm-dialog.component.html (1)
4-5: Add initial focus to the cancel button for safer UX.For destructive confirmation dialogs, best practice is to focus the cancel button initially to prevent accidental confirmations via keyboard (e.g., pressing Enter). Use
cdkFocusInitialon the cancel button.♻️ Proposed fix
- <button mat-button (click)="onCancel()"> + <button mat-button cdkFocusInitial (click)="onCancel()"> {{ data.cancelText ?? "Cancelar" }} </button>Ensure
A11yModule(from@angular/cdk/a11y) is imported in the component.As per coding guidelines: "Accessibility Requirements - It MUST pass all AXE checks. It MUST follow all WCAG AA minimums, including focus management."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/shared/components/confirm-dialog/confirm-dialog.component.html` around lines 4 - 5, Add initial focus to the cancel button by adding the cdkFocusInitial directive on the cancel button element in the ConfirmDialogComponent template (the button that calls onCancel() and displays data.cancelText ?? "Cancelar"); also ensure A11yModule from `@angular/cdk/a11y` is imported into the NgModule that declares ConfirmDialogComponent so cdkFocusInitial is available at runtime.
🤖 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 37-42: The component currently mixes constructor injection and the
project's inject() pattern—refactor CategoryService and MatDialog to use
Angular's inject() like the other two services: add import { inject } from
'@angular/core' and replace the constructor-based params with class properties
such as private categoryService = inject(CategoryService), private dialog =
inject(MatDialog), private confirmDialog = inject(ConfirmDialogService), and
private feedback = inject(FeedbackService); then remove the constructor entirely
so all four dependencies consistently use inject().
In
`@frontend/src/app/list/components/list-share-dialog/list-share-dialog.component.ts`:
- Around line 98-99: Replace the unaccented Portuguese feedback strings passed
to this.feedback.error and this.feedback.success in list-share-dialog.component
(e.g., the messages used in the methods where this.feedback.error('Erro ao
carregar permissoes'), this.feedback.success('Permissoes atualizadas'), etc.)
with correctly accented versions (e.g., "Erro ao carregar permissões",
"Permissões atualizadas", "Permissão removida", "Não foi possível ...", etc.),
keeping the same message intent and interpolation; update all occurrences
referenced in the review (the calls around the this.feedback.* usages and
related success/error branches at the locations noted) so copy is consistent
across the component while leaving the surrounding logic (e.g.,
this.isLoading.set(false), error handling) unchanged.
In
`@frontend/src/app/list/components/shopping-list-dialog/shopping-list-dialog.component.ts`:
- Around line 76-77: Update the Portuguese error message string passed to
this.feedback.error in ShoppingListDialogComponent to include proper accents:
change "Usuario nao identificado. Por favor, faca login novamente." to "Usuário
não identificado. Por favor, faça login novamente." so the UX text is correctly
localized; locate the call to this.feedback.error in
shopping-list-dialog.component (the authentication error branch) and replace the
literal string accordingly.
In `@frontend/src/app/list/components/shopping-list/shopping-list.component.ts`:
- Around line 40-45: The constructor currently injects dependencies via
constructor parameters (ShoppingListService, MatDialog, ConfirmDialogService,
FeedbackService) which must be migrated to Angular's inject() pattern for
consistency; replace the constructor DI by removing those parameters and instead
create private readonly fields initialized with inject() (e.g., private readonly
shoppingListService = inject(ShoppingListService), private readonly dialog =
inject(MatDialog), private readonly confirmDialog =
inject(ConfirmDialogService), private readonly feedback =
inject(FeedbackService)) and ensure any usages of these fields in
ShoppingListComponent methods remain unchanged.
In
`@frontend/src/app/list/components/units-management/unit-dialog/unit-dialog.component.ts`:
- Line 88: Replace the misspelled localized message in the unit dialog error
call: update the string passed to this.feedback.error in unit-dialog.component
(the call currently using 'Erro ao carregar unidades para validacao') to the
correct Portuguese form 'Erro ao carregar unidades para validação' so the
accented characters are used properly.
---
Outside diff comments:
In `@frontend/src/app/list/components/list-details/list-details.component.ts`:
- Around line 95-102: The updateListItem call in list-details.component.ts
currently only shows an error on failure; add a success feedback call in the
success branch so users get confirmation when a status toggle succeeds. Modify
the subscribe handlers on listItemService.updateListItem(this.listId,
item.idListItem, updatedItem) to call this.feedback.success('Item atualizado com
sucesso') (or the appropriate success message) inside the next handler before or
after invoking this.loadData(), and keep the existing error handler
this.feedback.error('Erro ao atualizar status do item').
---
Duplicate comments:
In
`@frontend/src/app/list/components/category-dialog/category-dialog.component.ts`:
- Line 93: Fix the Portuguese spelling in the error message inside
CategoryDialogComponent (the line calling this.feedback.error). Replace "Erro ao
carregar categorias para validacao" with the correctly accented "Erro ao
carregar categorias para validação" so the user-facing message uses proper
orthography.
In `@frontend/src/app/list/components/shopping-list/shopping-list.component.ts`:
- Around line 101-105: When handling deleteShoppingList(id) in the subscribe
next handler, remove the deleted item from the local collection
(this.shoppingLists) before calling feedback.success so the UI reflects the
deletion immediately; then call loadLists() or refresh if needed after updating
local state. Update the code around shoppingListService.deleteShoppingList,
next: (), feedback.success and loadLists to perform the optimistic local removal
(filter out the id) first, then show feedback.success('Lista excluida com
sucesso') and finally trigger loadLists() only if a full refresh is required.
In `@frontend/src/app/shared/components/category/category.component.ts`:
- Line 130: The success message string in CategoryComponent (where
this.feedback.success('Categoria excluida com sucesso') is called) has a missing
accent; update the literal to use the correct Portuguese spelling "Categoria
excluída com sucesso" so the UI shows the accented 'í' in the delete
confirmation message.
In `@frontend/src/app/shared/components/unit/unit.component.ts`:
- Line 133: Update the success toast message in the UnitComponent where
this.feedback.success(...) is called to use the correct Portuguese accent:
replace the string "Unidade excluida com sucesso" with "Unidade excluída com
sucesso" (locate the call to this.feedback.success in unit.component.ts).
---
Nitpick comments:
In
`@frontend/src/app/shared/components/confirm-dialog/confirm-dialog.component.html`:
- Around line 4-5: Add initial focus to the cancel button by adding the
cdkFocusInitial directive on the cancel button element in the
ConfirmDialogComponent template (the button that calls onCancel() and displays
data.cancelText ?? "Cancelar"); also ensure A11yModule from `@angular/cdk/a11y` is
imported into the NgModule that declares ConfirmDialogComponent so
cdkFocusInitial is available at runtime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: af3dbda4-19c9-4984-ba83-4e4539c83b62
📒 Files selected for processing (20)
frontend/src/app/list/components/categories-management/categories-management.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/items-management/items-management.component.tsfrontend/src/app/list/components/list-details/list-details.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/shopping-list.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/confirm-dialog/confirm-dialog.component.htmlfrontend/src/app/shared/components/confirm-dialog/confirm-dialog.component.scssfrontend/src/app/shared/components/confirm-dialog/confirm-dialog.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/confirm-dialog.service.tsfrontend/src/app/shared/services/feedback.service.tsfrontend/src/styles.scss
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/src/app/list/components/units-management/units-management.component.ts
- frontend/src/app/shared/components/confirm-dialog/confirm-dialog.component.ts
- frontend/src/app/shared/services/confirm-dialog.service.ts
- frontend/src/app/list/components/items-management/items-management.component.ts
- frontend/src/app/shared/components/list-item/list-item.component.ts
- frontend/src/app/list/components/items-management/item-dialog/item-dialog.component.ts
Comment on lines
+98
to
99
| this.feedback.error('Erro ao carregar permissoes'); | ||
| this.isLoading.set(false); |
Contributor
There was a problem hiding this comment.
Polish Portuguese strings (accentuation) in user-facing feedback.
There are regressions in localized copy quality (missing accents), which reduces UX polish and consistency.
✏️ Suggested patch
- this.feedback.error('Erro ao carregar permissoes');
+ this.feedback.error('Erro ao carregar permissões');
@@
- this.feedback.error('Erro ao carregar usuarios');
+ this.feedback.error('Erro ao carregar usuários');
@@
- this.feedback.error('Usuario nao encontrado com este e-mail');
+ this.feedback.error('Usuário não encontrado com este e-mail');
@@
- title: 'Remover permissao',
- message: 'Tem certeza que deseja remover esta permissao?',
+ title: 'Remover permissão',
+ message: 'Tem certeza que deseja remover esta permissão?',
@@
- this.feedback.success('Permissao removida com sucesso');
+ this.feedback.success('Permissão removida com sucesso');
@@
- this.feedback.error('Erro ao remover permissao');
+ this.feedback.error('Erro ao remover permissão');Also applies to: 110-110, 121-121, 147-149, 158-163
🤖 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 98 - 99, Replace the unaccented Portuguese feedback strings passed
to this.feedback.error and this.feedback.success in list-share-dialog.component
(e.g., the messages used in the methods where this.feedback.error('Erro ao
carregar permissoes'), this.feedback.success('Permissoes atualizadas'), etc.)
with correctly accented versions (e.g., "Erro ao carregar permissões",
"Permissões atualizadas", "Permissão removida", "Não foi possível ...", etc.),
keeping the same message intent and interpolation; update all occurrences
referenced in the review (the calls around the this.feedback.* usages and
related success/error branches at the locations noted) so copy is consistent
across the component while leaving the surrounding logic (e.g.,
this.isLoading.set(false), error handling) unchanged.
…unit-dialog.component.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
omatheusmesmo
approved these changes
Mar 13, 2026
Owner
omatheusmesmo
left a comment
omatheusmesmo
left a comment
There was a problem hiding this comment.
LGTM, @matheusandre1 ! Thanks.
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.
draft, which closes #37
I'll put this up for review after some of the pull requests that were made have been merged, so we don't have to manually resolve conflicts.
Summary by CodeRabbit