fix(webhooks): accept 2xx status codes in webhook test endpoint#7992
fix(webhooks): accept 2xx status codes in webhook test endpoint#7992Marnie0415 wants to merge 1 commit into
Conversation
|
@Marnie0415 is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
Estimated code review effort: 2 (Simple) | ~5 minutes Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/webhooks/views.py (1)
50-58: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAlign the webhook error payload with the existing test assertions.
detailandbodynow differ fromapi/tests/unit/webhooks/test_unit_webhooks.py, so this change will fail the current webhook test unless the response text and assertions are updated together.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f9f7ee49-c4fd-41aa-8ca0-d1ad7707e07b
📒 Files selected for processing (1)
api/webhooks/views.py
The webhook test endpoint previously treated any non-200 response as a failure. This is incorrect — HTTP 201 (Created), 202 (Accepted), and 204 (No Content) are all valid webhook responses. Change the condition from \status_code != 200\ to \status_code >= 400\ so that any 2xx response is considered successful. Also update the error message to include the actual status code instead of telling users to return a 200 OK.
Marnie0415
force-pushed
the
fix/webhook-test-status-codes
branch
from
8888e4c to
c595f84
Compare
|
Note: the existing test in \ est_send_test_webhook__various_error_status_codes__returns_correct_response\ has been updated to match the new error message and body format. The test now also validates that the actual status code from the external webhook is included in the response. |
|
Updated test assertions in test_unit_webhooks.py to match the new error messages. |
1 participant
Changes
The webhook test endpoint (
POST /api/v1/webhooks/test/) previously treated any non-200 response as a failure. This is incorrect - HTTP 201 (Created), 202 (Accepted), and 204 (No Content) are all valid and common webhook responses.Before: Users testing a webhook that returns 201 or 204 would see
Webhook returned invalid status - Please check the webhook endpoint to validate it returns a 200 OK.even though their webhook is working correctly.After: Any 2xx response (200-299) is treated as successful. Only 4xx/5xx responses are reported as failures, with the actual status code included in the error message.
How did you test this code?
!= 200->>= 400)test_unit_webhooks.pyto match the new error messages