fix(issues): Reject mismatched status/substatus on group update#115181
fix(issues): Reject mismatched status/substatus on group update#115181mrduncan wants to merge 2 commits into
Conversation
A PUT /api/0/issues/{id}/ body with valid-but-mismatched status and
substatus (e.g. {status: ignored, substatus: ongoing}) used to issue an
UPDATE that violated the substatus_is_valid_for_status DB check
constraint and surfaced as a 500.
GroupValidator.validate() now rejects mismatched pairs with a 400.
infer_substatus drops a caller-supplied substatus that doesn't match
new_status as a backstop, falling through to inference instead.
Fixes SENTRY-5MW7
github-actions
Bot
added
the
Scope: Backend
Combine the two near-duplicate tests into a single test using the canonical bug case (ignored + ongoing) with consistent post-call assertions that the group's status is unchanged.
nora-shap
left a comment
nora-shap
left a comment
There was a problem hiding this comment.
the 2 functions have 2 different strategies - validate rejects a status/substatus mismatch but infer_substatus will fix the mismatch for you.
your pr description acknowledges this, and I agree with your strategy to reject at the validation step and give the caller correct feedback, then use this graceful backstop to avoid a 500, but I think it would be a good idea to leave a comment (probably in infer_substatus) disclosing the choice+reasoning to handle mismatches differently.
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
2 participants
A PUT /api/0/issues/{id}/ body with valid-but-mismatched status and substatus (e.g. {status: ignored, substatus: ongoing}) used to issue an UPDATE that violated the substatus_is_valid_for_status DB check constraint and surfaced as a 500.
GroupValidator.validate() now rejects mismatched pairs with a 400. infer_substatus drops a caller-supplied substatus that doesn't match new_status as a backstop, falling through to inference instead.
Fixes SENTRY-5MW7