-
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 connection phase fix #1155
Conversation
WalkthroughThe pull request focuses on enhancing the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
🪧 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 (
|
|
@@ -174,17 +174,19 @@ export class QbdDirectOnboardingConnectorComponent implements OnInit { | |||
this.connectionStatus = QBDConnectionStatus.INCORRECT_COMPANY_PATH; | |||
this.warningDialogText = 'Incorrect company file path detected. Please check and try again.'; | |||
this.isDialogVisible = true; | |||
this.isConnectionLoading = false; |
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.
this change is done, because the step 2.2 is not waiting or show loading until it hit the terminal condition as we have the isConnectionLoading
false in the else case.
can see the video attached below for reference
Screen.Recording.2025-01-20.at.11.49.42.AM.mov
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.ts (3)
177-177
: Consider refactoring repeated loading state management.The loading state management is duplicated across different status handling branches. Consider moving
this.isConnectionLoading = false
to the end of the method to reduce code duplication.handleStatus(status: QbdDirectWorkspace): void { const onboardingState = status.onboarding_state; if (onboardingState === QbdDirectOnboardingState.INCORRECT_COMPANY_PATH) { this.connectionStatus = QBDConnectionStatus.INCORRECT_COMPANY_PATH; this.warningDialogText = 'Incorrect company file path detected. Please check and try again.'; this.isDialogVisible = true; - this.isConnectionLoading = false; } else if (onboardingState === QbdDirectOnboardingState.INCORRECT_PASSWORD) { this.connectionStatus = QBDConnectionStatus.IN_CORRECT_PASSWORD; this.warningDialogText = 'Incorrect password detected. Please check and try again.'; this.isDialogVisible = true; - this.isConnectionLoading = false; } else if (onboardingState === QbdDirectOnboardingState.DESTINATION_SYNC_IN_PROGRESS || onboardingState === QbdDirectOnboardingState.DESTINATION_SYNC_COMPLETE) { this.connectionStatus = QBDConnectionStatus.SUCCESS; this.isConnectionCTAEnabled = true; - this.isConnectionLoading = false; } + this.isConnectionLoading = false; }Also applies to: 183-183, 188-188
262-262
: Good use of Object.assign to prevent mutations.Creating a shallow copy prevents unintended mutations of the original workspace response. Consider using a more descriptive variable name like
workspaceResponseCopy
instead ofoldWorkspaceResponse
to better reflect its purpose.-const oldWorkspaceResponse = Object.assign({}, workspaceResponse); +const workspaceResponseCopy = Object.assign({}, workspaceResponse);
Line range hint
74-91
: Consider implementing a retry limit for connection attempts.While the error handling is comprehensive, there's no limit on how many times a user can retry the connection. Consider implementing a maximum retry count to prevent infinite attempts and provide appropriate feedback when the limit is reached.
Example implementation:
private readonly MAX_RETRY_ATTEMPTS = 3; private retryCount = 0; onConnectionDone(event: CheckBoxUpdate) { if (this.retryCount >= this.MAX_RETRY_ATTEMPTS) { this.toastService.displayToastMessage( ToastSeverity.ERROR, 'Maximum retry attempts reached. Please contact support.' ); return; } this.isConnectionLoading = true; if (event.value) { this.retryCount++; // ... rest of the existing code } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/app/integrations/qbd-direct/qbd-direct-onboarding/qbd-direct-onboarding-connector/qbd-direct-onboarding-connector.component.ts
(2 hunks)
Description
fix: qbd direct connection phase fix
Clickup
https://app.clickup.com/
Summary by CodeRabbit