chore: improve error handling when user lacks permission in incoming integrations - #41090
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (5)
|
| Layer / File(s) | Summary |
|---|---|
Separate missing-user and permission errors apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts, apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts, packages/i18n/src/locales/en.i18n.json, apps/meteor/tests/end-to-end/api/incoming-integrations.ts |
The add and update incoming integration methods now throw error-invalid-user / error-invalid-post-as-user only when the resolved user is missing, and error-user-lacks-message-impersonate-permission when the user exists but lacks message-impersonate; the English locale adds the matching message, and the incoming integration end-to-end tests now assert that error. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
- RocketChat/Rocket.Chat#41017: Modifies the same incoming integration validation paths and
message-impersonatepermission handling.
Suggested labels
type: chore
Suggested reviewers
- gabriellsh
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title clearly matches the main change: more specific error handling for missing permissions in incoming integrations. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
Warning
Review ran into problems
🔥 Problems
Errors were encountered while retrieving linked issues.
Errors (1)
- CORE-2350: Request failed with status code 401
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-8.6.0 #41090 +/- ##
================================================
Coverage ? 70.17%
================================================
Files ? 3365
Lines ? 129983
Branches ? 22552
================================================
Hits ? 91221
Misses ? 35454
Partials ? 3308
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Not much to add here, just a question of narrowing error-handling, making them clearer for the end user.
Issue(s)
CORE-2350 https://rocketchat.atlassian.net/browse/CORE-2350
Steps to test or reproduce
Further comments
Summary by CodeRabbit