fix: support marking an integration back as active #684
Conversation
WalkthroughThe changes primarily focus on enhancing the functionality of workspace management and QuickBooks Online (QBO) integration within the Changes
Poem
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 (
|
ashwin1111
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
apps/workspaces/actions.py (2)
Line range hint
198-201: Avoid Hardcoding Values in Test SetupIn the
setup_e2e_testsfunction, theDestinationAttribute.create_or_update_destination_attributemethod is called with hardcoded values fordestination_id,value, and other details. Hardcoding these values may lead to issues if the test environment changes or if these values differ across environments. Consider parameterizing these values or fetching them dynamically to make the tests more resilient and maintainable.Apply this diff to parameterize the values:
- DestinationAttribute.create_or_update_destination_attribute( - {'attribute_type': 'ACCOUNT', 'display_name': 'Account', 'value': 'Activity', 'destination_id': '900', 'active': True, 'detail': {"account_type": "Expense", "fully_qualified_name": "Activity"}}, workspace.id - ) + # Retrieve or parameterize account details + account_details = get_test_account_details() + DestinationAttribute.create_or_update_destination_attribute( + {'attribute_type': 'ACCOUNT', 'display_name': account_details['display_name'], 'value': account_details['value'], 'destination_id': account_details['destination_id'], 'active': True, 'detail': account_details['detail']}, workspace.id + )
Line range hint
252-268: Catch Specific Exceptions for Better Error HandlingIn the
post_to_integration_settingsfunction, catching all exceptions usingexcept Exception as errorcan obscure the actual issue and make debugging more difficult. It's a best practice to catch specific exceptions that you expect might occur. This approach enhances error transparency and helps in handling different error scenarios appropriately.Apply this diff to catch specific exceptions and re-raise unexpected ones:
- except Exception as error: + except (RequestException, SomeSpecificException) as error: logger.error(error) + # Handle specific exceptions if necessary + else: + # Re-raise any unexpected exceptions + raiseReplace
RequestExceptionandSomeSpecificExceptionwith the actual exceptions thatpost_requestmight raise.
| if workspace.onboarding_state == 'COMPLETE': | ||
| post_to_integration_settings(workspace_id, True) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Integration Settings Update on Reconnection
The addition of post_to_integration_settings(workspace_id, True) when workspace.onboarding_state == 'COMPLETE' ensures that the integration settings are updated when the onboarding process is complete. However, consider whether the integration settings should also be updated when a user reconnects their QuickBooks Online account, even if the onboarding state is not 'COMPLETE'. This would ensure that the integration status is accurate in all scenarios.
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Description
Please add PR description here, add screenshots if needed
Clickup
Clickup
Summary by CodeRabbit
New Features
Bug Fixes
Refactor