fix: qbo incorrect account warning content fix #1620
Conversation
WalkthroughThe QBO onboarding incorrect-account warning now injects an Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts(1 hunks)src/assets/i18n/en.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
**/*.component.{ts,html}: Top-level object in i18n files must be the component or feature folder name, converted from the file name by removing the prefixes 'feature', 'ui', or 'component' (in that order) and converting from kebab-case to camelCase.
Keys inside the top-level i18n object must not be nested beyond the first level; only one level of keys is allowed.
Keys inside the top-level i18n object must use meaningful, semantic, and context-aware names (e.g., 'title', 'accountDisabledError', 'submitButton'); avoid generic names like 'label1', 'text1', 'message1'.
Do not use prefixes like 'ui', 'feature', or 'component' in the top-level i18n object name; the object name should be the context.
Do not translate strings that are already translation keys, tracking/analytics strings, strings with only special characters, comments, non-user-visible text, or variable bindings and expressions.
One i18n object per component or feature; do not combine multiple components/features into a single i18n object.
Files:
src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts
src/**/*.component.{ts,html}
📄 CodeRabbit inference engine (.cursor/rules/component-i18n-key-naming.mdc)
For files in 'src/**', add or update i18n keys in the corresponding translation files located at 'src/assets/i18n/{lang}.json', and ensure the key is present in every supported language file.
Files:
src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts
**/*.component.ts
📄 CodeRabbit inference engine (.cursor/rules/component-creation.mdc)
Always create Angular standalone components (
--standalone=true), i.e., components must declarestandalone: truein@Componentmetadata
Files:
src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts
src/assets/i18n/*.json
📄 CodeRabbit inference engine (.cursor/rules/atomics-i18n-key-pattern.mdc)
src/assets/i18n/*.json: Group i18n keys under the correct atomic root: 'services', 'pipes', or 'directives' in the translation JSON files
Add i18n keys to every supported language file in 'src/assets/i18n/{lang}.json'
Keep child keys alphabetically sorted within each group in translation JSON files to reduce merge conflicts
Use only the three roots: services, pipes, directives for i18n key grouping
Files:
src/assets/i18n/en.json
🧠 Learnings (2)
📓 Common learnings
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has confirmed the suggestion to replace the hardcoded reference to 'Sage Intacct' with a dynamic reference to the target system using the variable `targetSystemName` in the subLabel string within the `business-central-export-settings.component.html` file.
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-10-08T15:51:28.972Z
Learning: The user ashwin1111 has confirmed the suggestion to replace the hardcoded reference to 'Sage Intacct' with a dynamic reference to the target system using the variable `targetSystemName` in the subLabel string within the `business-central-export-settings.component.html` file.
📚 Learning: 2024-06-10T19:13:15.470Z
Learnt from: ashwin1111
PR: fylein/fyle-integrations-app#343
File: src/app/integrations/business-central/business-central-shared/business-central-export-settings/business-central-export-settings.component.html:0-0
Timestamp: 2024-06-10T19:13:15.470Z
Learning: The user ashwin1111 has confirmed the suggestion to replace the hardcoded reference to 'Sage Intacct' with a dynamic reference to the target system using the variable `targetSystemName` in the subLabel string within the `business-central-export-settings.component.html` file.
Applied to files:
src/assets/i18n/en.json
🔇 Additional comments (1)
src/assets/i18n/en.json (1)
533-533: Good improvement - dynamic account name injection.The addition of the
{{accountName}}template parameter makes the warning message more specific and user-friendly by showing the actual account name. This aligns with the learnings about preferring dynamic references over hardcoded values.Based on learnings
| if (errorMessage === 'Please choose the correct QuickBooks Online account') { | ||
| this.warningHeaderText = this.translocoService.translate('qboOnboardingConnector.incorrectAccountWarningHeader'); | ||
| this.warningContextText = this.translocoService.translate('qboOnboardingConnector.incorrectAccountWarningContent'); | ||
| this.warningContextText = this.translocoService.translate('qboOnboardingConnector.incorrectAccountWarningContent', { accountName: this.qboCompanyName || 'a different' }); |
There was a problem hiding this comment.
🧩 Analysis chain
Hardcoded English fallback breaks i18n consistency.
The fallback value 'a different' is hardcoded in English, which will appear in English regardless of the user's locale setting. This breaks internationalization consistency.
Consider checking if the error response contains the company name (similar to line 223 where error.error.company_name is used), or use a translatable fallback string.
Verify if the error object at line 198 contains a company_name field that could be used instead of the hardcoded fallback:
🏁 Script executed:
#!/bin/bash
# Search for error response structure in QBO connector service and related files
rg -n -A5 -B5 "Please choose the correct QuickBooks Online account" --type=tsLength of output: 3497
---
🏁 Script executed:
```shell
#!/bin/bash
# Find qboCompanyName initialization and assignments
rg -n "qboCompanyName\s*=" --type=ts src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts
Length of output: 179
🏁 Script executed:
#!/bin/bash
# Find all references to company_name in error responses
rg -n "company_name" --type=ts -C3 src/app/integrations/qbo/Length of output: 3582
🏁 Script executed:
#!/bin/bash
# Check for translation keys related to 'a different' or fallback
rg -n "a different" --type=json src/assets/i18n/Length of output: 1500
🏁 Script executed:
#!/bin/bash
# Get the full error handler context around line 198-201
sed -n '190,210p' src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.tsLength of output: 1301
🏁 Script executed:
#!/bin/bash
# Check line 223 referenced in scratchpad to see company_name usage
sed -n '215,230p' src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.tsLength of output: 469
🏁 Script executed:
#!/bin/bash
# Search for translation keys containing 'ifferent'
rg -n "ifferent" --type=json src/assets/i18n/Length of output: 2119
Hardcoded English fallback breaks i18n consistency.
At line 201, the fallback value 'a different' is hardcoded in English and does not exist in any translation files. This will always display in English regardless of the user's locale setting.
The error response object contains company_name (as shown in line 223's error handler pattern), so consider extracting it from error.error.company_name instead of the hardcoded fallback:
accountName: error.error?.company_name || this.translocoService.translate('qboOnboardingConnector.fallbackAccountName')
Create a translation key for the fallback scenario to maintain i18n consistency across all locales.
🤖 Prompt for AI Agents
In
src/app/integrations/qbo/qbo-onboarding/qbo-onboarding-connector/qbo-onboarding-connector.component.ts
around line 201, the fallback account name is hardcoded as the English string 'a
different', breaking i18n; replace that hardcoded fallback by pulling the
company name from the error object when available (e.g.,
error.error?.company_name) and otherwise use a translated fallback key via
this.translocoService.translate('qboOnboardingConnector.fallbackAccountName');
add the new translation key 'qboOnboardingConnector.fallbackAccountName' to all
locale files and update the error handler to pass accountName:
error.error?.company_name ||
this.translocoService.translate('qboOnboardingConnector.fallbackAccountName') so
all locales show a proper translated fallback.
🔁 Code Duplication Report - Angular
📊 No significant change in code duplication. |
2 participants
Description
fix: qbo incorrect account warning content fix
Clickup
https://app.clickup.com
Summary by CodeRabbit