diff --git a/backend/api/serializers/user.py b/backend/api/serializers/user.py index 3e67200b..c0170f31 100644 --- a/backend/api/serializers/user.py +++ b/backend/api/serializers/user.py @@ -65,31 +65,6 @@ def validate_password(self, value: str): return value - def validate_requesting_to_join_class(self, value: str): - # NOTE: Error message is purposefully ambiguous to prevent class - # enumeration. - error_message = "Class does not exist or does not accept join requests." - - if value is not None: - try: - klass = Class.objects.get(access_code=value) - except Class.DoesNotExist as ex: - raise serializers.ValidationError( - error_message, code="does_not_exist" - ) from ex - - if klass.accept_requests_until is None: - raise serializers.ValidationError( - error_message, code="does_not_accept_requests" - ) - - if klass.accept_requests_until < timezone.now(): - raise serializers.ValidationError( - error_message, code="no_longer_accepts_requests" - ) - - return value - def update(self, instance, validated_data): password = validated_data.pop("password", None) if password is not None: @@ -130,6 +105,31 @@ class Meta(_UserSerializer.Meta): "password": {"write_only": True, "required": False}, } + def validate_requesting_to_join_class(self, value: str): + # NOTE: Error message is purposefully ambiguous to prevent class + # enumeration. + error_message = "Class does not exist or does not accept join requests." + + if value is not None: + try: + klass = Class.objects.get(access_code=value) + except Class.DoesNotExist as ex: + raise serializers.ValidationError( + error_message, code="does_not_exist" + ) from ex + + if klass.accept_requests_until is None: + raise serializers.ValidationError( + error_message, code="does_not_accept_requests" + ) + + if klass.accept_requests_until < timezone.now(): + raise serializers.ValidationError( + error_message, code="no_longer_accepts_requests" + ) + + return value + def validate(self, attrs): if self.instance: # Updating if self.view.action != "reset_password": diff --git a/backend/api/tests/serializers/test_user.py b/backend/api/tests/serializers/test_user.py index a5276996..9f1f465d 100644 --- a/backend/api/tests/serializers/test_user.py +++ b/backend/api/tests/serializers/test_user.py @@ -58,54 +58,6 @@ def setUp(self): self.class_2 = Class.objects.get(name="Class 2 @ School 1") self.class_3 = Class.objects.get(name="Class 3 @ School 1") - def test_validate__first_name_not_unique_per_class_in_data(self): - """First name must be unique per class in data.""" - self.assert_validate( - attrs=[ - { - "first_name": "Peter", - "new_student": { - "class_field": { - "access_code": "ZZ111", - }, - }, - }, - { - "first_name": "Peter", - "new_student": { - "class_field": { - "access_code": "ZZ111", - }, - }, - }, - ], - error_code="first_name_not_unique_per_class_in_data", - many=True, - ) - - def test_validate__first_name_not_unique_per_class_in_db(self): - """First name must be unique per class in database.""" - klass = Class.objects.get(name="Class 1 @ School 1") - assert klass is not None - - student = Student.objects.filter(class_field=klass).first() - assert student is not None - - self.assert_validate( - attrs=[ - { - "first_name": student.new_user.first_name, - "new_student": { - "class_field": { - "access_code": klass.access_code, - }, - }, - }, - ], - error_code="first_name_not_unique_per_class_in_db", - many=True, - ) - def test_validate__create__teacher_and_student(self): """Cannot create a user with both teacher and student attributes.""" self.assert_validate( diff --git a/backend/api/tests/views/test_user.py b/backend/api/tests/views/test_user.py index 4d5727eb..83f72261 100644 --- a/backend/api/tests/views/test_user.py +++ b/backend/api/tests/views/test_user.py @@ -44,13 +44,13 @@ def setUp(self): email="teacher@noschool.com" ) - self.admin_school1_teacher_user = AdminSchoolTeacherUser.objects.get( + self.admin_school_1_teacher_user = AdminSchoolTeacherUser.objects.get( email="admin.teacher@school1.com" ) - self.non_admin_school1_teacher_user = ( + self.non_admin_school_1_teacher_user = ( NonAdminSchoolTeacherUser.objects.get(email="teacher@school1.com") ) - self.admin_school2_teacher_user = AdminSchoolTeacherUser.objects.get( + self.admin_school_2_teacher_user = AdminSchoolTeacherUser.objects.get( email="admin.teacher@school2.com" ) @@ -59,8 +59,7 @@ def setUp(self): email="indy.requester@email.com" ) - self.class_1 = Class.objects.get(name="Class 1 @ School 1") - self.class_2 = Class.objects.get(name="Class 2 @ School 1") + self.class_1_at_school_1 = Class.objects.get(name="Class 1 @ School 1") def _get_pk_and_token_for_user(self, email: str): user = User.objects.get(email__iexact=email) @@ -139,7 +138,7 @@ def test_get_queryset__independents__handle_join_class_request__admin(self): """Handling a join class request can only target the independent students who made a request to any class in the teacher's school.""" self._test_get_queryset__independents__handle_join_class_request( - user=self.admin_school1_teacher_user + user=self.admin_school_1_teacher_user ) def test_get_queryset__independents__handle_join_class_request__non_admin( @@ -148,16 +147,16 @@ def test_get_queryset__independents__handle_join_class_request__non_admin( """Handling a join class request can only target the independent students who made a request to one of the teacher's classes.""" self._test_get_queryset__independents__handle_join_class_request( - user=self.non_school_teacher_user + user=self.non_admin_school_1_teacher_user ) def test_get_queryset__destroy(self): """Destroying a user can only target the user making the request.""" self.assert_get_queryset( - [self.admin_school1_teacher_user], + [self.admin_school_1_teacher_user], action="destroy", request=self.client.request_factory.delete( - user=self.admin_school1_teacher_user + user=self.admin_school_1_teacher_user ), ) @@ -174,46 +173,42 @@ def test_get_queryset__partial_update__student(self): def test_independents__handle_join_class_request__accept(self): """Teacher can successfully accept a class join request.""" - self.client.login_as(self.admin_school1_teacher_user) + indy_user = self.indy_user_requesting_to_join_class - viewname = self.reverse_action( - "independents--handle-join-class-request", - kwargs={ - "pk": self.indy_user.pk, - }, + self.client.login_as(self.admin_school_1_teacher_user) + self.client.patch( + path=self.reverse_action( + "independents--handle-join-class-request", + kwargs={"pk": indy_user.pk}, + ), + data={"accept": True}, ) - pending_class_request = self.indy_user.student.pending_class_request - - self.client.patch(viewname, data={"accept": True}) - - username = self.indy_user.username - - self.indy_user.refresh_from_db() - - assert self.indy_user.new_student.pending_class_request is None - assert self.indy_user.new_student.class_field == pending_class_request - assert self.indy_user.last_name == "" - assert self.indy_user.email == "" - assert self.indy_user.username != username + pending_class_request = indy_user.student.pending_class_request + username = indy_user.username + indy_user.refresh_from_db() + assert indy_user.student.pending_class_request is None + assert indy_user.student.class_field == pending_class_request + assert indy_user.last_name == "" + assert indy_user.email == "" + assert indy_user.username != username def test_independents__handle_join_class_request__reject(self): """Teacher can successfully reject a class join request.""" - self.client.login_as(self.admin_school1_teacher_user) + indy_user = self.indy_user_requesting_to_join_class - viewname = self.reverse_action( - "independents--handle-join-class-request", - kwargs={ - "pk": self.indy_user.pk, - }, + self.client.login_as(self.admin_school_1_teacher_user) + self.client.patch( + path=self.reverse_action( + "independents--handle-join-class-request", + kwargs={"pk": indy_user.pk}, + ), + data={"accept": False}, ) - self.client.patch(viewname, data={"accept": False}) - - self.indy_user.refresh_from_db() - - assert self.indy_user.new_student.pending_class_request is None - assert self.indy_user.new_student.class_field is None + indy_user.refresh_from_db() + assert indy_user.student.pending_class_request is None + assert indy_user.student.class_field is None # test: reset password actions @@ -332,15 +327,16 @@ def test_reset_password__patch__indy(self): # test: generic actions + # TODO: move this logic and test to TeacherViewSet def test_partial_update__teacher(self): """Admin-school-teacher can update another teacher's profile.""" - self.client.login_as(self.admin_school1_teacher_user) + self.client.login_as(self.admin_school_1_teacher_user) other_school_teacher_user = ( SchoolTeacherUser.objects.filter( - new_teacher__school=self.admin_school1_teacher_user.teacher.school + new_teacher__school=self.admin_school_1_teacher_user.teacher.school ) - .exclude(pk=self.admin_school1_teacher_user.pk) + .exclude(pk=self.admin_school_1_teacher_user.pk) .first() ) assert other_school_teacher_user @@ -361,7 +357,7 @@ def test_partial_update__indy__send_join_request(self): self.client.partial_update( self.indy_user, - {"requesting_to_join_class": self.class_1.access_code}, + {"requesting_to_join_class": self.class_1_at_school_1.access_code}, ) def test_partial_update__indy__revoke_join_request(self): @@ -424,7 +420,7 @@ def _test_destroy( def test_destroy__class_teacher(self): """Class-teacher-users can anonymize themselves and their classes.""" - user = self.non_admin_school1_teacher_user + user = self.non_admin_school_1_teacher_user assert user.teacher.class_teacher.exists() class_names = list( user.teacher.class_teacher.values_list("name", flat=True) @@ -440,7 +436,7 @@ def test_destroy__school_teacher__last_teacher(self): School-teacher-users can anonymize themselves and their school if they are the last teacher. """ - user = self.admin_school1_teacher_user + user = self.admin_school_1_teacher_user assert user.teacher.class_teacher.exists() class_names = list( user.teacher.class_teacher.values_list("name", flat=True) @@ -464,7 +460,7 @@ def test_destroy__school_teacher__last_admin_teacher(self): admin teachers. """ self._test_destroy( - self.admin_school1_teacher_user, + self.admin_school_1_teacher_user, status_code_assertion=status.HTTP_409_CONFLICT, )