diff --git a/course/enrollment.py b/course/enrollment.py index 976a2e4e5..8aeb526bd 100644 --- a/course/enrollment.py +++ b/course/enrollment.py @@ -259,15 +259,17 @@ def email_suffix_matches(email, suffix): roles = ParticipationRole.objects.filter( course=course, is_default_for_new_participants=True) + tags = None if preapproval is not None: roles = list(preapproval.roles.all()) + tags = list(preapproval.tags.all()) try: if course.enrollment_approval_required and preapproval is None: participation = handle_enrollment_request( course, user, participation_status.requested, - roles, request) + roles, tags, request) assert participation is not None @@ -305,7 +307,7 @@ def email_suffix_matches(email, suffix): "by email once your request has been acted upon.")) else: handle_enrollment_request(course, user, participation_status.active, - roles, request) + roles, tags, request) messages.add_message(request, messages.SUCCESS, _("Successfully enrolled.")) @@ -318,8 +320,8 @@ def email_suffix_matches(email, suffix): @transaction.atomic -def handle_enrollment_request(course, user, status, roles, request=None): - # type: (Course, Any, Text, Optional[List[ParticipationRole]], Optional[http.HttpRequest]) -> Participation # noqa +def handle_enrollment_request(course, user, status, roles, tags, request=None): + # type: (Course, Any, Text, Optional[List[ParticipationRole]], Optional[List[ParticipationTag]], Optional[http.HttpRequest]) -> Participation # noqa participations = Participation.objects.filter(course=course, user=user) assert participations.count() <= 1 @@ -338,6 +340,9 @@ def handle_enrollment_request(course, user, status, roles, request=None): if roles is not None: participation.roles.set(roles) + if tags is not None: + participation.tags.set(tags) + if status == participation_status.active: send_enrollment_decision(participation, True, request) elif status == participation_status.denied: @@ -447,6 +452,12 @@ def __init__(self, course, *args, **kwargs): .filter(course=course) ), label=_("Roles")) + self.fields["tags"] = forms.ModelMultipleChoiceField( + queryset=( + ParticipationTag.objects + .filter(course=course) + ), + label=_("Tags"), required=False) self.fields["preapproval_type"] = forms.ChoiceField( choices=( ("email", _("Email")), diff --git a/course/migrations/0114_add_tags_and_full_name_to_preapp.py b/course/migrations/0114_add_tags_and_full_name_to_preapp.py new file mode 100644 index 000000000..a7ce683d4 --- /dev/null +++ b/course/migrations/0114_add_tags_and_full_name_to_preapp.py @@ -0,0 +1,26 @@ +# Generated by Django 3.0.8 on 2020-10-28 07:23 + +import course.models +import django.core.validators +from django.db import migrations, models +import jsonfield.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('course', '0113_merge_20190919_1408'), + ] + + operations = [ + migrations.AddField( + model_name='participationpreapproval', + name='known_name', + field=models.CharField(blank=True, help_text='Known name of the participant', max_length=254, null=True, verbose_name='Known name'), + ), + migrations.AddField( + model_name='participationpreapproval', + name='tags', + field=models.ManyToManyField(blank=True, to='course.ParticipationTag', verbose_name='Participation tags'), + ), + ] diff --git a/course/models.py b/course/models.py index 2926fe810..94babb155 100644 --- a/course/models.py +++ b/course/models.py @@ -618,11 +618,12 @@ class ParticipationPreapproval(models.Model): verbose_name=_("Role (unused)"),) roles = models.ManyToManyField(ParticipationRole, blank=True, verbose_name=_("Roles"), related_name="+") - creator = models.ForeignKey(settings.AUTH_USER_MODEL, null=True, verbose_name=_("Creator"), on_delete=models.SET_NULL) creation_time = models.DateTimeField(default=now, db_index=True, verbose_name=_("Creation time")) + tags = models.ManyToManyField(ParticipationTag, blank=True, + verbose_name=_("Participation tags")) def __unicode__(self): if self.email: diff --git a/course/receivers.py b/course/receivers.py index 889fe678f..22b591d74 100644 --- a/course/receivers.py +++ b/course/receivers.py @@ -34,7 +34,9 @@ ParticipationPreapproval, ) -from typing import List, Union, Text, Optional, Tuple, Any # noqa +from typing import List, Union, Text, Optional, Tuple, Any, TYPE_CHECKING # noqa +if TYPE_CHECKING: + from course.models import ParticipationTag, ParticipationRole # noqa # {{{ Update enrollment status when a User/Course instance is saved @@ -51,6 +53,8 @@ def update_requested_participation_status(sender, created, instance, user_updated = False course_updated = False + course = None + user = None if isinstance(instance, Course): course_updated = True @@ -71,20 +75,22 @@ def update_requested_participation_status(sender, created, instance, assert user_updated course = requested.course - may_preapprove, roles = may_preapprove_role(course, user) + assert course is not None + assert user is not None + may_preapprove, roles, tags = may_preapprove_role_and_tag(course, user) if may_preapprove: from course.enrollment import handle_enrollment_request handle_enrollment_request( - course, user, participation_status.active, roles) + course, user, participation_status.active, roles, tags) -def may_preapprove_role(course, user): - # type: (Course, User) -> Tuple[bool, Optional[List[Text]]] +def may_preapprove_role_and_tag(course, user): + # type: (Course, User) -> Tuple[bool, Optional[List[ParticipationRole]], Optional[List[ParticipationTag]]] # noqa if not user.is_active: - return False, None + return False, None, None preapproval = None if user.email: @@ -105,9 +111,9 @@ def may_preapprove_role(course, user): pass if preapproval: - return True, list(preapproval.roles.all()) + return True, list(preapproval.roles.all()), list(preapproval.tags.all()) else: - return False, None + return False, None, None # }}} diff --git a/tests/factories.py b/tests/factories.py index 63752e8dd..d74de3e40 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -263,6 +263,21 @@ def roles(self, create, extracted, **kwargs): role = ParticipationRoleFactory(course=self.course) self.roles.set([role]) + @factory.post_generation + def tags(self, create, extracted, **kwargs): + if not create: + # Simple build, do nothing. + return + if extracted: + for tag in extracted: + if isinstance(tag, str): + tag = ParticipationTagFactory( + course=self.course, name=tag) + else: + assert isinstance(tag, models.ParticipationTag) + self.tags.set([tag]) + return + def generate_random_hash(): import hashlib diff --git a/tests/test_enrollment.py b/tests/test_enrollment.py index d2dc83f1b..962bdf7aa 100644 --- a/tests/test_enrollment.py +++ b/tests/test_enrollment.py @@ -422,6 +422,34 @@ def test_preapprved_user_updated_inst_id_after_req_enrollment_roles_match(self): self.assertIn(expected_role_identifier, [role.identifier for role in user_participation.roles.all()]) + def test_preapprved_user_updated_inst_id_after_req_enrollment_tags_match(self): + self.update_require_approval_course( + preapproval_require_verified_inst_id=True) + + user = factories.UserFactory() + inst_id = user.institutional_id + + # Temporarily remove his/her inst_id + user.institutional_id = None + user.save() + + expected_tag_name = "test_student_tag" + + self.get_test_preapproval( + institutional_id=inst_id, tags=[expected_tag_name]) + + with self.temporarily_switch_to_user(user): + self.c.post(self.enroll_request_url) + + # Add back the inst_id + user.institutional_id = inst_id + user.institutional_id_verified = True + user.save() + + user_participation = Participation.objects.get(user=user) + self.assertIn(expected_tag_name, + [tag.name for tag in user_participation.tags.all()]) + def test_course_require_inst_id_verified_user_inst_id_not_verified1(self): # thought matched self.update_require_approval_course( @@ -553,7 +581,7 @@ def test_approve_new(self): request = mock.MagicMock() participation = enrollment.handle_enrollment_request( - self.course, user, status, roles, request=request) + self.course, user, status, roles, None, request=request) self.assertEqual(participation.user, user) self.assertEqual(participation.status, status) @@ -570,7 +598,7 @@ def test_approve_new_none_roles(self): request = mock.MagicMock() participation = enrollment.handle_enrollment_request( - self.course, user, status, roles, request=request) + self.course, user, status, roles, None, request=request) self.assertEqual(participation.user, user) self.assertEqual(participation.status, status) @@ -589,7 +617,7 @@ def test_deny_new(self): request = mock.MagicMock() participation = enrollment.handle_enrollment_request( - self.course, user, status, roles, request=request) + self.course, user, status, roles, None, request=request) self.assertEqual(participation.user, user) self.assertEqual(participation.status, status) @@ -611,7 +639,7 @@ def test_approve_requested(self): request = mock.MagicMock() participation = enrollment.handle_enrollment_request( - self.course, user, status, roles, request=request) + self.course, user, status, roles, None, request=request) self.assertEqual(participation.user, user) self.assertEqual(participation.status, status) @@ -634,7 +662,7 @@ def test_deny_requested(self): request = mock.MagicMock() participation = enrollment.handle_enrollment_request( - self.course, user, status, roles, request=request) + self.course, user, status, roles, None, request=request) self.assertEqual(participation.user, user) self.assertEqual(participation.status, status) @@ -1002,75 +1030,6 @@ def test_edit_participation_view_save_integrity_error(self): self.assertEqual(len(mail.outbox), 0) -class EnrollmentPreapprovalTestMixin(LocmemBackendTestsMixin, - EnrollmentTestBaseMixin): - - @classmethod - def setUpTestData(cls): # noqa - super(EnrollmentPreapprovalTestMixin, cls).setUpTestData() - cls.non_ptcp_active_user1.institutional_id_verified = True - cls.non_ptcp_active_user1.save() - cls.non_ptcp_active_user2.institutional_id_verified = False - cls.non_ptcp_active_user2.save() - - @property - def preapprove_data_emails(self): - preapproved_user = [self.non_ptcp_active_user1, - self.non_ptcp_active_user2] - preapproved_data = [u.email for u in preapproved_user] - preapproved_data.insert(1, " ") # empty line - preapproved_data.insert(0, " ") # empty line - return preapproved_data - - @property - def preapprove_data_institutional_ids(self): - preapproved_user = [self.non_ptcp_active_user1, - self.non_ptcp_active_user2, - self.non_ptcp_unconfirmed_user1] - preapproved_data = [u.institutional_id for u in preapproved_user] - preapproved_data.insert(1, " ") # empty line - preapproved_data.insert(0, " ") # empty line - return preapproved_data - - @property - def preapproval_url(self): - return reverse("relate-create_preapprovals", - args=[self.course.identifier]) - - @property - def default_preapprove_role(self): - role, _ = (ParticipationRole.objects.get_or_create( - course=self.course, identifier="student")) - return [str(role.pk)] - - def post_preapproval(self, preapproval_type, preapproval_data=None, - force_login_instructor=True): - if preapproval_data is None: - if preapproval_type == "email": - preapproval_data = self.preapprove_data_emails - elif preapproval_type == "institutional_id": - preapproval_data = self.preapprove_data_institutional_ids - - assert preapproval_data is not None - assert isinstance(preapproval_data, list) - - data = { - "preapproval_type": [preapproval_type], - "preapproval_data": ["\n".join(preapproval_data)], - "roles": self.student_role_post_data, - "submit": [""] - } - if not force_login_instructor: - approver = self.get_logged_in_user() - else: - approver = self.instructor_participation.user - with self.temporarily_switch_to_user(approver): - return self.c.post(self.preapproval_url, data, follow=True) - - def get_preapproval_count(self): - return ParticipationPreapproval.objects.all().count() - - class CreatePreapprovalsTest(EnrollmentTestMixin, SingleCourseTestMixin, TestCase): # test enrollment.create_preapprovals