-
Notifications
You must be signed in to change notification settings - Fork 0
fix: QA fixes #1078
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: QA fixes #1078
Changes from all commits
77b39c7
e2f390d
9640a93
9c7f344
a5eaa57
a561310
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 |
|---|---|---|
|
|
@@ -131,7 +131,8 @@ export const fyleKbArticles: KbArticle[string] = { | |
| ADVANCED_SETTING: `${brandingConfig.helpArticleDomain}/en/articles/8394683-how-to-configure-the-fyle-sage-intacct-integration#h_3f6718633c`, | ||
| LANDING: `${brandingConfig.helpArticleDomain}/en/articles/8394683-how-to-configure-the-fyle-sage-intacct-integration`, | ||
| CONNECTOR: `${brandingConfig.helpArticleDomain}/en/articles/8394683-how-to-configure-the-fyle-sage-intacct-integration#h_38e0c9bea6`, | ||
| SKIP_EXPORT: `${brandingConfig.helpArticleDomain}/en/articles/7882821-how-to-skip-exporting-specific-expenses-from-fyle-to-sage-intacct` | ||
| SKIP_EXPORT: `${brandingConfig.helpArticleDomain}/en/articles/7882821-how-to-skip-exporting-specific-expenses-from-fyle-to-sage-intacct`, | ||
| HELPER_ARTICLE: `${brandingConfig.helpArticleDomain}/en/articles/7882821-how-to-skip-exporting-specific-expenses-from-fyle-to-sage-intacct` | ||
|
Comment on lines
+134
to
+135
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 All help article URLs in QBD_DIRECT section are incorrectly pointing to Sage Intacct documentation The entire QBD_DIRECT section in the branding config is using Sage Intacct documentation URLs for all its properties:
This appears to be a copy-paste error where the entire Sage Intacct section was duplicated for QBD_DIRECT without updating the URLs to point to the correct QuickBooks Desktop documentation. 🔗 Analysis chainIncorrect documentation URLs for QuickBooks Desktop The SKIP_EXPORT and HELPER_ARTICLE URLs under QBD_DIRECT are currently pointing to Sage Intacct documentation. This needs to be updated with the correct QuickBooks Desktop documentation URLs. Let's verify if similar URL mismatches exist in other sections: Let me verify the URLs in the QBD_DIRECT section by checking the branding config file: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for potential documentation URL mismatches
# Look for URLs that contain product names in sections for different products
echo "Checking for potential URL mismatches..."
rg -n "intacct" | grep -i "qbd\|quickbooks"
rg -n "sage" | grep -i "qbd\|quickbooks"
Length of output: 10043 Script: #!/bin/bash
# Check the QBD_DIRECT section in the branding config
rg -A 10 "QBD_DIRECT" src/app/branding/fyle-branding-config.ts
Length of output: 2282 |
||
| } | ||
| } | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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[] { | ||
|
|
@@ -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
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. 🛠️ Refactor suggestion Add error handling and use a more maintainable approach for field mapping. The current implementation has potential issues:
Consider this more robust approach: + private static readonly MEMO_FIELD_MAPPING = {
+ 'Expense/Report ID': 'expense_key'
+ } as const;
static constructPayload(advancedSettingForm: FormGroup, adminEmails: EmailOption[]): QbdDirectAdvancedSettingsPost {
const topMemo: string[] = advancedSettingForm.controls.topMemoStructure.value;
- const index = topMemo.indexOf('Expense/Report ID');
- topMemo[index] = 'expense_key';
+ const mappedMemo = topMemo.map(field =>
+ QbdDirectAdvancedSettingsModel.MEMO_FIELD_MAPPING[field] || field
+ );This approach:
|
||
| const allSelectedEmails: EmailOption[] = advancedSettingForm.get('email')?.value; | ||
|
|
||
| const selectedEmailsEmails = allSelectedEmails?.filter((email: EmailOption) => adminEmails.includes(email)); | ||
|
|
||
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 incorrect help article URL for QBD_DIRECT
The
HELPER_ARTICLEURL points to a Sage Intacct article ("how-to-skip-exporting-specific-expenses-from-fyle-to-sage-intacct") instead of a QuickBooks Desktop article. This could mislead users by providing incorrect documentation.Please update the URL to point to the correct QBD documentation.