WEB-810 Create Stepper for Savings Withdrawal Transaction#3310
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Template (Stepper) src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html |
Rewrote template to a 3-step mat-stepper: step 1 collects transaction inputs with optional payment details; step 2 displays a read-only confirmation; step 3 shows success/receipt with Print/Done and per-step navigation. |
Component Logic src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts |
Added @ViewChild('stepper') stepper: MatStepper, transactionResponse, isSubmitting, and methods: proceedToConfirmation(), goBack(), confirmTransaction(), done(), getPaymentTypeName(), printReceipt(). Retains previous submit() but integrates stepper flow and imports (MatStepperModule, FaIconComponent). |
Styling src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scss |
New SCSS rules for stepper, confirmation and success layouts, detail rows, amount styling, responsive adjustments and print media rules. |
Translations (multi-locale) src/assets/translations/{cs-CS,de-DE,en-US,es-CL,es-MX,fr-FR,it-IT,ko-KO,lt-LT,lv-LV,ne-NE,pt-PT,sw-SW}.json |
Added PRINT, DONE, Submitting keys and a cluster of transaction-related keys (Transaction Details, Confirm Transaction Details, Transaction Complete, Transaction Successful, Id) across locales; removed/relocated prior Transaction Details entries in several files. |
Sequence Diagram
sequenceDiagram
participant User as User
participant UIForm as "Form Validator"
participant Stepper as "MatStepper"
participant Component as "SavingsAccountTransactionsComponent"
participant Service as "Transaction Service"
participant Backend as "Backend API"
User->>UIForm: Enter transaction details
UIForm->>UIForm: Validate step 1
User->>Stepper: Click "Proceed to Confirmation"
Stepper->>Component: proceedToConfirmation()
Component->>Stepper: Advance to step 2
Stepper->>User: Show confirmation summary
User->>Stepper: Click "Submit"
Stepper->>Component: confirmTransaction()
Component->>Component: set isSubmitting = true / build payload
Component->>Service: submit transaction payload
Service->>Backend: POST /transactions
Backend-->>Service: return transactionResponse
Service-->>Component: response
Component->>Component: store transactionResponse / set isSubmitting = false
Component->>Stepper: Advance to step 3
Stepper->>User: Show transaction complete (ID)
User->>Component: Click "Print"
Component->>Component: printReceipt()
User->>Component: Click "Done"
Component->>Component: done() / navigate back
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- WEB-770 Validation message not displayed for payment type in deposit and withdrawal forms #3203 — Modifies Payment Type handling and validation in the same savings-account-transactions form; likely to overlap with stepper/form controls.
- WEB-771 feat: sync 70+ remittance translation keys across all locales #3201 — Adds/edits translation JSON keys similar to the new transaction/print/submitting keys added here.
Suggested reviewers
- IOhacker
- alberto-art3ch
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title accurately summarizes the main change: implementing a Material stepper for savings withdrawal transactions. It is concise, specific, and directly reflects the primary objective demonstrated in the PR description and code changes. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (5)
src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scss (1)
143-146: Prefer MDC class selector in print styles for consistency with Angular Material v20+.Line 144 targets
mat-card-actionsby element selector; use.mat-mdc-card-actionsto align with the MDC-based DOM structure in Angular Material v20 and maintain consistency with the other selectors in this block.💡 Proposed fix
`@media` print { .mat-stepper-horizontal-line, - mat-card-actions, + .mat-mdc-card-actions, .mat-stepper-horizontal { display: none !important; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scss` around lines 143 - 146, The print stylesheet hides stepper and card action elements using an element selector `mat-card-actions`; update it to use the MDC class selector `.mat-mdc-card-actions` so it matches the other selectors (`.mat-stepper-horizontal-line`, `.mat-stepper-horizontal`) and Angular Material v20+ DOM structure—replace `mat-card-actions` with `.mat-mdc-card-actions` in the same rule that sets `display: none !important;`.src/assets/translations/cs-CS.json (1)
1331-1332: Polish Czech casing for UI consistency.
"Transakce Dokončena"and"Transakce Úspěšná"read a bit unnatural in Czech UI copy; sentence case is more idiomatic.💬 Suggested localization tweak
- "Transaction Complete": "Transakce Dokončena", - "Transaction Successful": "Transakce Úspěšná", + "Transaction Complete": "Transakce dokončena", + "Transaction Successful": "Transakce úspěšná",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/cs-CS.json` around lines 1331 - 1332, Update the Czech translations for the keys "Transaction Complete" and "Transaction Successful" to use sentence case for UI consistency; change the value for "Transaction Complete" from "Transakce Dokončena" to "Transakce dokončena" and the value for "Transaction Successful" from "Transakce Úspěšná" to "Transakce úspěšná" so they read naturally in Czech UI copy.src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html (1)
61-61: Track payment options by stable key (id), not object reference.Using
track paymentType.idprevents unnecessary DOM churn when option objects are recreated.♻️ Suggested change
- `@for` (paymentType of paymentTypeOptions; track paymentType) { + `@for` (paymentType of paymentTypeOptions; track paymentType.id) {As per coding guidelines: "For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html` at line 61, The template is tracking options by object reference (track paymentType) which causes unnecessary DOM churn; change the ngFor tracking to use a stable key (e.g., track paymentType.id) and ensure the backing array paymentTypeOptions (and its element type) exposes a unique id property so the view uses that stable identifier for change tracking.src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts (2)
175-202:confirmTransaction()and deprecatedsubmit()now duplicate the same payload/submission flow.Please extract a shared payload builder + execute helper to avoid divergence between the stepper path and legacy path.
Also applies to: 231-249
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts` around lines 175 - 202, confirmTransaction() and the deprecated submit() duplicate payload construction and submission logic; extract a shared private helper (e.g., buildTransactionPayload(savingAccountTransactionForm: FormGroup): any) that builds the data object (handles locale, dateFormat, formats transactionDate via dateUtils.formatDate, coerces transactionAmount) and another helper (e.g., executeTransactionCommand(payload: any)) that calls this.savingsService.executeSavingsAccountTransactionsCommand(this.savingAccountId, this.transactionCommand, payload). Replace the duplicate code in confirmTransaction() and submit() to call buildTransactionPayload(...) then executeTransactionCommand(...) and ensure both helpers update this.isSubmitting, this.transactionResponse and stepper.next()/error handling consistently.
81-84: Avoid introducing a newany-typed transaction response state.Use a narrow response interface for the fields consumed by the template (
resourceId,savingsId) to keep the new stepper flow type-safe.♻️ Suggested change
+interface SavingsTransactionResponse { + resourceId?: number | string; + savingsId?: number | string; +} + export class SavingsAccountTransactionsComponent implements OnInit { @@ - transactionResponse: any = null; + transactionResponse: SavingsTransactionResponse | null = null;As per coding guidelines: "For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts` around lines 81 - 84, Replace the broad any-typed transactionResponse with a narrow interface describing only the fields the template consumes (e.g., interface TransactionResponse { resourceId: string; savingsId: string; }), then change the component property declaration from transactionResponse: any = null to transactionResponse: TransactionResponse | null = null and update any code that assigns to it to satisfy the new shape (or map/pluck those fields from the backend response). Also ensure template accesses use optional chaining (transactionResponse?.resourceId / ?.savingsId) if needed so nullability is handled safely.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html`:
- Around line 258-260: The transaction type label is hardcoded in the template;
update the span that uses transactionType.withdrawal in
savings-account-transactions.component.html to output translated strings instead
of plain English. Replace the literal 'Withdrawal' and 'Deposit' with calls to
the translate pipe using your i18n keys (e.g. the appropriate keys for
withdrawal/deposit such as 'labels.inputs.Withdrawal' and
'labels.inputs.Deposit' or your project's existing keys) so the expression
becomes conditional but uses {{ key | translate }}; keep the conditional on
transactionType.withdrawal to choose the correct translation key.
- Around line 88-89: The account identifier input uses type="number" which
strips leading zeros; update the template control with
formControlName="accountNumber" in savings-account-transactions.component.html
to use a text input (type="text" or omit type) so identifiers preserve leading
zeros, and adjust any validators or parsing logic (e.g., numeric pattern checks
or conversions) in the SavingsAccountTransactions component or its FormGroup to
treat accountNumber as a string rather than coercing to a number.
- Around line 75-82: The slide-toggle control isn't receiving keyboard events
because the click handler is on the parent span; move the interaction to the
mat-slide-toggle itself by wiring its change event and binding its checked state
to the component flag: remove the (click) on the span and add
(change)="addPaymentDetails($event)" (or a new onPaymentDetailsToggle(event:
MatSlideToggleChange | boolean) that sets addPaymentDetailsFlag = event.checked
|| !!event) and set [checked]="addPaymentDetailsFlag" on the <mat-slide-toggle>,
then update the addPaymentDetails handler in the component (or add a new
handler) to accept the event and update addPaymentDetailsFlag accordingly so
keyboard (Space/Enter) toggles stay in sync with addPaymentDetailsFlag.
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scss`:
- Around line 149-151: Replace the deprecated CSS property in the
.success-container rule: remove page-break-after and use the modern
fragmentation property by adding break-after: page; so the .success-container
uses the current standard instead of the deprecated page-break-after.
In `@src/assets/translations/de-DE.json`:
- Around line 1332-1334: Update the German translations for the keys
"Transaction Complete", "Transaction Successful", and "Id" to use proper German
casing and acronym form: change "Transaktion Abgeschlossen" → "Transaktion
abgeschlossen", "Transaktion Erfolgreich" → "Transaktion erfolgreich", and "Id"
→ "ID" so the JSON values follow natural German capitalization and standard
acronym usage.
In `@src/assets/translations/it-IT.json`:
- Around line 1329-1333: Remove the duplicate "Id": "Id" entry from the
transaction details block in the it-IT translation JSON (the block containing
"Transaction Details", "Confirm Transaction Details", "Transaction Complete",
"Transaction Successful"); keep the original "Id" definition in the ID document
types section and delete the second "Id" key to avoid silent overwrite when
parsing the translations.
In `@src/assets/translations/ko-KO.json`:
- Line 1335: The translation key "Id" is inconsistent with the locale's existing
"ID" casing; update the key/value pair that currently reads "Id": "Id" to use
"ID" (i.e., change the identifier and its translation to "ID") so it matches the
other locale entries and preserves UI consistency — locate the "Id" key in the
ko-KO JSON and replace it with "ID".
In `@src/assets/translations/lt-LT.json`:
- Line 1328: The Lithuanian translation for the key "Transaction Details" is
misspelled; update the value for the "Transaction Details" key in lt-LT.json
from "Sandorio detalai" to the correct form "Sandorio detalės" so the UI shows
the proper Lithuanian text.
In `@src/assets/translations/ne-NE.json`:
- Around line 1328-1332: The "Id" translation is left as English which will show
mixed-language UI; update the value for the "Id" key in the ne-NE.json
translation object (keep the key "Id" unchanged) to an appropriate Nepali label
(for example "आईडी" or "पहिचान"), ensuring the rest of the transaction-related
keys like "Transaction Details", "Confirm Transaction Details", "Transaction
Complete", and "Transaction Successful" remain intact.
In `@src/assets/translations/pt-PT.json`:
- Around line 611-613: The translation key "DONE" currently maps to a
past-tense/status string "Concluído"; change its value to an imperative/action
verb suitable for a button (e.g., "Concluir") so the UI shows a clear action
label. Update the "DONE" entry in the translations JSON (key "DONE") to use the
imperative Portuguese phrase and run a quick UI/text check to ensure it reads
correctly in the context where the DONE key is used.
---
Nitpick comments:
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html`:
- Line 61: The template is tracking options by object reference (track
paymentType) which causes unnecessary DOM churn; change the ngFor tracking to
use a stable key (e.g., track paymentType.id) and ensure the backing array
paymentTypeOptions (and its element type) exposes a unique id property so the
view uses that stable identifier for change tracking.
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scss`:
- Around line 143-146: The print stylesheet hides stepper and card action
elements using an element selector `mat-card-actions`; update it to use the MDC
class selector `.mat-mdc-card-actions` so it matches the other selectors
(`.mat-stepper-horizontal-line`, `.mat-stepper-horizontal`) and Angular Material
v20+ DOM structure—replace `mat-card-actions` with `.mat-mdc-card-actions` in
the same rule that sets `display: none !important;`.
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts`:
- Around line 175-202: confirmTransaction() and the deprecated submit()
duplicate payload construction and submission logic; extract a shared private
helper (e.g., buildTransactionPayload(savingAccountTransactionForm: FormGroup):
any) that builds the data object (handles locale, dateFormat, formats
transactionDate via dateUtils.formatDate, coerces transactionAmount) and another
helper (e.g., executeTransactionCommand(payload: any)) that calls
this.savingsService.executeSavingsAccountTransactionsCommand(this.savingAccountId,
this.transactionCommand, payload). Replace the duplicate code in
confirmTransaction() and submit() to call buildTransactionPayload(...) then
executeTransactionCommand(...) and ensure both helpers update this.isSubmitting,
this.transactionResponse and stepper.next()/error handling consistently.
- Around line 81-84: Replace the broad any-typed transactionResponse with a
narrow interface describing only the fields the template consumes (e.g.,
interface TransactionResponse { resourceId: string; savingsId: string; }), then
change the component property declaration from transactionResponse: any = null
to transactionResponse: TransactionResponse | null = null and update any code
that assigns to it to satisfy the new shape (or map/pluck those fields from the
backend response). Also ensure template accesses use optional chaining
(transactionResponse?.resourceId / ?.savingsId) if needed so nullability is
handled safely.
In `@src/assets/translations/cs-CS.json`:
- Around line 1331-1332: Update the Czech translations for the keys "Transaction
Complete" and "Transaction Successful" to use sentence case for UI consistency;
change the value for "Transaction Complete" from "Transakce Dokončena" to
"Transakce dokončena" and the value for "Transaction Successful" from "Transakce
Úspěšná" to "Transakce úspěšná" so they read naturally in Czech UI copy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 90e3681f-60d3-4dd8-9055-c2cde2897edc
📒 Files selected for processing (16)
src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.htmlsrc/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scsssrc/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
6255adb to
3094a27
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html (1)
95-95:⚠️ Potential issue | 🟠 MajorKeep cheque identifiers as text to preserve exact value.
type="number"can normalize/strip leading zeros for cheque identifiers. Use text input with numeric keyboard hint instead.🔧 Suggested change
- <input type="number" formControlName="checkNumber" matInput /> + <input type="text" inputmode="numeric" formControlName="checkNumber" matInput />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html` at line 95, The input using formControlName="checkNumber" currently uses type="number" which can strip leading zeros; change the input to type="text" and add a numeric input hint (e.g., inputmode="numeric" and/or pattern="[0-9]*") so cheque identifiers are preserved as exact strings while still presenting a numeric keyboard on mobile; update the template element with formControlName="checkNumber" accordingly and ensure any validation/parsing logic in the component that expects a number handles the value as a string.
🧹 Nitpick comments (2)
src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html (1)
61-63: Use a stable track expression for payment options.Prefer tracking by
paymentType.idinstead of object reference to avoid unnecessary DOM churn when option arrays are recreated.♻️ Suggested change
- `@for` (paymentType of paymentTypeOptions; track paymentType) { + `@for` (paymentType of paymentTypeOptions; track paymentType.id) {As per coding guidelines: "For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html` around lines 61 - 63, The template currently tracks options by object reference ("track paymentType"); change the ngFor to use a stable key by replacing the track expression with "trackBy: trackByPaymentType" and add a trackByPaymentType(index: number, paymentType: PaymentType) { return paymentType.id; } method on the SavingsAccountTransactionsComponent (or the component class that defines paymentTypeOptions) so the loop uses the paymentType.id stable identifier instead of the object reference; ensure the method signature and PaymentType typing match your codebase.src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts (1)
175-202: Remove duplicated submission logic betweenconfirmTransaction()and deprecatedsubmit().Both methods rebuild the same payload and call the same API. Extract a shared payload builder + submit helper to reduce divergence risk.
♻️ Suggested refactor sketch
+ private buildTransactionPayload() { + const formData = this.savingAccountTransactionForm.value; + const locale = this.settingsService.language.code; + const dateFormat = this.settingsService.dateFormat; + const prevTransactionDate: Date = formData.transactionDate; + if (formData.transactionDate instanceof Date) { + formData.transactionDate = this.dateUtils.formatDate(prevTransactionDate, dateFormat); + } + return { + ...formData, + dateFormat, + locale, + transactionAmount: formData.transactionAmount * 1 + }; + }Also applies to: 231-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts` around lines 175 - 202, The confirmTransaction() and deprecated submit() duplicate payload construction and API call; refactor by extracting a shared helper (e.g., buildTransactionPayload(form: FormGroup) and submitTransaction(payload)) that both confirmTransaction() and submit() call: move logic that reads this.savingAccountTransactionForm.value, formats transactionDate using this.dateUtils.formatDate and settingsService (locale/dateFormat), coerces transactionAmount to number, and merges dateFormat/locale into buildTransactionPayload, then have submitTransaction call this.savingsService.executeSavingsAccountTransactionsCommand(this.savingAccountId, this.transactionCommand, payload) and handle subscription side-effects (set this.isSubmitting, this.transactionResponse, stepper.next(), error handling) so the duplicated code in confirmTransaction() and submit() is removed and they delegate to the new helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts`:
- Around line 175-201: Add an early-return guard at the start of
confirmTransaction(): if (this.isSubmitting) return; to prevent re-entry on
rapid clicks, then keep setting this.isSubmitting = true before calling
executeSavingsAccountTransactionsCommand; also ensure isSubmitting is always
cleared by using RxJS finalize (or handling complete/error) on the observable
returned by executeSavingsAccountTransactionsCommand so this.isSubmitting =
false is executed on both success and error (and still call stepper.next() on
success as currently done).
In `@src/assets/translations/sw-SW.json`:
- Around line 1327-1331: The Swahili translation keys for the stepper
("Transaction Details", "Confirm Transaction Details", "Transaction Complete",
"Transaction Successful") currently live under labels.catalogs but the UI reads
labels.heading.*, so move those four keys into labels.heading with their
translations and delete the duplicate catalogs.Id entry (ensure only one
catalogs.Id remains) so the heading lookups resolve correctly.
---
Duplicate comments:
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html`:
- Line 95: The input using formControlName="checkNumber" currently uses
type="number" which can strip leading zeros; change the input to type="text" and
add a numeric input hint (e.g., inputmode="numeric" and/or pattern="[0-9]*") so
cheque identifiers are preserved as exact strings while still presenting a
numeric keyboard on mobile; update the template element with
formControlName="checkNumber" accordingly and ensure any validation/parsing
logic in the component that expects a number handles the value as a string.
---
Nitpick comments:
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html`:
- Around line 61-63: The template currently tracks options by object reference
("track paymentType"); change the ngFor to use a stable key by replacing the
track expression with "trackBy: trackByPaymentType" and add a
trackByPaymentType(index: number, paymentType: PaymentType) { return
paymentType.id; } method on the SavingsAccountTransactionsComponent (or the
component class that defines paymentTypeOptions) so the loop uses the
paymentType.id stable identifier instead of the object reference; ensure the
method signature and PaymentType typing match your codebase.
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts`:
- Around line 175-202: The confirmTransaction() and deprecated submit()
duplicate payload construction and API call; refactor by extracting a shared
helper (e.g., buildTransactionPayload(form: FormGroup) and
submitTransaction(payload)) that both confirmTransaction() and submit() call:
move logic that reads this.savingAccountTransactionForm.value, formats
transactionDate using this.dateUtils.formatDate and settingsService
(locale/dateFormat), coerces transactionAmount to number, and merges
dateFormat/locale into buildTransactionPayload, then have submitTransaction call
this.savingsService.executeSavingsAccountTransactionsCommand(this.savingAccountId,
this.transactionCommand, payload) and handle subscription side-effects (set
this.isSubmitting, this.transactionResponse, stepper.next(), error handling) so
the duplicated code in confirmTransaction() and submit() is removed and they
delegate to the new helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a13d1a0e-c72e-41af-96c1-fff304abcf09
📒 Files selected for processing (16)
src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.htmlsrc/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scsssrc/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (8)
- src/assets/translations/ko-KO.json
- src/assets/translations/en-US.json
- src/assets/translations/lv-LV.json
- src/assets/translations/it-IT.json
- src/assets/translations/cs-CS.json
- src/assets/translations/fr-FR.json
- src/assets/translations/pt-PT.json
- src/assets/translations/de-DE.json
|
@JaySoni1 for keeping the consistency, can it be vertical stepper instead of vertical? |
|
@IOhacker Ok Victor . I will update the PR with these changes and will update you |
|
Thank you |
IOhacker
left a comment
There was a problem hiding this comment.
It has to be vertical stepper
3094a27 to
9130c2f
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html (1)
93-96:⚠️ Potential issue | 🟠 MajorUse a text input for cheque numbers too.
type="number"will still strip leading zeros and coerce large cheque identifiers. The account number field was fixed already; this field has the same problem.💡 Suggested fix
- <input type="number" formControlName="checkNumber" matInput /> + <input type="text" inputmode="numeric" formControlName="checkNumber" matInput />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html` around lines 93 - 96, The cheque number input uses type="number" which strips leading zeros and can coerce large identifiers; update the <input> in savings-account-transactions.component.html that uses formControlName="checkNumber" (inside the mat-form-field) to use a text input instead (e.g., type="text" and optionally inputmode="numeric" or a pattern) so cheque identifiers preserve leading zeros and large values are not coerced.
🧹 Nitpick comments (3)
src/assets/translations/es-MX.json (1)
610-613: Reuse the existing
labels.buttons.Printalready exists on Line 523, so addingDoneinstead ofDONE).Suggested cleanup
"Retry": "Reintentar", - "PRINT": "Imprimir", - "DONE": "Hecho", + "Done": "Hecho", "Submitting": "Enviando"Update the component to keep using
buttons.Printfor the print action.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/es-MX.json` around lines 610 - 613, Replace the duplicated all-caps JSON keys by reusing the existing title-cased button keys: remove the "PRINT" entry and ensure the translation uses the existing "Print" key (labels.buttons.Print) for the print action; likewise change "DONE" to "Done" to match the title-case button convention used elsewhere and update any component lookups to reference labels.buttons.Print and labels.buttons.Done instead of the uppercase variants.src/assets/translations/en-US.json (1)
612-612: Reuse the existingLine 527 already defines
labels.buttons.Print. Keeping both♻️ Suggested cleanup
- "PRINT": "Print",Update the consuming component to use
labels.buttons.Print.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/assets/translations/en-US.json` at line 612, Remove the duplicate top-level "PRINT" key and update references to use the existing labels.buttons.Print key; specifically, delete the "PRINT" entry from the JSON and change any consuming component or new stepper UI code that currently reads "PRINT" to instead read labels.buttons.Print so all translations reuse the canonical labels.buttons.Print identifier.src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html (1)
61-64: Track the payment-type options byid.
track paymentTypefalls back to object identity. If the options array is refreshed, Angular will recreate everymat-optioneven thoughpaymentType.idis already the stable key you bind as the value.As per coding guidelines, "For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns."💡 Suggested tweak
- `@for` (paymentType of paymentTypeOptions; track paymentType) { + `@for` (paymentType of paymentTypeOptions; track paymentType.id) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html` around lines 61 - 64, The template uses "track paymentType" which falls back to object identity; replace it with a trackBy function (e.g., change "*ngFor='let paymentType of paymentTypeOptions; track paymentType'" to use "trackBy: trackByPaymentTypeId") and implement a corresponding method named trackByPaymentTypeId in the component class (savings-account-transactions.component) that accepts index and a paymentType item and returns paymentType.id (ensuring types align with your PaymentType interface) so Angular can preserve mat-option elements when the array is refreshed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html`:
- Around line 271-274: In the savings-account-transactions component template,
avoid rendering empty parentheses by conditionally appending the parenthesized
symbol only when currency?.displaySymbol is non-empty; update the span that
currently contains "{{ currency?.name }} ({{ currency?.displaySymbol }})" to
render either "{{ currency?.name }} ({{ currency?.displaySymbol }})" via an
*ngIf around the parenthesis or use a ternary/interpolation that yields "{{
currency?.name }}{{ currency?.displaySymbol ? ' (' + currency?.displaySymbol +
')' : '' }}", ensuring you reference currency?.displaySymbol in the conditional.
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scss`:
- Around line 32-34: The SCSS targets the horizontal stepper with deprecated
::ng-deep and dead selectors; replace occurrences of "::ng-deep
.mat-stepper-horizontal" (and the print-rule targeting the horizontal stepper at
the second block) with a selector that targets the rendered vertical stepper
(e.g., ".mat-stepper-vertical" or ":host .mat-stepper-vertical") and remove the
"::ng-deep" token; also update the print media rule that hides the chrome to
target ".mat-stepper-vertical" so the margin and print-hiding behavior apply to
the actual vertical stepper rendered by the template.
In `@src/assets/translations/fr-FR.json`:
- Around line 1333-1337: Update the French translations to match the locale's
glossary: change the "Id" value to "Identifiant" and replace the "Confirm
Transaction Details" value (currently an imperative/action) with a header-style
label such as "Confirmation des détails de la transaction" so it reads
consistently with "Transaction Details", "Transaction Complete", and
"Transaction Successful".
- Around line 610-613: The French strings for the CTA/progress keys are using
status nouns instead of action/progress wording; update the "DONE" key from
"Terminé" to an imperative/action like "Terminer" (for the success button) and
change "Submitting" from "Soumission" to a progress phrase such as "Envoi en
cours" or "En cours d'envoi" so the submit-in-progress state reads like an
active process rather than a static status; update the translations for the keys
"DONE" and "Submitting" accordingly in the JSON.
In `@src/assets/translations/lt-LT.json`:
- Around line 1330-1331: Update the translation values for the keys "Transaction
Complete" and "Transaction Successful" so they use sentence case in Lithuanian
(e.g., "Sandoris baigtas" and "Sandoris sėkmingas") instead of capitalizing the
mid-phrase adjectives; modify the values associated with those exact keys in the
lt-LT.json translations object to reflect the sentence-case strings.
In `@src/assets/translations/ne-NE.json`:
- Around line 1328-1332: Remove the duplicate "Id" key from the labels.catalogs
section in the ne-NE.json file (the later occurrence around the Transaction
labels block); keep the original "Id" definition already present earlier and
delete the repeated `"Id": "आईडी",` entry to avoid shadowing and JSON ambiguity.
In `@src/assets/translations/pt-PT.json`:
- Around line 1329-1333: The pt-PT locale currently maps
labels.catalogs.Withdrawal to "Cancelamento", which is incorrect for the new
receipt flow; update the translation entry for labels.catalogs.Withdrawal in
src/assets/translations/pt-PT.json to the correct Portuguese term (e.g.,
"Levantamento" or "Retirada") so withdrawal receipts display the proper label
instead of "Cancelamento".
---
Duplicate comments:
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html`:
- Around line 93-96: The cheque number input uses type="number" which strips
leading zeros and can coerce large identifiers; update the <input> in
savings-account-transactions.component.html that uses
formControlName="checkNumber" (inside the mat-form-field) to use a text input
instead (e.g., type="text" and optionally inputmode="numeric" or a pattern) so
cheque identifiers preserve leading zeros and large values are not coerced.
---
Nitpick comments:
In
`@src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.html`:
- Around line 61-64: The template uses "track paymentType" which falls back to
object identity; replace it with a trackBy function (e.g., change "*ngFor='let
paymentType of paymentTypeOptions; track paymentType'" to use "trackBy:
trackByPaymentTypeId") and implement a corresponding method named
trackByPaymentTypeId in the component class
(savings-account-transactions.component) that accepts index and a paymentType
item and returns paymentType.id (ensuring types align with your PaymentType
interface) so Angular can preserve mat-option elements when the array is
refreshed.
In `@src/assets/translations/en-US.json`:
- Line 612: Remove the duplicate top-level "PRINT" key and update references to
use the existing labels.buttons.Print key; specifically, delete the "PRINT"
entry from the JSON and change any consuming component or new stepper UI code
that currently reads "PRINT" to instead read labels.buttons.Print so all
translations reuse the canonical labels.buttons.Print identifier.
In `@src/assets/translations/es-MX.json`:
- Around line 610-613: Replace the duplicated all-caps JSON keys by reusing the
existing title-cased button keys: remove the "PRINT" entry and ensure the
translation uses the existing "Print" key (labels.buttons.Print) for the print
action; likewise change "DONE" to "Done" to match the title-case button
convention used elsewhere and update any component lookups to reference
labels.buttons.Print and labels.buttons.Done instead of the uppercase variants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3b217bc2-1c29-48a4-96fd-4a007447e776
📒 Files selected for processing (16)
src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.htmlsrc/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.scsssrc/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.tssrc/assets/translations/cs-CS.jsonsrc/assets/translations/de-DE.jsonsrc/assets/translations/en-US.jsonsrc/assets/translations/es-CL.jsonsrc/assets/translations/es-MX.jsonsrc/assets/translations/fr-FR.jsonsrc/assets/translations/it-IT.jsonsrc/assets/translations/ko-KO.jsonsrc/assets/translations/lt-LT.jsonsrc/assets/translations/lv-LV.jsonsrc/assets/translations/ne-NE.jsonsrc/assets/translations/pt-PT.jsonsrc/assets/translations/sw-SW.json
🚧 Files skipped from review as they are similar to previous changes (4)
- src/assets/translations/lv-LV.json
- src/app/savings/saving-account-actions/savings-account-transactions/savings-account-transactions.component.ts
- src/assets/translations/de-DE.json
- src/assets/translations/it-IT.json
9130c2f to
846d8ee
Compare
|
@IOhacker I have updated the PR as per the chnages you have requsted and also updated the screenshot in the PR descrption . Can you please review ? |
|
@IOhacker Thank You for the review |
Changes Made :-
-Implemented 3-step Material stepper for savings account withdrawal/deposit transactions with progressive flow: Transaction Details → Confirmation → Success screen with transaction summary and receipt printing .
-Added comprehensive multi-language support .
WEB-810
Before :-
After :-
Summary by CodeRabbit
New Features
Localization