Skip to content

Commit

Permalink
[#2441] Conditionally disable email actions based on feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
pbanaszkiewicz committed Jun 24, 2023
1 parent e418ec1 commit 79465b6
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
15 changes: 15 additions & 0 deletions amy/emails/actions.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from datetime import timedelta
import logging
from typing import Any, TypedDict

from django.conf import settings
from django.contrib import messages
from django.dispatch import receiver
from django.http import HttpRequest
Expand All @@ -12,6 +14,8 @@
from emails.signals import persons_merged_signal
from workshops.models import Person

logger = logging.getLogger("amy")


class PersonsMergedKwargs(TypedDict):
request: HttpRequest
Expand All @@ -20,8 +24,19 @@ class PersonsMergedKwargs(TypedDict):
selected_person_id: int


def check_feature_flag() -> bool:
"""Receivers will be connected no matter if EMAIL_MODULE_ENABLED is set or not.
This function helps check if the receiver should exit early when the feature flag
is disabled."""
return settings.EMAIL_MODULE_ENABLED is True


@receiver(persons_merged_signal)
def persons_merged_receiver(sender: Any, **kwargs: Unpack[PersonsMergedKwargs]) -> None:
if not check_feature_flag():
logger.debug("EMAIL_MODULE_ENABLED not set, skipping persons_merged_receiver")
return

request = kwargs["request"]
selected_person_id = kwargs["selected_person_id"]

Expand Down
6 changes: 5 additions & 1 deletion amy/emails/tests/actions/test_persons_merged.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from urllib.parse import urlencode
import weakref

from django.test import RequestFactory, TestCase
from django.test import RequestFactory, TestCase, override_settings
from django.urls import reverse

from emails.models import EmailTemplate, ScheduledEmail
Expand Down Expand Up @@ -50,6 +50,7 @@ def test_signal_received(self) -> None:
# Finally
persons_merged_signal.receivers = _copied_receivers[:]

@override_settings(EMAIL_MODULE_ENABLED=True)
def test_action_triggered(self) -> None:
# Arrange
person = Person.objects.create()
Expand Down Expand Up @@ -80,6 +81,7 @@ def test_action_triggered(self) -> None:
request, f"Action was scheduled: {scheduled_email.get_absolute_url()}."
)

@override_settings(EMAIL_MODULE_ENABLED=True)
@mock.patch("emails.actions.messages")
@mock.patch("emails.actions.timezone")
def test_email_scheduled(
Expand Down Expand Up @@ -114,6 +116,7 @@ def test_email_scheduled(
to_header=[person.email],
)

@override_settings(EMAIL_MODULE_ENABLED=True)
@mock.patch("emails.actions.messages")
def test_missing_template(self, mock_messages: mock.MagicMock) -> None:
# Arrange
Expand All @@ -138,6 +141,7 @@ def test_missing_template(self, mock_messages: mock.MagicMock) -> None:


class TestPersonsMergedSignalReceiverIntegration(TestBase):
@override_settings(EMAIL_MODULE_ENABLED=True)
def test_integration(self) -> None:
# Arrange
self._setUpUsersAndLogin()
Expand Down

0 comments on commit 79465b6

Please sign in to comment.