From f8c0fa11ea3af105725364487c68745070794af2 Mon Sep 17 00:00:00 2001 From: Eli Chadwick Date: Mon, 30 Oct 2023 08:38:00 +0000 Subject: [PATCH 1/7] Add soft code validation to TR update form --- amy/extforms/forms.py | 1 + amy/extrequests/forms.py | 73 ++++++++++++++++++++++++++++++++++++++++ amy/extrequests/views.py | 6 ++++ 3 files changed, 80 insertions(+) diff --git a/amy/extforms/forms.py b/amy/extforms/forms.py index 072effd06..98516d55d 100644 --- a/amy/extforms/forms.py +++ b/amy/extforms/forms.py @@ -92,6 +92,7 @@ class Meta: } def __init__(self, *args, **kwargs): + # request is required for ENFORCE_MEMBER_CODES flag self.request_http = kwargs.pop("request", None) super().__init__(*args, **kwargs) diff --git a/amy/extrequests/forms.py b/amy/extrequests/forms.py index 498b4bce9..3fca20195 100644 --- a/amy/extrequests/forms.py +++ b/amy/extrequests/forms.py @@ -1407,6 +1407,79 @@ class Meta: "state": forms.RadioSelect(), } + def __init__(self, *args, **kwargs): + # request is required for ENFORCE_MEMBER_CODES flag + self.request_http = kwargs.pop("request", None) + super().__init__(*args, **kwargs) + + def clean(self): + super().clean() + + errors = self.validate_member_code(request=self.request_http) + if errors: + raise ValidationError(errors) + + @feature_flag_enabled("ENFORCE_MEMBER_CODES") + def validate_member_code( + self, request: HttpRequest + ) -> 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) + + if not member_code: + return None + + member_code_valid, error_detail = 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 + elif not member_code_valid: + if not member_code_override: + # user must either correct the code or tick the override + error_msg = ( + "This code is invalid: " + f"{error_detail} " + "Tick the checkbox below to ignore this message." + ) + errors["member_code"] = ValidationError(error_msg) + + return errors + + def member_code_valid(self, code: str) -> tuple[bool, str]: + """Returns True if the code matches an active Membership with available seats, + including a grace period of 90 days before and after the Membership dates. + If there is no match, returns False and a detailed error. + """ + try: + # find relevant membership - may raise Membership.DoesNotExist + membership = Membership.objects.get(registration_code=code) + except Membership.DoesNotExist: + return False, f"No membership found for code {code}." + + # confirm that membership was active at the time the request was submitted + # grace period: 90 days before and after + if not membership.active_on_date( + self.instance.created_at.date(), grace_before=90, grace_after=90 + ): + return ( + False, + "Membership is inactive " + f"(start {membership.agreement_start}, " + f"end {membership.agreement_end}).", + ) + + # confirm that membership has training seats remaining + if ( + membership.public_instructor_training_seats_remaining + + membership.inhouse_instructor_training_seats_remaining + <= 0 + ): + return False, "Membership has no training seats remaining." + + return True, "" + class TrainingRequestsSelectionForm(forms.Form): trainingrequest_a = forms.ModelChoiceField( diff --git a/amy/extrequests/views.py b/amy/extrequests/views.py index 1ae24b821..b48bf71b6 100644 --- a/amy/extrequests/views.py +++ b/amy/extrequests/views.py @@ -825,6 +825,12 @@ class TrainingRequestUpdate(RedirectSupportMixin, OnlyForAdminsMixin, AMYUpdateV form_class = TrainingRequestUpdateForm template_name = "generic_form_with_comments.html" + def get_form_kwargs(self): + # request is required for ENFORCE_MEMBER_CODES flag + kwargs = super().get_form_kwargs() + kwargs["request"] = self.request + return kwargs + @admin_required @permission_required( From 65bc9bf9fbdbcb8239cb517bf2d50621ec922f8c Mon Sep 17 00:00:00 2001 From: Eli Chadwick Date: Mon, 30 Oct 2023 09:37:06 +0000 Subject: [PATCH 2/7] ensure TR char/text fields use default="" --- ...trainingrequest_default_for_text_fields.py | 30 +++++++++++++++++++ amy/workshops/models.py | 6 ++-- 2 files changed, 33 insertions(+), 3 deletions(-) create mode 100644 amy/workshops/migrations/0269_alter_trainingrequest_default_for_text_fields.py diff --git a/amy/workshops/migrations/0269_alter_trainingrequest_default_for_text_fields.py b/amy/workshops/migrations/0269_alter_trainingrequest_default_for_text_fields.py new file mode 100644 index 000000000..ac48387e7 --- /dev/null +++ b/amy/workshops/migrations/0269_alter_trainingrequest_default_for_text_fields.py @@ -0,0 +1,30 @@ +# Generated by Django 3.2.20 on 2023-10-30 09:20 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + replaces = [('workshops', '0269_alter_trainingrequest_nonprofit_teaching_experience'), ('workshops', '0270_auto_20231030_0919')] + + dependencies = [ + ('workshops', '0268_remove_workshoprequest_member_affiliation'), + ] + + operations = [ + migrations.AlterField( + model_name='trainingrequest', + name='nonprofit_teaching_experience', + field=models.CharField(blank=True, default='', help_text="Provide details or leave blank if this doesn't apply to you.", max_length=255, verbose_name='I have been an active contributor to other volunteer or non-profit groups with significant teaching or training components.'), + ), + migrations.AlterField( + model_name='trainingrequest', + name='previous_experience_explanation', + field=models.TextField(blank=True, default='', verbose_name='Description of your previous experience in teaching'), + ), + migrations.AlterField( + model_name='trainingrequest', + name='previous_training_explanation', + field=models.TextField(blank=True, default='', verbose_name='Description of your previous training in teaching'), + ), + ] diff --git a/amy/workshops/models.py b/amy/workshops/models.py index 079356dd6..024fa18a2 100644 --- a/amy/workshops/models.py +++ b/amy/workshops/models.py @@ -2255,7 +2255,7 @@ class TrainingRequest( nonprofit_teaching_experience = models.CharField( max_length=STR_LONGEST, blank=True, - null=True, + default="", verbose_name="I have been an active contributor to other volunteer or" " non-profit groups with significant teaching or training" " components.", @@ -2285,8 +2285,8 @@ class TrainingRequest( ) previous_training_explanation = models.TextField( verbose_name="Description of your previous training in teaching", - null=True, blank=True, + default="", ) # this part changed a little bit, mostly wording and choices @@ -2310,8 +2310,8 @@ class TrainingRequest( ) previous_experience_explanation = models.TextField( verbose_name="Description of your previous experience in teaching", - null=True, blank=True, + default="", ) PROGRAMMING_LANGUAGE_USAGE_FREQUENCY_CHOICES = ( From 212bbc869c67201b1af64c543fa61975074bcb0a Mon Sep 17 00:00:00 2001 From: Eli Chadwick Date: Mon, 30 Oct 2023 09:41:54 +0000 Subject: [PATCH 3/7] add tests for soft validation on update form --- amy/extrequests/forms.py | 2 +- amy/extrequests/tests/test_forms.py | 286 ++++++++++++++++++++++++++++ 2 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 amy/extrequests/tests/test_forms.py diff --git a/amy/extrequests/forms.py b/amy/extrequests/forms.py index 3fca20195..981d620d4 100644 --- a/amy/extrequests/forms.py +++ b/amy/extrequests/forms.py @@ -1456,7 +1456,7 @@ def member_code_valid(self, code: str) -> tuple[bool, str]: # find relevant membership - may raise Membership.DoesNotExist membership = Membership.objects.get(registration_code=code) except Membership.DoesNotExist: - return False, f"No membership found for code {code}." + return False, f'No membership found for code "{code}".' # confirm that membership was active at the time the request was submitted # grace period: 90 days before and after diff --git a/amy/extrequests/tests/test_forms.py b/amy/extrequests/tests/test_forms.py new file mode 100644 index 000000000..c71790296 --- /dev/null +++ b/amy/extrequests/tests/test_forms.py @@ -0,0 +1,286 @@ +from datetime import date, timedelta + +from django.test import override_settings +from django.urls import reverse + +from extrequests.tests.test_training_request import create_training_request +from workshops.models import Event, Membership, Role, Tag, Task, TrainingRequest +from workshops.tests.base import TestBase + + +class TestTrainingRequestUpdateForm(TestBase): + INVALID_MEMBER_CODE_ERROR = "This code is invalid." + + def setUp(self): + self._setUpUsersAndLogin() + self._setUpRoles() + self.request = create_training_request( + state="p", person=None, open_review=False + ) + + def setUpMembership(self): + self.membership = Membership.objects.create( + name="Alpha Organization", + variant="bronze", + agreement_start=date.today() - timedelta(weeks=26), + agreement_end=date.today() + timedelta(weeks=26), + contribution_type="financial", + registration_code="valid123", + public_instructor_training_seats=1, + inhouse_instructor_training_seats=1, + ) + + def setUpUsedSeats(self): + # set up some prior seat usage + super().setUp() + self._setUpTags() + ttt_event = Event.objects.create(slug="ttt-event", host=self.org_alpha) + ttt_event.tags.add(Tag.objects.get(name="TTT")) + learner = Role.objects.get(name="learner") + self.task_public = Task.objects.create( + event=ttt_event, + person=self.spiderman, + role=learner, + seat_membership=self.membership, + seat_public=True, + ) + self.task_inhouse = Task.objects.create( + event=ttt_event, + person=self.blackwidow, + role=learner, + seat_membership=self.membership, + seat_public=False, + ) + + @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("trainingrequest_edit", args=[self.request.pk]), 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): + """Valid member code should pass.""" + # Arrange + self.setUpMembership() + data = { + "review_process": "preapproved", + "member_code": "valid123", + } + + # Act + rv = self.client.post( + reverse("trainingrequest_edit", args=[self.request.pk]), 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_invalid(self): + """Invalid member code should not pass.""" + # Arrange + data = { + "review_process": "preapproved", + "member_code": "invalid", + } + + # Act + rv = self.client.post( + reverse("trainingrequest_edit", args=[self.request.pk]), data=data + ) + + # Assert + self.assertEqual(rv.status_code, 200) + self.assertContains(rv, "No membership found for code "invalid".") + + @override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]}) + def test_member_code_validation__code_inactive_early(self): + """Code used >90 days before membership start date should not pass.""" + # Arrange + self.setUpMembership() + self.membership.agreement_start = date.today() + timedelta(days=91) + self.membership.save() + data = { + "review_process": "preapproved", + "member_code": "valid123", + } + + # Act + rv = self.client.post( + reverse("trainingrequest_edit", args=[self.request.pk]), data=data + ) + + # Assert + self.assertEqual(rv.status_code, 200) + self.assertContains( + rv, + f"Membership is inactive (start {self.membership.agreement_start}, " + f"end {self.membership.agreement_end}).", + ) + + @override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]}) + def test_member_code_validation__code_inactive_late(self): + """Code used >90 days after membership end date should not pass.""" + # Arrange + self.setUpMembership() + self.membership.agreement_end = date.today() - timedelta(days=91) + self.membership.save() + data = { + "review_process": "preapproved", + "member_code": "valid123", + } + + # Act + rv = self.client.post( + reverse("trainingrequest_edit", args=[self.request.pk]), data=data + ) + + # Assert + self.assertEqual(rv.status_code, 200) + self.assertContains( + rv, + f"Membership is inactive (start {self.membership.agreement_start}, " + f"end {self.membership.agreement_end}).", + ) + + @override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]}) + def test_member_code_validation__code_no_seats_remaining(self): + """Code with no seats remaining should not pass.""" + # Arrange + self.setUpMembership() + self.setUpUsedSeats() + data = { + "review_process": "preapproved", + "member_code": "valid123", + } + + # Act + rv = self.client.post( + reverse("trainingrequest_edit", args=[self.request.pk]), data=data + ) + + # Assert + self.assertEqual(rv.status_code, 200) + self.assertContains(rv, "Membership has no training seats remaining.") + + @override_settings(FLAGS={"ENFORCE_MEMBER_CODES": [("boolean", True)]}) + def test_member_code_validation__code_only_public_seats_remaining(self): + """Code with only public seats remaining should pass.""" + # Arrange + self.setUpMembership() + self.setUpUsedSeats() + self.task_public.delete() + data = { + "review_process": "preapproved", + "member_code": "valid123", + } + + # Act + rv = self.client.post( + reverse("trainingrequest_edit", args=[self.request.pk]), 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_only_inhouse_seats_remaining(self): + """Code with only inhouse seats remaining should pass.""" + # Arrange + self.setUpMembership() + self.setUpUsedSeats() + self.task_inhouse.delete() + data = { + "review_process": "preapproved", + "member_code": "valid123", + } + + # Act + rv = self.client.post( + reverse("trainingrequest_edit", args=[self.request.pk]), 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_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("trainingrequest_edit", args=[self.request.pk]), 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_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("trainingrequest_edit", args=[self.request.pk]), 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_override_full_request(self): + """Override should be quietly changed to False if a valid code is used + in a successful submission.""" + # Arrange + super().setUp() + self.setUpMembership() + data = self.request.__dict__ + data["person_id"] = self.spiderman.pk + data["member_code"] = "valid123" + data["member_code_override"] = True + data.pop("score_manual") # can't encode None in POST request, so omit + + # Act + rv = self.client.post( + reverse("trainingrequest_edit", args=[self.request.pk]), + data=data, + follow=True, + ) + + # Assert + self.assertEqual(rv.status_code, 200) + self.assertEqual(rv.resolver_match.view_name, "trainingrequest_details") + self.assertFalse( + TrainingRequest.objects.get(member_code="valid123").member_code_override + ) From 304815be2f01f35ff31f5118d246bc3942788f3d Mon Sep 17 00:00:00 2001 From: Eli Chadwick Date: Mon, 6 Nov 2023 14:19:16 +0000 Subject: [PATCH 4/7] remove reference to uncommitted migrations --- .../0269_alter_trainingrequest_default_for_text_fields.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/amy/workshops/migrations/0269_alter_trainingrequest_default_for_text_fields.py b/amy/workshops/migrations/0269_alter_trainingrequest_default_for_text_fields.py index ac48387e7..0613df0f5 100644 --- a/amy/workshops/migrations/0269_alter_trainingrequest_default_for_text_fields.py +++ b/amy/workshops/migrations/0269_alter_trainingrequest_default_for_text_fields.py @@ -5,8 +5,6 @@ class Migration(migrations.Migration): - replaces = [('workshops', '0269_alter_trainingrequest_nonprofit_teaching_experience'), ('workshops', '0270_auto_20231030_0919')] - dependencies = [ ('workshops', '0268_remove_workshoprequest_member_affiliation'), ] From 1e4fdaeee987fd8f363edaf79aa60f318ab29de0 Mon Sep 17 00:00:00 2001 From: Eli Chadwick Date: Mon, 6 Nov 2023 15:58:23 +0000 Subject: [PATCH 5/7] move member_code_valid to a util function --- amy/extforms/forms.py | 47 +++++++++++----------------------- amy/extrequests/forms.py | 54 +++++++++++----------------------------- amy/extrequests/utils.py | 47 ++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 73 deletions(-) create mode 100644 amy/extrequests/utils.py diff --git a/amy/extforms/forms.py b/amy/extforms/forms.py index 98516d55d..b555243ef 100644 --- a/amy/extforms/forms.py +++ b/amy/extforms/forms.py @@ -15,13 +15,14 @@ WorkshopInquiryRequestBaseForm, WorkshopRequestBaseForm, ) +from extrequests.utils import MemberCodeValidationError, member_code_valid from workshops.fields import ( CheckboxSelectMultipleWithOthers, RadioSelectWithOther, Select2Widget, ) from workshops.forms import BootstrapHelper -from workshops.models import Membership, TrainingRequest +from workshops.models import TrainingRequest from workshops.utils.feature_flags import feature_flag_enabled @@ -226,13 +227,18 @@ def validate_member_code( 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: + # check code validity + # grace period: 90 days before and after + try: + member_code_is_valid = member_code_valid( + member_code, date.today(), grace_before=90, grace_after=90 + ) + if member_code_is_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) + except MemberCodeValidationError: self.set_display_member_code_override(visible=True) if not member_code_override: # user must either correct the code or tick the override @@ -240,31 +246,6 @@ def validate_member_code( return errors - def member_code_valid(self, code: str) -> bool: - """Returns True if the code matches an active Membership with available seats, - 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=code) - except Membership.DoesNotExist: - return False - - # 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 - - # confirm that membership has training seats remaining - if ( - membership.public_instructor_training_seats_remaining - + membership.inhouse_instructor_training_seats_remaining - <= 0 - ): - return False - - return True - def clean(self): super().clean() errors = dict() diff --git a/amy/extrequests/forms.py b/amy/extrequests/forms.py index 981d620d4..350119852 100644 --- a/amy/extrequests/forms.py +++ b/amy/extrequests/forms.py @@ -13,6 +13,7 @@ SelfOrganisedSubmission, WorkshopInquiryRequest, ) +from extrequests.utils import MemberCodeValidationError, member_code_valid from workshops.fields import ( CheckboxSelectMultipleWithOthers, CurriculumModelMultipleChoiceField, @@ -1430,56 +1431,29 @@ def validate_member_code( if not member_code: return None - member_code_valid, error_detail = 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 - elif not member_code_valid: + try: + member_code_is_valid = member_code_valid( + member_code, + self.instance.created_at.date(), + grace_before=90, + grace_after=90, + ) + if member_code_is_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 + except MemberCodeValidationError as e: if not member_code_override: # user must either correct the code or tick the override error_msg = ( "This code is invalid: " - f"{error_detail} " + f"{e.message} " "Tick the checkbox below to ignore this message." ) errors["member_code"] = ValidationError(error_msg) return errors - def member_code_valid(self, code: str) -> tuple[bool, str]: - """Returns True if the code matches an active Membership with available seats, - including a grace period of 90 days before and after the Membership dates. - If there is no match, returns False and a detailed error. - """ - try: - # find relevant membership - may raise Membership.DoesNotExist - membership = Membership.objects.get(registration_code=code) - except Membership.DoesNotExist: - return False, f'No membership found for code "{code}".' - - # confirm that membership was active at the time the request was submitted - # grace period: 90 days before and after - if not membership.active_on_date( - self.instance.created_at.date(), grace_before=90, grace_after=90 - ): - return ( - False, - "Membership is inactive " - f"(start {membership.agreement_start}, " - f"end {membership.agreement_end}).", - ) - - # confirm that membership has training seats remaining - if ( - membership.public_instructor_training_seats_remaining - + membership.inhouse_instructor_training_seats_remaining - <= 0 - ): - return False, "Membership has no training seats remaining." - - return True, "" - class TrainingRequestsSelectionForm(forms.Form): trainingrequest_a = forms.ModelChoiceField( diff --git a/amy/extrequests/utils.py b/amy/extrequests/utils.py new file mode 100644 index 000000000..de779e09b --- /dev/null +++ b/amy/extrequests/utils.py @@ -0,0 +1,47 @@ +from datetime import date + +from django.core.exceptions import ValidationError + +from workshops.models import Membership + + +class MemberCodeValidationError(ValidationError): + pass + + +def member_code_valid( + code: str, date: date, grace_before: int = 0, grace_after: int = 0 +) -> tuple[bool, str]: + """Returns True if `code` matches an Membership that is active on `date`, + including a grace period of `grace_before` days before and `grace_after` + days after the Membership dates. + If there is no match, raises an Exception with a detailed error. + """ + try: + # find relevant membership - may raise Membership.DoesNotExist + membership = Membership.objects.get(registration_code=code) + except Membership.DoesNotExist as e: + raise MemberCodeValidationError( + f'No membership found for code "{code}".' + ) from e + + # confirm that membership was active at the time the request was submitted + # grace period: 90 days before and after + if not membership.active_on_date( + date, grace_before=grace_before, grace_after=grace_after + ): + raise MemberCodeValidationError( + "Membership is inactive " + f"(start {membership.agreement_start}, " + f"end {membership.agreement_end})." + ) + + # confirm that membership has training seats remaining + if ( + membership.public_instructor_training_seats_remaining + + membership.inhouse_instructor_training_seats_remaining + <= 0 + ): + raise MemberCodeValidationError("Membership has no training seats remaining.") + + return True From 65ebc44edf3fd978edde4c3e38cf0a0b523a299f Mon Sep 17 00:00:00 2001 From: Eli Chadwick Date: Mon, 6 Nov 2023 15:58:39 +0000 Subject: [PATCH 6/7] add tests for member_code_valid util function --- amy/extrequests/tests/test_utils.py | 242 ++++++++++++++++++++++++++++ 1 file changed, 242 insertions(+) create mode 100644 amy/extrequests/tests/test_utils.py diff --git a/amy/extrequests/tests/test_utils.py b/amy/extrequests/tests/test_utils.py new file mode 100644 index 000000000..40b5194b7 --- /dev/null +++ b/amy/extrequests/tests/test_utils.py @@ -0,0 +1,242 @@ +from datetime import date, timedelta + +from extrequests.utils import MemberCodeValidationError, member_code_valid +from workshops.models import Event, Membership, Role, Tag, Task +from workshops.tests.base import TestBase + + +class TestMemberCodeValid(TestBase): + def setUp(self): + self._setUpRoles() + self.date = date.today() + + def setUpMembership(self): + self.valid_code = "valid123" + self.membership = Membership.objects.create( + name="Alpha Organization", + variant="bronze", + agreement_start=date.today() - timedelta(weeks=26), + agreement_end=date.today() + timedelta(weeks=26), + contribution_type="financial", + registration_code=self.valid_code, + public_instructor_training_seats=1, + inhouse_instructor_training_seats=1, + ) + + def setUpUsedSeats(self): + # set up some prior seat usage + super().setUp() + self._setUpTags() + ttt_event = Event.objects.create(slug="ttt-event", host=self.org_alpha) + ttt_event.tags.add(Tag.objects.get(name="TTT")) + learner = Role.objects.get(name="learner") + self.task_public = Task.objects.create( + event=ttt_event, + person=self.spiderman, + role=learner, + seat_membership=self.membership, + seat_public=True, + ) + self.task_inhouse = Task.objects.create( + event=ttt_event, + person=self.blackwidow, + role=learner, + seat_membership=self.membership, + seat_public=False, + ) + + def test_code_valid(self): + """Valid member code should pass.""" + # Arrange + self.setUpMembership() + code = self.valid_code + + # Act + result = member_code_valid( + code=code, + date=self.date, + ) + + # Assert + self.assertTrue(result) + + def test_code_invalid(self): + """Invalid member code should not pass.""" + # Arrange + code = "invalid" + + # Act & Assert + with self.assertRaises( + MemberCodeValidationError, msg='No membership found for code "invalid".' + ): + member_code_valid( + code=code, + date=self.date, + ) + + def test__code_inactive_early(self): + """Code used before membership start date should not pass.""" + # Arrange + self.setUpMembership() + self.membership.agreement_start = date.today() + timedelta(days=91) + self.membership.save() + code = self.valid_code + test_date = date.today() - timedelta(weeks=30) + + # Act & Assert + with self.assertRaises( + MemberCodeValidationError, + msg=( + "Membership is inactive " + f"(start {self.membership.agreement_start}, " + f"end {self.membership.agreement_end})." + ), + ): + member_code_valid( + code=code, + date=test_date, + ) + + def test__code_inactive_late(self): + """Code used after membership end date should not pass.""" + # Arrange + self.setUpMembership() + self.membership.agreement_start = date.today() + timedelta(days=91) + self.membership.save() + code = self.valid_code + test_date = date.today() + timedelta(weeks=30) + + # Act & Assert + with self.assertRaises( + MemberCodeValidationError, + msg=( + "Membership is inactive " + f"(start {self.membership.agreement_start}, " + f"end {self.membership.agreement_end})." + ), + ): + member_code_valid( + code=code, + date=test_date, + ) + + def test_code_valid_within_grace_before(self): + """Code used within a grace period should pass.""" + # Arrange + self.setUpMembership() + # we will use a 30-day grace period + # so set up a membership so that it starts <30 days from today + self.membership.agreement_start = date.today() + timedelta(days=29) + self.membership.save() + code = self.valid_code + + # Act + result = member_code_valid(code=code, date=self.date, grace_before=30) + + # Assert + self.assertTrue(result) + + def test_code_valid_within_grace_after(self): + """Code used within a grace period should pass.""" + # Arrange + self.setUpMembership() + # we will use a 30-day grace period + # so set up a membership so that it ends <30 days before today + self.membership.agreement_end = date.today() - timedelta(days=29) + self.membership.save() + code = self.valid_code + + # Act + result = member_code_valid(code=code, date=self.date, grace_after=30) + + # Assert + self.assertTrue(result) + + def test_code_invalid_beyond_grace_before(self): + """Code used outside a grace period should not pass.""" + # Arrange + self.setUpMembership() + # we will use a 30-day grace period + # so set up a membership so that it starts >30 days from today + self.membership.agreement_start = date.today() + timedelta(days=31) + self.membership.save() + code = self.valid_code + + # Act & Assert + with self.assertRaises( + MemberCodeValidationError, + msg=( + "Membership is inactive " + f"(start {self.membership.agreement_start}, " + f"end {self.membership.agreement_end})." + ), + ): + member_code_valid(code=code, date=self.date, grace_before=30) + + def test_code_valid_beyond_grace_after(self): + """Code used outside a grace period should not pass.""" + # Arrange + self.setUpMembership() + # we will use a 30-day grace period + # so set up a membership so that it ends >30 days before today + self.membership.agreement_end = date.today() - timedelta(days=31) + self.membership.save() + code = self.valid_code + + # Act & Assert + with self.assertRaises( + MemberCodeValidationError, + msg=( + "Membership is inactive " + f"(start {self.membership.agreement_start}, " + f"end {self.membership.agreement_end})." + ), + ): + member_code_valid(code=code, date=self.date, grace_after=30) + + def test_code_no_seats_remaining(self): + """Code with no seats remaining should not pass.""" + # Arrange + self.setUpMembership() + self.setUpUsedSeats() + code = self.valid_code + + # Act & Assert + with self.assertRaises( + MemberCodeValidationError, msg="Membership has no training seats remaining." + ): + member_code_valid(code=code, date=self.date) + + def test_code_only_public_seats_remaining(self): + """Code with only public seats remaining should pass.""" + # Arrange + self.setUpMembership() + self.setUpUsedSeats() + self.task_public.delete() + code = self.valid_code + + # Act + result = member_code_valid( + code=code, + date=self.date, + ) + + # Assert + self.assertTrue(result) + + def test_member_code_validation__code_only_inhouse_seats_remaining(self): + """Code with only inhouse seats remaining should pass.""" + # Arrange + self.setUpMembership() + self.setUpUsedSeats() + self.task_inhouse.delete() + code = self.valid_code + + # Act + result = member_code_valid( + code=code, + date=self.date, + ) + + # Assert + self.assertTrue(result) From 7cf642595b1198628d706fc03d80d57fedbf3a48 Mon Sep 17 00:00:00 2001 From: Eli Chadwick Date: Mon, 6 Nov 2023 16:06:09 +0000 Subject: [PATCH 7/7] move training seat checks to a second util function --- amy/extforms/forms.py | 4 ++-- amy/extrequests/forms.py | 4 ++-- amy/extrequests/tests/test_utils.py | 12 +++++++---- amy/extrequests/utils.py | 31 +++++++++++++++++++++++++++-- 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/amy/extforms/forms.py b/amy/extforms/forms.py index b555243ef..c9f775c86 100644 --- a/amy/extforms/forms.py +++ b/amy/extforms/forms.py @@ -15,7 +15,7 @@ WorkshopInquiryRequestBaseForm, WorkshopRequestBaseForm, ) -from extrequests.utils import MemberCodeValidationError, member_code_valid +from extrequests.utils import MemberCodeValidationError, member_code_valid_training from workshops.fields import ( CheckboxSelectMultipleWithOthers, RadioSelectWithOther, @@ -230,7 +230,7 @@ def validate_member_code( # check code validity # grace period: 90 days before and after try: - member_code_is_valid = member_code_valid( + member_code_is_valid = member_code_valid_training( member_code, date.today(), grace_before=90, grace_after=90 ) if member_code_is_valid and member_code_override: diff --git a/amy/extrequests/forms.py b/amy/extrequests/forms.py index 350119852..aa969a522 100644 --- a/amy/extrequests/forms.py +++ b/amy/extrequests/forms.py @@ -13,7 +13,7 @@ SelfOrganisedSubmission, WorkshopInquiryRequest, ) -from extrequests.utils import MemberCodeValidationError, member_code_valid +from extrequests.utils import MemberCodeValidationError, member_code_valid_training from workshops.fields import ( CheckboxSelectMultipleWithOthers, CurriculumModelMultipleChoiceField, @@ -1432,7 +1432,7 @@ def validate_member_code( return None try: - member_code_is_valid = member_code_valid( + member_code_is_valid = member_code_valid_training( member_code, self.instance.created_at.date(), grace_before=90, diff --git a/amy/extrequests/tests/test_utils.py b/amy/extrequests/tests/test_utils.py index 40b5194b7..0d05efae3 100644 --- a/amy/extrequests/tests/test_utils.py +++ b/amy/extrequests/tests/test_utils.py @@ -1,6 +1,10 @@ from datetime import date, timedelta -from extrequests.utils import MemberCodeValidationError, member_code_valid +from extrequests.utils import ( + MemberCodeValidationError, + member_code_valid, + member_code_valid_training, +) from workshops.models import Event, Membership, Role, Tag, Task from workshops.tests.base import TestBase @@ -205,7 +209,7 @@ def test_code_no_seats_remaining(self): with self.assertRaises( MemberCodeValidationError, msg="Membership has no training seats remaining." ): - member_code_valid(code=code, date=self.date) + member_code_valid_training(code=code, date=self.date) def test_code_only_public_seats_remaining(self): """Code with only public seats remaining should pass.""" @@ -216,7 +220,7 @@ def test_code_only_public_seats_remaining(self): code = self.valid_code # Act - result = member_code_valid( + result = member_code_valid_training( code=code, date=self.date, ) @@ -233,7 +237,7 @@ def test_member_code_validation__code_only_inhouse_seats_remaining(self): code = self.valid_code # Act - result = member_code_valid( + result = member_code_valid_training( code=code, date=self.date, ) diff --git a/amy/extrequests/utils.py b/amy/extrequests/utils.py index de779e09b..c9c78ba0a 100644 --- a/amy/extrequests/utils.py +++ b/amy/extrequests/utils.py @@ -13,8 +13,8 @@ def member_code_valid( code: str, date: date, grace_before: int = 0, grace_after: int = 0 ) -> tuple[bool, str]: """Returns True if `code` matches an Membership that is active on `date`, - including a grace period of `grace_before` days before and `grace_after` - days after the Membership dates. + including a grace period of `grace_before` days before + and `grace_after` days after the Membership dates. If there is no match, raises an Exception with a detailed error. """ try: @@ -45,3 +45,30 @@ def member_code_valid( raise MemberCodeValidationError("Membership has no training seats remaining.") return True + + +def member_code_valid_training( + code: str, date: date, grace_before: int = 0, grace_after: int = 0 +) -> tuple[bool, str]: + """Returns True if `code` matches an active Membership with training seats remaining. + If there is no match, raises an Exception with a detailed error.""" + # first ensure the code matches an active membership + try: + member_code_valid( + code=code, date=date, grace_before=grace_before, grace_after=grace_after + ) + except MemberCodeValidationError: + raise + + # find relevant membership - should definitely exist + membership = Membership.objects.get(registration_code=code) + + # confirm that membership has training seats remaining + if ( + membership.public_instructor_training_seats_remaining + + membership.inhouse_instructor_training_seats_remaining + <= 0 + ): + raise MemberCodeValidationError("Membership has no training seats remaining.") + + return True