diff --git a/app/dao/unsubscribe_request_dao.py b/app/dao/unsubscribe_request_dao.py index 0a03ffacef..43ef48be64 100644 --- a/app/dao/unsubscribe_request_dao.py +++ b/app/dao/unsubscribe_request_dao.py @@ -22,7 +22,7 @@ def create_unsubscribe_request_dao(unsubscribe_data): def get_unsubscribe_request_by_notification_id_dao(notification_id): - return UnsubscribeRequest.query.filter_by(notification_id=notification_id).one() + return UnsubscribeRequest.query.filter_by(notification_id=notification_id).one_or_none() def get_unsubscribe_requests_statistics_dao(service_id): diff --git a/app/one_click_unsubscribe/rest.py b/app/one_click_unsubscribe/rest.py index 4beaa4525e..b3664e20d5 100644 --- a/app/one_click_unsubscribe/rest.py +++ b/app/one_click_unsubscribe/rest.py @@ -8,6 +8,8 @@ from app.dao.unsubscribe_request_dao import ( create_unsubscribe_request_dao, get_unbatched_unsubscribe_requests_dao, + get_unsubscribe_request_by_notification_id_dao, + get_unsubscribe_request_report_by_id_dao, get_unsubscribe_request_reports_dao, ) from app.errors import InvalidRequest, register_errors @@ -30,6 +32,10 @@ def one_click_unsubscribe(notification_id, token): errors = {"unsubscribe request": "This is not a valid unsubscribe link."} raise InvalidRequest(errors, status_code=404) from e + # Check if the unsubscribe request is a duplicate + if is_duplicate_unsubscribe_request(notification_id): + return jsonify(result="success", message="Unsubscribe successful"), 200 + if notification := get_notification_by_id(notification_id): unsubscribe_data = get_unsubscribe_request_data(notification, email_address) @@ -69,3 +75,22 @@ def create_unsubscribe_request_reports_summary(service_id): UnsubscribeRequestReport.serialize_unbatched_requests(unbatched_unsubscribe_requests) ] + unsubscribe_request_reports return unsubscribe_request_reports + + +def is_duplicate_unsubscribe_request(notification_id): + """ + A duplicate unsubscribe request is being defined as an unsubscribe_request that has + the same notification_id of a previously received unsubscribe request that has not yet been processed + by the service that initiated the notification. + """ + unsubscribe_request = get_unsubscribe_request_by_notification_id_dao(notification_id) + + if not unsubscribe_request: + return False + + report_id = unsubscribe_request.unsubscribe_request_report_id + + if report_id and get_unsubscribe_request_report_by_id_dao(report_id).processed_by_service_at: + return False + + return True diff --git a/tests/app/db.py b/tests/app/db.py index 74f2ca0f81..fa719014b9 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -80,6 +80,7 @@ User, WebauthnCredential, ) +from app.utils import midnight_n_days_ago def create_user(*, mobile_number="+447700900986", email=None, state="active", id_=None, name="Test User", **kwargs): @@ -1322,3 +1323,38 @@ def create_unsubscribe_request_report( ) create_unsubscribe_request_reports_dao(report) return report + + +def create_unsubscribe_request_and_return_the_notification_id( + sample_service, sample_template, is_batched, processed_by_service +): + notification = create_notification( + template=sample_template, + to_field="example@example.com", + sent_at=datetime.now() - timedelta(days=4), + ) + + unsubscribe_request_report_id = None + if is_batched: + # Create processed unsubscribe request report + unsubscribe_request_report = create_unsubscribe_request_report( + sample_service, + earliest_timestamp=midnight_n_days_ago(4), + latest_timestamp=midnight_n_days_ago(2), + processed_by_service_at=midnight_n_days_ago(1) if processed_by_service else None, + ) + unsubscribe_request_report_id = unsubscribe_request_report.id + + # Create an unsubscribe request + create_unsubscribe_request_dao( + { + "notification_id": notification.id, + "template_id": notification.template_id, + "template_version": notification.template_version, + "service_id": sample_service.id, + "email_address": notification.to, + "created_at": datetime.now(), + "unsubscribe_request_report_id": unsubscribe_request_report_id, + } + ) + return notification.id diff --git a/tests/app/one_click_unsubscribe/test_one_click_unsubscribe.py b/tests/app/one_click_unsubscribe/test_one_click_unsubscribe.py index 0d765dd9c8..d42973e0bf 100644 --- a/tests/app/one_click_unsubscribe/test_one_click_unsubscribe.py +++ b/tests/app/one_click_unsubscribe/test_one_click_unsubscribe.py @@ -1,13 +1,16 @@ import uuid from unittest.mock import call +import pytest from flask import current_app from notifications_utils.url_safe_token import generate_token from app.constants import EMAIL_TYPE from app.dao.templates_dao import dao_update_template from app.dao.unsubscribe_request_dao import get_unsubscribe_request_by_notification_id_dao -from tests.app.db import create_notification, create_template +from app.models import UnsubscribeRequest +from app.one_click_unsubscribe.rest import is_duplicate_unsubscribe_request +from tests.app.db import create_notification, create_template, create_unsubscribe_request_and_return_the_notification_id def unsubscribe_url_post(client, notification_id, token): @@ -36,6 +39,21 @@ def test_valid_one_click_unsubscribe_url(mocker, client, sample_email_notificati ] +def test_duplicate_unsubscribe_requests(mocker, client, sample_email_notification): + token = generate_token( + sample_email_notification.to, current_app.config["SECRET_KEY"], current_app.config["DANGEROUS_SALT"] + ) + # first unsubscribe request + unsubscribe_url_post(client, sample_email_notification.id, token) + # duplicate unsubscribe request + unsubscribe_url_post(client, sample_email_notification.id, token) + + result = UnsubscribeRequest.query.filter_by(notification_id=sample_email_notification.id).all() + + # The duplicate unsubscribe request shouldn't be added to the unsubscribe_request table + assert len(result) == 1 + + def test_unsubscribe_request_object_refers_to_correct_template_version_after_template_updated(client, sample_service): test_template = create_template( sample_service, @@ -119,3 +137,26 @@ def test_invalid_one_click_unsubscribe_url_notification_id(client, sample_email_ response_json_data = response.get_json() assert response.status_code == 404 assert response_json_data["message"] == {"unsubscribe request": "This is not a valid unsubscribe link."} + + +@pytest.mark.parametrize( + "create_previous_unsubscribe_request,is_batched, processed_by_service, expected_result", + [(False, False, False, False), (True, True, True, False), (True, False, False, True), (True, True, False, True)], +) +def test_is_duplicate_unsubscribe_request( + sample_service, + sample_email_template, + create_previous_unsubscribe_request, + is_batched, + processed_by_service, + expected_result, +): + if create_previous_unsubscribe_request: + notification_id = create_unsubscribe_request_and_return_the_notification_id( + sample_service, sample_email_template, is_batched, processed_by_service + ) + else: + notification_id = uuid.uuid4() + + result = is_duplicate_unsubscribe_request(notification_id) + assert result == expected_result