-
Notifications
You must be signed in to change notification settings - Fork 15
feat: update Fyle brand name to Sage Expense Management #3977
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: Fyle-86d0k46y9
Are you sure you want to change the base?
Conversation
| [header]="'You have not been assigned with any mileage rates.'" | ||
| [image]="'../../../assets/svg/alert-circled.svg'" | ||
| [message]="'Please contact your Fyle-Administrator to resolve this issue.'" | ||
| [message]="'Please contact your administrator to resolve this issue.'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought this is apt.
WalkthroughRebrands user-facing copy from "Fyle" to "Sage Expense Management" across templates, specs, i18n, mock data, and app title. No behavioral, logic, or control-flow changes; tests updated to match new strings. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)src/assets/i18n/*.json📄 CodeRabbit inference engine (.cursor/rules/atomics-i18n-key-pattern.mdc)
Files:
🧠 Learnings (1)📚 Learning: 2025-07-28T05:57:50.701ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
| @@ -1,8 +1,8 @@ | |||
| <!doctype html> | |||
| <html lang="en"> | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the above logo added for now, will need design help on its size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
src/assets/svg/sage-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (20)
src/app/auth/pending-verification/pending-verification.page.html(1 hunks)src/app/auth/sign-in/error/error.component.spec.ts(2 hunks)src/app/core/services/back-button.service.spec.ts(1 hunks)src/app/fyle/add-edit-expense/add-edit-expense.page.html(1 hunks)src/app/fyle/add-edit-mileage/add-edit-mileage.page.html(1 hunks)src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.html(2 hunks)src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.html(1 hunks)src/app/fyle/my-profile/info-card/info-card.component.spec.ts(1 hunks)src/app/fyle/my-profile/my-profile.page.spec.ts(1 hunks)src/app/fyle/my-profile/my-profile.page.ts(2 hunks)src/app/fyle/personal-cards/personal-cards.page.spec.ts(2 hunks)src/app/fyle/personal-cards/personal-cards.page.ts(2 hunks)src/app/post-verification/invited-user/invited-user.page.html(1 hunks)src/app/shared/components/capture-receipt/capture-receipt.component.spec.ts(3 hunks)src/app/shared/components/fy-header/fy-header.component.spec.ts(1 hunks)src/app/shared/components/fy-location/fy-location-modal/fy-location-modal.component.spec.ts(2 hunks)src/app/shared/components/fy-location/fy-location.component.spec.ts(1 hunks)src/app/shared/components/popup-with-bullets/popup-with-bullets.component.spec.ts(1 hunks)src/assets/i18n/en.json(10 hunks)src/index.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/assets/i18n/*.json
📄 CodeRabbit inference engine (.cursor/rules/atomics-i18n-key-pattern.mdc)
src/assets/i18n/*.json: Keep child keys alphabetically sorted within each group in the translation JSON to reduce merge conflicts
Add new i18n keys to every supported language file (e.g., en.json, es.json) in src/assets/i18n/
Files:
src/assets/i18n/en.json
🧠 Learnings (2)
📚 Learning: 2024-12-03T04:18:27.019Z
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3289
File: src/app/post-verification/invited-user/invited-user.page.html:11-11
Timestamp: 2024-12-03T04:18:27.019Z
Learning: In the `src/app/post-verification/invited-user/invited-user.page.html` file, for the "Join" page, displaying a loading template when `fg` (form group) is undefined is not necessary.
Applied to files:
src/app/post-verification/invited-user/invited-user.page.html
📚 Learning: 2024-10-23T11:45:16.974Z
Learnt from: bistaastha
PR: fylein/fyle-mobile-app#3240
File: src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.spec.ts:293-354
Timestamp: 2024-10-23T11:45:16.974Z
Learning: In 'src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.spec.ts', the flags 'isVisaRTFEnabled$', 'isMastercardRTFEnabled$', and 'isYodleeEnabled$' do not have rapidly changing values, so tests for rapid changes in these flags are unnecessary.
Applied to files:
src/app/fyle/personal-cards/personal-cards.page.spec.tssrc/app/fyle/my-profile/info-card/info-card.component.spec.ts
🪛 GitHub Check: Run linters
src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.html
[warning] 65-65:
Hard-coded string "Add corporate cards to sync your transactions on S" should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
src/app/auth/pending-verification/pending-verification.page.html
[warning] 17-17:
Hard-coded string "Enter your registered email address to receive a n" should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
src/index.html
[warning] 5-5:
Hard-coded string "Sage Expense Management mobile app" should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.html
[warning] 818-818:
Hard-coded string "Please contact your administrator to resolve this " should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
[warning] 829-829:
Hard-coded string "Please contact your administrator to resolve this " should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
src/app/fyle/add-edit-mileage/add-edit-mileage.page.html
[warning] 792-792:
Hard-coded string "Please contact your administrator to resolve this " should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (14)
src/app/post-verification/invited-user/invited-user.page.html (1)
11-11: Header rebrand looks solid. The new copy fits the Sage Expense Management branding perfectly and leaves the rest of the flow untouched. Style stays sharp, just like the Superstar likes it.src/app/shared/components/fy-location/fy-location-modal/fy-location-modal.component.spec.ts (1)
130-130: LGTM, test translations are superb-aah!The translation mocks are updated perfectly to reflect Sage Expense Management branding. Like how I adjust my style for every role, these tests now match the new brand identity flawlessly!
Also applies to: 888-888
src/app/shared/components/fy-header/fy-header.component.spec.ts (1)
40-40: Superstar branding update da!The translation mock is spot-on! From 'Fyle' to 'Sage Expense Management'—smooth like my entry in every movie, no drama!
src/app/fyle/personal-cards/personal-cards.page.ts (1)
451-451: Thalaivaa! Perfect execution of branding update!The loader message and success messages are now branded correctly with Sage Expense Management. Like how I deliver my punch dialogues—timing and consistency, both perfect!
Also applies to: 460-460
src/index.html (1)
5-5: Boss-level title change!The page title is now "Sage Expense Management mobile app"—very good! The linter is complaining about hard-coded strings, but for HTML
<title>tags, this is often acceptable since it's the entry point meta-data. However, if your project has a pattern for i18n in theindex.html, you should follow it da!Verify if your project uses translation keys for the
<title>tag inindex.html. If yes, then update accordingly. If not, this change is perfect as-is!src/app/shared/components/popup-with-bullets/popup-with-bullets.component.spec.ts (1)
80-80: Mass update in test data!The test data is now referencing Sage Expense Management—correct and consistent like my signature style! No confusion, clear message!
src/app/shared/components/fy-location/fy-location.component.spec.ts (1)
64-64: Semma update da!Translation mock updated perfectly to Sage Expense Management! Like my dialogue delivery—powerful and consistent every time!
src/app/core/services/back-button.service.spec.ts (1)
18-18: Perfect-ah! Test mock updated correctly.The translation mock now reflects "Sage Expense Management" branding. Superstar move updating the test expectations to match the new brand name!
src/app/fyle/my-profile/info-card/info-card.component.spec.ts (1)
24-24: Semma! Test data updated correctly.Component test now uses "Sage Expense Management" in the receipt forwarding message. Clean and consistent with the rebranding effort!
src/app/auth/sign-in/error/error.component.spec.ts (1)
38-38: Mass-ah! Test translations updated correctly.Both the mock translation and test expectation now correctly reflect "Sage Expense Management" branding for the restricted access error message.
src/app/fyle/my-profile/my-profile.page.spec.ts (1)
798-798: Superstar style test update!Test expectation correctly updated to verify "Sage Expense Management" appears in the mobile receipts message. Perfectly aligned with the component changes!
src/app/shared/components/capture-receipt/capture-receipt.component.spec.ts (1)
239-241: Thalaiva-level consistency in test mocks!Permission message translations updated perfectly - both camera and gallery permission messages now reference "Sage Expense Management". Test setup is rock solid!
src/app/fyle/personal-cards/personal-cards.page.spec.ts (1)
287-287: LGTM! Style is superb, macha!The test expectations have been updated perfectly to match the new Sage Expense Management branding. Just like how I change my style effortlessly, you've changed these strings smoothly!
Also applies to: 318-318
src/assets/i18n/en.json (1)
42-42: Branding update rock-solid – only en.json exists
Even if I search every corner, no other locale files live in src/assets/i18n/.
| <div class="pending-verification__page-header">The invitation has expired</div> | ||
| <div class="pending-verification__sub-header"> | ||
| Enter your registered email address to receive a new invitation and set up your account on Fyle. | ||
| Enter your registered email address to receive a new invitation and set up your account on Sage Expense Management. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enna da rascal-aa, use translation keys!
The branding update to Sage Expense Management is excellent, but this hard-coded string should be moved to a translation key, machaan! Just like how I never compromise on my dialogue delivery, don't compromise on i18n best practices!
As per static analysis hints, this message should use a translation key:
-Enter your registered email address to receive a new invitation and set up your account on Sage Expense Management.
+{{ 'pendingVerification.invitationMessage' | transloco }}Then add the corresponding entry in your en.json translation file.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Enter your registered email address to receive a new invitation and set up your account on Sage Expense Management. | |
| {{ 'pendingVerification.invitationMessage' | transloco }} |
🧰 Tools
🪛 GitHub Check: Run linters
[warning] 17-17:
Hard-coded string "Enter your registered email address to receive a n" should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
🤖 Prompt for AI Agents
In src/app/auth/pending-verification/pending-verification.page.html around line
17, the hard-coded copy "Enter your registered email address to receive a new
invitation and set up your account on Sage Expense Management." should be
replaced with a translation key (e.g. pendingVerification.enterEmail) used via
the app's i18n mechanism (translate pipe or i18n directive consistent with the
project). Then add the matching entry in the en.json translations file with the
same text as the value. Ensure the key name follows existing naming conventions
and update any unit/visual tests if they assert on the literal string.
| 'Sage Expense Management will automatically merge this expense with the corresponding corporate card transaction when it flows into Sage Expense Management.' | ||
| " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Boss, this hardcoded message needs translation key treatment!
You've updated the branding correctly, but this alert message should be using a translation key for proper i18n support, just like other messages in this same file (see how other alerts use 'addEditExpense.xxx' | transloco).
Apply this diff:
- 'Sage Expense Management will automatically merge this expense with the corresponding corporate card transaction when it flows into Sage Expense Management.'
+ {{ 'addEditExpense.cccAutoMergeInfo' | transloco }}Then add to src/assets/i18n/en.json:
"addEditExpense": {
"cccAutoMergeInfo": "Sage Expense Management will automatically merge this expense with the corresponding corporate card transaction when it flows into Sage Expense Management."
}🤖 Prompt for AI Agents
In src/app/fyle/add-edit-expense/add-edit-expense.page.html around lines
542-543, replace the hardcoded alert message with the transloco translation key
'addEditExpense.cccAutoMergeInfo' (use the same `'addEditExpense.xxx' |
transloco` pattern used elsewhere in this file) and then add the corresponding
entry under "addEditExpense"."cccAutoMergeInfo" in src/assets/i18n/en.json with
the provided English string so the message is localized.
| [header]="'You have not been assigned with any mileage rates.'" | ||
| [image]="'../../../assets/svg/alert-circled.svg'" | ||
| [message]="'Please contact your Fyle-Administrator to resolve this issue.'" | ||
| [message]="'Please contact your administrator to resolve this issue.'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small adjustment needed, thalaiva!
The static analysis tool is absolutely right, boss! This hard-coded string should be using a translation key for proper internationalization. Even Rajinikanth follows the rules when needed!
As per coding guidelines, add this string to the i18n files and reference it here:
- [message]="'Please contact your administrator to resolve this issue.'"
+ [message]="'ZERO_STATE.CONTACT_ADMIN_TO_RESOLVE' | transloco"Then add to src/assets/i18n/en.json:
"zeroState": {
"contactAdminToResolve": "Please contact your administrator to resolve this issue."
}Don't forget to add the same key to all other language files (es.json, etc.) as per the coding guidelines!
Based on static analysis hints.
🧰 Tools
🪛 GitHub Check: Run linters
[warning] 792-792:
Hard-coded string "Please contact your administrator to resolve this " should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
🤖 Prompt for AI Agents
In src/app/fyle/add-edit-mileage/add-edit-mileage.page.html around line 792 you
have a hard-coded message string; replace it with a translation key reference
and add that key to the i18n files. Add "zeroState.contactAdminToResolve" with
the provided English value to src/assets/i18n/en.json and add equivalent entries
to all other language files (es.json, etc.), then update the template to use the
translation key (via your app's translate pipe/translation binding) instead of
the hard-coded English text.
| [header]="'You have not been assigned with any per diem rates.'" | ||
| [image]="'../../../assets/svg/alert-circled.svg'" | ||
| [message]="'Please contact your Fyle-Administrator to resolve this issue.'" | ||
| [message]="'Please contact your administrator to resolve this issue.'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Machaan, these strings need translation keys for i18n!
Style-aah! The branding update from "Fyle-Administrator" to "administrator" is perfect. But listen carefully da—these hard-coded strings should be replaced with translation keys to support internationalization. The linter is warning you for a reason, just like how I never ignore a villain's threat!
As per static analysis hints, both messages at lines 818 and 829 should reference translation keys instead of hard-coded English text. This ensures proper i18n support across all languages.
-[message]="'Please contact your administrator to resolve this issue.'"
+[message]="'addEditPerDiem.noRatesAdministratorMessage' | transloco"Do the same for line 829, my friend!
Also applies to: 829-829
🧰 Tools
🪛 GitHub Check: Run linters
[warning] 818-818:
Hard-coded string "Please contact your administrator to resolve this " should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
🤖 Prompt for AI Agents
In src/app/fyle/add-edit-per-diem/add-edit-per-diem.page.html around lines 818
and 829, two hard-coded English messages are used; replace them with i18n
translation keys (e.g., use the existing translation pipe or key lookup used
project-wide) so they reference entries in your translation files instead of
literal strings, add the corresponding keys and translated values to the
appropriate locale JSON files, and ensure the template uses the translation
binding (matching the app’s i18n pattern) for both places.
| <div class="manage-corporate-cards__zero-state-heading">No corporate card added yet!</div> | ||
| <div class="manage-corporate-cards__zero-state-subheading"> | ||
| Add corporate cards to sync your transactions on Fyle. | ||
| Add corporate cards to sync your transactions on Sage Expense Management. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Machaan, this hardcoded string won't fly in i18n world!
Style-ah, you updated "Fyle" to "Sage Expense Management", but this hero string is still hardcoded directly in the template. For proper internationalization support, this should be using a translation key like other messages in the app (see how addEditExpense namespace uses transloco pipe).
Based on static analysis.
Apply this diff to use translation key:
- Add corporate cards to sync your transactions on Sage Expense Management.
+ {{ 'manageCorporateCards.zeroStateSubheading' | transloco }}Then add the corresponding key to src/assets/i18n/en.json:
"manageCorporateCards": {
"zeroStateSubheading": "Add corporate cards to sync your transactions on Sage Expense Management."
}🧰 Tools
🪛 GitHub Check: Run linters
[warning] 65-65:
Hard-coded string "Add corporate cards to sync your transactions on S" should be replaced with a translation key. Refer to https://www.notion.so/fyleuniverse/i18n-translation-file-structure-1ea2ed8bfcb3803da113d3bfc2774ec1#1ea2ed8bfcb3809a9a0ddc05a548eb49 for more details
🤖 Prompt for AI Agents
In src/app/fyle/manage-corporate-cards/manage-corporate-cards.page.html around
line 65, the hero/subheading string is hardcoded; replace the literal text with
the appropriate Transloco translation pipe using a key (e.g.
"manageCorporateCards.zeroStateSubheading") consistent with the app's
translation patterns, and then add that key-entry to src/assets/i18n/en.json
under "manageCorporateCards": { "zeroStateSubheading": "Add corporate cards to
sync your transactions on Sage Expense Management." } so the template uses the
translation key instead of the raw string.
| { | ||
| title: 'Email receipts', | ||
| content: `Forward your receipts to Fyle at ${fyleEmail}.`, | ||
| content: `Forward your receipts to Sage Expense Management at ${fyleEmail}.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Thalaivar says: Hardcoded strings-ah? Not in my production code!
The email receipts content string is hardcoded here. For proper i18n support, this should use the TranslocoService that's already injected in this component.
Consider refactoring to use translation keys:
{
title: this.translocoService.translate('myProfile.emailReceiptsTitle'),
content: this.translocoService.translate('myProfile.emailReceiptsContent', { email: fyleEmail }),
contentToCopy: fyleEmail,
toastMessageContent: this.translocoService.translate('myProfile.emailCopiedToast'),
isShown: true,
}Add corresponding keys to src/assets/i18n/en.json:
"myProfile": {
"emailReceiptsTitle": "Email receipts",
"emailReceiptsContent": "Forward your receipts to Sage Expense Management at {{email}}.",
"emailCopiedToast": "Email copied successfully"
}🤖 Prompt for AI Agents
In src/app/fyle/my-profile/my-profile.page.ts around line 465, the email receipt
content is currently a hardcoded string; replace it with
this.translocoService.translate(...) using a translation key (e.g.,
'myProfile.emailReceiptsContent') and pass the fyleEmail as an interpolation
param; also replace the title and toast message with translation keys and set
contentToCopy to fyleEmail; finally add the corresponding keys and values to
src/assets/i18n/en.json (myProfile.emailReceiptsTitle,
myProfile.emailReceiptsContent with {{email}} placeholder, and
myProfile.emailCopiedToast).
| { | ||
| icon: 'envelope', | ||
| text: `Message your receipts to Fyle at ${fyleMobileNumber} and we will create an expense for you.`, | ||
| text: `Message your receipts to Sage Expense Management at ${fyleMobileNumber} and we will create an expense for you.`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Style-illa solluren, use translation keys da!
The mobile message text is hardcoded. This should use TranslocoService for consistency with the rest of the app's i18n approach.
Refactor to use translation:
{
icon: 'envelope',
text: this.translocoService.translate('myProfile.messageReceiptsInfo', { number: fyleMobileNumber }),
textToCopy: fyleMobileNumber,
}Add to src/assets/i18n/en.json:
"myProfile": {
"messageReceiptsInfo": "Message your receipts to Sage Expense Management at {{number}} and we will create an expense for you."
}🤖 Prompt for AI Agents
In src/app/fyle/my-profile/my-profile.page.ts around line 493 the mobile message
text is hardcoded; replace the literal string with a TranslocoService
translation call that uses a key like 'myProfile.messageReceiptsInfo' and
interpolate the fyleMobileNumber (e.g.
this.translocoService.translate('myProfile.messageReceiptsInfo', { number:
fyleMobileNumber })), keeping textToCopy as the phone number; also add the
corresponding key and value to src/assets/i18n/en.json under "myProfile": {
"messageReceiptsInfo": "Message your receipts to Sage Expense Management at
{{number}} and we will create an expense for you." }.
🔁 Code Duplication Report - Angular
📊 No significant change in code duplication. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/assets/i18n/en.json (1)
41-46: Alphabetize backButton keys, boss. Style + order = peace.Keep child keys sorted to reduce merge conflicts.
Apply this diff to sort keys alphabetically within backButton. As per coding guidelines.
- "backButton": { - "title": "Exit Sage Expense Management App", - "message": "Are you sure you want to exit the app?", - "primaryCtaText": "OK", - "secondaryCtaText": "Cancel" - }, + "backButton": { + "message": "Are you sure you want to exit the app?", + "primaryCtaText": "OK", + "secondaryCtaText": "Cancel", + "title": "Exit Sage Expense Management App" + },src/app/fyle/add-edit-expense/add-edit-expense.page.html (1)
528-534: One more step: localize this info message too. Punch perfection.Use the existing key
addEditExpense.outstandingBalanceInfoinstead of hardcoded text.- <app-fy-alert-info - [message]="'You have outstanding balance in your advance account(s)'" - [type]="'information'" - > + <app-fy-alert-info + [message]="'addEditExpense.outstandingBalanceInfo' | transloco" + [type]="'information'" + > </app-fy-alert-info>
♻️ Duplicate comments (1)
src/app/fyle/add-edit-expense/add-edit-expense.page.html (1)
541-542: Solid move to i18n, boss.Replaced hardcoded text with
'addEditExpense.autoMergeInfo' | transloco. Clean and correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/app/fyle/add-edit-expense/add-edit-expense.page.html(1 hunks)src/assets/i18n/en.json(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/assets/i18n/*.json
📄 CodeRabbit inference engine (.cursor/rules/atomics-i18n-key-pattern.mdc)
src/assets/i18n/*.json: Keep child keys alphabetically sorted within each group in the translation JSON to reduce merge conflicts
Add new i18n keys to every supported language file (e.g., en.json, es.json) in src/assets/i18n/
Files:
src/assets/i18n/en.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
src/assets/i18n/en.json (2)
477-481: Support email still points to fylehq.com — confirm intent.If rebrand affects support channels, update email; else, carry on.
1508-1509: addEditExpense.autoMergeInfo stands tall in all locale files. No soldier left behind!src/app/fyle/add-edit-expense/add-edit-expense.page.html (1)
541-542: Verify translation presence across locales forautoMergeInfo.Don’t let other languages miss the party. Use the i18n check script from my en.json comment.
| "fyle": "Sage Expense Management", | ||
| "cancel": "Cancel" | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Sort fyHeader keys — keep it sharp and clean.
Child keys should be alphabetically ordered.
Apply:
"fyHeader": {
- "fyle": "Sage Expense Management",
- "cancel": "Cancel"
+ "cancel": "Cancel",
+ "fyle": "Sage Expense Management"
},As per coding guidelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "fyle": "Sage Expense Management", | |
| "cancel": "Cancel" | |
| }, | |
| "fyHeader": { | |
| "cancel": "Cancel", | |
| "fyle": "Sage Expense Management" | |
| }, |
🤖 Prompt for AI Agents
In src/assets/i18n/en.json around lines 546 to 548, the child keys under the
"fyHeader" object are not alphabetically ordered; reorder all keys within the
"fyHeader" object in ascending alphabetical order (A–Z) while preserving their
values and JSON structure, ensuring commas and indentation remain valid after
reordering.
…to Fyle-86d0k46xx
Clickup
https://app.clickup.com/t/86d0k46xx
Code Coverage
Please add code coverage here
UI Preview
Please add screenshots for UI changes
Summary by CodeRabbit
Style
Tests