feat: existing file path flow#1764
feat: existing file path flow#1764JustARatherRidiculouslyLongUsername wants to merge 1 commit intoqwc-regen-back-buttonfrom
Conversation
If 'existing file path' option is selected, - Skip the download prompt and go straight to the connector - Download the existing qwc file - Hide step number - Show warnings based on existing / new flow in the connector (step 2)
WalkthroughUpdates QBD Direct onboarding to support two flow types (new and existing integrations) with service-level caching. Implements cache control via decorators on connector settings methods, renames the post method for clarity, adds conditional UI rendering based on flow type, updates branding styles, and introduces warning messages for user guidance. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QbdRegenerateCmp as QbdRegenerate<br/>Component
participant ConnectorSvc as Connector<br/>Service
participant Cache as Cache Layer
participant Upstream as Upstream State
rect rgba(100, 150, 200, 0.5)
Note over User,Upstream: NEW Flow Path
User->>QbdRegenerateCmp: Load (flowType: NEW)
QbdRegenerateCmp->>QbdRegenerateCmp: Initialize UI (no preload)
User->>QbdRegenerateCmp: Click Prerequisites Continue
QbdRegenerateCmp->>QbdRegenerateCmp: handlePrerequisitesContinue()
QbdRegenerateCmp->>QbdRegenerateCmp: Set state → DOWNLOAD
end
rect rgba(150, 200, 100, 0.5)
Note over User,Upstream: EXISTING Flow Path
User->>QbdRegenerateCmp: Load (flowType: EXISTING)
QbdRegenerateCmp->>ConnectorSvc: getQBDConnectorSettings()
ConnectorSvc->>Cache: Check cache
Cache-->>ConnectorSvc: Return cached settings (or fetch new)
ConnectorSvc-->>QbdRegenerateCmp: password, xmlFileContent
QbdRegenerateCmp->>QbdRegenerateCmp: Populate UI with credentials
User->>QbdRegenerateCmp: Click Prerequisites Continue
QbdRegenerateCmp->>Upstream: Set onboarding state → PENDING_QWC_UPLOAD
QbdRegenerateCmp->>QbdRegenerateCmp: Transition → SETUP_CONNECTION
QbdRegenerateCmp->>ConnectorSvc: postQbdConnectorSettings(payload)
ConnectorSvc->>Cache: Bust cache via notifier
ConnectorSvc-->>QbdRegenerateCmp: Updated settings
QbdRegenerateCmp->>QbdRegenerateCmp: Trigger download
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
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/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts (1)
246-255:⚠️ Potential issue | 🟠 MajorEXISTING flow gets stuck on connection error —
handleStatusneeds flow-aware recoveryWhen connection fails with
INCORRECT_COMPANY_PATHorCOMPANY_NAME_MISMATCH, both branches setstatetoQwcFlowState.DOWNLOAD. In the EXISTING flow this leaves the UI blank after the user dismisses the dialog:
- The download section is guarded by
@if (flowType === QwcRegenerationFlowType.NEW)in the template → not rendered.- The setup-connection block has
[showSection]="state() >= QwcFlowState.SETUP_CONNECTION"→ hidden when state isDOWNLOAD.
closeDialog()only setsisDialogVisibleto false and doesn't change state, so the user is stranded on an empty page with no recovery path.For the EXISTING flow these error states should keep (or restore) state to
QwcFlowState.SETUP_CONNECTIONso the user can retry.🐛 Proposed fix
} else if (state === QbdDirectOnboardingState.INCORRECT_COMPANY_PATH) { this.warningDialogText.set(this.translocoService.translate('qbdDirectRegenerateQwcFile.incorrectCompanyPathMessage')); this.isDialogVisible.set(true); - this.state.set(QwcFlowState.DOWNLOAD); - this.isCompanyPathInvalid.set(true); + if (this.flowType === QwcRegenerationFlowType.NEW) { + this.state.set(QwcFlowState.DOWNLOAD); + this.isCompanyPathInvalid.set(true); + } else { + this.state.set(QwcFlowState.SETUP_CONNECTION); + } } else if (state === QbdDirectOnboardingState.COMPANY_NAME_MISMATCH) { this.warningDialogText.set(this.translocoService.translate('qbdDirectRegenerateQwcFile.companyNameMismatchMessage')); this.isDialogVisible.set(true); - this.state.set(QwcFlowState.DOWNLOAD); - this.isCompanyPathInvalid.set(true); + if (this.flowType === QwcRegenerationFlowType.NEW) { + this.state.set(QwcFlowState.DOWNLOAD); + this.isCompanyPathInvalid.set(true); + } else { + this.state.set(QwcFlowState.SETUP_CONNECTION); + } }🤖 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-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts` around lines 246 - 255, The EXISTING flow gets stuck because both INCORRECT_COMPANY_PATH and COMPANY_NAME_MISMATCH branches set state to QwcFlowState.DOWNLOAD (which hides the setup UI); change those branches in handleStatus so they restore/keep the flow in the retryable step: instead of unconditionally calling state.set(QwcFlowState.DOWNLOAD), set state to QwcFlowState.SETUP_CONNECTION when this.flowType === QwcRegenerationFlowType.EXISTING (otherwise keep DOWNLOAD for NEW), keep setting warningDialogText/isDialogVisible/isCompanyPathInvalid as before, so after the dialog is dismissed the setup-connection UI is shown and the user can retry.
🧹 Nitpick comments (1)
src/app/shared/components/configuration/configuration-info-label/configuration-info-label.component.ts (1)
36-36:readMoreTextdefault should be the translation key, not the pre-translated stringThe template applies
{{ readMoreText | transloco }}, which expectsreadMoreTextto be a translation key. The current default callsthis.translocoService.translate('common.readMoreText')synchronously at construction time.The issue arises because loading translations is asynchronous, while
translate()is synchronous. If translations haven't loaded yet, callingtranslate()at inject/constructor time can return empty results because the translation is yet to be ready. This meansreadMoreTextmay end up as''or the raw key, and then the| translocopipe receives the wrong value.The simplest and most correct fix is to set the default to the key literal so the reactive
| translocopipe handles it:♻️ Proposed fix
- `@Input`() readMoreText: string = this.translocoService.translate('common.readMoreText'); + `@Input`() readMoreText: string = 'common.readMoreText';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/shared/components/configuration/configuration-info-label/configuration-info-label.component.ts` at line 36, The Input property readMoreText is being initialized by calling this.translocoService.translate('common.readMoreText') which can run before translations load and yields incorrect values; change the default initializer to the translation key literal (e.g. 'common.readMoreText') so the template's {{ readMoreText | transloco }} pipe resolves it reactively, and remove the synchronous this.translocoService.translate call from the property initializer (leave translocoService usage elsewhere if needed).
🤖 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-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`:
- Line 285: Remove the redundant assignment "this.isLoading = false" in the
ngOnInit method of QbdDirectRegenerateQwcFileComponent: since isLoading is
initialized to false and never set to true in ngOnInit, delete that line to
clean up dead code and avoid misleading state resets; ensure no other logic
relies on setting isLoading there and run unit/behavioral checks after removal.
- Around line 278-289: ngOnInit currently triggers getQBDConnectorSettings()
without setting isLoading or handling errors, causing race conditions with
handlePrerequisitesContinue()/handleManualDownload(); fix by setting
this.isLoading = true before calling
qbdDirectConnectorService.getQBDConnectorSettings(), add an error handler on
that subscription to set this.isLoading = false and surface/log the error and a
user-facing message, assign password and xmlFileContent only on success and then
set isLoading = false, and ensure handlePrerequisitesContinue() waits for
settings to be loaded before calling handleManualDownload() (either by chaining
the settings request into the continue flow or gating the continue button when
isLoading is true) so xmlFileContent and password are guaranteed populated.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-setup-connection/qbd-direct-setup-connection.component.html`:
- Around line 40-49: In the qbd-direct-setup-connection component template the
container div uses an invalid class "tw-pt-16-pdx"; replace that typo with valid
Tailwind spacing classes (e.g., split into "tw-pt-16 tw-px-4" or whatever
horizontal padding value is appropriate) so the padding is applied; update the
class on the div wrapping <app-configuration-info-label> accordingly.
In `@src/assets/i18n/en.json`:
- Around line 2796-2798: Reorder the child keys inside the
qbdDirectRegenerateQwcFile object so they are alphabetically sorted: place
"somethingWentWrong" first, then "warningTextExistingPath", and then
"warningTextNewPath"; update the JSON object in en.json accordingly so the child
keys follow this alphabetical order to reduce merge conflicts.
---
Outside diff comments:
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`:
- Around line 246-255: The EXISTING flow gets stuck because both
INCORRECT_COMPANY_PATH and COMPANY_NAME_MISMATCH branches set state to
QwcFlowState.DOWNLOAD (which hides the setup UI); change those branches in
handleStatus so they restore/keep the flow in the retryable step: instead of
unconditionally calling state.set(QwcFlowState.DOWNLOAD), set state to
QwcFlowState.SETUP_CONNECTION when this.flowType ===
QwcRegenerationFlowType.EXISTING (otherwise keep DOWNLOAD for NEW), keep setting
warningDialogText/isDialogVisible/isCompanyPathInvalid as before, so after the
dialog is dismissed the setup-connection UI is shown and the user can retry.
---
Nitpick comments:
In
`@src/app/shared/components/configuration/configuration-info-label/configuration-info-label.component.ts`:
- Line 36: The Input property readMoreText is being initialized by calling
this.translocoService.translate('common.readMoreText') which can run before
translations load and yields incorrect values; change the default initializer to
the translation key literal (e.g. 'common.readMoreText') so the template's {{
readMoreText | transloco }} pipe resolves it reactively, and remove the
synchronous this.translocoService.translate call from the property initializer
(leave translocoService usage elsewhere if needed).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/app/branding/c1/style-config.tssrc/app/branding/fyle/style-config.tssrc/app/core/services/qbd-direct/qbd-direct-configuration/qbd-direct-connector.service.tssrc/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.tssrc/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-data-sync/qbd-direct-data-sync.component.htmlsrc/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-download-file/qbd-direct-download-file.component.htmlsrc/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.htmlsrc/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-qwc-file/qbd-direct-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.tssrc/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-setup-connection/qbd-direct-setup-connection.component.htmlsrc/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-setup-connection/qbd-direct-setup-connection.component.tssrc/app/shared/components/configuration/configuration-info-label/configuration-info-label.component.htmlsrc/app/shared/components/configuration/configuration-info-label/configuration-info-label.component.scsssrc/app/shared/components/configuration/configuration-info-label/configuration-info-label.component.tssrc/assets/i18n/en.json
| ngOnInit(): void { | ||
| this.route.data.subscribe(data => { | ||
| this.flowType = data.flowType; | ||
| if (this.flowType === QwcRegenerationFlowType.EXISTING) { | ||
| this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe((qbdConnectorSettings) => { | ||
| this.password.set(qbdConnectorSettings.password); | ||
| this.xmlFileContent = qbdConnectorSettings.qwc; | ||
| this.isLoading = false; | ||
| }); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Race condition: xmlFileContent may be empty when handleManualDownload() fires; also missing error handling
ngOnInit fires getQBDConnectorSettings() without setting isLoading = true first, so no spinner is shown and there is no guarantee the response has arrived before the user clicks through the prerequisites and triggers handlePrerequisitesContinue(). That method calls handleManualDownload() immediately after updateWorkspaceOnboardingState() resolves — independently of the connector settings call — so the downloaded .qwc file and the displayed password can both be empty if the first request is still in-flight.
Additionally, there is no error handler on getQBDConnectorSettings(): a failure silently leaves xmlFileContent and password empty for the rest of the session.
🐛 Proposed fix — show loading state and handle errors
ngOnInit(): void {
this.route.data.subscribe(data => {
this.flowType = data.flowType;
if (this.flowType === QwcRegenerationFlowType.EXISTING) {
+ this.isLoading = true;
this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe({
- next: (qbdConnectorSettings) => {
+ next: (qbdConnectorSettings) => {
this.password.set(qbdConnectorSettings.password);
this.xmlFileContent = qbdConnectorSettings.qwc;
this.isLoading = false;
},
+ error: (error) => {
+ console.error('Error loading connector settings:', error);
+ this.toastService.displayToastMessage(ToastSeverity.ERROR, this.translocoService.translate('qbdDirectRegenerateQwcFile.somethingWentWrong'));
+ this.isLoading = false;
+ }
});
}
});
}Also ensure handlePrerequisitesContinue() does not call handleManualDownload() until the settings are confirmed loaded (e.g., by using a combineLatest / chaining the two calls, or gating the continue button while isLoading is true).
🤖 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-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`
around lines 278 - 289, ngOnInit currently triggers getQBDConnectorSettings()
without setting isLoading or handling errors, causing race conditions with
handlePrerequisitesContinue()/handleManualDownload(); fix by setting
this.isLoading = true before calling
qbdDirectConnectorService.getQBDConnectorSettings(), add an error handler on
that subscription to set this.isLoading = false and surface/log the error and a
user-facing message, assign password and xmlFileContent only on success and then
set isLoading = false, and ensure handlePrerequisitesContinue() waits for
settings to be loaded before calling handleManualDownload() (either by chaining
the settings request into the continue flow or gating the continue button when
isLoading is true) so xmlFileContent and password are guaranteed populated.
| this.qbdDirectConnectorService.getQBDConnectorSettings().subscribe((qbdConnectorSettings) => { | ||
| this.password.set(qbdConnectorSettings.password); | ||
| this.xmlFileContent = qbdConnectorSettings.qwc; | ||
| this.isLoading = false; |
There was a problem hiding this comment.
Redundant this.isLoading = false
isLoading is initialised to false and is never set to true anywhere in ngOnInit, so this assignment is a no-op and can be removed.
♻️ Proposed fix
next: (qbdConnectorSettings) => {
this.password.set(qbdConnectorSettings.password);
this.xmlFileContent = qbdConnectorSettings.qwc;
- 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.
| this.isLoading = false; | |
| next: (qbdConnectorSettings) => { | |
| this.password.set(qbdConnectorSettings.password); | |
| this.xmlFileContent = qbdConnectorSettings.qwc; | |
| }, |
🤖 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-regenerate-qwc-file/qbd-direct-regenerate-qwc-file.component.ts`
at line 285, Remove the redundant assignment "this.isLoading = false" in the
ngOnInit method of QbdDirectRegenerateQwcFileComponent: since isLoading is
initialized to false and never set to true in ngOnInit, delete that line to
clean up dead code and avoid misleading state resets; ensure no other logic
relies on setting isLoading there and run unit/behavioral checks after removal.
| @if (warningMessage) { | ||
| <div class="tw-pt-16-pdx"> | ||
| <app-configuration-info-label | ||
| [iconSrc]="'warning-filled'" | ||
| [customBackgroundColorClass]="brandingStyle.configuration.infoLabelWarningBackground" | ||
| [customStyleClass]="brandingStyle.configuration.infoLabelWarningIcon" | ||
| [infoText]="warningMessage" | ||
| [readMoreLink]="warningArticleLink" | ||
| ></app-configuration-info-label> | ||
| </div> |
There was a problem hiding this comment.
Fix typo in spacing class to avoid missing padding (Line 41).
tw-pt-16-pdx looks like a typo and won’t match Tailwind’s px suffix.
🩹 Suggested fix
- <div class="tw-pt-16-pdx">
+ <div class="tw-pt-16-px">📝 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.
| @if (warningMessage) { | |
| <div class="tw-pt-16-pdx"> | |
| <app-configuration-info-label | |
| [iconSrc]="'warning-filled'" | |
| [customBackgroundColorClass]="brandingStyle.configuration.infoLabelWarningBackground" | |
| [customStyleClass]="brandingStyle.configuration.infoLabelWarningIcon" | |
| [infoText]="warningMessage" | |
| [readMoreLink]="warningArticleLink" | |
| ></app-configuration-info-label> | |
| </div> | |
| `@if` (warningMessage) { | |
| <div class="tw-pt-16-px"> | |
| <app-configuration-info-label | |
| [iconSrc]="'warning-filled'" | |
| [customBackgroundColorClass]="brandingStyle.configuration.infoLabelWarningBackground" | |
| [customStyleClass]="brandingStyle.configuration.infoLabelWarningIcon" | |
| [infoText]="warningMessage" | |
| [readMoreLink]="warningArticleLink" | |
| ></app-configuration-info-label> | |
| </div> |
🤖 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-setup-connection/qbd-direct-setup-connection.component.html`
around lines 40 - 49, In the qbd-direct-setup-connection component template the
container div uses an invalid class "tw-pt-16-pdx"; replace that typo with valid
Tailwind spacing classes (e.g., split into "tw-pt-16 tw-px-4" or whatever
horizontal padding value is appropriate) so the padding is applied; update the
class on the div wrapping <app-configuration-info-label> accordingly.
| "somethingWentWrong": "Something went wrong. Please try again later.", | ||
| "warningTextNewPath": "Remove the previous integration from the Web Connector and change the OwnerID/FileID to avoid QBWC1039 error.", | ||
| "warningTextExistingPath": "Change the OwnerID/FileID to avoid QBWC1039 Error." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check missing keys for qbdDirectRegenerateQwcFile across locales
python - <<'PY'
import json, glob, sys
base = "src/assets/i18n"
with open(f"{base}/en.json") as f:
en = json.load(f)
keys = set(en.get("qbdDirectRegenerateQwcFile", {}).keys())
missing = {}
for path in glob.glob(f"{base}/*.json"):
if path.endswith("/en.json"):
continue
with open(path) as f:
data = json.load(f)
obj = data.get("qbdDirectRegenerateQwcFile", {})
miss = keys - set(obj.keys())
if miss:
missing[path] = sorted(miss)
if missing:
print("Missing keys in other locales:")
for k, v in sorted(missing.items()):
print(f"{k}: {v}")
sys.exit(1)
print("✓ All locales contain all qbdDirectRegenerateQwcFile keys.")
PYRepository: fylein/fyle-integrations-app
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Check alphabetical ordering of child keys in qbdDirectRegenerateQwcFile
python - <<'PY'
import json
with open("src/assets/i18n/en.json") as f:
en = json.load(f)
obj = en.get("qbdDirectRegenerateQwcFile", {})
if not obj:
print("qbdDirectRegenerateQwcFile not found")
else:
keys = list(obj.keys())
sorted_keys = sorted(obj.keys())
print(f"Current order: {keys}")
print(f"Alphabetical order: {sorted_keys}")
if keys == sorted_keys:
print("✓ Keys are properly sorted alphabetically")
else:
print("✗ Keys are NOT sorted alphabetically")
for i, (current, expected) in enumerate(zip(keys, sorted_keys)):
if current != expected:
print(f" Position {i}: '{current}' should be '{expected}'")
PYRepository: fylein/fyle-integrations-app
Length of output: 1777
Reorder qbdDirectRegenerateQwcFile keys alphabetically.
The child keys in qbdDirectRegenerateQwcFile are not alphabetically sorted. The entire object needs to be reordered to comply with the requirement: "Keep i18n child keys alphabetically sorted to reduce merge conflicts." The three new keys (lines 2796–2798) should be positioned at: somethingWentWrong, then warningTextExistingPath, then warningTextNewPath.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/assets/i18n/en.json` around lines 2796 - 2798, Reorder the child keys
inside the qbdDirectRegenerateQwcFile object so they are alphabetically sorted:
place "somethingWentWrong" first, then "warningTextExistingPath", and then
"warningTextNewPath"; update the JSON object in en.json accordingly so the child
keys follow this alphabetical order to reduce merge conflicts.
Description
If 'existing file path' option is selected,
Clickup
https://app.clickup.com/t/86d1x6zz4
Summary by CodeRabbit
Release Notes
New Features
Improvements