Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 2 additions & 2 deletions app/clients/airtable/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,12 @@ def get_latest_newsletter_templates(cls):
if not cls.table_exists():
cls.create_table()

results = cls.all(sort=["-Created at"], max_records=1)
results = cls.all(sort=["-Created at"], max_records=3)
if not results:
response = Response()
response.status_code = 404
raise HTTPError(response=response)
return results[0]
return results

@classmethod
def get_table_schema(cls) -> Dict[str, Any]:
Expand Down
61 changes: 53 additions & 8 deletions app/newsletter/rest.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
from uuid import UUID

from flask import Blueprint, current_app, jsonify, request
from requests import HTTPError
from sqlalchemy.exc import SQLAlchemyError

from app.clients.airtable.models import LatestNewsletterTemplate, NewsletterSubscriber
from app.config import QueueNames
Expand Down Expand Up @@ -133,7 +136,7 @@ def send_latest_newsletter(subscriber_id):
subscriber = NewsletterSubscriber.from_id(record_id=subscriber_id)
except HTTPError as e:
if e.response.status_code == 404:
raise InvalidRequest("Subscriber not found", status_code=404)
raise InvalidRequest(f"Subscriber not found: {e.response}", status_code=404)
raise InvalidRequest(f"Failed to fetch subscriber: {e.response.text}", status_code=e.response.status_code)

if subscriber.status != NewsletterSubscriber.Statuses.SUBSCRIBED.value:
Expand Down Expand Up @@ -167,7 +170,7 @@ def get_subscriber():


def _send_latest_newsletter(subscriber_id, recipient_email, language):
# Get the current newsletter template IDs from Airtable
# Get the current newsletter template IDs from Airtable (up to 3 most recent)
try:
latest_newsletter_templates = LatestNewsletterTemplate.get_latest_newsletter_templates()
except HTTPError as e:
Expand All @@ -177,12 +180,54 @@ def _send_latest_newsletter(subscriber_id, recipient_email, language):
f"Failed to fetch latest newsletter templates: {e.response.text}", status_code=e.response.status_code
)

# Fetch the template from the DB depending on the subscriber's language
template = (
dao_get_template_by_id(latest_newsletter_templates.template_id_en)
if language == NewsletterSubscriber.Languages.EN.value
else dao_get_template_by_id(latest_newsletter_templates.template_id_fr)
)
template = None
template_id = None
first_template_id = None

# Iterate through the 3 latest newsletter templates from Airtable and use the most recent
# template that currently exists in the DB
for index, newsletter_template in enumerate(latest_newsletter_templates):
# Get the appropriate template ID based on language
template_id = (
newsletter_template.template_id_en
if language == NewsletterSubscriber.Languages.EN.value
else newsletter_template.template_id_fr
)

# Validate that template_id is a valid UUID before attempting to use it
try:
UUID(template_id)
except (ValueError, TypeError, AttributeError):
current_app.logger.warning(
f"Template ID {template_id} is not a valid UUID, continuing search for a valid newsletter template_id."
)
if index == 0:
first_template_id = template_id
continue

try:
template = dao_get_template_by_id(template_id)
# Keep track of and log when we cannot use the latest template_id found in Airtable
if first_template_id and index > 0:
current_app.logger.warning(
f"Most recent template from Airtable: {first_template_id} not found in database. "
f"Using fallback template: {template_id} for subscriber: {subscriber_id}"
)
break # Successfully found a template
except SQLAlchemyError:
current_app.logger.warning(f"Template {template_id} not found in database, trying next template.")
if index == 0:
first_template_id = template_id
continue # Try the next template pair

# If we didn't find any valid template, raise an error
if template is None:
current_app.logger.error("No valid newsletter templates found in database.")
raise InvalidRequest(
f"Latest newsletter was not sent to {subscriber_id}. No valid newsletter templates found in database.",
status_code=500,
)

service = Service.query.get(current_app.config["NOTIFY_SERVICE_ID"])

# Save and send the notification
Expand Down
6 changes: 3 additions & 3 deletions tests/app/clients/test_airtable.py
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,8 @@ def test_get_latest_newsletter_templates_success(self, mocker):

result = LatestNewsletterTemplate.get_latest_newsletter_templates()

assert result == mock_template
mock_all.assert_called_once_with(sort=["-Created at"], max_records=1)
assert result == [mock_template]
mock_all.assert_called_once_with(sort=["-Created at"], max_records=3)

def test_get_latest_newsletter_templates_creates_table_if_not_exists(self, mocker):
"""Test get_latest_newsletter_templates creates table if it doesn't exist."""
Expand All @@ -299,7 +299,7 @@ def test_get_latest_newsletter_templates_creates_table_if_not_exists(self, mocke
result = LatestNewsletterTemplate.get_latest_newsletter_templates()

mock_create_table.assert_called_once()
assert result == mock_template
assert result == [mock_template]

def test_get_latest_newsletter_templates_not_found(self, mocker):
"""Test get_latest_newsletter_templates raises HTTPError when no templates found."""
Expand Down
128 changes: 127 additions & 1 deletion tests/app/newsletter/test_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ def test_send_latest_newsletter_subscriber_not_found(self, admin_request, mocker
response = admin_request.get("newsletter.send_latest_newsletter", subscriber_id="rec999999", _expected_status=404)

assert response["result"] == "error"
assert response["message"] == "Subscriber not found"
assert "Subscriber not found" in response["message"]

def test_send_latest_newsletter_api_error(self, admin_request, mocker):
mock_response = Response()
Expand All @@ -342,6 +342,132 @@ def test_send_latest_newsletter_api_error(self, admin_request, mocker):
assert response["result"] == "error"
assert "Failed to fetch subscriber" in response["message"]

def test_send_latest_newsletter_all_templates_not_found(self, admin_request, mocker, mock_subscriber):
from sqlalchemy.exc import SQLAlchemyError

mock_subscriber.status = NewsletterSubscriber.Statuses.SUBSCRIBED.value
mocker.patch("app.newsletter.rest.NewsletterSubscriber.from_id", return_value=mock_subscriber)

# Mock LatestNewsletterTemplate to return a list of 3 template pairs
mock_newsletter_template_1 = Mock()
mock_newsletter_template_1.template_id_en = "template-en-123"
mock_newsletter_template_1.template_id_fr = "template-fr-123"

mock_newsletter_template_2 = Mock()
mock_newsletter_template_2.template_id_en = "template-en-456"
mock_newsletter_template_2.template_id_fr = "template-fr-456"

mock_newsletter_template_3 = Mock()
mock_newsletter_template_3.template_id_en = "template-en-789"
mock_newsletter_template_3.template_id_fr = "template-fr-789"

mocker.patch(
"app.newsletter.rest.LatestNewsletterTemplate.get_latest_newsletter_templates",
return_value=[mock_newsletter_template_1, mock_newsletter_template_2, mock_newsletter_template_3],
)

# Mock dao_get_template_by_id to always raise SQLAlchemyError
mocker.patch("app.newsletter.rest.dao_get_template_by_id", side_effect=SQLAlchemyError("Template not found"))

response = admin_request.get("newsletter.send_latest_newsletter", subscriber_id="rec123456", _expected_status=500)

assert response["result"] == "error"
assert "Latest newsletter was not sent" in response["message"]
assert "No valid newsletter templates found in database" in response["message"]

@pytest.mark.parametrize(
"template_1_en, template_1_fr, template_2_en, template_2_fr, template_3_en, template_3_fr, expected_dao_calls",
[
# All valid UUIDs - first template fails, second succeeds
(
"a290f1ee-6c54-4b01-90e6-d701748f0851",
"a520c123-4d32-4c01-80f5-e801648f0962",
"b290f1ee-6c54-4b01-90e6-d701748f0851",
"b520c123-4d32-4c01-80f5-e801648f0962",
"c290f1ee-6c54-4b01-90e6-d701748f0851",
"c520c123-4d32-4c01-80f5-e801648f0962",
2, # First template queried but fails, second succeeds
),
# First two invalid UUIDs, third valid - only third is queried
(
"not-a-valid-uuid",
"also-not-valid",
"still-invalid",
"nope",
"d290f1ee-6c54-4b01-90e6-d701748f0851",
"d520c123-4d32-4c01-80f5-e801648f0962",
1, # Only the third template is queried
),
],
)
def test_send_latest_newsletter_uses_fallback_template(
self,
admin_request,
mocker,
mock_subscriber,
template_1_en,
template_1_fr,
template_2_en,
template_2_fr,
template_3_en,
template_3_fr,
expected_dao_calls,
):
from sqlalchemy.exc import SQLAlchemyError

mock_subscriber.status = NewsletterSubscriber.Statuses.SUBSCRIBED.value
mocker.patch("app.newsletter.rest.NewsletterSubscriber.from_id", return_value=mock_subscriber)

# Mock LatestNewsletterTemplate to return a list of 3 template pairs
mock_newsletter_template_1 = Mock()
mock_newsletter_template_1.template_id_en = template_1_en
mock_newsletter_template_1.template_id_fr = template_1_fr

mock_newsletter_template_2 = Mock()
mock_newsletter_template_2.template_id_en = template_2_en
mock_newsletter_template_2.template_id_fr = template_2_fr

mock_newsletter_template_3 = Mock()
mock_newsletter_template_3.template_id_en = template_3_en
mock_newsletter_template_3.template_id_fr = template_3_fr

mocker.patch(
"app.newsletter.rest.LatestNewsletterTemplate.get_latest_newsletter_templates",
return_value=[mock_newsletter_template_1, mock_newsletter_template_2, mock_newsletter_template_3],
)

# Mock template object
mock_template = Mock()
mock_template.id = template_3_en if expected_dao_calls == 1 else template_2_en
mock_template.version = 1

# Setup dao mock based on test scenario
mock_dao = mocker.patch("app.newsletter.rest.dao_get_template_by_id")
if expected_dao_calls == 2:
# First valid UUID fails, second succeeds
mock_dao.side_effect = [
SQLAlchemyError("Template not found"),
mock_template,
]
else:
# Only third template is queried (first two are invalid UUIDs)
mock_dao.return_value = mock_template

# Mock Service query
mock_service = Mock()
mock_query = Mock()
mock_query.get.return_value = mock_service
mocker.patch("app.newsletter.rest.Service.query", mock_query)

mocker.patch("app.newsletter.rest.persist_notification", return_value=Mock())
mocker.patch("app.newsletter.rest.send_notification_to_queue")

response = admin_request.get("newsletter.send_latest_newsletter", subscriber_id="rec123456", _expected_status=200)

assert response["result"] == "success"
# Verify that dao_get_template_by_id was called the expected number of times
assert mock_dao.call_count == expected_dao_calls


class TestGetSubscriber:
def test_get_subscriber_by_id_success(self, admin_request, mocker, mock_subscriber):
Expand Down
Loading