-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(integration): Add endpoint to validate a manually entered channel #101508
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
feat(integration): Add endpoint to validate a manually entered channel #101508
Conversation
src/sentry/integrations/api/endpoints/organization_integration_channel_validate.py
Fixed
Show fixed
Hide fixed
src/sentry/integrations/api/endpoints/organization_integration_channel_validate.py
Fixed
Show fixed
Hide fixed
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #101508 +/- ##
===========================================
+ Coverage 79.87% 80.99% +1.11%
===========================================
Files 8697 8705 +8
Lines 386780 386842 +62
Branches 24514 24471 -43
===========================================
+ Hits 308958 313306 +4348
+ Misses 77474 73185 -4289
- Partials 348 351 +3 |
leeandher
left a comment
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.
Looks great! Minor comments, but logically looks excellent, great tests too 🔥
src/sentry/integrations/api/endpoints/organization_integration_channel_validate.py
Fixed
Show fixed
Hide fixed
| except Exception as e: | ||
| return Response( | ||
| {"valid": False, "detail": str(e) or "Unexpected error"}, |
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.
I don't think we need this large wrapping try/except, If there are individual parts we worry about failing or timing out (maybe the slack util get_channel_id, and the other equivalents) then we can just wrap those portions so it's easier to understand what we're worrying about breaking.
That said anything that isn't an API call out to an integration seems super straight forward to me so I don't think this is necessary!
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.
Thanks for the feedback! I updated the code so it no longer exposes raw responses, filters out known exceptions (ApiError, ValidationError, etc.), and uses sentry_sdk.capture_exception just for unexpected errors. I kept a single try/except around everything for simplicity - hope that is fine!
…r-custom-channel-validation
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
c0a642c to
375e89f
Compare
…r-custom-channel-validation
…r-custom-channel-validation
…r-custom-channel-validation
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues: |
Problem
The Slack API has some limitations (see the PR discussion). We list up to 1,000 channels in the dropdown, but users might not find the one they want. To address this, we will allow users to manually enter a channel. If the channel was typed instead of selected from the list, we want to validate it on the component’s onBlur event.
Solution
We’re introducing a separate endpoint specifically for channel validation, since the form might not be complete and we cannot fire the same request used when submitting the form.
This validation may not be necessary for MS Teams or Discord, but we can include it for consistency.
Contributes to https://linear.app/getsentry/issue/TET-1229/implement-dropdown-or-validation-for-slack-field