feat: back button#1763
feat: back button#1763JustARatherRidiculouslyLongUsername wants to merge 1 commit intoqwc-regen-new-path-flowfrom
Conversation
WalkthroughThe pull request adds back-navigation support to the QBD Direct onboarding flow. Changes include introducing a new DONE CTA enum value, adding back-navigation inputs/outputs and event handlers across multiple components, refactoring footer visibility logic in the setup connection component, and implementing loading-aware back button disabling with error messaging. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant RQC as QbdDirectRegenerateQwcFileComponent
participant DLF as QbdDirectDownloadFileComponent
participant SCN as QbdDirectSetupConnectionComponent
participant CSF as ConfigurationStepFooter
participant API as Workspace Service
User->>DLF: Clicks back button
DLF->>DLF: onBackClick()
DLF->>RQC: Emits back event
RQC->>RQC: handleBackClick()
rect rgba(100, 149, 237, 0.5)
alt State > SETUP_CONNECTION
RQC->>API: Update to PENDING_QWC_UPLOAD
API-->>RQC: Success/Error
opt On Error
RQC->>User: Show error toast
end
RQC->>RQC: Set state to SETUP_CONNECTION
else State > DOWNLOAD
RQC->>RQC: Set state to DOWNLOAD
else
RQC->>RQC: Set state to PREREQUISITES
end
end
alt showAssistedSetup = true
RQC->>SCN: Render with showAssistedSetup=true
SCN->>CSF: Hide back button
else showAssistedSetup = false
RQC->>SCN: Render with showAssistedSetup=false
SCN->>CSF: Show back button
alt isLoading = true
CSF->>CSF: Disable back button
end
end
User->>CSF: Back button click
CSF->>SCN: Emit navigateToPreviousStep
SCN->>RQC: Emit back event
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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: 1
🧹 Nitpick comments (2)
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)
131-149: Enum-ordering dependency makes back navigation fragile to refactoring.The current code relies on
QwcFlowState's numeric ordering (SETUP_CONNECTION = 4,DOWNLOAD = 1, etc.) for back navigation. While this works today, the logic is implicit and brittle: reordering enum values would silently break navigation without compiler warnings. Consider using an explicit state-to-previous-state mapping for clarity and maintainability.💡 Example: Explicit state mapping
handleBackClick(): void { - if (this.state() > QwcFlowState.SETUP_CONNECTION) { + const backTargetByState: Record<QwcFlowState, QwcFlowState> = { + [QwcFlowState.CONNECTION_DONE]: QwcFlowState.SETUP_CONNECTION, + [QwcFlowState.CONNECTION_IN_PROGRESS]: QwcFlowState.SETUP_CONNECTION, + [QwcFlowState.SETUP_CONNECTION]: QwcFlowState.DOWNLOAD, + [QwcFlowState.DOWNLOAD_DONE]: QwcFlowState.DOWNLOAD, + [QwcFlowState.DOWNLOAD_IN_PROGRESS]: QwcFlowState.DOWNLOAD, + [QwcFlowState.DOWNLOAD]: QwcFlowState.PREREQUISITES, + [QwcFlowState.PREREQUISITES]: QwcFlowState.PREREQUISITES + }; + + if (this.state() === QwcFlowState.CONNECTION_DONE || this.state() === QwcFlowState.CONNECTION_IN_PROGRESS) { this.workspaceService.updateWorkspaceOnboardingState({ onboarding_state: QbdDirectOnboardingState.PENDING_QWC_UPLOAD }).subscribe({ next: () => { this.state.set(QwcFlowState.SETUP_CONNECTION); }, error: (error) => { console.error('Error updating workspace onboarding state:', error); this.toastService.displayToastMessage(ToastSeverity.ERROR, this.translocoService.translate('qbdDirectRegenerateQwcFile.backError')); } }); - } else if (this.state() > QwcFlowState.DOWNLOAD) { - this.state.set(QwcFlowState.DOWNLOAD); } else { - this.state.set(QwcFlowState.PREREQUISITES); + this.state.set(backTargetByState[this.state()] ?? QwcFlowState.PREREQUISITES); }🤖 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 131 - 149, The back-navigation currently compares numeric enum ordering (in handleBackClick using QwcFlowState) which is brittle; replace it with an explicit mapping from each QwcFlowState value to its previous state (e.g., const PREVIOUS_STATE: Record<QwcFlowState, QwcFlowState | null>), then in handleBackClick look up the previous = PREVIOUS_STATE[this.state()] and if previous is null use a safe fallback; if previous equals QwcFlowState.SETUP_CONNECTION still perform workspaceService.updateWorkspaceOnboardingState({ onboarding_state: QbdDirectOnboardingState.PENDING_QWC_UPLOAD }).subscribe(...) before calling this.state.set(previous) and otherwise just call this.state.set(previous); ensure mapping covers all QwcFlowState values so future enum reordering won’t break navigation.src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-setup-connection/qbd-direct-setup-connection.component.html (1)
141-141: Duplicate!isStepCompletedcondition.Line 141 checks
!isStepCompletedtwice in the same@ifexpression. While this is pre-existing, it's worth cleaning up.- `@if` (!isLoading && connectionStatus !== qbdConnectionStatus.SUCCESS && showSection && !isStepCompleted) { + `@if` (!isLoading && !isStepCompleted && connectionStatus !== qbdConnectionStatus.SUCCESS && showSection) {🤖 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` at line 141, The template `@if` condition contains a duplicated check for !isStepCompleted; update the `@if` expression in the qbd-direct-setup-connection component template to remove the redundant !isStepCompleted so it reads something like: check !isLoading, !isStepCompleted (only once), connectionStatus === qbdConnectionStatus.SUCCESS, and showSection; keep the other symbols unchanged (isLoading, isStepCompleted, connectionStatus, qbdConnectionStatus.SUCCESS, showSection) to preserve logic.
🤖 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/assets/i18n/en.json`:
- Around line 2795-2796: The keys inside the qbdDirectRegenerateQwcFile
translation block are out of alphabetical order; reorder the keys so they are
sorted lexicographically (e.g., ensure "backError" appears before
"companyNameMismatchMessage", "connectionError", etc.) by locating the
qbdDirectRegenerateQwcFile object in src/assets/i18n/en.json and reordering its
property keys alphabetically while preserving their values and commas.
---
Nitpick 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 131-149: The back-navigation currently compares numeric enum
ordering (in handleBackClick using QwcFlowState) which is brittle; replace it
with an explicit mapping from each QwcFlowState value to its previous state
(e.g., const PREVIOUS_STATE: Record<QwcFlowState, QwcFlowState | null>), then in
handleBackClick look up the previous = PREVIOUS_STATE[this.state()] and if
previous is null use a safe fallback; if previous equals
QwcFlowState.SETUP_CONNECTION still perform
workspaceService.updateWorkspaceOnboardingState({ onboarding_state:
QbdDirectOnboardingState.PENDING_QWC_UPLOAD }).subscribe(...) before calling
this.state.set(previous) and otherwise just call this.state.set(previous);
ensure mapping covers all QwcFlowState values so future enum reordering won’t
break navigation.
In
`@src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-setup-connection/qbd-direct-setup-connection.component.html`:
- Line 141: The template `@if` condition contains a duplicated check for
!isStepCompleted; update the `@if` expression in the qbd-direct-setup-connection
component template to remove the redundant !isStepCompleted so it reads
something like: check !isLoading, !isStepCompleted (only once), connectionStatus
=== qbdConnectionStatus.SUCCESS, and showSection; keep the other symbols
unchanged (isLoading, isStepCompleted, connectionStatus,
qbdConnectionStatus.SUCCESS, showSection) to preserve logic.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/app/core/models/enum/enum.model.tssrc/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-download-file/qbd-direct-download-file.component.tssrc/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-step-footer/configuration-step-footer.component.htmlsrc/app/shared/components/configuration/configuration-step-footer/configuration-step-footer.component.tssrc/assets/i18n/en.json
| "connectionSuccessToast": "QuickBooks Desktop connection successful", | ||
| "backError": "Something went wrong. Please try again later." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, list all locale files
ls -la src/assets/i18n/*.jsonRepository: fylein/fyle-integrations-app
Length of output: 146
🏁 Script executed:
#!/bin/bash
# Run the provided verification script
python - <<'PY'
import json, glob
key_path = ["qbdDirectRegenerateQwcFile", "backError"]
for path in sorted(glob.glob("src/assets/i18n/*.json")):
with open(path, "r", encoding="utf-8") as f:
data = json.load(f)
d = data
found = True
for k in key_path:
if k not in d:
print(f"Missing {'.'.join(key_path)} in {path}")
found = False
break
d = d[k]
if found:
print(f"✓ {'.'.join(key_path)} exists in {path}")
PYRepository: fylein/fyle-integrations-app
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Check the structure and sorting of qbdDirectRegenerateQwcFile block in en.json
python - <<'PY'
import json
with open("src/assets/i18n/en.json", "r", encoding="utf-8") as f:
data = json.load(f)
if "qbdDirectRegenerateQwcFile" in data:
block = data["qbdDirectRegenerateQwcFile"]
keys = list(block.keys())
sorted_keys = sorted(keys)
print("Current order of keys in qbdDirectRegenerateQwcFile:")
for i, key in enumerate(keys):
print(f" {i+1}. {key}")
print("\nExpected alphabetical order:")
for i, key in enumerate(sorted_keys):
print(f" {i+1}. {key}")
is_sorted = keys == sorted_keys
print(f"\nIs block alphabetically sorted? {is_sorted}")
if not is_sorted:
print("\nSorting issues found:")
for i in range(len(keys) - 1):
if keys[i] > keys[i+1]:
print(f" '{keys[i]}' should come after '{keys[i+1]}'")
PYRepository: fylein/fyle-integrations-app
Length of output: 1234
Fix the alphabetical ordering of keys in the qbdDirectRegenerateQwcFile block.
The keys in this block are not alphabetically sorted. backError should appear first, followed by companyNameMismatchMessage, connectionError, etc.
Alphabetically sorted block
"qbdDirectRegenerateQwcFile": {
- "headerTitleExistingPath": "Existing QWC file path",
- "headerTitleNewPath": "New QWC file path",
- "headerSubTitleExistingPath": "Current file stored in Fyle",
- "headerSubTitleNewPath": "New file to be generated",
- "downloadError": "Error downloading file. Please try again",
- "connectionFailed": "Connection failed",
- "gotIt": "Got it",
- "connectionError": "Connection error",
- "incorrectCompanyPathMessage": "Incorrect company path",
- "incorrectPasswordMessage": "Incorrect password",
- "companyNameMismatchMessage": "Company name mismatch",
- "connectionSuccessToast": "QuickBooks Desktop connection successful",
- "backError": "Something went wrong. Please try again later."
+ "backError": "Something went wrong. Please try again later.",
+ "companyNameMismatchMessage": "Company name mismatch",
+ "connectionError": "Connection error",
+ "connectionFailed": "Connection failed",
+ "connectionSuccessToast": "QuickBooks Desktop connection successful",
+ "downloadError": "Error downloading file. Please try again",
+ "gotIt": "Got it",
+ "headerSubTitleExistingPath": "Current file stored in Fyle",
+ "headerSubTitleNewPath": "New file to be generated",
+ "headerTitleExistingPath": "Existing QWC file path",
+ "headerTitleNewPath": "New QWC file path",
+ "incorrectCompanyPathMessage": "Incorrect company path",
+ "incorrectPasswordMessage": "Incorrect password"
}🤖 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 2795 - 2796, The keys inside the
qbdDirectRegenerateQwcFile translation block are out of alphabetical order;
reorder the keys so they are sorted lexicographically (e.g., ensure "backError"
appears before "companyNameMismatchMessage", "connectionError", etc.) by
locating the qbdDirectRegenerateQwcFile object in src/assets/i18n/en.json and
reordering its property keys alphabetically while preserving their values and
commas.
Description
Clickup
https://app.clickup.com/t/86d1x6zz4
Summary by CodeRabbit
Release Notes
New Features
Improvements