Skip to content

Commit

Permalink
Merge pull request #1151 from maykinmedia/feature/2291-use-verified-e…
Browse files Browse the repository at this point in the history
…mail

[#2291 #2273 #2233] Use verified email for Laposta newsletters and Qmatic appointments
  • Loading branch information
alextreme authored Apr 12, 2024
2 parents a3d1e00 + 5164ae4 commit b54d732
Show file tree
Hide file tree
Showing 7 changed files with 142 additions and 25 deletions.
40 changes: 39 additions & 1 deletion src/open_inwoner/accounts/tests/test_profile_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,10 @@ def setUp(self):
cms_tools.create_apphook_page(ProfileApphook)

self.profile_url = reverse("profile:detail")
self.user = DigidUserFactory()
self.user = DigidUserFactory(
email="[email protected]", verified_email="[email protected]"
)
self.assertTrue(self.user.has_verified_email())

self.config = LapostaConfig.get_solo()
self.config.api_root = "https://laposta.local/api/v2/"
Expand Down Expand Up @@ -1153,6 +1156,31 @@ def test_render_form_limit_newsletters_to_admin_selection(self, m):
# Second field was excluded by `LapostaConfig.limit_list_selection_to`
self.assertNotIn("Nieuwsbrief2: bar", response.text)

def test_do_not_render_form_if_email_not_verified(self, m):
self.setUpMocks(m)

self.user.verified_email = ""
self.user.save()
self.assertFalse(self.user.has_verified_email())

self.config.limit_list_selection_to = ["list1"]
self.config.save()

m.get(
f"{self.config.api_root}member/{self.user.email}?list_id=list1",
json={
"member": MemberFactory.build(
list_id="list1",
member_id="1234567",
email=self.user.email,
custom_fields=None,
).model_dump()
},
)
response = self.app.get(self.profile_url, user=self.user)

self.assertNotIn("newsletter-form", response.forms)


@requests_mock.Mocker()
@override_settings(
Expand All @@ -1165,6 +1193,7 @@ def setUp(self):
super().setUp()

self.data = QmaticMockData()
self.assertTrue(self.data.user.has_verified_email())

def test_do_not_render_list_if_config_is_missing(self, m):
self.data.config.service = None
Expand Down Expand Up @@ -1194,6 +1223,15 @@ def test_do_not_render_list_if_validation_error(self, m):

self.assertIn(_("Geen afspraken beschikbaar"), response.text)

def test_do_not_render_list_if_email_not_verified(self, m):
self.data.user.verified_email = ""
self.data.user.save()
self.assertFalse(self.data.user.has_verified_email())

response = self.app.get(self.appointments_url, user=self.data.user)

self.assertIn(_("Geen afspraken beschikbaar"), response.text)

def test_render_list_if_appointments_are_found(self, m):
self.data.setUpMocks(m)

Expand Down
25 changes: 16 additions & 9 deletions src/open_inwoner/accounts/views/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,22 +338,29 @@ def form_valid(self, form):


class UserAppointmentsView(
LogMixin, LoginRequiredMixin, CommonPageMixin, BaseBreadcrumbMixin, TemplateView
LogMixin,
LoginRequiredMixin,
CommonPageMixin,
BaseBreadcrumbMixin,
TemplateView,
):
template_name = "pages/profile/appointments.html"

def get_context_data(self, **kwargs) -> dict[str, Any]:
context = super().get_context_data(**kwargs)
# TODO email should be verified
try:
client = QmaticClient()
except NoServiceConfigured:
logger.exception("Error occurred while creating Qmatic client")
user: User = self.request.user
if not user.has_verified_email():
context["appointments"] = []
else:
context["appointments"] = client.list_appointments_for_customer(
self.request.user.email
)
try:
client = QmaticClient()
except NoServiceConfigured:
logger.exception("Error occurred while creating Qmatic client")
context["appointments"] = []
else:
context["appointments"] = client.list_appointments_for_customer(
user.verified_email
)
return context

@cached_property
Expand Down
19 changes: 10 additions & 9 deletions src/open_inwoner/cms/plugins/cms_plugins/appointments.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,18 @@ class UserAppointmentsPlugin(CMSPluginBase):

def render(self, context, instance, placeholder):
request = context["request"]
# TODO email should be verified
if not request.user.is_authenticated:
return context

try:
client = QmaticClient()
except NoServiceConfigured:
logger.exception("Error occurred while creating Qmatic client")
if not request.user.is_authenticated or not request.user.has_verified_email():
appointments = []
else:
appointments = client.list_appointments_for_customer(request.user.email)
try:
client = QmaticClient()
except NoServiceConfigured:
logger.exception("Error occurred while creating Qmatic client")
appointments = []
else:
appointments = client.list_appointments_for_customer(
request.user.verified_email
)

context.update(
{
Expand Down
18 changes: 18 additions & 0 deletions src/open_inwoner/cms/plugins/tests/test_appointments.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ def test_plugin(self, m):
data = QmaticMockData()
data.setUpMocks(m)

self.assertTrue(data.user.has_verified_email())

html, context = cms_tools.render_plugin(
UserAppointmentsPlugin, plugin_data={}, user=data.user
)
Expand Down Expand Up @@ -46,3 +48,19 @@ def test_plugin(self, m):

action_url = items[0].attrib["href"]
self.assertEqual(action_url, reverse("profile:appointments"))

def test_plugin__email_not_verified(self, m):
data = QmaticMockData()
data.setUpMocks(m)
data.user.verified_email = ""
data.user.save()
self.assertFalse(data.user.has_verified_email())

html, context = cms_tools.render_plugin(
UserAppointmentsPlugin, plugin_data={}, user=data.user
)

appointments = context["appointments"]

# zero results
self.assertEqual(len(appointments), 0)
18 changes: 13 additions & 5 deletions src/open_inwoner/laposta/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from open_inwoner.laposta.models import LapostaConfig
from open_inwoner.utils.api import ClientError

from ..accounts.models import User
from .choices import get_list_choices
from .client import create_laposta_client

Expand All @@ -27,6 +28,9 @@ class NewsletterSubscriptionForm(forms.Form):
def __init__(self, user=None, *args, **kwargs):
super().__init__(**kwargs)

if not user.has_verified_email():
return

if laposta_client := create_laposta_client():
lists = laposta_client.get_lists()
self.fields["newsletters"].choices = get_list_choices(lists)
Expand All @@ -40,27 +44,31 @@ def __init__(self, user=None, *args, **kwargs):
self.fields[
"newsletters"
].initial = laposta_client.get_subscriptions_for_email(
limited_to, user.email
limited_to, user.verified_email
)

def save(self, request, *args, **kwargs):
newsletters = self.cleaned_data["newsletters"]
user: User = request.user
if not user.has_verified_email():
return

client = create_laposta_client()
if not client:
return

newsletters = self.cleaned_data["newsletters"]

list_name_mapping = dict(self.fields["newsletters"].choices)
user_data = UserData(
ip=get_client_ip(request)[0],
email=request.user.email,
email=user.verified_email,
source_url=None,
custom_fields=None,
options=None,
)
limited_to = LapostaConfig.get_solo().limit_list_selection_to
existing_subscriptions = set(
client.get_subscriptions_for_email(limited_to, request.user.email)
client.get_subscriptions_for_email(limited_to, user.verified_email)
)
for list_id in newsletters:
if list_id in existing_subscriptions:
Expand All @@ -85,7 +93,7 @@ def save(self, request, *args, **kwargs):
unsubscribe_from_ids = existing_subscriptions - set(newsletters)
for list_id in unsubscribe_from_ids:
try:
client.remove_subscription(list_id, request.user.email)
client.remove_subscription(list_id, user.verified_email)
except (RequestException, ClientError):
logger.exception(
"Something went wrong while trying to delete subscription"
Expand Down
42 changes: 42 additions & 0 deletions src/open_inwoner/laposta/tests/test_forms.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from unittest.mock import patch
from urllib.parse import parse_qs

from django.test import RequestFactory, TestCase
Expand All @@ -21,6 +22,9 @@ def setUp(self):
super().setUp()

self.user = DigidUserFactory()
self.user.verified_email = self.user.email
self.user.save()
self.assertTrue(self.user.has_verified_email())

self.request = RequestFactory().get("/")
self.request.user = self.user
Expand Down Expand Up @@ -289,3 +293,41 @@ def test_save_form_raises_errors(self, m):
]
},
)

@patch("open_inwoner.laposta.forms.create_laposta_client")
def test_form__disables_when_email_not_verified(self, m, mock_create_client):
"""
Verify that form actions cannot be performed if the user's email is not verified
"""
self.setUpMocks(m)

self.user.verified_email = ""
self.user.save()
self.assertFalse(self.user.has_verified_email())

m.get(
f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=123",
json={
"member": MemberFactory.build(
list_id="123",
member_id="1234567",
email=self.user.email,
custom_fields=None,
).model_dump()
},
)

form = NewsletterSubscriptionForm(data={}, user=self.user)

# We don't get any newletters if not using verified email
self.assertEqual(form["newsletters"].initial, None)

# Because client was never built (and no request-mock exception was thrown)
mock_create_client.assert_not_called()

# Check forced save() doesn't do anything
form = NewsletterSubscriptionForm(data={"newsletters": ["123"]}, user=self.user)
form.save(self.request)

# Client was never built (and no request-mock exception was thrown)
mock_create_client.assert_not_called()
5 changes: 4 additions & 1 deletion src/open_inwoner/qmatic/tests/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
class QmaticMockData:
def __init__(self):
self.appointments_url = reverse("profile:appointments")
self.user = DigidUserFactory()
self.user = DigidUserFactory(
email="[email protected]",
verified_email="[email protected]",
)

self.config = QmaticConfig.get_solo()
self.config.booking_base_url = "https://qmatic.local/"
Expand Down

0 comments on commit b54d732

Please sign in to comment.