Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Adds tests for email endpoints and dispatch service #1112

Merged
merged 4 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/fidesops/ops/api/v1/endpoints/email_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
HTTP_400_BAD_REQUEST,
HTTP_404_NOT_FOUND,
HTTP_422_UNPROCESSABLE_ENTITY,
HTTP_500_INTERNAL_SERVER_ERROR,
)

from fidesops.ops.api import deps
Expand Down Expand Up @@ -76,7 +77,7 @@ def post_config(
except Exception as exc:
logger.warning(f"Create failed for email config {email_config.key}: {exc}")
raise HTTPException(
status_code=HTTP_422_UNPROCESSABLE_ENTITY,
status_code=HTTP_500_INTERNAL_SERVER_ERROR,
detail=f"Config with key {email_config.key} failed to be added",
)

Expand Down Expand Up @@ -107,7 +108,7 @@ def patch_config_by_key(
except Exception as exc:
pattisdr marked this conversation as resolved.
Show resolved Hide resolved
logger.warning(f"Patch failed for email config {email_config.key}: {exc}")
raise HTTPException(
status_code=HTTP_422_UNPROCESSABLE_ENTITY,
status_code=HTTP_500_INTERNAL_SERVER_ERROR,
detail=f"Config with key {email_config.key} failed to be added",
)

Expand Down
32 changes: 0 additions & 32 deletions src/fidesops/ops/schemas/email/email.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,38 +94,6 @@ class Config:
use_enum_values = False
orm_mode = True

@validator("details", pre=True, always=True)
def validate_details(
cls,
v: Dict[str, str],
values: Dict[str, Any],
) -> Dict[str, str]:
"""
Custom validation logic for the `details` field.
"""
service_type = values.get("service_type")
if not service_type:
raise ValueError("A `service_type` field must be specified.")

try:
schema = {
EmailServiceType.MAILGUN: EmailServiceDetailsMailgun,
}[service_type]
except KeyError:
raise ValueError(
f"`service type` {service_type} has no supported `details` validation."
)

try:
schema.parse_obj(v) # type: ignore
except ValidationError as exc:
# Pydantic requires validators raise either a ValueError, TypeError, or AssertionError
# so this exception is cast into a `ValueError`.
errors = [f"{err['msg']} {str(err['loc'])}" for err in exc.errors()]
raise ValueError(errors)

return v


class EmailConfigResponse(BaseModel):
"""Email Config Response Schema"""
Expand Down
4 changes: 2 additions & 2 deletions src/fidesops/ops/service/email/email_dispatch_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,5 +103,5 @@ def _mailgun_dispatcher(
f"Email failed to send with status code {response.status_code}"
)
except Exception as e:
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}")
Comment on lines -106 to +107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

#837

7 changes: 4 additions & 3 deletions tests/ops/api/v1/endpoints/test_email_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,10 @@ def test_post_email_config_with_invalid_mailgun_details(
auth_header = generate_auth_header([EMAIL_CREATE_OR_UPDATE])
response = api_client.post(url, headers=auth_header, json=payload)
assert 422 == response.status_code
assert json.loads(response.text)["detail"][0]["msg"] == "field required"
assert (
json.loads(response.text)["detail"][0]["msg"]
== "[\"field required ('domain',)\", \"extra fields not permitted ('invalid',)\"]"
json.loads(response.text)["detail"][1]["msg"]
== "extra fields not permitted"
)

def test_post_email_config_with_not_supported_service_type(
Expand Down Expand Up @@ -179,7 +180,7 @@ def test_post_email_config_missing_detail(
assert response.status_code == 422
errors = response.json()["detail"]
assert "details" in errors[0]["loc"]
assert errors[0]["msg"] == "[\"field required ('domain',)\"]"
assert errors[0]["msg"] == "field required"

def test_post_email_config_already_exists(
self,
Expand Down
28 changes: 28 additions & 0 deletions tests/ops/service/email/email_dispatch_service_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
from unittest.mock import Mock

import pytest
import requests.exceptions
import requests_mock
from fastapi import HTTPException
from sqlalchemy.orm import Session

from fidesops.ops.common_exceptions import EmailDispatchException
Expand Down Expand Up @@ -87,3 +90,28 @@ def test_email_dispatch_mailgun_config_no_secrets(
mock_mailgun_dispatcher.assert_not_called()

email_config.delete(db)
pattisdr marked this conversation as resolved.
Show resolved Hide resolved


def test_email_dispatch_mailgun_failed_email(db: Session, email_config) -> None:
with requests_mock.Mocker() as mock_response:
mock_response.post(
f"https://api.mailgun.net/{email_config.details[EmailServiceDetails.API_VERSION.value]}/{email_config.details[EmailServiceDetails.DOMAIN.value]}/messages",
json={
"message": "Rejected: IP <id-address> can’t be used to send the message",
"id": "<[email protected]>",
},
status_code=403,
)
with pytest.raises(EmailDispatchException) as exc:
dispatch_email(
db=db,
action_type=EmailActionType.SUBJECT_IDENTITY_VERIFICATION,
to_email="[email protected]",
email_body_params=SubjectIdentityVerificationBodyParams(
access_code="2348"
),
)
assert (
exc.value.args[0]
== "Email failed to send due to: Email failed to send with status code 403"
)