feat: qbd direct onboarding landing page #1056
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces several updates across multiple files to enhance branding configurations and onboarding processes. Key changes include dynamic incorporation of Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OnboardingComponent
participant WorkspaceService
participant API
User->>OnboardingComponent: Initiate connection
OnboardingComponent->>WorkspaceService: connectQbdDirect()
WorkspaceService->>API: PATCH update onboarding state
API-->>WorkspaceService: Response
WorkspaceService-->>OnboardingComponent: Update state
OnboardingComponent-->>User: Show onboarding progress
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -6,7 +6,7 @@ export const brandingConfig: BrandingConfiguration = config as BrandingConfigura | |||
| export const c1Contents = { | |||
| qbd_direct: { | |||
| landing: { | |||
| contentText: 'Import chart of accounts and projects from NetSuite and export expenses from your Expense Management account.', | |||
| contentText: 'Import data from QuickBooks Desktop to Fyle and export expenses from Fyle to QuickBooks Desktop', | |||
There was a problem hiding this comment.
don't use "Fyle" keyword in c1 file, use expense management / use brandName secret
| @@ -7,3 +7,7 @@ export type Workspace = { | |||
| created_at: Date; | |||
| updated_at: Date; | |||
| } | |||
|
|
|||
| export type updateWorkspaceOnboardingStatePost = { | |||
There was a problem hiding this comment.
WorkspaceOnboardingState
| @@ -74,4 +73,8 @@ export class WorkspaceService { | |||
| getWorkspaceGeneralSettings(): Observable<any> { | |||
| return this.apiService.get(`/workspaces/${this.getWorkspaceId()}/settings/general/`, {}); | |||
| } | |||
|
|
|||
| updateWorkspaceOnboardingState(payload: updateWorkspaceOnboardingStatePost): Observable<any> { | |||
There was a problem hiding this comment.
Workspace is a generic type, we will miss some properties, so using Workspace will create errors, while calling API we can use our model for that
There was a problem hiding this comment.
API would return whole workspace model
There was a problem hiding this comment.
yes, but in Workspace model we are only having the common keys only, and we interface with workspace model and write the integration respective workspace mode. in the Workspace we don't have onboarding_state key. I am using it to store the onboarding state in local storage. that why
There was a problem hiding this comment.
okay sorry, I meant QBDDirectWorkspace
|
|
||
| embedVideoLink = brandingDemoVideoLinks.onboarding.QBD_DIRECT; | ||
|
|
||
| isQbdDirectConnectionInProcess = false; |
There was a problem hiding this comment.
isQbdConnectionInProgress
| @@ -40,9 +40,10 @@ | |||
| <button *ngIf="isIntegrationConnected && (appName === AppName.BAMBOO_HR || appName === AppName.TRAVELPERK)" pButton type="button" class="p-button danger-outline" (click)="disconnect()"> | |||
| {{ isConnectionInProgress ? 'Disconnecting' : 'Disconnect' }} | |||
| </button> | |||
| <button *ngIf="!isIntegrationConnected && (appName === AppName.QBD || appName === AppName.NETSUITE || appName === AppName.INTACCT || appName === AppName.SAGE300 || appName === AppName.BUSINESS_CENTRAL)" pButton type="button" class="p-button-raised" (click)="connect()"> | |||
| <button *ngIf="!isIntegrationConnected && (appName === AppName.QBD || appName === AppName.NETSUITE || appName === AppName.INTACCT || appName === AppName.SAGE300 || appName === AppName.BUSINESS_CENTRAL || appName.includes(AppName.QBD_DIRECT))" pButton type="button" class="p-button-raised" (click)="connect()"> | |||
There was a problem hiding this comment.
why not appName === AppName.QBD_DIRECT ?
There was a problem hiding this comment.
this is due to the name, we use app name here as 'QuickBooks Desktop - Direct Integration'
| {{buttonText}} | ||
| <app-svg-icon *ngIf="appName === AppName.QBD && brandingFeatureConfig.isIconsInsideButtonAllowed" [svgSource]="'arrow-tail-right-medium'" [width]="'18px'" [height]="'18px'" [styleClasses]="'tw-pl-10-px tw-pt-2-px !tw-text-12-px'"></app-svg-icon> | ||
| <app-svg-icon *ngIf="(appName === AppName.QBD || appName.includes(AppName.QBD_DIRECT)) && brandingFeatureConfig.isIconsInsideButtonAllowed" [svgSource]="'arrow-tail-right-medium'" [width]="'18px'" [height]="'18px'" [styleClasses]="'tw-pl-10-px tw-pt-2-px !tw-text-12-px'"></app-svg-icon> |
| @@ -90,8 +90,7 @@ export class AppLandingPageHeaderComponent implements OnInit { | |||
| } | |||
|
|
|||
| connect(): void { | |||
|
|
|||
| if (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO) { | |||
| if (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO || this.appName.includes(AppName.QBD_DIRECT)) { | |||
src/assets/logos/qbd-direct-logo.svg
Outdated
There was a problem hiding this comment.
use QBO logo from the repo present already
There was a problem hiding this comment.
QA wont affect, if we use different logo,
There was a problem hiding this comment.
let's use this assets/logos/qbo.png
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (10)
src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-landing/qbd-direct-onboarding-landing.component.ts (1)
19-29: Add explicit types to improve type safety.While the properties are initialized correctly, adding explicit types would improve code maintainability and catch potential type-related issues earlier.
Consider applying these type annotations:
- redirectLink = brandingKbArticles.topLevelArticles.QBD_DIRECT; - embedVideoLink = brandingDemoVideoLinks.onboarding.QBD_DIRECT; + redirectLink: string = brandingKbArticles.topLevelArticles.QBD_DIRECT; + embedVideoLink: string = brandingDemoVideoLinks.onboarding.QBD_DIRECT;src/app/core/services/common/workspace.service.ts (1)
78-80: Consider adding error handlingWhile the method implementation is clean and well-typed, consider adding error handling for:
- Invalid workspace IDs
- Failed PATCH requests
- Invalid onboarding state values
Example enhancement:
updateWorkspaceOnboardingState(payload: WorkspaceOnboardingState): Observable<QbdDirectWorkspace> { + const workspaceId = this.getWorkspaceId(); + if (!workspaceId) { + throw new Error('Workspace ID not found'); + } - return this.apiService.patch(`/workspaces/${this.getWorkspaceId()}/onboarding_state/`, payload); + return this.apiService.patch(`/workspaces/${workspaceId}/onboarding_state/`, payload) + .pipe( + catchError((error: any) => { + console.error('Failed to update workspace onboarding state:', error); + throw error; + }) + ); }src/app/shared/components/helper/app-landing-page-header/app-landing-page-header.component.ts (1)
Line range hint
93-102: Add tracking event for QBD Direct OAuth flow.While tracking events exist for other QBD flows (e.g., export_settings), there's no tracking for the new QBD Direct OAuth flow. Consider adding a tracking event to maintain consistency in analytics.
Add the tracking before initiating OAuth:
if (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO || this.appName === AppName.QBD_DIRECT) { + if (this.appName === AppName.QBD_DIRECT) { + this.trackingService.onClickEvent(TrackingApp.QBD, ClickEvent.CONNECT_QBD_DIRECT); + } this.initiateOAuth(); return; }src/app/shared/components/helper/app-landing-page-header/app-landing-page-header.component.html (2)
43-43: Consider refactoring the lengthy condition into a component method.The template contains a long list of conditions that could be moved to the component class for better maintainability. This would also make it easier to test and modify the logic.
Consider refactoring like this:
-*ngIf="!isIntegrationConnected && (appName === AppName.QBD || appName === AppName.NETSUITE || appName === AppName.INTACCT || appName === AppName.SAGE300 || appName === AppName.BUSINESS_CENTRAL || appName.includes(AppName.QBD_DIRECT))" +*ngIf="!isIntegrationConnected && showConnectButton"Add to component class:
get showConnectButton(): boolean { return [ AppName.QBD, AppName.NETSUITE, AppName.INTACCT, AppName.SAGE300, AppName.BUSINESS_CENTRAL ].includes(this.appName) || this.appName.includes(AppName.QBD_DIRECT); }
45-45: Consider refactoring the icon condition for consistency.Similar to the button condition, this logic could be moved to the component class to maintain consistency and improve maintainability.
Consider refactoring like this:
-*ngIf="(appName === AppName.QBD || appName.includes(AppName.QBD_DIRECT)) && brandingFeatureConfig.isIconsInsideButtonAllowed" +*ngIf="showQbdIcon && brandingFeatureConfig.isIconsInsideButtonAllowed"Add to component class:
get showQbdIcon(): boolean { return this.appName === AppName.QBD || this.appName.includes(AppName.QBD_DIRECT); }src/app/branding/c1-contents-config.ts (3)
9-9: Use template literals for better readabilityReplace string concatenation with template literals for improved readability and maintainability.
- contentText: 'Import data from QuickBooks Desktop to ' + brandingConfig.brandName + ' and export expenses from ' + brandingConfig.brandName + ' to QuickBooks Desktop', + contentText: `Import data from QuickBooks Desktop to ${brandingConfig.brandName} and export expenses from ${brandingConfig.brandName} to QuickBooks Desktop`,
81-81: Fix typo and use template literalsThe text contains a capitalization error and uses string concatenation.
- customizeSectionSubLabel: 'In this section, you can customize the data that you\'d like to export from ' + brandingConfig.brandName + ' to QuickBooks Desktop You can choose what data points need to be exported and what shouldn\'t be.', + customizeSectionSubLabel: `In this section, you can customize the data that you'd like to export from ${brandingConfig.brandName} to QuickBooks Desktop. you can choose what data points need to be exported and what shouldn't be.`,
96-96: Remove unnecessary string concatenationThe line starts with an empty string concatenation which is unnecessary.
- autoCreateMerchantsAsVendorsSubLabel: '' + brandingConfig.brandName + ' will auto-create a new vendor in QuickBooks Desktop if a merchant added by an employee does not have a corresponding match in QuickBooks Desktop. ', + autoCreateMerchantsAsVendorsSubLabel: `${brandingConfig.brandName} will auto-create a new vendor in QuickBooks Desktop if a merchant added by an employee does not have a corresponding match in QuickBooks Desktop.`,src/app/branding/fyle-contents-config.ts (1)
Line range hint
1-624: Consider architectural improvements for better maintainabilityThe configuration file could benefit from the following improvements:
- Extract common text patterns into reusable templates
- Create constants for frequently used phrases
- Standardize casing conventions (e.g., "Auto-Create" vs "Auto create")
Example approach:
// Create a template system for common patterns const templates = { importContent: (source: string) => `Import data from ${source} to ${brandingConfig.brandName} and export expenses from ${brandingConfig.brandName} to ${source}`, autoCreate: (entity: string) => `Auto-Create ${entity}` }; // Use templates in config qbd_direct: { landing: { contentText: templates.importContent('QuickBooks Desktop'), // ... } }tailwind.config.js (1)
1134-1134: Consider using CSS variables for fixed dimensions.While adding the fixed height value is valid, consider using CSS variables for better maintainability and consistency across the design system.
- '45-px': '45px', + '45-px': 'var(--height-45)',This approach would:
- Centralize dimension management
- Make system-wide updates easier
- Maintain consistency with other variable-based values in the config
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (5)
src/assets/flow-charts/fyle-qbd-direct-flow-chart.svgis excluded by!**/*.svgsrc/assets/icons/downloadqbd.svgis excluded by!**/*.svgsrc/assets/icons/expand.svgis excluded by!**/*.svgsrc/assets/logos/qbd-direct-logo.svgis excluded by!**/*.svgsrc/assets/sprites/sprite.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
src/app/branding/c1-contents-config.ts(3 hunks)src/app/branding/fyle-contents-config.ts(1 hunks)src/app/core/models/db/workspaces.model.ts(1 hunks)src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-connector.model.ts(1 hunks)src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-onboarding.model.ts(1 hunks)src/app/core/services/common/workspace.service.ts(2 hunks)src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-landing/qbd-direct-onboarding-landing.component.html(1 hunks)src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-landing/qbd-direct-onboarding-landing.component.ts(1 hunks)src/app/shared/components/helper/app-landing-page-header/app-landing-page-header.component.html(1 hunks)src/app/shared/components/helper/app-landing-page-header/app-landing-page-header.component.ts(1 hunks)tailwind.config.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-connector.model.ts
🔇 Additional comments (10)
src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-landing/qbd-direct-onboarding-landing.component.ts (3)
1-16: LGTM! Well-structured component setup.
The component follows Angular best practices with proper standalone configuration and necessary module imports.
31-34: LGTM! Proper dependency injection.
The constructor follows Angular's dependency injection best practices with appropriate access modifiers.
41-41: Verify the navigation path in router configuration.
Ensure that the navigation path /integrations/qbd_direct/onboarding/pre_requisite/ is correctly defined in the router configuration.
✅ Verification successful
Let me generate another script to check the specific route configuration in the QBD Direct onboarding module since we found a reference to the 'pre_requisite' path.
Navigation path is correctly configured in the router
The navigation path /integrations/qbd_direct/onboarding/pre_requisite/ is correctly mapped in the QBD Direct onboarding routing module. The route configuration shows that 'pre_requisite' is a valid child route that maps to the QbdDirectOnboardingPreRequisiteComponent.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for router configuration files and route definitions
rg -g "*.ts" -A 5 "RouterModule.forChild|RouterModule.forRoot"
rg -g "*.ts" "path.*['\"]pre_requisite['\"]"
Length of output: 34674
Script:
#!/bin/bash
# Check the complete route configuration in QBD Direct onboarding module
cat src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-routing.module.ts
Length of output: 2248
src/app/core/services/common/workspace.service.ts (1)
8-9: LGTM! Appropriate type imports added
The specific workspace types are correctly imported, following the previous review discussion about avoiding generic types.
src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-onboarding.model.ts (3)
20-22: LGTM! Class name accurately reflects its purpose.
The rename to QbdDirectOnboardingModel better represents its specific role in QBD direct onboarding.
Line range hint 24-61: LGTM! Improved visual flow with directional icons
The updated icon configuration creates a better visual representation of the onboarding flow:
- Starting with an up arrow
- Using directional arrows and link icons to show progression
- Ending with a gear icon for settings
Line range hint 62-84: Verify complete onboarding state coverage
Let's ensure all QBD Direct onboarding states are properly mapped and handled.
✅ Verification successful
All QBD Direct onboarding states are properly mapped and handled
After verification, all states from the QbdDirectOnboardingState enum are correctly mapped in both:
- The onboarding state step map (with step numbers 1-6)
- The component routing map (with corresponding URLs)
The mapping is complete and consistent across the codebase with proper state transitions in the onboarding flow:
- YET_TO_START/CONFIRM_PRE_REQUISITES (Step 1)
- CONNECTION states including error states (Step 2)
- EXPORT_SETTINGS (Step 3)
- IMPORT_SETTINGS (Step 4)
- ADVANCED_SETTINGS (Step 5)
- COMPLETE (Step 6)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify QBD Direct onboarding state handling
# Check for any unmapped states in enum
ast-grep --pattern 'enum QbdDirectOnboardingState {
$$$
}'
# Check for state usage in other files
rg "QbdDirectOnboardingState\." -A 2
Length of output: 9041
src/app/shared/components/helper/app-landing-page-header/app-landing-page-header.component.html (1)
46-46: LGTM! Good user feedback implementation.
The loader provides clear visual feedback during the connection process, following Angular best practices.
src/app/branding/c1-contents-config.ts (1)
Line range hint 1-500: Verify consistent branding across the file
The file contains multiple instances of direct references to specific brands. Ensure all brand references are using the dynamic brandingConfig.brandName.
tailwind.config.js (1)
1200-1200: LGTM! Width value matches height for consistency.
The addition of the 45px width value maintains symmetry with the height value, which is a good practice for consistent sizing options.
| export type WorkspaceOnboardingState = { | ||
| onboarding_state: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using string literal union type for better type safety
The onboarding_state property would benefit from being more strictly typed to prevent invalid states. Consider using a string literal union type to explicitly define all possible onboarding states.
Here's a suggested improvement:
+/**
+ * Represents the onboarding state of a workspace in QBD direct integration
+ */
export type WorkspaceOnboardingState = {
- onboarding_state: string;
+ onboarding_state: 'LANDING_PAGE' | 'IN_PROGRESS' | 'COMPLETE'; // Add all valid states
}This change will:
- Provide better type safety by restricting possible values
- Make the code more self-documenting
- Help catch potential errors at compile time
Committable suggestion skipped: line range outside the PR's diff.
| <p>qbd-direct-onboarding-landing works!</p> | ||
| <div> | ||
| <div> | ||
| <app-landing-page-header [buttonText]="'Start'" [iconPath]="'assets/logos/qbo.png'" [appName]="'QuickBooks Desktop - Direct Integration'" [appDescription]="brandingContent.contentText" [isConnectionInProgress]="isQbdConnectionInProgress" [isLoading]="false" [isIntegrationConnected]="false" [redirectLink]="redirectLink" (connectIntegration)="connectQbdDirect()" [logoStyleClasses]="'tw-p-16-px'"></app-landing-page-header> |
There was a problem hiding this comment.
Fix incorrect QuickBooks logo and improve configuration.
There are several issues in the header configuration:
- The icon path points to "qbo.png" (QuickBooks Online) instead of a QuickBooks Desktop logo
- The app name is hardcoded in the template
- Tailwind CSS classes should be managed through component's stylesheet
Apply these improvements:
-<app-landing-page-header [buttonText]="'Start'" [iconPath]="'assets/logos/qbo.png'" [appName]="'QuickBooks Desktop - Direct Integration'" [appDescription]="brandingContent.contentText" [isConnectionInProgress]="isQbdConnectionInProgress" [isLoading]="false" [isIntegrationConnected]="false" [redirectLink]="redirectLink" (connectIntegration)="connectQbdDirect()" [logoStyleClasses]="'tw-p-16-px'">
+<app-landing-page-header
+ [buttonText]="'Start'"
+ [iconPath]="'assets/logos/qbd.png'"
+ [appName]="brandingContent.appName"
+ [appDescription]="brandingContent.contentText"
+ [isConnectionInProgress]="isQbdConnectionInProgress"
+ [isLoading]="false"
+ [isIntegrationConnected]="false"
+ [redirectLink]="redirectLink"
+ (connectIntegration)="connectQbdDirect()">
+</app-landing-page-header>📝 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.
| <app-landing-page-header [buttonText]="'Start'" [iconPath]="'assets/logos/qbo.png'" [appName]="'QuickBooks Desktop - Direct Integration'" [appDescription]="brandingContent.contentText" [isConnectionInProgress]="isQbdConnectionInProgress" [isLoading]="false" [isIntegrationConnected]="false" [redirectLink]="redirectLink" (connectIntegration)="connectQbdDirect()" [logoStyleClasses]="'tw-p-16-px'"></app-landing-page-header> | |
| <app-landing-page-header | |
| [buttonText]="'Start'" | |
| [iconPath]="'assets/logos/qbd.png'" | |
| [appName]="brandingContent.appName" | |
| [appDescription]="brandingContent.contentText" | |
| [isConnectionInProgress]="isQbdConnectionInProgress" | |
| [isLoading]="false" | |
| [isIntegrationConnected]="false" | |
| [redirectLink]="redirectLink" | |
| (connectIntegration)="connectQbdDirect()"> | |
| </app-landing-page-header> |
| <app-landing-page-header [buttonText]="'Start'" [iconPath]="'assets/logos/qbo.png'" [appName]="'QuickBooks Desktop - Direct Integration'" [appDescription]="brandingContent.contentText" [isConnectionInProgress]="isQbdConnectionInProgress" [isLoading]="false" [isIntegrationConnected]="false" [redirectLink]="redirectLink" (connectIntegration)="connectQbdDirect()" [logoStyleClasses]="'tw-p-16-px'"></app-landing-page-header> | ||
| </div> | ||
| <div> | ||
| <app-landing-page-body [headlineText]="brandingContent.guideHeaderText" [headerText]="'A quick guide to help you set up your QuickBooks Online integration.'" [embedVideo]="embedVideoLink" [embedImage]="'assets/co/coQBOLanding.svg'" [svgPath]="'assets/flow-charts/' + brandingConfig.brandId +'-qbd-direct-flow-chart.svg'" [appName]="appName"></app-landing-page-body> |
There was a problem hiding this comment.
💡 Codebase verification
Fix incorrect QuickBooks product reference and update landing image
The review comment is correct. The component needs to be updated as it's using QBO-specific text and image for a QBD (QuickBooks Desktop) integration. This is confirmed by:
- The component's app name is set to
QBD_DIRECT - The header mentions "QuickBooks Desktop - Direct Integration"
- Currently using
coQBOLanding.svgwhich is meant for QuickBooks Online - No existing QBD-specific landing image, but other QBD assets exist in different directories
The suggested fix is correct, but needs a slight modification:
- The text change from "Online" to "Desktop" is correct
- The image path should be updated, but since
coQBDLanding.svgdoesn't exist in assets/co/, we should create a new ticket to add this asset - For now, we can use the generic QuickBooks Desktop logo:
assets/logos/quickbooks-desktop.svg
🔗 Analysis chain
Fix incorrect QuickBooks product reference in header text.
The header text incorrectly mentions "QuickBooks Online" instead of "QuickBooks Desktop".
Apply this fix:
-<app-landing-page-body [headlineText]="brandingContent.guideHeaderText" [headerText]="'A quick guide to help you set up your QuickBooks Online integration.'" [embedVideo]="embedVideoLink" [embedImage]="'assets/co/coQBOLanding.svg'" [svgPath]="'assets/flow-charts/' + brandingConfig.brandId +'-qbd-direct-flow-chart.svg'" [appName]="appName">
+<app-landing-page-body
+ [headlineText]="brandingContent.guideHeaderText"
+ [headerText]="'A quick guide to help you set up your QuickBooks Desktop integration.'"
+ [embedVideo]="embedVideoLink"
+ [embedImage]="'assets/co/coQBDLanding.svg'"
+ [svgPath]="'assets/flow-charts/' + brandingConfig.brandId + '-qbd-direct-flow-chart.svg'"
+ [appName]="appName">
+</app-landing-page-body>Also, verify if the landing image path needs to be updated:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if QBD-specific landing image exists
fd "coQBDLanding.svg" assets/co/
Length of output: 130
Script:
#!/bin/bash
# Let's first check what files exist in the assets directory
fd . assets
# Also search for any QBD or QuickBooks Desktop related SVG files
fd -e svg . assets
# Let's also check the component file to understand the context better
rg -A 5 -B 5 "QuickBooks.*integration" src/
Length of output: 6538
Script:
#!/bin/bash
# Let's check the component's TypeScript file to understand if there are any specific QBD-related configurations
rg -A 5 -B 5 "class QbdDirectOnboardingLandingComponent" src/
# Also check for any references to image paths in the component
rg "coQBOLanding|coQBDLanding" src/
# Let's also check the directory structure to find the correct assets location
find . -type d -name "assets" -o -name "co"
Length of output: 2904
Script:
#!/bin/bash
# Let's check what SVG files exist in the assets/co directory
ls -la ./src/assets/co/*.svg 2>/dev/null || true
# Also check if there are any QBD specific assets
find ./src/assets -type f -name "*qbd*.svg" -o -name "*quickbooks-desktop*.svg" 2>/dev/null || true
# Let's also verify the app name constant
rg "enum AppName" src/ -A 10
Length of output: 2248
| connectQbdDirect() { | ||
| this.isQbdConnectionInProgress = true; | ||
| this.workspaceService.updateWorkspaceOnboardingState({"onboarding_state": QbdDirectOnboardingState.CONFIRM_PRE_REQUISITES}).subscribe((workspaceResponse: QbdDirectWorkspace) => { | ||
| this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state); | ||
| this.isQbdConnectionInProgress = false; | ||
| this.router.navigate([`/integrations/qbd_direct/onboarding/pre_requisite/`]); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Add error handling and improve state management.
The current implementation has several potential issues:
- Missing error handling for the HTTP request
- Hardcoded navigation path
- No cleanup of
isQbdConnectionInProgressif the request fails
Consider implementing this improved version:
connectQbdDirect() {
this.isQbdConnectionInProgress = true;
this.workspaceService.updateWorkspaceOnboardingState({
"onboarding_state": QbdDirectOnboardingState.CONFIRM_PRE_REQUISITES
- }).subscribe((workspaceResponse: QbdDirectWorkspace) => {
- this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
- this.isQbdConnectionInProgress = false;
- this.router.navigate([`/integrations/qbd_direct/onboarding/pre_requisite/`]);
- });
+ }).subscribe({
+ next: (workspaceResponse: QbdDirectWorkspace) => {
+ this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
+ this.router.navigate(['/integrations', this.appName.toLowerCase(), 'onboarding', 'pre_requisite']);
+ },
+ error: (error: any) => {
+ console.error('Failed to update workspace state:', error);
+ // Handle error appropriately (e.g., show error message to user)
+ },
+ complete: () => {
+ this.isQbdConnectionInProgress = false;
+ }
+ });
}Committable suggestion skipped: line range outside the PR's diff.
| updateWorkspaceOnboardingState(payload: WorkspaceOnboardingState): Observable<QbdDirectWorkspace> { | ||
| return this.apiService.patch(`/workspaces/${this.getWorkspaceId()}/onboarding_state/`, payload); | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistency found between onboarding state enums
The verification reveals a mismatch between the onboarding states. The code uses QbdDirectOnboardingState enum in the actual implementation, which has different states (CONNECTION, EXPORT_SETTINGS, IMPORT_SETTINGS, ADVANCED_SETTINGS, COMPLETE) compared to QBDOnboardingState (CONNECTION, EXPORT_SETTINGS, FIELD_MAPPINGS, ADVANCED_SETTINGS, COMPLETE).
- The workspace service is using
WorkspaceOnboardingStatetype which accepts any string, potentially leading to inconsistencies - The component is using
QbdDirectOnboardingStatewhile the stepper is usingQBDOnboardingState - These enums have different states:
QbdDirectOnboardingStatehasIMPORT_SETTINGSwhileQBDOnboardingStatehasFIELD_MAPPINGS
🔗 Analysis chain
Verify onboarding state values consistency
Ensure that the WorkspaceOnboardingState values align with QBDOnboardingState enum values used in the service.
Let me gather more information about the QBD onboarding states to verify the consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for QBDOnboardingState enum definition and WorkspaceOnboardingState usage
echo "Searching for QBDOnboardingState enum definition:"
rg -A 5 "QBDOnboardingState\s*=" src/
echo "\nSearching for WorkspaceOnboardingState usage:"
rg -A 2 "WorkspaceOnboardingState" src/
Length of output: 2763
Script:
#!/bin/bash
# Search for QBD onboarding state enums and their values
echo "Searching for QBD onboarding state enum definitions:"
rg -B2 -A5 "enum.*OnboardingState" src/
# Search for actual usage of onboarding states in the codebase
echo "\nSearching for onboarding state assignments:"
rg "onboarding_state.*=.*" src/
Length of output: 6119
| @@ -90,8 +90,7 @@ export class AppLandingPageHeaderComponent implements OnInit { | |||
| } | |||
|
|
|||
| connect(): void { | |||
|
|
|||
| if (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO) { | |||
| if (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO || this.appName.includes(AppName.QBD_DIRECT)) { | |||
There was a problem hiding this comment.
Replace includes() with strict equality check for consistency.
The condition this.appName.includes(AppName.QBD_DIRECT) uses a different comparison method than other app names, which could lead to false positives if another app name contains "QBD_DIRECT" as a substring.
Apply this diff to maintain consistency:
- if (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO || this.appName.includes(AppName.QBD_DIRECT)) {
+ if (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO || this.appName === AppName.QBD_DIRECT) {📝 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 (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO || this.appName.includes(AppName.QBD_DIRECT)) { | |
| if (this.appName === AppName.TRAVELPERK || this.appName === AppName.BUSINESS_CENTRAL || this.appName === AppName.BAMBOO_HR || this.appName === AppName.XERO || this.appName === AppName.QBD_DIRECT) { |
| route: '/integrations/qbo/onboarding/pre_requisite', | ||
| styleClasses: ['step-name-connector--text'] |
There was a problem hiding this comment.
Critical: Update routing paths for QBD Direct integration
The routes are currently pointing to QBO integration paths (/integrations/qbo/onboarding/), but they should be updated for QBD Direct integration.
Apply this fix to all route definitions:
- route: '/integrations/qbo/onboarding/pre_requisite',
+ route: '/integrations/qbd/onboarding/pre_requisite',Similar updates needed for connector, export_settings, import_settings, and advanced_settings routes.
Also applies to: 35-36, 42-43, 49-50, 56-57
| @@ -6,7 +6,7 @@ export const brandingConfig: BrandingConfiguration = config as BrandingConfigura | |||
| export const fyleContents = { | |||
| qbd_direct: { | |||
| landing: { | |||
| contentText: 'Import data from NetSuite to ' + brandingConfig.brandName + ' and export expenses from ' + brandingConfig.brandName + ' to NetSuite. ', | |||
| contentText: 'Import data from QuickBooks Desktop to Fyle and export expenses from Fyle to QuickBooks Desktop', | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistency by using dynamic branding
The content text should use dynamic branding (brandingConfig.brandName) to maintain consistency with other integration sections.
Apply this change:
- contentText: 'Import data from QuickBooks Desktop to Fyle and export expenses from Fyle to QuickBooks Desktop',
+ contentText: 'Import data from QuickBooks Desktop to ' + brandingConfig.brandName + ' and export expenses from ' + brandingConfig.brandName + ' to QuickBooks Desktop',📝 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.
| contentText: 'Import data from QuickBooks Desktop to Fyle and export expenses from Fyle to QuickBooks Desktop', | |
| contentText: 'Import data from QuickBooks Desktop to ' + brandingConfig.brandName + ' and export expenses from ' + brandingConfig.brandName + ' to QuickBooks Desktop', |
* feat: qbd-direct-onboarding-pre-requisite implementation * styling changes * unit test fix * step footer contentt fix * pre requisite Ui updation * PR comments fix * PR comments fix * feat: Download qwd file UI changes (#1059) * feat: Download qwd file UI changes * download file Ui updation * download file Ui updation * download file Ui updation * download file Ui updation * feat: qbd connector setup UI changes (#1060) * feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]>
ashwin1111
merged commit 5ce97c7
into
qbd-onboarding-model-service
* feat: onboarding basic setup * feat: qbd direct onboarding landing page (#1056) * feat: qbd direct onboarding landing page * feat: qbd-direct-onboarding-pre-requisite implementation * PR comments fix * PR fix * updateWorkspaceOnboardingState service return type update * qbd direct logo update * feat: qbd-direct onboarding prerequisite UI implementation (#1058) * feat: qbd-direct-onboarding-pre-requisite implementation * styling changes * unit test fix * step footer contentt fix * pre requisite Ui updation * PR comments fix * PR comments fix * feat: Download qwd file UI changes (#1059) * feat: Download qwd file UI changes * download file Ui updation * download file Ui updation * download file Ui updation * download file Ui updation * feat: qbd connector setup UI changes (#1060) * feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]>
* feat: checkbox button creation * PR comments fix * feat: onboarding basic setup (#1055) * feat: onboarding basic setup * feat: qbd direct onboarding landing page (#1056) * feat: qbd direct onboarding landing page * feat: qbd-direct-onboarding-pre-requisite implementation * PR comments fix * PR fix * updateWorkspaceOnboardingState service return type update * qbd direct logo update * feat: qbd-direct onboarding prerequisite UI implementation (#1058) * feat: qbd-direct-onboarding-pre-requisite implementation * styling changes * unit test fix * step footer contentt fix * pre requisite Ui updation * PR comments fix * PR comments fix * feat: Download qwd file UI changes (#1059) * feat: Download qwd file UI changes * download file Ui updation * download file Ui updation * download file Ui updation * download file Ui updation * feat: qbd connector setup UI changes (#1060) * feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]>
* fead: folder creation * PR comments fix * feat: Qbd checkbox button creation (#1054) * feat: checkbox button creation * PR comments fix * feat: onboarding basic setup (#1055) * feat: onboarding basic setup * feat: qbd direct onboarding landing page (#1056) * feat: qbd direct onboarding landing page * feat: qbd-direct-onboarding-pre-requisite implementation * PR comments fix * PR fix * updateWorkspaceOnboardingState service return type update * qbd direct logo update * feat: qbd-direct onboarding prerequisite UI implementation (#1058) * feat: qbd-direct-onboarding-pre-requisite implementation * styling changes * unit test fix * step footer contentt fix * pre requisite Ui updation * PR comments fix * PR comments fix * feat: Download qwd file UI changes (#1059) * feat: Download qwd file UI changes * download file Ui updation * download file Ui updation * download file Ui updation * download file Ui updation * feat: qbd connector setup UI changes (#1060) * feat: qbd connector setup UI changes * Merge branch qbd-direct-onboarding-download-file-UI into qbd-direct-step-connector-UI * feat: Qbd direct connection data sync UI changes (#1061) * feat: Qbd direct connection data sync UI changes * input made required * svg update * feat: qbd direct pre requisite ts changes (#1062) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes (#1063) * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes (#1064) * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes (#1065) * PR comment fix * PR comment fix * Qbd direct connector data sync up ts (#1070) * feat: qbd direct pre requisite ts changes * feat: qbd direct pre requisite ts changes * feat: qbd direct connector download file ts changes * feat: qbd direct connector setup connector ts changes * feat: qbd direct connector data sync up ts changes * feat: QBD direct main connection page business logic (#1066) * feat: QBD direct main connection page business logic * onboarding connection ts changes * onboarding connection ts changes --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> --------- Co-authored-by: ashwin1111 <[email protected]> * checkbox button fix * QA fixes * qa fixes * qa fixes * fix: qbd direct export settings onboarding changes and content changes (#1082) * qbd direct export settings onboarding changes and content changes * fix: qbd direct import settings onboarding changes and content changes (#1083) * qbd direct import settings onboarding changes and content changes * qbd direct advanced settings onboarding changes and content changes (#1084) * QA fixes * QBD direct bug fixes * QBD direct bug fixes * JE changes in export settings and mapping * PR comments fix --------- Co-authored-by: ashwin1111 <[email protected]>
Description
feat: qbd direct onboarding landing page
Clickup
https://app.clickup.com/t/86cwzcf6x
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Refactor