-
Notifications
You must be signed in to change notification settings - Fork 0
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 bug fixes #1101
Changes from all commits
b8a9c0f
1da613e
0bfe78e
522be3f
287b3a3
c25229f
fed0d68
1762b05
3b7e2b5
4acb11a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -9,7 +9,7 @@ export type QbdDirectAdvancedSettingsPost = { | |||||
emails_added: EmailOption[], | ||||||
interval_hours: number, | ||||||
auto_create_merchant_as_vendor: boolean | ||||||
auto_create_reimbursable_enitity: boolean, | ||||||
auto_create_reimbursable_entity: boolean, | ||||||
} | ||||||
|
||||||
export interface QbdDirectAdvancedSettingsGet extends QbdDirectAdvancedSettingsPost { | ||||||
|
@@ -56,7 +56,7 @@ export class QbdDirectAdvancedSettingsModel extends AdvancedSettingsModel { | |||||
exportSchedule: new FormControl(advancedSettings?.schedule_is_enabled ? advancedSettings?.schedule_is_enabled : false), | ||||||
email: new FormControl(advancedSettings?.emails_selected ? advancedSettings?.emails_selected : null), | ||||||
exportScheduleFrequency: new FormControl(advancedSettings?.schedule_is_enabled ? advancedSettings?.interval_hours : 1), | ||||||
autoCreateReimbursableEnitity: new FormControl(advancedSettings?.auto_create_reimbursable_enitity ? advancedSettings?.auto_create_reimbursable_enitity : false), | ||||||
autoCreateReimbursableEnitity: new FormControl(advancedSettings?.auto_create_reimbursable_entity ? advancedSettings?.auto_create_reimbursable_entity : false), | ||||||
autoCreateMerchantsAsVendors: new FormControl(advancedSettings?.auto_create_merchant_as_vendor ? advancedSettings?.auto_create_merchant_as_vendor : false), | ||||||
skipExport: new FormControl(isSkipExportEnabled), | ||||||
searchOption: new FormControl('') | ||||||
|
@@ -79,7 +79,7 @@ export class QbdDirectAdvancedSettingsModel extends AdvancedSettingsModel { | |||||
schedule_is_enabled: advancedSettingForm.get('exportSchedule')?.value ? advancedSettingForm.get('exportSchedule')?.value : false, | ||||||
emails_selected: advancedSettingForm.get('exportSchedule')?.value ? selectedEmailsEmails : [], | ||||||
interval_hours: advancedSettingForm.get('exportSchedule')?.value ? advancedSettingForm.get('exportScheduleFrequency')?.value : null, | ||||||
auto_create_reimbursable_enitity: advancedSettingForm.get('autoCreateReimbursableEnitity')?.value ? advancedSettingForm.get('autoCreateReimbursableEnitity')?.value : false, | ||||||
auto_create_reimbursable_entity: advancedSettingForm.get('autoCreateReimbursableEnitity')?.value ? advancedSettingForm.get('autoCreateReimbursableEnitity')?.value : false, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix form control access in payload construction The payload construction attempts to access the misspelled form control while assigning to the correctly spelled property. Apply this fix: - auto_create_reimbursable_entity: advancedSettingForm.get('autoCreateReimbursableEnitity')?.value ? advancedSettingForm.get('autoCreateReimbursableEnitity')?.value : false,
+ auto_create_reimbursable_entity: advancedSettingForm.get('autoCreateReimbursableEntity')?.value ? advancedSettingForm.get('autoCreateReimbursableEntity')?.value : false, 📝 Committable suggestion
Suggested change
|
||||||
auto_create_merchant_as_vendor: advancedSettingForm.get('autoCreateMerchantsAsVendors')?.value ? advancedSettingForm.get('autoCreateMerchantsAsVendors')?.value : false, | ||||||
emails_added: advancedSettingForm.get('exportSchedule')?.value ? additionalEmails : [] | ||||||
}; | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
<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> | ||
<app-landing-page-header [buttonText]="'Start'" [iconPath]="'assets/logos/qbo.png'" [appName]="'QuickBooks Desktop '" [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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Based on the search results, I can now generate the final response: Remove trailing space from QuickBooks Desktop string The trailing space in
The proper fix would be to:
🔗 Analysis chainVerify the impact of appName change The appName has been changed to 'QuickBooks Desktop ' to match the new condition checks. However:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for other instances of QuickBooks Desktop string to ensure consistency
rg -l "QuickBooks Desktop" | while read -r file; do
echo "=== $file ==="
rg "QuickBooks Desktop" "$file"
done
Length of output: 25142 |
||
</div> | ||
<div> | ||
<app-landing-page-body [headlineText]="brandingContent.guideHeaderText" [headerText]="'A quick guide to help you set up your Fyle-QuickBooks Desktop integration.'" [embedVideo]="embedVideoLink" [embedImage]="'assets/co/coQBOLanding.svg'" [svgPath]="'assets/flow-charts/' + brandingConfig.brandId +'-qbd-direct-flow-chart.svg'" [appName]="appName" [redirectLink]="redirectLink"></app-landing-page-body> | ||
<app-landing-page-body [headlineText]="brandingContent.guideHeaderText" [headerText]="'A quick guide to help you set up your Fyle-QuickBooks Desktop 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> | ||
</div> | ||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -203,11 +203,11 @@ | |
(searchOptionsDropdown)="searchOptionsDropdown($event)" | ||
[optionLabel]="'value'" | ||
[isFieldMandatory]="true" | ||
[mandatoryErrorListName]="'accounts payable'" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
[mandatoryErrorListName]="'a default credit account'" | ||
[label]="brandingContent.corporateCard.defaultCCCAccountPayableLabel + (exportSettingsForm.get('creditCardExportType')?.value | snakeCaseToSpaceCase | titlecase)" | ||
[subLabel]="brandingContent.corporateCard.defaultCCCAccountPayableSubLabel + (exportSettingsForm.get('creditCardExportType')?.value | snakeCaseToSpaceCase | titlecase) + ' ,while debit lines will reflect the category chosen by the employee for each respective expense'" | ||
[iconPath]="'list'" | ||
[placeholder]="'Select accounts payable'" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same |
||
[placeholder]="'Select default account'" | ||
[isMultiLineOption]="true" | ||
[formControllerName]="'defaultCCCAccountsPayableAccountName'"> | ||
</app-configuration-select-field> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,8 @@ export class DashboardErrorSectionComponent implements OnInit { | |
|
||
isMultiLineOption: boolean; | ||
|
||
detailAccountType: string[] | undefined; | ||
|
||
constructor( | ||
private dashboardService: DashboardService, | ||
private mappingService: MappingService, | ||
|
@@ -147,10 +149,12 @@ export class DashboardErrorSectionComponent implements OnInit { | |
this.mappingService.getGroupedDestinationAttributes([this.destinationField], 'v2').subscribe(groupedDestinationResponse => { | ||
if (this.sourceField === 'EMPLOYEE') { | ||
this.destinationOptions = this.destinationField === FyleField.EMPLOYEE ? groupedDestinationResponse.EMPLOYEE : groupedDestinationResponse.VENDOR; | ||
this.detailAccountType = undefined; | ||
} else if (this.sourceField === 'CATEGORY') { | ||
if (this.destinationField === 'EXPENSE_TYPE') { | ||
this.destinationOptions = groupedDestinationResponse.EXPENSE_TYPE; | ||
} else { | ||
this.detailAccountType = this.chartOfAccounts; | ||
this.destinationOptions = this.appName !== AppName.QBD_DIRECT ? groupedDestinationResponse.ACCOUNT : this.destinationOptionsWatcher( this.chartOfAccounts, groupedDestinationResponse.ACCOUNT as QbdDirectDestinationAttribute[]); | ||
} | ||
} | ||
|
@@ -212,8 +216,9 @@ export class DashboardErrorSectionComponent implements OnInit { | |
} | ||
|
||
handleResolvedMappingStat(): void { | ||
this.dashboardService.getExportErrors(this.errorsVersion, this.appName).subscribe((errors) => { | ||
const argument = this.errorsVersion === 'v1' ? errors : (errors as ErrorResponse).results; | ||
const errorVersion = this.appName === AppName.QBD_DIRECT ? this.appName : this.errorsVersion; | ||
this.dashboardService.getExportErrors(errorVersion).subscribe((errors) => { | ||
const argument = errorVersion === 'v1' ? errors : (errors as ErrorResponse).results; | ||
Comment on lines
+219
to
+221
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling for export errors retrieval. The service call to - this.dashboardService.getExportErrors(errorVersion).subscribe((errors) => {
+ this.dashboardService.getExportErrors(errorVersion).subscribe({
+ next: (errors) => {
const argument = errorVersion === 'v1' ? errors : (errors as ErrorResponse).results;
const newError: AccountingGroupedErrors = this.formatErrors(argument);
+ },
+ error: (error) => {
+ console.error('Failed to fetch export errors:', error);
+ // Consider showing an error message to the user
+ }
+ });
|
||
const newError: AccountingGroupedErrors = this.formatErrors(argument); | ||
|
||
if (this.errors.CATEGORY_MAPPING.length !== newError.CATEGORY_MAPPING.length) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix form control name to match type definition
The form control name uses the misspelled version
autoCreateReimbursableEnitity
while accessing a correctly spelled property from the API response.Apply this fix:
📝 Committable suggestion