Skip to content

Commit

Permalink
Merge pull request #2549 from carpentries/feature/2535-Member-codes-I…
Browse files Browse the repository at this point in the history
…mplement-soft-validation-for-member-codes

Implement soft validation for member codes in training requests
  • Loading branch information
elichad committed Oct 20, 2023
2 parents cad326e + 7dafdf9 commit 80389d8
Show file tree
Hide file tree
Showing 6 changed files with 187 additions and 15 deletions.
48 changes: 38 additions & 10 deletions amy/extforms/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class Meta:
fields = (
"review_process",
"member_code",
"member_code_override",
"personal",
"family",
"email",
Expand Down Expand Up @@ -109,6 +110,7 @@ def __init__(self, *args, **kwargs):
self.set_other_fields(self.helper.layout)
self.set_fake_required_fields()
self.set_accordion(self.helper.layout)
self.set_display_member_code_override(visible=False)
self.set_hr(self.helper.layout)

def set_other_field(self, field_name: str, layout: Layout) -> None:
Expand Down Expand Up @@ -143,6 +145,7 @@ def set_accordion(self, layout: Layout) -> None:
self["review_process"].field.widget.subfields = { # type: ignore
"preapproved": [
self["member_code"],
self["member_code_override"],
],
"open": [], # this option doesn't require any additional fields
}
Expand All @@ -155,6 +158,7 @@ def set_accordion(self, layout: Layout) -> None:

layout.fields.remove("review_process")
layout.fields.remove("member_code")
layout.fields.remove("member_code_override")

# insert div+field at previously saved position
layout.insert(
Expand All @@ -167,6 +171,10 @@ def set_accordion(self, layout: Layout) -> None:
),
)

def set_display_member_code_override(self, *, visible: bool) -> None:
widget = forms.CheckboxInput() if visible else forms.HiddenInput()
self.fields["member_code_override"].widget = widget

def set_hr(self, layout: Layout) -> None:
# add <HR> around "underrepresented*" fields
index = layout.fields.index("underrepresented")
Expand Down Expand Up @@ -202,30 +210,50 @@ def validate_member_code(
) -> None | dict[str, ValidationError]:
errors = dict()
member_code = self.cleaned_data.get("member_code", "")
member_code_override = self.cleaned_data.get("member_code_override", False)
error_msg = (
"This code is invalid. "
"This may be due to a typo, an expired code, "
"or a code that has not yet been activated. "
"Please confirm that you have copied the code correctly, "
"or confirm the code with the Membership Contact for your group."
"or confirm the code with the Membership Contact for your group. "
"If the code seems to be correct, tick the checkbox below to ignore "
"this message."
)

if not member_code:
return None

member_code_valid = self.member_code_valid(member_code)
if member_code_valid and member_code_override:
# case where a user corrects their code but ticks the box anyway
# checkbox doesn't need to be ticked, so correct it quietly and continue
self.cleaned_data["member_code_override"] = False
self.set_display_member_code_override(visible=False)
elif not member_code_valid:
self.set_display_member_code_override(visible=True)
if not member_code_override:
# user must either correct the code or tick the override
errors["member_code"] = ValidationError(error_msg)

return errors

def member_code_valid(self, code: str) -> bool:
"""Returns True if the code matches an active Membership,
including a grace period of 90 days before and after the Membership dates.
"""
try:
# find relevant membership - may raise Membership.DoesNotExist
membership = Membership.objects.get(registration_code=member_code)
# confirm that membership is currently active
# grace period: 90 days before and after
if not membership.active_on_date(
date.today(), grace_before=90, grace_after=90
):
errors["member_code"] = ValidationError(error_msg)
membership = Membership.objects.get(registration_code=code)
except Membership.DoesNotExist:
errors["member_code"] = ValidationError(error_msg)
return False

return errors
# confirm that membership is currently active
# grace period: 90 days before and after
if not membership.active_on_date(date.today(), grace_before=90, grace_after=90):
return False

return True

def clean(self):
super().clean()
Expand Down
112 changes: 112 additions & 0 deletions amy/extforms/tests/test_training_request_form.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from datetime import date, timedelta

from django.core import mail
from django.forms import CheckboxInput, HiddenInput
from django.test import override_settings
from django.urls import reverse

Expand Down Expand Up @@ -134,6 +135,11 @@ def test_review_process_validation__preapproved_code_empty(self):
rv,
"Registration code is required for pre-approved training review process.",
)
# test that override field is not visible
self.assertEqual(
rv.context["form"].fields["member_code_override"].widget.__class__,
HiddenInput,
)

def test_review_process_validation__open_code_nonempty(self):
"""Shouldn't pass when review_process requires *NO* member_code."""
Expand All @@ -151,6 +157,27 @@ def test_review_process_validation__open_code_nonempty(self):
self.assertContains(
rv, "Registration code must be empty for open training review process."
)
# test that override field is not visible
self.assertEqual(
rv.context["form"].fields["member_code_override"].widget.__class__,
HiddenInput,
)

@override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", False)]})
def test_member_code_validation__not_enforced(self):
"""Invalid code should pass if enforcement is not enabled."""
# Arrange
data = {
"review_process": "preapproved",
"member_code": "invalid",
}

# Act
rv = self.client.post(reverse("training_request"), data=data)

# Assert
self.assertEqual(rv.status_code, 200)
self.assertNotContains(rv, self.INVALID_MEMBER_CODE_ERROR)

@override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]})
def test_member_code_validation__code_valid(self):
Expand All @@ -168,6 +195,11 @@ def test_member_code_validation__code_valid(self):
# Assert
self.assertEqual(rv.status_code, 200)
self.assertNotContains(rv, self.INVALID_MEMBER_CODE_ERROR)
# test that override field is not visible
self.assertEqual(
rv.context["form"].fields["member_code_override"].widget.__class__,
HiddenInput,
)

@override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]})
def test_member_code_validation__code_invalid(self):
Expand All @@ -184,6 +216,11 @@ def test_member_code_validation__code_invalid(self):
# Assert
self.assertEqual(rv.status_code, 200)
self.assertContains(rv, self.INVALID_MEMBER_CODE_ERROR)
# test that override field is visible
self.assertEqual(
rv.context["form"].fields["member_code_override"].widget.__class__,
CheckboxInput,
)

@override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]})
def test_member_code_validation__code_inactive_early(self):
Expand All @@ -203,6 +240,11 @@ def test_member_code_validation__code_inactive_early(self):
# Assert
self.assertEqual(rv.status_code, 200)
self.assertContains(rv, self.INVALID_MEMBER_CODE_ERROR)
# test that override field is visible
self.assertEqual(
rv.context["form"].fields["member_code_override"].widget.__class__,
CheckboxInput,
)

@override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]})
def test_member_code_validation__code_inactive_late(self):
Expand All @@ -222,3 +264,73 @@ def test_member_code_validation__code_inactive_late(self):
# Assert
self.assertEqual(rv.status_code, 200)
self.assertContains(rv, self.INVALID_MEMBER_CODE_ERROR)
# test that override field is visible
self.assertEqual(
rv.context["form"].fields["member_code_override"].widget.__class__,
CheckboxInput,
)

@override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]})
def test_member_code_validation__code_invalid_override(self):
"""Invalid member code should be accepted when the override is ticked."""
# Arrange
data = {
"review_process": "preapproved",
"member_code": "invalid",
"member_code_override": True,
}

# Act
rv = self.client.post(reverse("training_request"), data=data)

# Assert
self.assertEqual(rv.status_code, 200)
self.assertNotContains(rv, self.INVALID_MEMBER_CODE_ERROR)
# test that override field is visible
self.assertEqual(
rv.context["form"].fields["member_code_override"].widget.__class__,
CheckboxInput,
)

@override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]})
def test_member_code_validation__code_valid_override(self):
"""Override should be quietly hidden if a valid code is used."""
# Arrange
self.setUpMembership()
data = {
"review_process": "preapproved",
"member_code": "valid123",
"member_code_override": True,
}

# Act
rv = self.client.post(reverse("training_request"), data=data)

# Assert
self.assertEqual(rv.status_code, 200)
self.assertNotContains(rv, self.INVALID_MEMBER_CODE_ERROR)
# test that override field is not visible
self.assertEqual(
rv.context["form"].fields["member_code_override"].widget.__class__,
HiddenInput,
)

@override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]})
def test_member_code_validation__code_valid_override_full_request(self):
"""Override should be quietly changed to False if a valid code is used
in a successful submission."""
# Arrange
self.setUpMembership()
self.data["member_code"] = "valid123"
self.data["member_code_override"] = True
self.passCaptcha(self.data)

# Act
rv = self.client.post(reverse("training_request"), data=self.data, follow=True)

# Assert
self.assertEqual(rv.status_code, 200)
self.assertEqual(rv.resolver_match.view_name, "training_request_confirm")
self.assertFalse(
TrainingRequest.objects.get(member_code="valid123").member_code_override
)
10 changes: 5 additions & 5 deletions amy/fiscal/tests/test_membership.py
Original file line number Diff line number Diff line change
Expand Up @@ -1129,11 +1129,11 @@ def test_comment_added(self):
comment = "Everything is awesome."
data = {"new_agreement_end": date(2021, 3, 31), "comment": comment}
today = date.today()
expected_comment = f"""Extended membership by 30 days on {today} (new end date: 2021-03-31).
----
{comment}"""
expected_comment = (
f"Extended membership by 30 days on {today} (new end date: 2021-03-31)."
"\n\n----\n\n"
f"{comment}"
)

# Act
self.client.post(reverse("membership_extend", args=[membership.pk]), data=data)
Expand Down
2 changes: 2 additions & 0 deletions amy/templates/includes/trainingrequest_details.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
<td>{{ object.get_review_process_display|default:"&mdash;" }}</td></tr>
<tr><th>Registration Code:</th>
<td>{{ object.member_code|default:"&mdash;" }}</td></tr>
<tr><th>Continue with registration code marked as invalid:</th>
<td>{{ object.member_code_override|yesno }}</td></tr>
<tr><th>Personal name:</th>
<td>{{ object.personal }}</td></tr>
<tr><th>Middle name:</th>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Generated by Django 3.2.20 on 2023-10-19 07:37

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("workshops", "0266_rename_group_name_trainingrequest_member_code"),
]

operations = [
migrations.AddField(
model_name="trainingrequest",
name="member_code_override",
field=models.BooleanField(
blank=True,
default=False,
help_text="A member of our team will check the code and follow up with you if there are any problems that require your attention.",
verbose_name="Continue with registration code marked as invalid",
),
),
]
8 changes: 8 additions & 0 deletions amy/workshops/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2123,6 +2123,14 @@ class TrainingRequest(
"a Carpentries member site or for a specific scheduled "
"event, please enter it here:",
)
member_code_override = models.BooleanField(
null=False,
default=False,
blank=True,
verbose_name="Continue with registration code marked as invalid",
help_text="A member of our team will check the code and follow up with you if "
"there are any problems that require your attention.",
)

personal = models.CharField(
max_length=STR_LONG,
Expand Down

0 comments on commit 80389d8

Please sign in to comment.