feat: qbd direct tracking for export and sync on click #1134
Conversation
WalkthroughThis pull request modifies the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)src/app/core/models/enum/enum.model.ts (2)
The addition of step numbers (2.1, 2.2, 2.3) in the descriptions provides better clarity on the sequence of actions in the QuickBooks Desktop Connector workflow.
The naming inconsistency with "Quickbooks" vs "QuickBooks" was previously identified. Please ensure consistent naming across the codebase. 🪧 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 (
|
|
PR description must contain a link to a ClickUp (case-insensitive) |
1 similar comment
|
PR description must contain a link to a ClickUp (case-insensitive) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.ts (1)
Line range hint
98-134: Consider refactoring pollExportStatus methodThe pollExportStatus method is quite long and handles multiple responsibilities. Consider breaking it down into smaller, more focused methods for better maintainability.
Suggested refactor:
- Extract the export progress calculation
- Extract the error handling logic
- Extract the export summary update logic
Example structure:
private updateExportProgress(processedTasks: number): void { this.processedCount = processedTasks; this.exportProgressPercentage = Math.round((this.processedCount / this.exportableAccountingExportIds.length) * 100); } private handleExportCompletion(taskResults: QbdDirectTaskLog[]): void { forkJoin([ this.getExportErrors$, this.getAccountingExportSummary$ ]).subscribe(this.updateExportSummary.bind(this)); } private updateExportSummary([errorResponse, exportSummary]: [ErrorResponse, AccountingExportSummary]): void { this.errors = DashboardModel.parseAPIResponseToGroupedError(errorResponse.results); this.groupedErrorStat = { EMPLOYEE_MAPPING: null, CATEGORY_MAPPING: null }; this.accountingExportSummary = AccountingExportSummaryModel.parseAPIResponseToAccountingSummaryForQbdDirect(exportSummary); // ... rest of the summary update logic } private pollExportStatus(exportableAccountingExportIds: number[] = []): void { 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) => { const processedTasks = res.results.filter((task: { status: TaskLogState; }) => (task.status !== TaskLogState.IN_PROGRESS && task.status !== TaskLogState.ENQUEUED)).length; this.updateExportProgress(processedTasks); if (res.results.filter((task: { status: TaskLogState; }) => (this.exportLogProcessingStates.includes(task.status))).length === 0) { this.handleExportCompletion(res.results); } }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/app/core/models/enum/enum.model.ts(1 hunks)src/app/core/services/qbd-direct/qbd-direct-core/qbd-direct-helper.service.ts(2 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.ts(2 hunks)src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-main.component.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/core/services/qbd-direct/qbd-direct-core/qbd-direct-helper.service.ts
[error] 21-21: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
🔇 Additional comments (5)
src/app/core/services/qbd-direct/qbd-direct-core/qbd-direct-helper.service.ts (2)
17-18: LGTM: TrackingService integration
The TrackingService dependency is correctly injected into the constructor.
26-26: LGTM: Sync tracking implementation
The tracking event is correctly implemented for the sync operation.
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-main.component.ts (1)
36-36: LGTM: Dependency cleanup
Good cleanup of unused service dependencies, improving code maintainability.
src/app/integrations/qbd-direct/qbd-direct-main/qbd-direct-dashboard/qbd-direct-dashboard.component.ts (2)
86-86: LGTM: TrackingService integration
The TrackingService dependency is correctly injected into the constructor.
92-92: LGTM: Export tracking implementation
The tracking event is correctly implemented for the export operation.
| @@ -13,14 +14,16 @@ export class QbdDirectHelperService { | |||
| constructor( | |||
| private apiService: ApiService, | |||
| private workspaceService: WorkspaceService, | |||
| private toastService: IntegrationsToastService | |||
| private toastService: IntegrationsToastService, | |||
| private trackingService: TrackingService | |||
| ) { } | |||
|
|
|||
| importQBDAttributes(refresh: boolean): Observable<{}> { | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace {} type with a more specific return type
Using {} as a return type is discouraged as it means "any non-nullable value". Consider defining a specific interface or type for the response.
- importQBDAttributes(refresh: boolean): Observable<{}> {
+ importQBDAttributes(refresh: boolean): Observable<void> {Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 21-21: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
| QBD_DIRECT_EXPORT = 'Direct Export Quickbooks Desktop', | ||
| QBD_DIRECT_SYNC = 'Sync Quickbooks Desktop Direct' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistent naming conventions for enum values
The new enum values have inconsistent naming patterns compared to existing QuickBooks-related entries:
- "Quickbooks" should be "QuickBooks" (two words) to match existing entries
- Word order should follow the established pattern
Apply this diff to maintain consistency:
- QBD_DIRECT_EXPORT = 'Direct Export Quickbooks Desktop',
- QBD_DIRECT_SYNC = 'Sync Quickbooks Desktop Direct'
+ QBD_DIRECT_EXPORT = 'Export QuickBooks Desktop Direct',
+ QBD_DIRECT_SYNC = 'Sync QuickBooks Desktop Direct'📝 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.
| QBD_DIRECT_EXPORT = 'Direct Export Quickbooks Desktop', | |
| QBD_DIRECT_SYNC = 'Sync Quickbooks Desktop Direct' | |
| QBD_DIRECT_EXPORT = 'Export QuickBooks Desktop Direct', | |
| QBD_DIRECT_SYNC = 'Sync QuickBooks Desktop Direct' |
|
PR description must contain a link to a ClickUp (case-insensitive) |
|
PR description must contain a link to a ClickUp (case-insensitive) |
|
* feat: qbd direct tracking for export and sync on click * wording change for connector page
Description
feat: qbd direct tracking for export and sync on click
Clickup
https://app.clickup.com
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Refactor