Adds tests for email endpoints and dispatch service #1112
Conversation
| assert 422 == response.status_code | ||
| assert ( | ||
| json.loads(response.text)["detail"][0]["msg"] | ||
| == "[\"field required ('domain',)\", \"extra fields not permitted ('invalid',)\"]" |
There was a problem hiding this comment.
Could you lean on Pydantic to take care of more of the validation for you and remove some of the redundant checks in EmailConfigRequest.validate_details? For example, if you remove your validate_details, you still get a 422 with the following response body:
{
"detail":[
{
"loc":[
"body",
"details",
"domain"
],
"msg":"field required",
"type":"value_error.missing"
},
{
"loc":[
"body",
"details",
"invalid"
],
"msg":"extra fields not permitted",
"type":"value_error.extra"
}
]
}
There was a problem hiding this comment.
Ah nice, taking a look at this now
| assert response.status_code == 422 | ||
| errors = response.json()["detail"] | ||
| assert "details" in errors[0]["loc"] | ||
| assert errors[0]["msg"] == "[\"field required ('domain',)\"]" |
There was a problem hiding this comment.
Same with this response, if you get rid of your custom validation in EmailConfigRequest.validate_details, you get the following response body, without the escaping issues:
{
"loc":[
"body",
"details",
"domain"
],
"msg":"field required",
"type":"value_error.missing"
}…testing on email dispatch service
|
@pattisdr back over to you, thanks for the review! |
| logger.error(e) | ||
| raise EmailDispatchException(format("Email failed to send: %s", str(e))) | ||
| logger.error("Email failed to send: %s", str(e)) | ||
| raise EmailDispatchException(f"Email failed to send due to: {e}") |
There was a problem hiding this comment.
I do think the %-style formatting you were using is better, there's a ticket to move to that at some point, but no need to change now since it's consistent with what we have now.
Purpose
Adds tests for email endpoints and dispatch service
Changes
Checklist
CHANGELOG.mdfileCHANGELOG.mdfile is being appended toUnreleasedsection in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.Run Unsafe PR Checkslabel has been applied, and checks have passed, if this PR touches any external servicesTicket
Fixes #1086