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

fix: qbd direct import settings onboarding changes and content changes #1083

Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 14 additions & 14 deletions src/app/branding/fyle-contents-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,38 +66,38 @@ export const fyleContents = {
stepName: 'Import Settings',
headerText: '',
contentText: 'In this section, you can choose the fields required to be imported from QuickBooks Desktop to ' + brandingConfig.brandName + '. ',
importCategoriesLabel: 'Import the Chart of Accounts as Categories in ' + brandingConfig.brandName,
importCategoriesSubLabel: 'Imported account will be available as Categories in ' + brandingConfig.brandName + '.',
importCategoriesLabel: 'Import Chart of Accounts as Categories in ' + brandingConfig.brandName,
importCategoriesSubLabel: 'Chart of Accounts in QuickBooks will be imported as Categories in Fyle, available in a dropdown for employees to select when coding expenses.',
importItemsLabel: 'Import Products/Services from QuickBooks Desktop',
importItemsSubLabel: 'Products/services from QuickBooks Desktop will be imported as Categories in ' + brandingConfig.brandName + '.',
importVendorsAsMerchantsLabel: 'Import Vendors from QuickBooks Desktop',
chartOfAccountTypes: 'Select the accounts from QuickBooks Desktop to import as categories in ' + brandingConfig.brandName,
chartOfAccountTypesSubLabel: 'By default expense will be selected. Open the dropdown to select more as per your requirements.'
importVendorsAsMerchantsLabel: 'Import Vendors as Merchants in Fyle',
chartOfAccountTypes: 'Choose the type of accounts to be imported',
chartOfAccountTypesSubLabel: 'By default \'Expense\' type accounts will be imported. Open the dropdown to add or modify selections based on your needs.'
},
advancedSettings: {
stepName: 'Advanced Settings',
contentText: 'In this section, you can customize the integration based on your accounting requirements. ',
automationLabel: 'Automation',
automationSubLabel: 'You can automate the export and sync of your data in this section.',
customizeSectionLabel: 'Customization',
customizeSectionSubLabel: 'In this section, you can customize the data that you\'d like to export from Fyle 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 Fyle to QuickBooks Desktop.',
scheduleAutoExportLabel: 'Schedule automatic export',
scheduleAutoExportSubLabel: 'Set up a schedule to frequently automate the export of expenses from ' + brandingConfig.brandName + ' to QuickBooks Desktop.',
autoExportfrequencyLabel: 'Set up export frequency',
autoExportfrequencySubLabel: 'Set a frequency based on how often you want your expenses in Fyle to be exported to QuickBooks Desktop.',
topLevelMemoStructureLabel: 'Select the top level memo field data for QuickBooks Desktop',
topLevelMemoStructureSubLabel: 'You can customize the <b>data point</b> you would like to export to QuickBooks Desktop\’s <b>top-level memo</b> field while exporting expenses from ' + brandingConfig.brandName + '.',
memoStructureLabel: 'Set the line-item level memo field data for QuickBooks Desktop.',
memoStructureSubLabel: 'You can customize the data set you would like to export to QuickBooks Desktop\’s <b>transaction line-item level memo</b> field while exporting expenses from ' + brandingConfig.brandName + '.',
topLevelMemoStructureLabel: 'Customize the Top-Level Memo Field',
topLevelMemoStructureSubLabel: 'Select the datapoints you\'d like to export to QuickBooks Desktop’s top-level memo field when exporting expenses from Fyle.',
memoStructureLabel: 'Customize the Line-Item Level Memo Field',
memoStructureSubLabel: 'Select the datapoints you\'d like to export to QuickBooks Desktop\’s line-item level memo field when exporting expenses from Fyle.',
previewDescriptionFieldLabel: 'Preview of the Description Field',
otherPreferencesLabel: 'Other preferences',
otherPreferencesSubLabel: 'Based on your preference, you can choose whether you want to create any new records in QuickBooks Desktop from ' + brandingConfig.brandName + '.',
autoCreateMerchantsAsVendorsLabel: 'Auto-create merchants as vendors',
autoCreateMerchantsAsVendorsSubLabel: 'Fyle 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. ',
skipExportLabel: 'Skip export of specific expenses from ' + brandingConfig.brandName + ' to QuickBooks Desktop',
skipExportSubLabel: 'You could choose to skip expenses from ' + brandingConfig.brandName + ' to QuickBooks Desktop by setting up a conditional rule. ',
autoCreateReimbursableEnitityLabel: 'Auto create reimbursable enitity',
autoCreateReimbursableEnititySubLabel: 'Do you want to create a reimbursable enitity if not present',
skipExportLabel: 'Skip Export of Specific Expenses from Fyle to QuickBooks Desktop',
skipExportSubLabel: 'Set up a conditional rule to selectively skip exporting certain expenses from Fyle to QuickBooks Desktop. ',
autoCreateReimbursableEnitityLabel: 'Auto-create fyle employees as ',
autoCreateReimbursableEnititySubLabel: 'If an employee in Fyle has no corresponding record in QuickBooks Desktop, the integration will automatically create a ',
accountingPeriodLabel: 'Post entries in the current accounting period',
accountingPeriodSubLabel: 'If there are expenses for which the accounting period is closed in QuickBooks Desktop, you can export those to the current month by enabling this option.'
}
Expand Down
2 changes: 1 addition & 1 deletion src/app/core/models/common/advanced-settings.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class AdvancedSettingsModel {
merchant: 'Pizza Hut',
report_number: 'C/2021/12/R/1',
spent_on: today.toLocaleDateString(),
expence_key: 'txDdlUFWkahX',
expense_key: 'txDdlUFWkahX',
expense_link: `${environment.fyle_app_url}/app/main/#/enterprise/view_expense/`
};
let memoPreviewText = '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class QbdDirectAdvancedSettingsModel extends AdvancedSettingsModel {
}

static defaultTopMemoOptions(): string[] {
return ["employee_name", "expense_key"];
return ["employee_name", "Expense/Report ID"];
}

static formatMemoStructure(memoStructure: string[], defaultMemoOptions: string[]): string[] {
Expand Down Expand Up @@ -58,6 +58,9 @@ export class QbdDirectAdvancedSettingsModel extends AdvancedSettingsModel {

const topMemo: string[] = advancedSettingForm.controls.topMemoStructure.value;

const index = topMemo.indexOf('Expense/Report ID');
topMemo[index] = 'expense_key';

Comment on lines +61 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add safety checks for the memo field transformation.

The current implementation could be more robust. Consider these improvements:

-        const index = topMemo.indexOf('Expense/Report ID');
-        topMemo[index] = 'expense_key';
+        if (topMemo) {
+            const index = topMemo.indexOf('Expense/Report ID');
+            if (index !== -1) {
+                // Create a new array instead of mutating the original
+                topMemo = [...topMemo];
+                topMemo[index] = 'expense_key';
+            }
+        }

This change:

  1. Adds null check for topMemo
  2. Verifies the element exists before replacement
  3. Avoids direct array mutation
  4. Makes the code more defensive against potential runtime errors

Committable suggestion skipped: line range outside the PR's diff.

const allSelectedEmails: EmailOption[] = advancedSettingForm.get('email')?.value;

const selectedEmailsEmails = allSelectedEmails?.filter((email: EmailOption) => adminEmails.includes(email));
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,4 @@
<p>qbd-direct-onboarding-advanced-settings works!</p>
<div class="tw-pb-48-px">
<app-onboarding-steppers [onboardingSteps]="onboardingSteps"></app-onboarding-steppers>
<app-qbd-direct-advanced-settings></app-qbd-direct-advanced-settings>
</div>
Original file line number Diff line number Diff line change
@@ -1,12 +1,31 @@
import { Component } from '@angular/core';
import { CommonModule } from '@angular/common';
import { Component, OnInit } from '@angular/core';
import { QbdDirectSharedModule } from '../../qbd-direct-shared/qbd-direct-shared.module';
import { SharedModule } from 'src/app/shared/shared.module';
import { QbdDirectAdvancedSettingsComponent } from "../../qbd-direct-shared/qbd-direct-advanced-settings/qbd-direct-advanced-settings.component";
import { QbdDirectOnboardingModel } from 'src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-onboarding.model';
import { brandingContent } from 'src/app/branding/branding-config';
import { OnboardingStepper } from 'src/app/core/models/misc/onboarding-stepper.model';
import { WorkspaceService } from 'src/app/core/services/common/workspace.service';

@Component({
selector: 'app-qbd-direct-onboarding-advanced-settings',
standalone: true,
imports: [],
imports: [CommonModule, QbdDirectSharedModule, SharedModule, QbdDirectAdvancedSettingsComponent],
templateUrl: './qbd-direct-onboarding-advanced-settings.component.html',
styleUrl: './qbd-direct-onboarding-advanced-settings.component.scss'
})
export class QbdDirectOnboardingAdvancedSettingsComponent {
export class QbdDirectOnboardingAdvancedSettingsComponent implements OnInit {

brandingContent = brandingContent.configuration.advancedSettings;

onboardingSteps: OnboardingStepper[] = new QbdDirectOnboardingModel().getOnboardingSteps(this.brandingContent.stepName, this.workspaceService.getOnboardingState());

constructor(
private workspaceService: WorkspaceService
) { }

ngOnInit(): void {
}

}
Original file line number Diff line number Diff line change
@@ -1 +1,8 @@
<p>qbd-direct-onboarding-import-settings works!</p>
<div class="tw-pb-48-px">
<div>
<app-onboarding-steppers [onboardingSteps]="onboardingSteps"></app-onboarding-steppers>
</div>
<div>
<app-qbd-direct-import-settings></app-qbd-direct-import-settings>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,12 +1,31 @@
import { Component } from '@angular/core';
import { CommonModule } from '@angular/common';
import { Component, OnInit } from '@angular/core';
import { SharedModule } from 'src/app/shared/shared.module';
import { QbdDirectSharedModule } from '../../qbd-direct-shared/qbd-direct-shared.module';
import { QbdDirectOnboardingModel } from 'src/app/core/models/qbd-direct/qbd-direct-configuration/qbd-direct-onboarding.model';
import { brandingContent } from 'src/app/branding/branding-config';
import { OnboardingStepper } from 'src/app/core/models/misc/onboarding-stepper.model';
import { WorkspaceService } from 'src/app/core/services/common/workspace.service';

@Component({
selector: 'app-qbd-direct-onboarding-import-settings',
standalone: true,
imports: [],
imports: [QbdDirectSharedModule, SharedModule, CommonModule],
templateUrl: './qbd-direct-onboarding-import-settings.component.html',
styleUrl: './qbd-direct-onboarding-import-settings.component.scss'
})
export class QbdDirectOnboardingImportSettingsComponent {
export class QbdDirectOnboardingImportSettingsComponent implements OnInit {

brandingContent = brandingContent.configuration.importSetting;

onboardingSteps: OnboardingStepper[] = new QbdDirectOnboardingModel().getOnboardingSteps(this.brandingContent.stepName, this.workspaceService.getOnboardingState());

Comment on lines +19 to +21
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

Move service-dependent initialization to ngOnInit

The initialization of onboardingSteps in the property declaration is using an injected service, which is an anti-pattern in Angular. This could cause issues during testing and potential race conditions.

Move the initialization to ngOnInit:

-  onboardingSteps: OnboardingStepper[] = new QbdDirectOnboardingModel().getOnboardingSteps(this.brandingContent.stepName, this.workspaceService.getOnboardingState());
+  onboardingSteps: OnboardingStepper[];
+
+  ngOnInit(): void {
+    this.onboardingSteps = new QbdDirectOnboardingModel().getOnboardingSteps(
+      this.brandingContent.stepName,
+      this.workspaceService.getOnboardingState()
+    );
+  }
📝 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
brandingContent = brandingContent.configuration.importSetting;
onboardingSteps: OnboardingStepper[] = new QbdDirectOnboardingModel().getOnboardingSteps(this.brandingContent.stepName, this.workspaceService.getOnboardingState());
brandingContent = brandingContent.configuration.importSetting;
onboardingSteps: OnboardingStepper[];
ngOnInit(): void {
this.onboardingSteps = new QbdDirectOnboardingModel().getOnboardingSteps(
this.brandingContent.stepName,
this.workspaceService.getOnboardingState()
);
}

constructor(
private workspaceService: WorkspaceService
) { }

ngOnInit(): void {
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@

<app-configuration-toggle-field *ngIf="isReimbursableExportTypePresent"
[form]="advancedSettingsForm"
[label]="brandingContent.autoCreateReimbursableEnitityLabel"
[label]="brandingContent.autoCreateReimbursableEnitityLabel + (employeeMapping | snakeCaseToSpaceCase | lowercase) + 's in QuickBooks Desktop'"
[iconPath]="'user-plus'"
[subLabel]="brandingContent.autoCreateReimbursableEnititySubLabel"
[subLabel]="brandingContent.autoCreateReimbursableEnititySubLabel + (employeeMapping | snakeCaseToSpaceCase | lowercase) + ' record for them, ensuring expenses are posted to this new record.'"
[formControllerName]="'autoCreateReimbursableEnitity'">
</app-configuration-toggle-field>

Expand Down Expand Up @@ -128,7 +128,7 @@
</div>

</div>
<app-configuration-step-footer [ctaText]="!saveInProgress ? (isOnboarding ? ConfigurationCtaText.SAVE_AND_CONTINUE : ConfigurationCtaText.SAVE) : ConfigurationCtaText.SAVING" [isButtonDisabled]="!advancedSettingsForm.valid || saveInProgress || !skipExportForm.valid" (save)="save()"></app-configuration-step-footer>
<app-configuration-step-footer [ctaText]="!saveInProgress ? (isOnboarding ? ConfigurationCtaText.SAVE_AND_CONTINUE : ConfigurationCtaText.SAVE) : ConfigurationCtaText.SAVING" [isButtonDisabled]="!advancedSettingsForm.valid || saveInProgress || !skipExportForm.valid" (save)="save()" [showBackButton]="true" (navigateToPreviousStep)="navigateToPreviousStep()"></app-configuration-step-footer>
</form>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ export class QbdDirectAdvancedSettingsComponent implements OnInit {
}
}

navigateToPreviousStep() {
this.router.navigate([`/integrations/qbd_direct/onboarding/import_settings`]);
}

save() {
this.saveInProgress = true;
this.saveSkipExport();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ <h5 class="lg:tw-w-3/5 md:tw-w-1/2 tw-text-slightly-normal-text-color tw-text-14
<app-configuration-toggle-field
[form]="importSettingForm"
[label]="brandingContent.importVendorsAsMerchantsLabel"
[subLabel]="'Vendors from QuickBooks Online will be imported as merchants in ' + brandingConfig.brandName + ' and will be a selectable field while creating an expense.'"
[subLabel]="'Vendors in QuickBooks will be imported as Merchants in Fyle, available in a dropdown for employees to select when coding expenses.'"
[formControllerName]="'importVendorsAsMerchants'"
[iconPath]="'arrow-tail-down'">
</app-configuration-toggle-field>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { QbdDirectDownloadFileComponent } from './qbd-direct-download-file/qbd-d
import { QbdDirectSetupConnectionComponent } from './qbd-direct-setup-connection/qbd-direct-setup-connection.component';
import { QbdDirectDataSyncComponent } from './qbd-direct-data-sync/qbd-direct-data-sync.component';
import { QbdDirectExportSettingsComponent } from './qbd-direct-export-settings/qbd-direct-export-settings.component';
import { QbdDirectImportSettingsComponent } from './qbd-direct-import-settings/qbd-direct-import-settings.component';
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

Component exists and follows Angular conventions

The QbdDirectImportSettingsComponent exists at the correct relative path and follows proper Angular component structure with appropriate decorators, selector, and metadata. However, I notice it's marked as standalone: true, which means it shouldn't be declared in NgModules. The shared module should use imports array instead of both imports and declarations.

🔗 Analysis chain

Verify the imported component exists

The import statement follows Angular conventions, but let's verify the component exists at the specified path.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify QbdDirectImportSettingsComponent exists and follows Angular component structure

# Check if the component file exists
fd "qbd-direct-import-settings.component.ts" 

# Verify component structure
ast-grep --pattern 'class QbdDirectImportSettingsComponent {
  $$$
}'

Length of output: 233


Script:

#!/bin/bash
# Let's verify the component's structure and decorators
rg -A 5 "@Component" src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-import-settings/qbd-direct-import-settings.component.ts

Length of output: 396


@NgModule({
declarations: [],
Expand All @@ -14,13 +15,15 @@ import { QbdDirectExportSettingsComponent } from './qbd-direct-export-settings/q
QbdDirectDownloadFileComponent,
QbdDirectSetupConnectionComponent,
QbdDirectDataSyncComponent,
QbdDirectExportSettingsComponent
QbdDirectExportSettingsComponent,
QbdDirectImportSettingsComponent
Comment on lines +18 to +19
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

Components are incorrectly configured in the shared module

The verification revealed that the components are incorrectly placed in the imports array instead of the declarations array of the shared module. Components should be declared in declarations array and then exported via exports array. The current configuration won't work as expected since Angular components cannot be imported directly.

  • Move QbdDirectExportSettingsComponent and QbdDirectImportSettingsComponent from imports to declarations array in src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-shared.module.ts
  • The same applies to other components currently in the imports array: QbdDirectDownloadFileComponent, QbdDirectSetupConnectionComponent, and QbdDirectDataSyncComponent
🔗 Analysis chain

LGTM! Module configuration follows best practices

The changes correctly:

  • Add the component to both imports and exports arrays
  • Follow the shared module pattern for component reusability
  • Maintain consistency with existing component configuration

Also applies to: 25-26


Let me verify the module configuration by checking the actual declarations in the routing and shared modules.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the component usage across the application

# Find where the shared module is imported
rg -l "QbdDirectSharedModule" 

# Find direct usage of the component
rg -l "QbdDirectImportSettingsComponent"

Length of output: 855


Script:

#!/bin/bash
# Check the component declarations in shared module
rg -A 10 "declarations:" src/app/integrations/qbd-direct/qbd-direct-shared/qbd-direct-shared.module.ts

# Check the component usage in routing module
rg -A 10 "const routes" src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-configuration/qbd-direct-configuration-routing.module.ts

# Check if component is used in templates
rg "<qbd-direct-import-settings" 
rg "<qbd-direct-export-settings"

Length of output: 833

],
exports: [
QbdDirectDownloadFileComponent,
QbdDirectSetupConnectionComponent,
QbdDirectDataSyncComponent,
QbdDirectExportSettingsComponent
QbdDirectExportSettingsComponent,
QbdDirectImportSettingsComponent
]
})
export class QbdDirectSharedModule { }
Loading