feat: qbd direct servey and info box in dashboard #1175
Conversation
WalkthroughThis pull request introduces enhancements that improve how a survey is triggered and how environment variables are configured. A new substitution for the placeholder Changes
Sequence Diagram(s)sequenceDiagram
participant Qbd as QbdDirectDashboardComponent
participant RS as refinerService
Qbd->>Qbd: Execute pollExportStatus
alt Export complete with no errors (failedExpenseGroupCount == 0)
Qbd->>RS: triggerSurvey(appName, surveyId, "export_done")
else Export has errors
Qbd->>Qbd: No survey triggered
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.ts (1)
103-142: Consider handling component destruction during polling.The polling subscription should be cleaned up when the component is destroyed to prevent memory leaks and potential errors.
Consider storing the subscription and cleaning it up in ngOnDestroy:
+private pollingSub: Subscription; private pollExportStatus(exportableAccountingExportIds: number[] = []): void { - interval(3000).pipe( + this.pollingSub = interval(3000).pipe( switchMap(() => from(this.dashboardService.getAllTasks([], exportableAccountingExportIds, [], AppName.QBD_DIRECT))), takeWhile((response: QbdDirectTaskResponse) => response.results.filter(task => (this.exportLogProcessingStates.includes(task.status)) ).length > 0, true ) ).subscribe((res: QbdDirectTaskResponse) => { // ... existing code ... }); } +ngOnDestroy(): void { + if (this.pollingSub) { + this.pollingSub.unsubscribe(); + } +}run.sh (1)
41-41: Fix potential issues with special characters in the environment variable.The current implementation might not handle special characters in
REFINER_QBD_EXPORT_DONE_SURVEY_IDcorrectly.Consider using printf for safer variable substitution:
- sed -i $SED_EXTRA_ARGS "s?{{REFINER_QBD_EXPORT_DONE_SURVEY_ID}}?${REFINER_QBD_EXPORT_DONE_SURVEY_ID}?g" $f; + printf -v safe_survey_id "%q" "${REFINER_QBD_EXPORT_DONE_SURVEY_ID}" + sed -i $SED_EXTRA_ARGS "s?{{REFINER_QBD_EXPORT_DONE_SURVEY_ID}}?${safe_survey_id}?g" $f;🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 41-41: Quotes/backslashes in this variable will not be respected.
(SC2090)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
run.sh(1 hunks)scripts/setup_env.js(1 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.ts(1 hunks)src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.html(1 hunks)src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.ts(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
run.sh
[warning] 41-41: Quotes/backslashes in this variable will not be respected.
(SC2090)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: unit-test
🔇 Additional comments (5)
src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.ts (1)
47-47: LGTM!The addition of the
AppNameproperty follows Angular best practices by exposing the enum to the template.scripts/setup_env.js (1)
44-44: LGTM!The addition of
export_done_survery_idfollows the established pattern for environment variables and maintains consistency with other refiner survey configurations.src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.ts (1)
134-138: LGTM!The survey trigger is well-implemented, with appropriate conditions to ensure it only fires after a successful export.
run.sh (1)
41-41: LGTM!The addition of the QBD export done survey ID substitution follows the established pattern.
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 41-41: Quotes/backslashes in this variable will not be respected.
(SC2090)
src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.html (1)
51-55: Conditional Rendering and Info Label Text FormattingThe new block conditionally renders an info label when
appNameequalsAppName.QBD_DIRECTandisExportInProgressis true. Please ensure that bothappNameandAppNameare properly defined and imported in the corresponding component's TypeScript file to avoid runtime issues. Additionally, theinfoTextattribute contains escaped smart quotes (\‘Update Selected\’), which may not render as intended in all browsers. Consider using standard single quotes or proper HTML entities for consistency.Suggested diff for clarity in the text formatting:
- [infoText]="'Click \‘Update Selected\’ in QuickBooks Web Connector after exporting to sync your expenses immediately'"> + [infoText]="'Click \'Update Selected\' in QuickBooks Web Connector after exporting to sync your expenses immediately'">This adjustment improves clarity and avoids potential issues with escape characters in Angular templates.
✅ Verification successful
Error: No shell scripts or web queries found
I'll start by gathering information with a shell script that checks the TypeScript file for the definitions of
appName,isExportInProgress, andAppName, and also examines the HTML file for any occurrences of the escaped smart quotes (\‘). This will help us confirm both that the component is set up correctly and that the text formatting issue is present.Please see the script below:
Once this script runs and returns its output, we can verify:
- That
appNameandAppName(as well asisExportInProgress) are correctly defined/imported in the corresponding TypeScript file.- That the HTML file contains the escaped smart quotes as mentioned, confirming the need to change them to standard ones as suggested.
Based on the results, we can then decide whether any adjustments are required beyond the recommended diff.
🏁 Script executed:
#!/bin/bash # Check if the TypeScript file exists and search for key identifiers FILE="src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.ts" echo "=== Checking $FILE for required definitions ===" if [ -f "$FILE" ]; then echo "File found: $FILE" echo "----- Occurrences of 'appName' -----" grep -H "appName" "$FILE" echo "----- Occurrences of 'isExportInProgress' -----" grep -H "isExportInProgress" "$FILE" echo "----- Occurrences of 'AppName' -----" grep -H "AppName" "$FILE" else echo "File not found: $FILE" fi # Check the HTML file for escaped smart quotes HTML_FILE="src/app/shared/components/dashboard/dashboard-export-section/dashboard-export-section.component.html" echo "=== Checking $HTML_FILE for escaped smart quotes -----" if [ -f "$HTML_FILE" ]; then grep -H "\\‘" "$HTML_FILE" || echo "No occurrences of escaped smart quotes found." else echo "File not found: $HTML_FILE" fiLength of output: 4428
Conditional Rendering & Smart Quote Correction
Both the
appNameandisExportInProgressinputs, along with theAppNameenum, are correctly defined and imported in the component’s TypeScript file. However, the HTML file still contains escaped smart quotes in theinfoTextattribute, which may lead to inconsistent rendering. Please update the escaped smart quotes according to the suggested diff:- [infoText]="'Click \‘Update Selected\’ in QuickBooks Web Connector after exporting to sync your expenses immediately'"> + [infoText]="'Click \'Update Selected\' in QuickBooks Web Connector after exporting to sync your expenses immediately'">
|
|
* qbd direct servey and info box in dashboard * adding padding
Description
feat: qbd direct servey and info box in dashboard
Clickup
https://app.clickup.com
Summary by CodeRabbit