feat: QWC file regen: landing page#1755
Open
JustARatherRidiculouslyLongUsername wants to merge 2 commits intoallow-qwc-regenerationfrom
Open
feat: QWC file regen: landing page#1755JustARatherRidiculouslyLongUsername wants to merge 2 commits intoallow-qwc-regenerationfrom
JustARatherRidiculouslyLongUsername wants to merge 2 commits intoallow-qwc-regenerationfrom
Conversation
Contributor
WalkthroughA new QWC (Quickbooks Company File) landing feature is introduced to the QBD Direct integration, encompassing a new component, routing, form model, localization strings, and styling updates. The feature enables users to manage QWC file regeneration by choosing between existing or new file paths, with metadata display and clipboard integration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
🔁 Code Duplication Report - Angular
📊 No significant change in code duplication. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/shared/components/core/svg-icon/svg-icon.component.ts (1)
4-31:⚠️ Potential issue | 🟠 MajorAdd mappings for icons used in QWC regeneration UI
The template uses
check-circle-fillandquestion-square-fillicons. When the C1 branding config enablesuseCustomIcon: true, these keys must be mapped throughICON_MAPPING(line 126); missing entries resolve toundefined, breaking the sprite URL. Add these mappings to the ICON_MAPPING object.
🤖 Fix all issues with AI agents
In
`@src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-configuration/qbd-direct-configuration.component.ts`:
- Line 32: The i18n key used in the navigation item is inconsistent: change the
translate key from 'qbd_direct.configuration.QWCFile.stepName' to
'qbd_direct.configuration.qwcFile.stepName' in
qbd-direct-configuration.component (the line referencing
this.translocoService.translate), and update the corresponding key in the
en.json (and any other locale files) to use "qwcFile" (camelCase) so it matches
the other keys like exportSetting/importSetting/advancedSettings.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.scss`:
- Around line 1-6: Update the .shadow-card &:hover rule to use modern color
notation and drop px on zero offsets: replace "box-shadow: 0px 4px 4px 0px
rgba(44, 48, 78, 0.10);" in the .shadow-card hover block with a lint-compliant
form such as "box-shadow: 0 4px 4px 0 rgb(44 48 78 / 0.1);" so it uses RGB
slash-alpha and no unit on zero values.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.spec.ts`:
- Around line 9-18: The spec fails because QbdDirectConnectorService (and other
deps used in ngOnInit) are not provided; update the TestBed for
QbdDirectRegenerateQWCFileComponent to either provide mocked providers for
QbdDirectConnectorService (mocking getQBDConnectorSettings to return an
observable), FormBuilder, BrandingService, IntegrationsToastService, and
TranslocoService (mirror the mocks used in
qbd-direct-assisted-setup.component.spec.ts), or mark the suite as skipped with
xdescribe to match other specs; ensure the mock for QbdDirectConnectorService is
referenced so ngOnInit’s call to getQBDConnectorSettings().subscribe() won’t
throw.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`:
- Around line 85-88: The subscription to
qbdDirectConnectorService.getQBDConnectorSettings() in the
QbdDirectRegenerateQwcFileComponent lacks error handling so isLoading can remain
true on failure; update the call (getQBDConnectorSettings()) to handle errors by
adding an error callback or using pipe/catchError, ensure this.isLoading is set
to false in the error branch, and surface the failure (e.g., call the
component's notification/toast method or log the error) while leaving
connectorSettings unchanged.
- Around line 42-44: The getter selectedNextStep can return null even though
it’s typed as NextStepOption; update it to accurately reflect possible null by
changing its signature to return NextStepOption | null (or provide a safe
default), and return the control value without force-casting (e.g. return
this.regenerateQwcForm.get('nextStep')?.value as NextStepOption | null or
coalesce to a default like ?? DEFAULT_OPTION). Ensure callers/templates handle
the nullable type accordingly or use a non-null default constant referenced
here.
In `@src/assets/i18n/en.json`:
- Around line 2733-2766: Reorder the child keys alphabetically in the two i18n
objects: under qbd_direct.configuration ensure the three keys are ordered as
advancedSettings, exportSetting, QWCFile (instead of exportSetting,
advancedSettings, QWCFile), and under qbdDirectRegenerateQwcFile sort all 23
keys alphabetically (so contentText appears before stepName); update the JSON
key ordering only (no text changes) to maintain alphabetical order for merge
hygiene.
🧹 Nitpick comments (2)
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts (2)
58-67:navigator.clipboard.writeTextcan reject — handle the promise.Clipboard access may be denied (e.g., non-HTTPS, permission policy). The unhandled promise rejection will silently fail while the success toast still shows.
Proposed fix
copyQwcFilePath() { if (!this.connectorSettings?.file_location) { return; } - navigator.clipboard.writeText(this.connectorSettings.file_location); - this.toastService.displayToastMessage( - ToastSeverity.SUCCESS, - this.translocoService.translate('qbdDirectRegenerateQwcFile.qwcFilePathCopied') - ); + navigator.clipboard.writeText(this.connectorSettings.file_location).then(() => { + this.toastService.displayToastMessage( + ToastSeverity.SUCCESS, + this.translocoService.translate('qbdDirectRegenerateQwcFile.qwcFilePathCopied') + ); + }); }
69-83: Group-level validator is over-engineered for a single required control.A simple
Validators.requiredon theFormControlachieves the same result more idiomatically.Simplified form setup
this.regenerateQwcForm = this.formBuilder.group<RegenerateQwcForm>({ - nextStep: new FormControl(null) - }, { - validators: (form) => { - const errors: ValidationErrors = {}; - - if (form.get('nextStep')?.value === null) { - errors.nextStep = { required: true }; - } - - return Object.keys(errors).length > 0 ? errors : null; - } + nextStep: new FormControl(null, { validators: [Validators.required] }) });Note: you'll need to add
Validatorsto the existing@angular/formsimport (it's already partially imported).
| {label: this.translocoService.translate('qbd_direct.configuration.importSetting.stepName'), routerLink: '/integrations/qbd_direct/main/configuration/import_settings', value: 'import_settings'}, | ||
| {label: this.translocoService.translate('qbd_direct.configuration.advancedSettings.stepName'), routerLink: '/integrations/qbd_direct/main/configuration/advanced_settings', value: 'advanced_settings'} | ||
| {label: this.translocoService.translate('qbd_direct.configuration.advancedSettings.stepName'), routerLink: '/integrations/qbd_direct/main/configuration/advanced_settings', value: 'advanced_settings'}, | ||
| {label: this.translocoService.translate('qbd_direct.configuration.QWCFile.stepName'), routerLink: '/integrations/qbd_direct/main/configuration/qwc_file', value: 'qwc_file'} |
Contributor
There was a problem hiding this comment.
i18n key QWCFile should be camelCase (qwcFile) for consistency.
The other keys in this array use camelCase (exportSetting, importSetting, advancedSettings), but this one uses PascalCase QWCFile. This also needs to match the key in en.json.
- {label: this.translocoService.translate('qbd_direct.configuration.QWCFile.stepName'), routerLink: '/integrations/qbd_direct/main/configuration/qwc_file', value: 'qwc_file'}
+ {label: this.translocoService.translate('qbd_direct.configuration.qwcFile.stepName'), routerLink: '/integrations/qbd_direct/main/configuration/qwc_file', value: 'qwc_file'}As per coding guidelines, top-level i18n object must be the component/feature folder name in camelCase, with keys using meaningful semantic names.
📝 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.
Suggested change
| {label: this.translocoService.translate('qbd_direct.configuration.QWCFile.stepName'), routerLink: '/integrations/qbd_direct/main/configuration/qwc_file', value: 'qwc_file'} | |
| {label: this.translocoService.translate('qbd_direct.configuration.qwcFile.stepName'), routerLink: '/integrations/qbd_direct/main/configuration/qwc_file', value: 'qwc_file'} |
🤖 Prompt for AI Agents
In
`@src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-configuration/qbd-direct-configuration.component.ts`
at line 32, The i18n key used in the navigation item is inconsistent: change the
translate key from 'qbd_direct.configuration.QWCFile.stepName' to
'qbd_direct.configuration.qwcFile.stepName' in
qbd-direct-configuration.component (the line referencing
this.translocoService.translate), and update the corresponding key in the
en.json (and any other locale files) to use "qwcFile" (camelCase) so it matches
the other keys like exportSetting/importSetting/advancedSettings.
Comment on lines
+1
to
+6
| .shadow-card { | ||
| @apply tw-transition-shadow; | ||
|
|
||
| &:hover { | ||
| box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10); | ||
| } |
Contributor
There was a problem hiding this comment.
Fix stylelint violations in hover shadow
Stylelint flags the hover box-shadow for legacy color notation and zero units. Update to modern rgb() slash-alpha and drop px on zeros to satisfy the lint rules.
Proposed fix
.shadow-card {
`@apply` tw-transition-shadow;
&:hover {
- box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10);
+ box-shadow: 0 4px 4px 0 rgb(44 48 78 / 10%);
}
}📝 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.
Suggested change
| .shadow-card { | |
| @apply tw-transition-shadow; | |
| &:hover { | |
| box-shadow: 0px 4px 4px 0px rgba(44, 48, 78, 0.10); | |
| } | |
| .shadow-card { | |
| `@apply` tw-transition-shadow; | |
| &:hover { | |
| box-shadow: 0 4px 4px 0 rgb(44 48 78 / 10%); | |
| } | |
| } |
🧰 Tools
🪛 Stylelint (17.2.0)
[error] 5-5: Expected "0.10" to be "10%" (alpha-value-notation)
(alpha-value-notation)
[error] 5-5: Expected "rgba" to be "rgb" (color-function-alias-notation)
(color-function-alias-notation)
[error] 5-5: Expected modern color-function notation (color-function-notation)
(color-function-notation)
[error] 5-5: Unexpected unit (length-zero-no-unit)
(length-zero-no-unit)
[error] 5-5: Unexpected unit (length-zero-no-unit)
(length-zero-no-unit)
🤖 Prompt for AI Agents
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.scss`
around lines 1 - 6, Update the .shadow-card &:hover rule to use modern color
notation and drop px on zero offsets: replace "box-shadow: 0px 4px 4px 0px
rgba(44, 48, 78, 0.10);" in the .shadow-card hover block with a lint-compliant
form such as "box-shadow: 0 4px 4px 0 rgb(44 48 78 / 0.1);" so it uses RGB
slash-alpha and no unit on zero values.
Comment on lines
9
to
18
| beforeEach(async () => { | ||
| await TestBed.configureTestingModule({ | ||
| imports: [QbdDirectRegenerateQWCFileComponent] | ||
| }) | ||
| .compileComponents(); | ||
|
|
||
| fixture = TestBed.createComponent(QbdDirectRegenerateQWCFileComponent); | ||
| component = fixture.componentInstance; | ||
| await fixture.whenStable(); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find src/app/integrations/qbd-direct/qbd-direct-shared -name "*.component.ts" | head -5Repository: fylein/fyle-integrations-app
Length of output: 642
🏁 Script executed:
cat -n src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.tsRepository: fylein/fyle-integrations-app
Length of output: 4067
🏁 Script executed:
find src/app/integrations/qbd-direct/qbd-direct-shared -name "*.spec.ts" -exec sh -c 'echo "=== {} ===" && head -35 "$1"' _ {} \;Repository: fylein/fyle-integrations-app
Length of output: 8818
Test will fail — missing provider for QbdDirectConnectorService.
ngOnInit immediately calls this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe() at line 85, but QbdDirectConnectorService is not provided in TestBed. Additionally, FormBuilder, BrandingService, IntegrationsToastService, and TranslocoService are not provided or mocked.
Either mock all dependencies (as done in qbd-direct-assisted-setup.component.spec.ts) or skip the spec file using xdescribe to match the project's convention—most other specs in this directory are skipped.
🤖 Prompt for AI Agents
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.spec.ts`
around lines 9 - 18, The spec fails because QbdDirectConnectorService (and other
deps used in ngOnInit) are not provided; update the TestBed for
QbdDirectRegenerateQWCFileComponent to either provide mocked providers for
QbdDirectConnectorService (mocking getQBDConnectorSettings to return an
observable), FormBuilder, BrandingService, IntegrationsToastService, and
TranslocoService (mirror the mocks used in
qbd-direct-assisted-setup.component.spec.ts), or mark the suite as skipped with
xdescribe to match other specs; ensure the mock for QbdDirectConnectorService is
referenced so ngOnInit’s call to getQBDConnectorSettings().subscribe() won’t
throw.
Comment on lines
+42
to
+44
| get selectedNextStep(): NextStepOption { | ||
| return this.regenerateQwcForm.get('nextStep')?.value as NextStepOption; | ||
| } |
Contributor
There was a problem hiding this comment.
Getter can return null despite NextStepOption return type.
When no option is selected, value is null but the cast as NextStepOption silently hides this. Template code relying on this getter may behave unexpectedly.
- get selectedNextStep(): NextStepOption {
- return this.regenerateQwcForm.get('nextStep')?.value as NextStepOption;
+ get selectedNextStep(): NextStepOption | null {
+ return this.regenerateQwcForm.get('nextStep')?.value ?? null;
}📝 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.
Suggested change
| get selectedNextStep(): NextStepOption { | |
| return this.regenerateQwcForm.get('nextStep')?.value as NextStepOption; | |
| } | |
| get selectedNextStep(): NextStepOption | null { | |
| return this.regenerateQwcForm.get('nextStep')?.value ?? null; | |
| } |
🤖 Prompt for AI Agents
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`
around lines 42 - 44, The getter selectedNextStep can return null even though
it’s typed as NextStepOption; update it to accurately reflect possible null by
changing its signature to return NextStepOption | null (or provide a safe
default), and return the control value without force-casting (e.g. return
this.regenerateQwcForm.get('nextStep')?.value as NextStepOption | null or
coalesce to a default like ?? DEFAULT_OPTION). Ensure callers/templates handle
the nullable type accordingly or use a non-null default constant referenced
here.
Comment on lines
+85
to
+88
| this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe((qbdConnectorSettings: QbdConnectorGet) => { | ||
| this.connectorSettings = qbdConnectorSettings; | ||
| this.isLoading = false; | ||
| }); |
Contributor
There was a problem hiding this comment.
Missing error handling — loading state stuck on failure.
If getQBDConnectorSettings() errors, isLoading remains true forever and the user is stuck on a spinner. Add error handling to stop loading and surface the failure.
Proposed fix
this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe((qbdConnectorSettings: QbdConnectorGet) => {
this.connectorSettings = qbdConnectorSettings;
this.isLoading = false;
- });
+ }, () => {
+ this.isLoading = false;
+ });📝 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.
Suggested change
| this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe((qbdConnectorSettings: QbdConnectorGet) => { | |
| this.connectorSettings = qbdConnectorSettings; | |
| this.isLoading = false; | |
| }); | |
| this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe((qbdConnectorSettings: QbdConnectorGet) => { | |
| this.connectorSettings = qbdConnectorSettings; | |
| this.isLoading = false; | |
| }, () => { | |
| this.isLoading = false; | |
| }); |
🤖 Prompt for AI Agents
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`
around lines 85 - 88, The subscription to
qbdDirectConnectorService.getQBDConnectorSettings() in the
QbdDirectRegenerateQwcFileComponent lacks error handling so isLoading can remain
true on failure; update the call (getQBDConnectorSettings()) to handle errors by
adding an error callback or using pipe/catchError, ensure this.isLoading is set
to false in the error branch, and surface the failure (e.g., call the
component's notification/toast method or log the error) while leaving
connectorSettings unchanged.
Comment on lines
2733
to
2766
| }, | ||
| "QWCFile": { | ||
| "stepName": "QWC file" | ||
| } | ||
| } | ||
| }, | ||
| "dashboard": { | ||
| "exportLogSubHeaderSuccessful": "These expenses have been successfully exported to your {{ appName }}." | ||
| }, | ||
| "qbdDirectRegenerateQwcFile": { | ||
| "stepName": "Regenerate QWC File", | ||
| "contentText": "Regenerate your QuickBooks Web Connector file or set up a new one if your company file path has changed.", | ||
| "currentFileHeader": "Current company file path", | ||
| "currentFileLastRegenerated": "Last regenerated", | ||
| "currentFileBy": " by ", | ||
| "currentFileOn": " on ", | ||
| "qwcFilePathCopied": "QWC file path copied!", | ||
| "nextStepHeader": "Choose your next step:", | ||
| "useExistingPath": "Use existing company file path", | ||
| "useExistingPathDescription": "Regenerate your QuickBooks Web Connector (.qwc) file and password for the current company file path.", | ||
| "useNewPath": "Use new company file path", | ||
| "useNewPathDescription": "Generate a new QuickBooks Web Connector (.qwc) file and password for a new or updated company file path.", | ||
| "whenToUse": "When to use:", | ||
| "existingWhenToUse1": "The QWC file was lost or corrupted", | ||
| "existingWhenToUse2": "Another team member needs access (not recommended)", | ||
| "newWhenToUse1": "The company file was moved or renamed", | ||
| "newWhenToUse2": "QuickBooks Desktop was reinstalled or updated", | ||
| "keyBenefits": "Key benefits:", | ||
| "existingBenefit1": "Easiest and safest way to restore your connection", | ||
| "existingBenefit2": "No changes to your integration configurations", | ||
| "newBenefit1": "Ensures your integration matches your current QuickBooks setup", | ||
| "newBenefit2": "Resolves path mismatch errors", | ||
| "readMore": "Read more" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, list all locale files
echo "=== Available locale files ==="
ls -la src/assets/i18n/*.json 2>/dev/null || echo "Directory not found"
# Check file count
echo ""
echo "=== Locale file count ==="
find src/assets/i18n -name "*.json" -type f | wc -lRepository: fylein/fyle-integrations-app
Length of output: 206
🏁 Script executed:
#!/bin/bash
# Run the verification script from the review
python - <<'PY'
import json, glob
def flatten(d, prefix=""):
out = {}
for k, v in d.items():
key = f"{prefix}.{k}" if prefix else k
if isinstance(v, dict):
out.update(flatten(v, key))
else:
out[key] = v
return out
files = sorted(glob.glob("src/assets/i18n/*.json"))
base = "src/assets/i18n/en.json"
with open(base) as f:
base_keys = set(flatten(json.load(f)).keys())
for path in files:
if path == base:
continue
with open(path) as f:
keys = set(flatten(json.load(f)).keys())
missing = sorted(base_keys - keys)
if missing:
print(f"{path}: missing {len(missing)} keys. Example: {missing[:20]}")
PYRepository: fylein/fyle-integrations-app
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check alphabetical ordering of keys in the affected sections
python - <<'PY'
import json
with open("src/assets/i18n/en.json") as f:
data = json.load(f)
# Check qbd_direct.configuration ordering
if "qbd_direct" in data and "configuration" in data["qbd_direct"]:
config_keys = list(data["qbd_direct"]["configuration"].keys())
sorted_keys = sorted(config_keys)
print("=== qbd_direct.configuration key order ===")
print("Current:", config_keys)
print("Sorted: ", sorted_keys)
print("In order:", config_keys == sorted_keys)
if config_keys != sorted_keys:
print("\nMisaligned keys:")
for i, (curr, sort) in enumerate(zip(config_keys, sorted_keys)):
if curr != sort:
print(f" Position {i}: have '{curr}', should be '{sort}'")
# Check qbdDirectRegenerateQwcFile ordering
if "qbdDirectRegenerateQwcFile" in data:
qwc_keys = list(data["qbdDirectRegenerateQwcFile"].keys())
sorted_keys = sorted(qwc_keys)
print("\n=== qbdDirectRegenerateQwcFile key order ===")
print("Current:", qwc_keys)
print("Sorted: ", sorted_keys)
print("In order:", qwc_keys == sorted_keys)
if qwc_keys != sorted_keys:
print("\nMisaligned keys:")
for i, (curr, sort) in enumerate(zip(qwc_keys, sorted_keys)):
if curr != sort:
print(f" Position {i}: have '{curr}', should be '{sort}'")
PYRepository: fylein/fyle-integrations-app
Length of output: 2683
🏁 Script executed:
#!/bin/bash
# List all files in i18n directory with more detail
echo "=== All files in i18n directory ==="
find src/assets/i18n -type f | sort
echo ""
echo "=== Check file sizes ==="
find src/assets/i18n -type f -exec wc -l {} +
echo ""
echo "=== Check if QWC keys exist in other locales ==="
for file in src/assets/i18n/*.json; do
echo "File: $file"
if grep -q "qbdDirectRegenerateQwcFile" "$file" 2>/dev/null; then
echo " ✓ Contains qbdDirectRegenerateQwcFile"
else
echo " ✗ Missing qbdDirectRegenerateQwcFile"
fi
doneRepository: fylein/fyle-integrations-app
Length of output: 447
🏁 Script executed:
#!/bin/bash
# Check what the other en.json files contain - are they full locales or partial configs?
echo "=== Content of co/en.json (first 30 lines) ==="
head -30 src/assets/i18n/co/en.json
echo ""
echo "=== Content of fyle/en.json (first 30 lines) ==="
head -30 src/assets/i18n/fyle/en.json
echo ""
echo "=== Check if co/en.json has dashboard key ==="
grep -c "dashboard" src/assets/i18n/co/en.json || echo "Not found"
echo ""
echo "=== Check if fyle/en.json has dashboard key ==="
grep -c "dashboard" src/assets/i18n/fyle/en.json || echo "Not found"
echo ""
echo "=== Check directory structure ==="
find src/assets/i18n -type dRepository: fylein/fyle-integrations-app
Length of output: 4495
🏁 Script executed:
#!/bin/bash
# Check if there are actual language-specific files (not org-specific)
echo "=== Check for language files at root level ==="
find src/assets/i18n -maxdepth 1 -name "*.json" -type f | sort
echo ""
echo "=== Check structure - are these org-specific variants? ==="
for file in src/assets/i18n/*/en.json; do
echo "File: $file"
tail -5 "$file"
echo "---"
done
echo ""
echo "=== Verify: Does qbdDirectRegenerateQwcFile exist in co/en.json? ==="
grep -c "qbdDirectRegenerateQwcFile" src/assets/i18n/co/en.json || echo "0"
echo ""
echo "=== Verify: Does qbdDirectRegenerateQwcFile exist in fyle/en.json? ==="
grep -c "qbdDirectRegenerateQwcFile" src/assets/i18n/fyle/en.json || echo "0"
echo ""
echo "=== Check if QWCFile key exists in qbd_direct.configuration for other files ==="
for file in src/assets/i18n/*/en.json; do
echo "File: $file"
grep -A50 '"configuration"' "$file" | grep -c "QWCFile" || echo " Not found"
doneRepository: fylein/fyle-integrations-app
Length of output: 762
🏁 Script executed:
#!/bin/bash
# Let's check if there's any documentation or configuration that explains the i18n structure
echo "=== Check for any i18n setup files ==="
find . -name "*i18n*" -o -name "*locale*" -o -name "*lang*" | grep -E "\.(ts|js|json|md)$" | head -20
echo ""
echo "=== Check the git log to see if co/ and fyle/ were recent additions ==="
git log --oneline --all -- src/assets/i18n/ | head -10
echo ""
echo "=== Check if there's a main en.json file that includes/extends the org variants ==="
head -5 src/assets/i18n/en.json
tail -5 src/assets/i18n/en.json
echo ""
echo "=== Look for translation/language setup in code ==="
rg -l "i18n|translation" src --type ts --type js 2>/dev/null | head -5Repository: fylein/fyle-integrations-app
Length of output: 1102
🏁 Script executed:
#!/bin/bash
# Check the transloco-http-loader to understand how these locale files are used
echo "=== Content of transloco-http-loader.ts ==="
cat src/app/transloco-http-loader.ts
echo ""
echo "=== Check if co/en.json and fyle/en.json contain all keys or just overrides ==="
echo "Number of top-level keys in en.json:"
jq 'keys | length' src/assets/i18n/en.json
echo ""
echo "Number of top-level keys in co/en.json:"
jq 'keys | length' src/assets/i18n/co/en.json
echo ""
echo "Number of top-level keys in fyle/en.json:"
jq 'keys | length' src/assets/i18n/fyle/en.json
echo ""
echo "=== Check if co and fyle are just partial overrides or complete locales ==="
echo "Top-level keys in co/en.json:"
jq 'keys' src/assets/i18n/co/en.json
echo ""
echo "Top-level keys in fyle/en.json:"
jq 'keys' src/assets/i18n/fyle/en.jsonRepository: fylein/fyle-integrations-app
Length of output: 2806
🏁 Script executed:
#!/bin/bash
# Verify one more thing - check if there are any other language files planned or in use
echo "=== Check if there are other language variants mentioned in code ==="
rg "i18n/(es|fr|de|it|pt|ja|zh)" src --type ts --type js 2>/dev/null | head -10
echo ""
echo "=== Check git history for language files ==="
git log --all --name-only --pretty=format: -- "src/assets/i18n/*.json" | sort -u
echo ""
echo "=== Final verification: Current en.json key order for the two sections ==="
python3 - <<'PY'
import json
with open("src/assets/i18n/en.json") as f:
data = json.load(f)
print("=== qbd_direct.configuration ===")
config = data["qbd_direct"]["configuration"]
keys = list(config.keys())
print(f"Current order: {keys}")
print(f"Sorted order: {sorted(keys)}")
print(f"Needs fixing: {keys != sorted(keys)}")
print("\n=== qbdDirectRegenerateQwcFile (showing first 10 keys) ===")
qwc = data["qbdDirectRegenerateQwcFile"]
keys = list(qwc.keys())
print(f"Current (first 10): {keys[:10]}")
sorted_keys = sorted(keys)
print(f"Sorted (first 10): {sorted_keys[:10]}")
print(f"Needs fixing: {keys != sorted_keys}")
PYRepository: fylein/fyle-integrations-app
Length of output: 1043
Sort i18n keys alphabetically in the two new sections
The keys in qbd_direct.configuration and qbdDirectRegenerateQwcFile are not alphabetically sorted. Per the coding guidelines, keep child keys alphabetically sorted to reduce merge conflicts.
qbd_direct.configuration: Should beadvancedSettings,exportSetting,QWCFile(currently:exportSetting,advancedSettings,QWCFile)qbdDirectRegenerateQwcFile: All 23 keys need reordering alphabetically starting withcontentTextinstead ofstepName
🤖 Prompt for AI Agents
In `@src/assets/i18n/en.json` around lines 2733 - 2766, Reorder the child keys
alphabetically in the two i18n objects: under qbd_direct.configuration ensure
the three keys are ordered as advancedSettings, exportSetting, QWCFile (instead
of exportSetting, advancedSettings, QWCFile), and under
qbdDirectRegenerateQwcFile sort all 23 keys alphabetically (so contentText
appears before stepName); update the JSON key ordering only (no text changes) to
maintain alphabetical order for merge hygiene.
JustARatherRidiculouslyLongUsername
changed the base branch from
master
to
allow-qwc-regeneration
ashwin1111
approved these changes
Feb 17, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.html (2)
145-154: Duplicate@if (!isLoading)— consolidate with the main content block.This is a second
@if (!isLoading)outside the root<div>. Consider moving the footer inside the@elseblock (per the refactor above) to avoid the duplicated condition check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.html` around lines 145 - 154, There is a duplicate conditional block checking isLoading for the footer; remove this extra "@if (!isLoading)" and move the <app-configuration-step-footer> into the main content's non-loading branch (the existing else/@if block that renders the page content) so the footer is rendered once when isLoading is false; ensure you keep the same inputs/outputs ([ctaText]="ConfigurationCtaText.CONTINUE", [showBackButton]="false", [isButtonDisabled]="regenerateQwcForm.invalid", (continue)="continue()") and delete the redundant conditional wrapper around the footer to consolidate the logic.
2-7: Use@elseinstead of a separate@if (!isLoading)block.Lines 2 and 7 are mutually exclusive conditions. Using
@if/@elseis cleaner and avoids evaluating both expressions.Proposed fix
`@if` (isLoading) { <div class="tw-flex tw-justify-center tw-items-center tw-pt-80-px tw-h-[75vh]"> <app-loader></app-loader> </div> - } - `@if` (!isLoading) { + } `@else` {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.html` around lines 2 - 7, Replace the two separate Razor/Angular-like conditionals that check isLoading with a single if/else to avoid evaluating both branches: update the template in qbd-direct-qwc-file-landing.component.html so the first conditional uses `@if` (isLoading) { ... } and the mutually exclusive branch becomes `@else` { ... } instead of `@if` (!isLoading) { ... }; adjust only the template markup around the isLoading blocks to use `@else` and keep the existing loader and content nodes (app-loader and the subsequent content) unchanged.src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts (2)
71-83: Custom group-level validator is over-engineered — useValidators.requiredon the control.A simple
Validators.requiredon theFormControlachieves the same effect and is more idiomatic for Angular reactive forms.Proposed fix
this.regenerateQwcForm = this.formBuilder.group<RegenerateQwcForm>({ - nextStep: new FormControl(null) - }, { - validators: (form) => { - const errors: ValidationErrors = {}; - - if (form.get('nextStep')?.value === null) { - errors.nextStep = { required: true }; - } - - return Object.keys(errors).length > 0 ? errors : null; - } + nextStep: new FormControl(null, { validators: [Validators.required] }) });Also,
ValidationErrorscan be removed from the import on Line 2 if no longer used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts` around lines 71 - 83, Replace the custom group-level validator on regenerateQwcForm with a simple required validator on the nextStep control: when creating the form via formBuilder.group<RegenerateQwcForm> pass nextStep: new FormControl(null, Validators.required) and remove the validators:{...} block; update imports to remove ValidationErrors if it becomes unused. Locate the form initialization in qbd-direct-qwc-file-landing.component (regenerateQwcForm, formBuilder.group, RegenerateQwcForm, nextStep) and make these changes so validation is handled at the control level.
54-56:continueis a JavaScript reserved keyword.While TypeScript allows reserved words as method names on classes and Angular resolves template bindings by string, using
continueas a method name is confusing and could trip up developers or certain tooling. Consider renaming to something likeonContinue()orhandleContinue().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts` around lines 54 - 56, The class method named continue() should be renamed to a non-reserved identifier (e.g., onContinue() or handleContinue()) to avoid confusion and tooling issues; update the method declaration in qbd-direct-qwc-file-landing component (replace continue() with onContinue()/handleContinue()) and then update all references (Angular templates, event bindings, unit tests, and any direct calls) to use the new name so behavior remains identical.
🤖 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/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.html`:
- Around line 54-61: The custom radio markup hides the native input
(formControlName="nextStep") with tw-opacity-0 but provides no visible focus
indicator for keyboard users; update the template/CSS so when the input (the
radio bound to NextStepOption.EXISTING / selectedNextStep) receives focus you
show a visible focus ring on the parent container or the custom icon (e.g.,
using input:focus + .custom-icon or input:focus ~ .icon-wrapper classes) and
ensure the focus style is accessible (contrast, visible outline) while
preserving current visuals for mouse users.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts`:
- Around line 58-67: The copyQwcFilePath method calls
navigator.clipboard.writeText but does not handle its returned Promise; wrap the
call in proper async handling (make copyQwcFilePath async and await writeText or
attach .then/.catch) and handle failures by catching the rejection and showing
an error toast via this.toastService (and optionally logging the error) while
preserving the existing success toast when the write succeeds; update references
to copyQwcFilePath accordingly.
---
Duplicate comments:
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.scss`:
- Around line 4-6: Update the &:hover box-shadow declaration to use modern
color-function notation and remove units on zero values: replace "box-shadow:
0px 4px 4px 0px rgba(44, 48, 78, 0.10);" with "box-shadow: 0 4px 4px 0 rgb(44 48
78 / 0.1);" in qbd-direct-qwc-file-landing.component.scss (target the &:hover
rule).
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts`:
- Around line 42-44: The getter selectedNextStep currently casts a possibly-null
form control value to NextStepOption; change its signature to return
NextStepOption | null and return the control value without forced casting to
honestly represent nullability (update references to selectedNextStep where
non-null is required to handle the null case or add explicit checks). Locate
selectedNextStep and the regenerateQwcForm.get('nextStep') usage in
qbd-direct-qwc-file-landing.component to apply the type change and adjust any
callsites accordingly.
- Around line 85-88: The subscription to
qbdDirectConnectorService.getQBDConnectorSettings() lacks error handling so
isLoading stays true on failure; update the call (in
qbd-direct-qwc-file-landing.component.ts) to handle errors by either providing
an error callback to subscribe or using pipe(finalize(...)) / catchError, ensure
this.isLoading = false in the error/finalize path, and surface the failure
(e.g., set a user-facing error flag/message or log via console/process logger)
while leaving this.connectorSettings unchanged or cleared as appropriate.
---
Nitpick comments:
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.html`:
- Around line 145-154: There is a duplicate conditional block checking isLoading
for the footer; remove this extra "@if (!isLoading)" and move the
<app-configuration-step-footer> into the main content's non-loading branch (the
existing else/@if block that renders the page content) so the footer is rendered
once when isLoading is false; ensure you keep the same inputs/outputs
([ctaText]="ConfigurationCtaText.CONTINUE", [showBackButton]="false",
[isButtonDisabled]="regenerateQwcForm.invalid", (continue)="continue()") and
delete the redundant conditional wrapper around the footer to consolidate the
logic.
- Around line 2-7: Replace the two separate Razor/Angular-like conditionals that
check isLoading with a single if/else to avoid evaluating both branches: update
the template in qbd-direct-qwc-file-landing.component.html so the first
conditional uses `@if` (isLoading) { ... } and the mutually exclusive branch
becomes `@else` { ... } instead of `@if` (!isLoading) { ... }; adjust only the
template markup around the isLoading blocks to use `@else` and keep the existing
loader and content nodes (app-loader and the subsequent content) unchanged.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts`:
- Around line 71-83: Replace the custom group-level validator on
regenerateQwcForm with a simple required validator on the nextStep control: when
creating the form via formBuilder.group<RegenerateQwcForm> pass nextStep: new
FormControl(null, Validators.required) and remove the validators:{...} block;
update imports to remove ValidationErrors if it becomes unused. Locate the form
initialization in qbd-direct-qwc-file-landing.component (regenerateQwcForm,
formBuilder.group, RegenerateQwcForm, nextStep) and make these changes so
validation is handled at the control level.
- Around line 54-56: The class method named continue() should be renamed to a
non-reserved identifier (e.g., onContinue() or handleContinue()) to avoid
confusion and tooling issues; update the method declaration in
qbd-direct-qwc-file-landing component (replace continue() with
onContinue()/handleContinue()) and then update all references (Angular
templates, event bindings, unit tests, and any direct calls) to use the new name
so behavior remains identical.
...d/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.html
Show resolved
Hide resolved
...red/qbd-direct-qwc-file/qbd-direct-qwc-file-landing/qbd-direct-qwc-file-landing.component.ts
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Clickup
https://app.clickup.com/t/86d1x1yra
Summary by CodeRabbit
New Features
Style