WEB-637 :- Display "No Data Available" message instead of empty tables on Client General Tab#3070
WEB-637 :- Display "No Data Available" message instead of empty tables on Client General Tab#3070Omar-Nabil2 wants to merge 1 commit intoopenMF:devfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
General Tab template src/app/clients/clients-view/general-tab/general-tab.component.html |
Adds matColumnDef="no-data" footer cell definitions and tr mat-footer-row footer rows with colspan bound to each table's columns; footer rows conditionally render when the corresponding collections are empty. |
General Tab styles src/app/clients/clients-view/general-tab/general-tab.component.scss |
Adds .no-data-message styling (centered, padded, secondary text color, 14px font) and resets p { margin: 0; }. |
General Tab logic src/app/clients/clients-view/general-tab/general-tab.component.ts |
Initializes several account/collection properties as empty arrays, imports Angular Material footer types, replaces direct route-data access with optional chaining + nullish coalescing, and supplies safe defaults to computePerformanceHistory. |
Localization (multiple locales) src/assets/translations/.../*.json en-US.json, de-DE.json, fr-FR.json, es-CL.json, es-MX.json, it-IT.json, pt-PT.json, cs-CS.json, ko-KO.json, lt-LT.json, lv-LV.json, ne-NE.json, sw-SW.json |
Adds new messages keys for empty-state texts (e.g., No Upcoming Charges Available, No Active/Closed [Account Type] Available, No Collateral Data Available) across locales; minor JSON formatting adjustments (trailing commas). |
Sequence Diagram(s)
(omitted)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- WEB-521 fix(i18n): add missing translations and apply translate pipes across … #2934 — Overlapping i18n changes that extend/add "No ... Available" translation keys used by client tables.
Suggested reviewers
- IOhacker
- gkbishnoi07
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The PR title clearly and specifically describes the main change: displaying a 'No Data Available' message instead of empty tables on the Client General Tab, which is directly supported by file summaries showing no-data footer additions, CSS styling, and corresponding translations across multiple languages. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
🧪 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.
Pull request overview
Updates the Client “General” tab to show localized empty-state messages (“No … Available”) instead of rendering empty Angular Material tables.
Changes:
- Added Angular Material footer row/cell support to each relevant table and rendered a “no data” footer row when the data source is empty.
- Initialized client account/charge/collateral collections with empty arrays to avoid empty-table rendering issues.
- Added new i18n message keys for the new empty-state messages across multiple locales.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/clients/clients-view/general-tab/general-tab.component.ts | Adds Material footer directives to standalone imports and initializes table data arrays with fallbacks. |
| src/app/clients/clients-view/general-tab/general-tab.component.html | Adds per-table “no data” footer rows and translation keys for empty states. |
| src/app/clients/clients-view/general-tab/general-tab.component.scss | Styles the empty-state message container. |
| src/assets/translations/en-US.json | Adds message keys for new empty states (English). |
| src/assets/translations/de-DE.json | Adds message keys for new empty states (German). |
| src/assets/translations/cs-CS.json | Adds message keys for new empty states (Czech). |
| src/assets/translations/es-CL.json | Adds message keys for new empty states (Spanish - Chile). |
| src/assets/translations/es-MX.json | Adds message keys for new empty states (Spanish - Mexico). |
| src/assets/translations/fr-FR.json | Adds message keys for new empty states (French). |
| src/assets/translations/it-IT.json | Adds message keys for new empty states (Italian). |
| src/assets/translations/pt-PT.json | Adds message keys for new empty states (Portuguese - Portugal). |
| src/assets/translations/lt-LT.json | Adds message keys for new empty states (Lithuanian). |
| src/assets/translations/lv-LV.json | Adds message keys for new empty states (Latvian). |
| src/assets/translations/ne-NE.json | Adds message keys for new empty states (Nepali). |
| src/assets/translations/sw-SW.json | Adds message keys for new empty states (Swahili). |
| src/assets/translations/ko-KO.json | Adds message keys for new empty states (but currently contains incorrect language values). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/assets/translations/ko-KO.json
Outdated
| "No Upcoming Charges Available": "Hakuna malipo yanayokuja yanapatikana", | ||
| "No Active Loan Accounts Available": "Hakuna akaunti za mkopo zinazofanya kazi zinapatikana", | ||
| "No Closed Loan Accounts Available": "Hakuna akaunti za mkopo zilizofungwa zinapatikana", | ||
| "No Active Saving Accounts Available": "Hakuna akaunti za akiba zinazofanya kazi zinapatikana", | ||
| "No Closed Saving Accounts Available": "Hakuna akaunti za akiba zilizofungwa zinapatikana", | ||
| "No Active Fixed Deposit Accounts Available": "Hakuna akaunti za amana za kudumu zinazofanya kazi zinapatikana", | ||
| "No Closed Fixed Deposit Accounts Available": "Hakuna akaunti za amana za kudumu zilizofungwa zinapatikana", | ||
| "No Active Recurring Deposit Accounts Available": "Hakuna akaunti za amana za kurudiwa zinazofanya kazi zinapatikana", | ||
| "No Closed Recurring Deposit Accounts Available": "Hakuna akaunti za amana za kurudiwa zilizofungwa zinapatikana", | ||
| "No Active Share Accounts Available": "Hakuna akaunti za hisa zinazofanya kazi zinapatikana", | ||
| "No Closed Share Accounts Available": "Hakuna akaunti za hisa zilizofungwa zinapatikana", | ||
| "No Collateral Data Available": "Hakuna data ya dhamana inayopatikana" |
There was a problem hiding this comment.
The new Korean (ko-KO) translations for these messages appear to be Swahili (copy/paste from sw-SW). This will show the wrong language when the UI is set to Korean. Please replace these values with Korean translations (or temporarily fall back to English) to avoid shipping incorrect locale strings.
| "No Upcoming Charges Available": "Hakuna malipo yanayokuja yanapatikana", | |
| "No Active Loan Accounts Available": "Hakuna akaunti za mkopo zinazofanya kazi zinapatikana", | |
| "No Closed Loan Accounts Available": "Hakuna akaunti za mkopo zilizofungwa zinapatikana", | |
| "No Active Saving Accounts Available": "Hakuna akaunti za akiba zinazofanya kazi zinapatikana", | |
| "No Closed Saving Accounts Available": "Hakuna akaunti za akiba zilizofungwa zinapatikana", | |
| "No Active Fixed Deposit Accounts Available": "Hakuna akaunti za amana za kudumu zinazofanya kazi zinapatikana", | |
| "No Closed Fixed Deposit Accounts Available": "Hakuna akaunti za amana za kudumu zilizofungwa zinapatikana", | |
| "No Active Recurring Deposit Accounts Available": "Hakuna akaunti za amana za kurudiwa zinazofanya kazi zinapatikana", | |
| "No Closed Recurring Deposit Accounts Available": "Hakuna akaunti za amana za kurudiwa zilizofungwa zinapatikana", | |
| "No Active Share Accounts Available": "Hakuna akaunti za hisa zinazofanya kazi zinapatikana", | |
| "No Closed Share Accounts Available": "Hakuna akaunti za hisa zilizofungwa zinapatikana", | |
| "No Collateral Data Available": "Hakuna data ya dhamana inayopatikana" | |
| "No Upcoming Charges Available": "예정된 청구가 없습니다", | |
| "No Active Loan Accounts Available": "활성 대출 계정이 없습니다", | |
| "No Closed Loan Accounts Available": "해지된 대출 계정이 없습니다", | |
| "No Active Saving Accounts Available": "활성 저축 계정이 없습니다", | |
| "No Closed Saving Accounts Available": "해지된 저축 계정이 없습니다", | |
| "No Active Fixed Deposit Accounts Available": "활성 정기 예금 계정이 없습니다", | |
| "No Closed Fixed Deposit Accounts Available": "해지된 정기 예금 계정이 없습니다", | |
| "No Active Recurring Deposit Accounts Available": "활성 적립식 예금 계정이 없습니다", | |
| "No Closed Recurring Deposit Accounts Available": "해지된 적립식 예금 계정이 없습니다", | |
| "No Active Share Accounts Available": "활성 주식 계정이 없습니다", | |
| "No Closed Share Accounts Available": "해지된 주식 계정이 없습니다", | |
| "No Collateral Data Available": "담보 데이터가 없습니다" |
| [routerLink]="['../', 'loans-accounts', row.id, 'general']" | ||
| class="select-row" | ||
| ></tr> | ||
| <tr mat-footer-row *matFooterRowDef="(loanAccounts | accountsFilter: 'loan' : 'open' : null)?.length === 0 ? ['no-data'] : []"></tr> |
There was a problem hiding this comment.
The footer-row empty-state check uses accountsFilter: 'loan' : 'open' : null, but the table dataSource uses loanAccounts | accountsFilter: 'loan'. Since the pipe only treats status === 'closed' specially, passing 'open' is currently a no-op and makes the condition diverge from the actual dataSource. Consider reusing the same pipe invocation as the dataSource (or omit the unused args) so the empty check can’t drift if the pipe logic changes.
| <tr mat-footer-row *matFooterRowDef="(loanAccounts | accountsFilter: 'loan' : 'open' : null)?.length === 0 ? ['no-data'] : []"></tr> | |
| <tr mat-footer-row *matFooterRowDef="(loanAccounts | accountsFilter: 'loan')?.length === 0 ? ['no-data'] : []"></tr> |
| *matRowDef="let row; columns: openSharesColumns" | ||
| [routerLink]="['../', 'shares-accounts', row.id, 'general']" | ||
| ></tr> | ||
| <tr mat-footer-row *matFooterRowDef="(shareAccounts | accountsFilter: 'share' : 'open' : null)?.length === 0 ? ['no-data'] : []"></tr> |
There was a problem hiding this comment.
Similar to the open-loans table: this table’s dataSource is shareAccounts | accountsFilter: 'share', but the empty-state condition calls accountsFilter: 'share' : 'open' : null. Because the pipe doesn’t interpret 'open' explicitly, this is misleading and can become incorrect if the pipe implementation is later extended. Reuse the same pipe invocation as the dataSource (or drop the unused args) for consistency.
| <tr mat-footer-row *matFooterRowDef="(shareAccounts | accountsFilter: 'share' : 'open' : null)?.length === 0 ? ['no-data'] : []"></tr> | |
| <tr mat-footer-row *matFooterRowDef="(shareAccounts | accountsFilter: 'share')?.length === 0 ? ['no-data'] : []"></tr> |
| this.savingAccounts = data.clientAccountsData.savingsAccounts || []; | ||
| this.loanAccounts = data.clientAccountsData.loanAccounts || []; | ||
| this.shareAccounts = data.clientAccountsData.shareAccounts || []; | ||
| this.upcomingCharges = data.clientChargesData.pageItems || []; | ||
| this.collaterals = data.clientCollateralData || []; |
There was a problem hiding this comment.
These assignments still assume data.clientAccountsData / data.clientChargesData are always non-null, but only the nested arrays are guarded. If a resolver ever returns null/undefined, data.clientChargesData.pageItems will throw before the || [] fallback runs. Consider using optional chaining + nullish coalescing (e.g., data.clientChargesData?.pageItems ?? [], data.clientAccountsData?.loanAccounts ?? []) to make the guards effective.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
272-283:⚠️ Potential issue | 🟡 MinorUse nullish coalescing for safer property access on resolved data.
While route resolvers (
ClientAccountsResolver,ClientChargesResolver,ClientCollateralResolver) guarantee the data arrives before the component is rendered, using nullish coalescing (??) instead of logical OR (||) provides better defense against unexpected null/undefined nested properties and improves code clarity. Consider assigning resolved data to local variables using the nullish coalescing operator (lines 274-279) to safeguard against resolver contract changes.
🤖 Fix all issues with AI agents
In `@src/assets/translations/es-CL.json`:
- Around line 3544-3546: The translation for the key "No Collateral Data
Available" uses "colateral" which is inconsistent with the project's ES-CL
vocabulary; update the value for "No Collateral Data Available" from "No hay
datos de colateral disponibles" to use the established term "garantía" or
"garantías" (e.g., "No hay datos de garantía disponibles") so it matches other
ES-CL strings using "Garantía/garantías".
In `@src/assets/translations/es-MX.json`:
- Around line 3537-3549: Update the translation value for the JSON key "No
Collateral Data Available" to use the existing app terminology
"Garantía/Garantías" for consistency; specifically replace "No hay datos de
colateral disponibles" with "No hay datos de garantías disponibles" (or the
singular form used elsewhere) so it matches other collateral translations in the
file.
In `@src/assets/translations/it-IT.json`:
- Around line 3534-3543: Update the Italian translations for the listed no‑data
messages to include "di" for consistency with existing phrasing (e.g., "Conto di
risparmio"); specifically revise the values for the keys "No Active Loan
Accounts Available", "No Closed Loan Accounts Available", "No Active Saving
Accounts Available", "No Closed Saving Accounts Available", "No Active Fixed
Deposit Accounts Available", "No Closed Fixed Deposit Accounts Available", "No
Active Recurring Deposit Accounts Available", "No Closed Recurring Deposit
Accounts Available", "No Active Share Accounts Available", and "No Closed Share
Accounts Available" to use phrases like "Nessun conto di risparmio attivo
disponibile" (and equivalent for other account types), matching existing
capitalization/word order conventions.
In `@src/assets/translations/ko-KO.json`:
- Around line 3534-3545: Replace the incorrect Swahili values with Korean
translations for the listed keys (e.g. "No Upcoming Charges Available", "No
Active Loan Accounts Available", "No Closed Loan Accounts Available", "No Active
Saving Accounts Available", "No Closed Saving Accounts Available", "No Active
Fixed Deposit Accounts Available", "No Closed Fixed Deposit Accounts Available",
"No Active Recurring Deposit Accounts Available", "No Closed Recurring Deposit
Accounts Available", "No Active Share Accounts Available", "No Closed Share
Accounts Available", "No Collateral Data Available"); update each value to
appropriate Korean strings (for example "예정된 요금이 없습니다" for "No Upcoming Charges
Available", "활성 대출 계좌가 없습니다" for "No Active Loan Accounts Available") following
the existing translation style (short, plain phrases like "데이터를 찾을 수 없음") and
ensure all JSON values remain quoted and properly comma-separated.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/assets/translations/lt-LT.json`:
- Around line 3537-3538: Update the two Lithuanian translations for the keys "No
Active Saving Accounts Available" and "No Closed Saving Accounts Available" to
use the same terminology as the rest of the file: replace "Nėra aktyvių taupymo
sąskaitų" and "Nėra uždarytų taupymo sąskaitų" with variants using "Taupomosios
sąskaitos" (e.g., "Nėra aktyvių taupomosios sąskaitos" or the grammatically
correct plural form consistent with other entries) so wording is consistent with
the existing "Savings Accounts" translations elsewhere in the JSON.
In `@src/assets/translations/ne-NE.json`:
- Around line 3533-3534: Replace the inconsistent Nepali word "रिण" with the
canonical translation "ऋण" for the two keys "No Active Loan Accounts Available"
and "No Closed Loan Accounts Available" so they match the existing translations
like "Loans" and "Loan Account"; update the values for those JSON keys to use
"कुनै सक्रिय ऋण खाताहरू उपलब्ध छैनन्" and "कुनै बन्द ऋण खाताहरू उपलब्ध छैनन्"
respectively.
|
Hi @IOhacker , I have created a new PR with a clean code. Could you please review this PR ? |
IOhacker
left a comment
IOhacker
left a comment
There was a problem hiding this comment.
Please fix the comments that coderabbit sent.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/assets/translations/cs-CS.json`:
- Around line 3537-3538: The two translation entries "No Active Fixed Deposit
Accounts Available" and "No Closed Fixed Deposit Accounts Available" use
"vkladové účty" which is inconsistent with the established term "Účty s pevným
vkladem"; update their Czech values to "Žádné aktivní účty s pevným vkladem k
dispozici" and "Žádné uzavřené účty s pevným vkladem k dispozici" respectively
so they match the existing "Fixed Deposit Accounts" wording.
In `@src/assets/translations/ne-NE.json`:
- Around line 3537-3543: The Nepali translations for keys "No Active Fixed
Deposit Accounts Available", "No Closed Fixed Deposit Accounts Available", "No
Active Recurring Deposit Accounts Available", "No Closed Recurring Deposit
Accounts Available", and "No Collateral Data Available" use alternative terms;
update their values to match the existing glossary terminology used elsewhere
(replace "कुनै सक्रिय निश्चित निक्षेप" / "कुनै बन्द निश्चित निक्षेप" with
"फिक्स्ड डिपोजिट", replace "आवर्ती निक्षेप" with "आवर्ती जम्मा", and replace
"धरौटी डाटा" with "संपार्श्विक") so the UI is consistent with labels elsewhere
(edit the string values for those exact keys in ne-NE.json).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/assets/translations/en-US.json`:
- Line 3643: The JSON entry with key "No Upcoming Charges Available" has an
extra leading space causing inconsistent indentation; open the translations JSON
and locate the string "No Upcoming Charges Available" and remove the extra
leading space before the key so it matches the surrounding entries' indentation
and formatting, ensuring the line aligns with other key/value pairs in that
section.
🧹 Nitpick comments (2)
src/assets/translations/cs-CS.json (1)
3541-3542: Minor terminology inconsistency for Share Accounts.The new translations use "účty podílů" (Lines 3541-3542), while elsewhere in the file (Line 1204) "Shares Accounts" is translated as "Účty akcií". Both "podíly" and "akcie" are valid Czech words for shares, but for consistency within the application, consider aligning with the existing terminology.
Optional: Align with existing terminology
- "No Active Share Accounts Available": "Žádné aktivní účty podílů k dispozici", - "No Closed Share Accounts Available": "Žádné uzavřené účty podílů k dispozici", + "No Active Share Accounts Available": "Žádné aktivní účty akcií k dispozici", + "No Closed Share Accounts Available": "Žádné uzavřené účty akcií k dispozici",src/app/clients/clients-view/general-tab/general-tab.component.html (1)
115-125: Consider using@ifto avoid creating unnecessary empty footer row DOM elements.The current ternary operator pattern
*matFooterRowDef="upcomingCharges?.length === 0 ? ['no-data'] : []"still instantiates a footer<tr>element even when the columns array is empty. While an empty footer row with zero cells has no visible height, the DOM element is still created unnecessarily. Use the@ifstructural directive instead to prevent the element from being created when not needed.Suggested pattern
- <tr mat-footer-row *matFooterRowDef="upcomingCharges?.length === 0 ? ['no-data'] : []"></tr> + `@if` (upcomingCharges?.length === 0) { + <tr mat-footer-row *matFooterRowDef="['no-data']"></tr> + }Apply to all footer rows: 268–285, 355–370, 474–488, 513–527, 607–621, 647–661, 745–760, 785–800, 882–896, 927–941, 998–1012.
|
@IOhacker Done |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
273-286:⚠️ Potential issue | 🟠 MajorGuard
computePerformanceHistoryagainst missing account data.
With the new optional chaining,clientAccountsDatacan beundefined, butcomputePerformanceHistorydereferences it. This can throw at runtime. Consider defaulting to an empty structure before calling.✅ Suggested fix
- this.clientAccountData = data.clientAccountsData; - this.savingAccounts = data.clientAccountsData?.savingsAccounts ?? []; - this.loanAccounts = data.clientAccountsData?.loanAccounts ?? []; - this.shareAccounts = data.clientAccountsData?.shareAccounts ?? []; + const accountsData = data.clientAccountsData ?? { loanAccounts: [], savingsAccounts: [], shareAccounts: [] }; + this.clientAccountData = accountsData; + this.savingAccounts = accountsData.savingsAccounts ?? []; + this.loanAccounts = accountsData.loanAccounts ?? []; + this.shareAccounts = accountsData.shareAccounts ?? []; this.upcomingCharges = data.clientChargesData?.pageItems ?? []; this.collaterals = data.clientCollateralData ?? []; @@ - this.computePerformanceHistory(data.clientAccountsData); + this.computePerformanceHistory(accountsData);
🤖 Fix all issues with AI agents
In `@src/assets/translations/lv-LV.json`:
- Line 3545: The Latvian translation for the string key "No Collateral Data
Available" uses "ķīlas" and should be changed to the consistent terminology
"Nodrošinājuma dati"; update the value for the "No Collateral Data Available"
key from "Nav pieejami ķīlas dati" to use "Nodrošinājuma" (e.g., "Nav pieejami
Nodrošinājuma dati") to match other translations that use "Nodrošinājuma dati".
- Line 3534: The translation for the key "No Upcoming Charges Available" uses
"maksājumu" (payments) instead of the project-preferred term "maksa(s)"; update
the value for "No Upcoming Charges Available" to use the consistent terminology
(e.g., replace "Nav gaidāmo maksājumu" with "Nav gaidāmo maksu" or another form
matching "Maksas/Gaidāmās maksas") so it aligns with existing translations.
🧹 Nitpick comments (1)
src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
226-234: Type these collections and verifytrackByusage in the template.
The newany[]fields drop type safety and they likely back*ngFortable rows; please use concrete account/collateral interfaces and ensure the template usestrackByto avoid unnecessary DOM churn.#!/bin/bash # Check for existing account/collateral model interfaces. rg -n --type=ts "interface .*Account|type .*Account|interface .*Collateral|type .*Collateral" src/app # Verify *ngFor usage and presence of trackBy in the General Tab template. rg -n --fixed-strings "*ngFor" src/app/clients/clients-view/general-tab/general-tab.component.html rg -n "trackBy" src/app/clients/clients-view/general-tab/general-tab.component.htmlAs per coding guidelines: For Angular code: verify component separation, trackBy on *ngFor, strict type safety, and clean observable patterns.
Also applies to: 250-250
|
@IOhacker Should i apply Prettier ? |
alberto-art3ch
left a comment
alberto-art3ch
left a comment
There was a problem hiding this comment.
Please fix the GA issue
|
@alberto-art3ch I have fixed it. Could you check it now ? |
|
@Omar-Nabil2 please check the comments provided by CodeRabbit, this is because there still suggestion in the translation files |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
275-286:⚠️ Potential issue | 🟠 MajorGuard against missing route data before computing performance history.
clientAccountsDatais now treated as optional; callingcomputePerformanceHistorywithundefinedwill throw.🛡️ Suggested fix
- this.computePerformanceHistory(data.clientAccountsData); + this.computePerformanceHistory( + data.clientAccountsData ?? { loanAccounts: [], savingsAccounts: [] } + );
🧹 Nitpick comments (2)
src/app/clients/clients-view/general-tab/general-tab.component.scss (1)
60-69: Consider using theme-aware color for dark mode compatibility.The hardcoded color
#666works well on light backgrounds but may have poor contrast in dark themes. Consider using a CSS variable or Angular Material's theming system for better adaptability.💡 Suggested improvement for theme compatibility
.no-data-message { text-align: center; padding: 40px 20px; - color: `#666`; + color: var(--mdc-theme-text-secondary-on-background, `#666`); font-size: 14px; p { margin: 0; } }Alternatively, if this app uses Angular Material theming, you could reference the theme palette to ensure the color adapts automatically to light/dark modes.
src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
228-234: Change array types fromanytoany[]for stricter type safety.These properties are assigned arrays and used as mat-table datasources, so enforce the array type to prevent accidental non-array assignments.
♻️ Suggested typing improvement
- loanAccounts: any = []; + loanAccounts: any[] = []; - savingAccounts: any = []; + savingAccounts: any[] = []; - shareAccounts: any = []; + shareAccounts: any[] = []; - upcomingCharges: any = []; + upcomingCharges: any[] = []; - collaterals: any = []; + collaterals: any[] = [];Also applies to: line 250
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@src/app/clients/clients-view/general-tab/general-tab.component.html`:
- Around line 373-376: Remove the unnecessary third argument ": null" passed to
the accountsFilter pipe in the loan and share closed-account footer row
definitions; update the mat-footer-row *matFooterRowDef expressions that
currently use "(loanAccounts | accountsFilter: 'loan' : 'closed' : null)" and
the analogous shareAccounts expression to use only two arguments "(... |
accountsFilter: 'loan' : 'closed')" (and similarly for 'share'), since the
checkSavings parameter is only relevant for savings and the extra null is
redundant and inconsistent with other filters.
In `@src/app/clients/clients-view/general-tab/general-tab.component.scss`:
- Line 63: The CSS fallback value for the color variable in
general-tab.component.scss is quoted, making it a string literal; remove the
quotes around the fallback so the color property uses a valid hex color as the
fallback for var(--mdc-theme-text-secondary-on-background) (update the color
declaration in general-tab.component.scss to use an unquoted hex fallback like
`#666`).
In `@src/assets/translations/de-DE.json`:
- Around line 3541-3543: Replace the German translation for the key "No
Collateral Data Available" to use clearer wording: change the value from "Keine
Sicherheitendaten verfügbar" to "Keine Sicherheiten verfügbar" (or similar using
"Sicherheiten") so it reads naturally and avoids the ambiguous
"Sicherheitendaten"; update the JSON entry for the "No Collateral Data
Available" key accordingly.
In `@src/assets/translations/sw-SW.json`:
- Around line 3535-3538: The Swahili translations for the four keys ("No Active
Fixed Deposit Accounts Available", "No Closed Fixed Deposit Accounts Available",
"No Active Recurring Deposit Accounts Available", "No Closed Recurring Deposit
Accounts Available") use "amana za kudumu/kurudiwa" and should be changed to
match existing terminology: replace "amana za kudumu" with "amana
zisizohamishika" for Fixed Deposit messages and replace "amana za kurudiwa" with
"amana za mara kwa mara" for Recurring Deposit messages so they align with other
strings in the file.
🧹 Nitpick comments (2)
src/app/clients/clients-view/general-tab/general-tab.component.ts (2)
271-288: AddtakeUntilto the route data subscription to prevent memory leaks.The component has a
destroy$Subject andngOnDestroylifecycle hook, but theroute.datasubscription in the constructor is not piped throughtakeUntil(this.destroy$). WhileActivatedRouteis typically tied to the component lifecycle, adding explicit cleanup ensures consistency and prevents potential memory leaks. As per coding guidelines for Angular: clean observable patterns.♻️ Proposed fix
constructor() { - this.route.data.subscribe( + this.route.data.pipe(takeUntil(this.destroy$)).subscribe( (data: { clientAccountsData: any; clientChargesData: any; clientSummary: any; clientCollateralData: any }) => {
382-386: Consider using safe assignment pattern ingetChargeData.For consistency with the constructor's defensive coding approach, consider using the nullish coalescing pattern here as well.
♻️ Proposed fix
getChargeData(clientId: string) { this.clientService.getClientChargesData(clientId).subscribe((data: any) => { - this.upcomingCharges = data.pageItems; + this.upcomingCharges = data?.pageItems ?? []; }); }
src/app/clients/clients-view/general-tab/general-tab.component.scss
Outdated
Show resolved
Hide resolved
Omar-Nabil2
force-pushed
the
msg
branch
2 times, most recently
from
42bb35d to
b3adbf7
Compare
|
@IOhacker Could you please check now ? |
|
Now it LGTM. As an advice/suggestion, about the Github's PR conversation tab, make sure to review the suggestions and if they are fixable do it if not, If you are taking another approach or is not relevan mark the conversation as resolved. 
|
|
@IOhacker I have handled the ": null" case, but removing ": 'open'" would result in incorrect logic. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 14 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <tr | ||
| mat-footer-row | ||
| *matFooterRowDef=" | ||
| (savingAccounts | accountsFilter: 'saving' : 'closed' : 'isSavings')?.length === 0 ? ['no-data'] : [] | ||
| " | ||
| ></tr> |
There was a problem hiding this comment.
The footer row condition re-runs the same accountsFilter pipe that is already used for the dataSource, and uses [] to hide the row (still renders an empty <tr>). Prefer computing the filtered list once and conditionally rendering the footer row only when empty.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/assets/translations/fr-FR.json`:
- Line 3532: The French translation for the key "No Data Found" is inconsistent
with other keys; update the value for the "No Data Found" JSON key in
src/assets/translations/fr-FR.json to "Aucune donnée disponible" so it matches
the phrasing used by "No Data Available" and other existing translations.
🧹 Nitpick comments (2)
src/assets/translations/cs-CS.json (1)
3531-3543: Align Czech terminology for “Collateral” and “Share Accounts” with existing entries.The new strings use “kolaterálu” and “účty podílů”, which diverge from terms already used in this file (e.g., “Údaje o zajištění” and “Účty akcií”). Consider normalizing to the established terminology to keep UI wording consistent.
🔧 Suggested wording alignment
- "No Active Share Accounts Available": "Žádné aktivní účty podílů k dispozici", - "No Closed Share Accounts Available": "Žádné uzavřené účty podílů k dispozici", - "No Collateral Data Available": "Žádná data o kolaterálu k dispozici" + "No Active Share Accounts Available": "Žádné aktivní akciové účty k dispozici", + "No Closed Share Accounts Available": "Žádné uzavřené akciové účty k dispozici", + "No Collateral Data Available": "Žádná data o zajištění k dispozici"src/app/clients/clients-view/general-tab/general-tab.component.ts (1)
228-234: Consider adding type interfaces for account data.The properties are typed as
any[]which is an improvement overany, but defining proper interfaces (e.g.,LoanAccount,SavingAccount,ShareAccount) would provide better type safety, IDE autocompletion, and catch potential runtime errors at compile time.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/assets/translations/pt-PT.json`:
- Around line 3533-3537: The PT‑PT strings use inconsistent terminology: replace
"Nenhum encargo futuro disponível" with "Nenhuma cobrança futura disponível"
(for key "No Upcoming Charges Available") and update saving account phrases to
include "de" as "Nenhuma conta de poupança ativa disponível" and "Nenhuma conta
de poupança encerrada disponível" (for keys "No Active Saving Accounts
Available" and "No Closed Saving Accounts Available") so they match existing
"cobrança" and "conta de poupança" usage; keep loan account strings unchanged.
🧹 Nitpick comments (1)
src/assets/translations/ko-KO.json (1)
3535-3544: Keep Korean account terminology consistent (“계좌” vs “계정”).These new messages use “계정”, while many existing account labels in this locale use “계좌”. Consider aligning for consistency (and standardizing “정기예금” spacing).
💡 Suggested wording consistency updates
- "No Active Loan Accounts Available": "활성 대출 계정이 없습니다", - "No Closed Loan Accounts Available": "해지된 대출 계정이 없습니다", - "No Active Saving Accounts Available": "활성 저축 계정이 없습니다", - "No Closed Saving Accounts Available": "해지된 저축 계정이 없습니다", - "No Active Fixed Deposit Accounts Available": "활성 정기 예금 계정이 없습니다", - "No Closed Fixed Deposit Accounts Available": "해지된 정기 예금 계정이 없습니다", - "No Active Recurring Deposit Accounts Available": "활성 적립식 예금 계정이 없습니다", - "No Closed Recurring Deposit Accounts Available": "해지된 적립식 예금 계정이 없습니다", - "No Active Share Accounts Available": "활성 주식 계정이 없습니다", - "No Closed Share Accounts Available": "해지된 주식 계정이 없습니다", + "No Active Loan Accounts Available": "활성 대출 계좌가 없습니다", + "No Closed Loan Accounts Available": "해지된 대출 계좌가 없습니다", + "No Active Saving Accounts Available": "활성 저축 계좌가 없습니다", + "No Closed Saving Accounts Available": "해지된 저축 계좌가 없습니다", + "No Active Fixed Deposit Accounts Available": "활성 정기예금 계좌가 없습니다", + "No Closed Fixed Deposit Accounts Available": "해지된 정기예금 계좌가 없습니다", + "No Active Recurring Deposit Accounts Available": "활성 적립식 예금 계좌가 없습니다", + "No Closed Recurring Deposit Accounts Available": "해지된 적립식 예금 계좌가 없습니다", + "No Active Share Accounts Available": "활성 주식 계좌가 없습니다", + "No Closed Share Accounts Available": "해지된 주식 계좌가 없습니다",
|
@IOhacker I think everything is okay right now. |
|
@IOhacker , could you please review this PR? Let me know if you’d like any changes. Also, if you feel the approach isn't the best fit for our current business requirements, I'm open to discussing alternatives! |
3 participants
Description
Display "No Data Available" message instead of empty tables on Client General Tab.
Screenshots, if any
after
after.mp4
before
before.mp4
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.Summary by CodeRabbit
New Features
Style
Localization
Bug Fixes