Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: qbd direct pre requisite ts changes #1062

Merged
merged 5 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<!-- <app-qbd-direct-download-file></app-qbd-direct-download-file> -->
<!-- <app-qbd-direct-setup-connection></app-qbd-direct-setup-connection> -->
<app-qbd-direct-data-sync></app-qbd-direct-data-sync>
<!-- <app-qbd-direct-data-sync></app-qbd-direct-data-sync> -->
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ import { brandingContent, brandingKbArticles } from 'src/app/branding/branding-c
import { brandingConfig } from 'src/app/branding/c1-contents-config';
import { BrandingConfiguration } from 'src/app/core/models/branding/branding-configuration.model';
import { checkBoxEmit } from 'src/app/core/models/common/helper.model';
import { ConfigurationCta, QBDPreRequisiteState } from 'src/app/core/models/enum/enum.model';
import { ConfigurationCta, QbdDirectOnboardingState, QBDPreRequisiteState } from 'src/app/core/models/enum/enum.model';
import { OnboardingStepper } from 'src/app/core/models/misc/onboarding-stepper.model';
import { QbdDirectWorkspace } from 'src/app/core/models/qbd-direct/db/qbd-direct-workspaces.model';
import { QBDPrerequisiteObject } from 'src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-connector.model';
import { QbdDirectOnboardingModel } from 'src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-onboarding.model';
import { IntegrationsToastService } from 'src/app/core/services/common/integrations-toast.service';
Expand Down Expand Up @@ -46,30 +47,36 @@ export class QbdDirectOnboardingPreRequisiteComponent {
caption: 'Download and install the QuickBooks Web Connector on the system where QuickBooks Desktop is installed.',
externalLink: 'https://qbd.com',
iconName: 'download-medium',
state: QBDPreRequisiteState.COMPLETE // INCOMPLETE
state: QBDPreRequisiteState.INCOMPLETE
},
{
id: 2,
label: 'Keep your Quickbooks company file open',
caption: 'Make sure the QuickBooks Company you want to connect to Fyle is open during the integration setup.',
externalLink: 'https://qbd.com',
iconName: 'expand',
state: QBDPreRequisiteState.INCOMPLETE // INCOMPLETE
state: QBDPreRequisiteState.INCOMPLETE
}
];

constructor(
private route: ActivatedRoute,
private router: Router,
private toastService: IntegrationsToastService,
private workspaceService: WorkspaceService,
private storageService: StorageService
private workspaceService: WorkspaceService
) { }

qbdWebConnectorStatus(status: checkBoxEmit): void {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updateConnectorStatus()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call the type as CheckBoxUpdate, update the name

this.preRequisitesteps[status.id-1].state = status.value ? QBDPreRequisiteState.COMPLETE : QBDPreRequisiteState.INCOMPLETE;
if (this.preRequisitesteps[0].state === QBDPreRequisiteState.COMPLETE && this.preRequisitesteps[1].state === QBDPreRequisiteState.COMPLETE) {
this.isContinueDisabled = false;
}
}

continueToNextStep(): void{
this.workspaceService.updateWorkspaceOnboardingState({"onboarding_state": QbdDirectOnboardingState.CONNECTION}).subscribe((workspaceResponse: QbdDirectWorkspace) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"" not required in onboarding_state

this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
this.saveInProgress = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mark saveInProgress as true before the call

this.router.navigate([`/integrations/qbd_direct/onboarding/connector`]);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Set saveInProgress before making the API call

As mentioned in the previous review, set saveInProgress = true before making the API call to properly indicate the loading state.

   continueToNextStep(): void{
+    this.saveInProgress = true;
     this.workspaceService.updateWorkspaceOnboardingState({"onboarding_state": QbdDirectOnboardingState.CONNECTION}).subscribe((workspaceResponse: QbdDirectWorkspace) => {
       this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
       this.saveInProgress = false;
       this.router.navigate([`/integrations/qbd_direct/onboarding/connector`]);
     });
   }
📝 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.workspaceService.updateWorkspaceOnboardingState({"onboarding_state": QbdDirectOnboardingState.CONNECTION}).subscribe((workspaceResponse: QbdDirectWorkspace) => {
this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
this.saveInProgress = false;
this.router.navigate([`/integrations/qbd_direct/onboarding/connector`]);
});
continueToNextStep(): void{
this.saveInProgress = true;
this.workspaceService.updateWorkspaceOnboardingState({"onboarding_state": QbdDirectOnboardingState.CONNECTION}).subscribe((workspaceResponse: QbdDirectWorkspace) => {
this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
this.saveInProgress = false;
this.router.navigate([`/integrations/qbd_direct/onboarding/connector`]);
});
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix state management and add error handling.

Several issues need to be addressed:

  1. saveInProgress should be set before the operation starts
  2. No error handling for failed state updates
  3. The onboarding_state property doesn't need quotes

Consider this implementation:

 continueToNextStep(): void {
+  this.saveInProgress = true;
-  this.workspaceService.updateWorkspaceOnboardingState({"onboarding_state": QbdDirectOnboardingState.CONNECTION}).subscribe((workspaceResponse: QbdDirectWorkspace) => {
+  this.workspaceService.updateWorkspaceOnboardingState({onboarding_state: QbdDirectOnboardingState.CONNECTION}).subscribe({
+    next: (workspaceResponse: QbdDirectWorkspace) => {
       this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
       this.saveInProgress = false;
       this.router.navigate([`/integrations/qbd_direct/onboarding/connector`]);
-    });
+    },
+    error: (error: any) => {
+      this.saveInProgress = false;
+      console.error('Failed to update workspace state:', error);
+    }
+  });
 }
📝 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
continueToNextStep(): void{
this.workspaceService.updateWorkspaceOnboardingState({"onboarding_state": QbdDirectOnboardingState.CONNECTION}).subscribe((workspaceResponse: QbdDirectWorkspace) => {
this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
this.saveInProgress = false;
this.router.navigate([`/integrations/qbd_direct/onboarding/connector`]);
});
continueToNextStep(): void {
this.saveInProgress = true;
this.workspaceService.updateWorkspaceOnboardingState({onboarding_state: QbdDirectOnboardingState.CONNECTION}).subscribe({
next: (workspaceResponse: QbdDirectWorkspace) => {
this.workspaceService.setOnboardingState(workspaceResponse.onboarding_state);
this.saveInProgress = false;
this.router.navigate([`/integrations/qbd_direct/onboarding/connector`]);
},
error: (error: any) => {
this.saveInProgress = false;
console.error('Failed to update workspace state:', error);
}
});
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ComponentFixture, TestBed } from '@angular/core/testing';

import { QbdDirectDataSyncComponent } from './qbd-direct-data-sync.component';

describe('QbdDirectDataSyncComponent', () => {
xdescribe('QbdDirectDataSyncComponent', () => {
let component: QbdDirectDataSyncComponent;
let fixture: ComponentFixture<QbdDirectDataSyncComponent>;

Expand Down
4 changes: 2 additions & 2 deletions src/app/integrations/qbd-direct/qbd-direct.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ export class QbdDirectComponent implements OnInit {

private navigate(): void {
const pathName = this.windowReference.location.pathname;
if (pathName === '/integrations/qbo') {
if (pathName === '/integrations/qbd_direct') {
const onboardingStateComponentMap = {
[QbdDirectOnboardingState.YET_TO_START]: '/integrations/qbd_direct/onboarding/landing',
[QbdDirectOnboardingState.CONFIRM_PRE_REQUISITES]: '/integrations/qbd_direct/onboarding/connector',
[QbdDirectOnboardingState.CONFIRM_PRE_REQUISITES]: '/integrations/qbd_direct/onboarding/pre_requisite',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Route inconsistency detected - Action required

The verification revealed inconsistencies in route usage that need to be addressed:

  • qbd-direct-export-settings.component.ts uses /integrations/qbd_direct/onboarding/connector as a navigation target in navigateToPreviousStep()
  • qbd-direct-onboarding-pre-requisite.component.ts navigates to /integrations/qbd_direct/onboarding/connector
  • qbd-direct-onboarding-landing.component.ts uses /integrations/qbd_direct/onboarding/pre_requisite/ (note the trailing slash)
  • The routing module defines the route as just pre_requisite

These inconsistencies could lead to navigation issues:

  1. Some components are still using the old connector route
  2. Inconsistent use of trailing slash in the pre_requisite route
  3. The route path in the routing module doesn't match the full paths used in navigation
🔗 Analysis chain

Verify route consistency across the application

The route for CONFIRM_PRE_REQUISITES state has been changed to '/pre_requisite'. Let's ensure this route change is consistently implemented across the application.


Let me check the specific content of these files to better understand the route usage and potential inconsistencies.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any hardcoded references to the old route and verify the new route implementation

# Search for any remaining references to the old route pattern
echo "Checking for old route pattern..."
rg -l "qbd_direct/onboarding/connector" --type ts

# Search for consistent usage of the new route pattern
echo "Checking new route pattern implementation..."
rg -l "qbd_direct/onboarding/pre_requisite" --type ts

# Look for potential route definitions in routing modules
echo "Checking route definitions..."
rg -l "path.*pre_requisite" --type ts

Length of output: 977


Script:

#!/bin/bash
# Check the content of files that reference the routes
echo "Content of qbd-direct.component.ts:"
rg -A 2 -B 2 "qbd_direct/onboarding/(connector|pre_requisite)" src/app/integrations/qbd-direct/qbd-direct.component.ts

echo -e "\nContent of qbd-direct-export-settings.component.ts:"
rg -A 2 -B 2 "qbd_direct/onboarding/connector" src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-export-settings/qbd-direct-export-settings.component.ts

echo -e "\nContent of qbd-direct-onboarding-pre-requisite.component.ts:"
rg -A 2 -B 2 "qbd_direct/onboarding/connector" src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-pre-requisite/qbd-direct-onboarding-pre-requisite.component.ts

echo -e "\nContent of qbd-direct-onboarding-landing.component.ts:"
rg -A 2 -B 2 "qbd_direct/onboarding/pre_requisite" src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-landing/qbd-direct-onboarding-landing.component.ts

echo -e "\nContent of routing module:"
rg -A 2 -B 2 "path.*pre_requisite" src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-routing.module.ts

Length of output: 3079

[QbdDirectOnboardingState.CONNECTION]: '/integrations/qbd_direct/onboarding/connector',
[QbdDirectOnboardingState.PENDING_QWC_UPLOAD]: '/integrations/qbd_direct/onboarding/connector',
[QbdDirectOnboardingState.INCORRECT_COMPANY_PATH]: '/integrations/qbd_direct/onboarding/connector',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const ICON_MAPPING = {
'upload': 'grv-upload',
'lock': 'grv-lock',
'eye-slash-medium': 'grv-show-hide-medium',
'eye-medium': 'grv-show-medium',
'eye-medium': 'grv-show-medium'
};

@Component({
Expand Down
Loading