Skip to content

Commit

Permalink
Release students (#309)
Browse files Browse the repository at this point in the history
* reset students' password

* move fixtures to py package

* add school teacher invitation fixtures

* use school teacher invitation fixtures

* reset login id also

* new py package

* feedback

* use new py package

* remove comment

* new python package

* anonymize independent- or teacher-user

* Merge branch 'development' into anonymize_user

* remove duplicate test

* new py package

* release students

* merge from dev

* TestReleaseStudentUserSerializer

* new py package

* first name
  • Loading branch information
SKairinos authored Feb 23, 2024
1 parent cc76c1a commit 698e750
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 25 deletions.
4 changes: 2 additions & 2 deletions backend/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ name = "pypi"
# Before adding a new package, check it's not listed under [packages] at
# https://github.com/ocadotechnology/codeforlife-package-python/blob/{ref}/Pipfile
# Replace "{ref}" in the above URL with the ref set below.
codeforlife = {ref = "v0.13.10", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"}
codeforlife = {ref = "v0.14.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"}
# TODO: check if we need the below packages
whitenoise = "==6.5.0"
django-pipeline = "==2.0.8"
Expand All @@ -34,7 +34,7 @@ google-cloud-container = "==2.3.0"
# Before adding a new package, check it's not listed under [dev-packages] at
# https://github.com/ocadotechnology/codeforlife-package-python/blob/{ref}/Pipfile
# Replace "{ref}" in the above URL with the ref set below.
codeforlife = {ref = "v0.13.10", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]}
codeforlife = {ref = "v0.14.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git", extras = ["dev"]}
# TODO: check if we need the below packages
django-selenium-clean = "==0.3.3"
django-test-migrations = "==1.2.0"
Expand Down
6 changes: 3 additions & 3 deletions backend/Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion backend/api/serializers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@
from .school_teacher_invitation import SchoolTeacherInvitationSerializer
from .student import StudentSerializer
from .teacher import TeacherSerializer
from .user import UserSerializer
from .user import ReleaseStudentUserSerializer, UserSerializer
4 changes: 2 additions & 2 deletions backend/api/serializers/teacher.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

import typing as t

from codeforlife.user.models import User
from codeforlife.user.models import Teacher, User
from codeforlife.user.serializers import TeacherSerializer as _TeacherSerializer
from rest_framework import serializers


# pylint: disable-next=missing-class-docstring,too-many-ancestors
class TeacherSerializer(_TeacherSerializer):
class TeacherSerializer(_TeacherSerializer[Teacher]):
class Meta(_TeacherSerializer.Meta):
extra_kwargs = {
**_TeacherSerializer.Meta.extra_kwargs,
Expand Down
48 changes: 44 additions & 4 deletions backend/api/serializers/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@
from .student import StudentSerializer
from .teacher import TeacherSerializer

# pylint: disable=missing-class-docstring,too-many-ancestors


# pylint: disable-next=missing-class-docstring
class UserListSerializer(ModelListSerializer[User]):
def create(self, validated_data):
classes = {
Expand Down Expand Up @@ -86,8 +87,7 @@ def get_first_name(user_fields: t.Dict[str, t.Any]):
return attrs


# pylint: disable-next=missing-class-docstring,too-many-ancestors
class UserSerializer(_UserSerializer):
class UserSerializer(_UserSerializer[User]):
student = StudentSerializer(source="new_student", required=False)
teacher = TeacherSerializer(source="new_teacher", required=False)
current_password = serializers.CharField(
Expand All @@ -106,7 +106,11 @@ class Meta(_UserSerializer.Meta):
extra_kwargs = {
**_UserSerializer.Meta.extra_kwargs,
"first_name": {"read_only": False},
"last_name": {"read_only": False, "required": False},
"last_name": {
"read_only": False,
"required": False,
"min_length": 1,
},
"email": {"read_only": False},
"password": {"write_only": True, "required": False},
}
Expand Down Expand Up @@ -197,3 +201,39 @@ def to_representation(self, instance: User):
representation["password"] = password

return representation


class ReleaseStudentUserListSerializer(ModelListSerializer[StudentUser]):
def update(self, instance, validated_data):
for student_user, data in zip(instance, validated_data):
student_user.student.class_field = None
student_user.student.save(update_fields=["class_field"])

student_user.email = data["email"]
student_user.save(update_fields=["email"])

return instance


class ReleaseStudentUserSerializer(_UserSerializer[StudentUser]):
"""Convert a student to an independent learner."""

class Meta(_UserSerializer.Meta):
extra_kwargs = {
"first_name": {
"min_length": 1,
"read_only": False,
"required": False,
},
"email": {"read_only": False},
}
list_serializer_class = ReleaseStudentUserListSerializer

# pylint: disable-next=missing-function-docstring
def validate_email(self, value: str):
if User.objects.filter(email__iexact=value).exists():
raise serializers.ValidationError(
"Already exists.", code="already_exists"
)

return value
34 changes: 31 additions & 3 deletions backend/api/tests/serializers/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
"""

from codeforlife.tests import ModelSerializerTestCase
from codeforlife.user.models import Class, Student, User
from codeforlife.user.models import Class, Student, StudentUser, User

from ...serializers import UserSerializer
from ...serializers import ReleaseStudentUserSerializer, UserSerializer

# pylint: disable=missing-class-docstring


# pylint: disable-next=missing-class-docstring
class TestUserSerializer(ModelSerializerTestCase[User]):
model_serializer_class = UserSerializer
fixtures = ["school_1"]
Expand Down Expand Up @@ -61,3 +62,30 @@ def test_validate__first_name_not_unique_per_class_in_db(self):
error_code="first_name_not_unique_per_class_in_db",
many=True,
)


class TestReleaseStudentUserSerializer(ModelSerializerTestCase[StudentUser]):
model_serializer_class = ReleaseStudentUserSerializer
fixtures = ["school_1"]

def setUp(self):
student_user = StudentUser.objects.first()
assert student_user
self.student_user = student_user

def test_validate_email__already_exists(self):
"""Cannot release a student with an email that already exists."""
user_fields = User.objects.values("email").first()
assert user_fields

self.assert_validate_field(
"email", user_fields["email"], error_code="already_exists"
)

def test_update(self):
"""The student-user is converted in an independent-user."""
self.assert_update_many(
instance=[self.student_user],
validated_data=[{"email": f"{self.student_user.pk}@email.com"}],
new_data=[{"student": {"class_field": None}}],
)
50 changes: 50 additions & 0 deletions backend/api/tests/views/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from django.db.models.query import QuerySet
from rest_framework import status

from ...serializers import ReleaseStudentUserSerializer
from ...views import UserViewSet

# NOTE: type hint to help Intellisense.
Expand Down Expand Up @@ -76,6 +77,15 @@ def test_get_permissions__students__reset_password(self):
action="students__reset_password",
)

def test_get_permissions__students__release(self):
"""
Only admin-teachers or class-teachers can release students.
"""
self.assert_get_permissions(
[OR(IsTeacher(is_admin=True), IsTeacher(in_class=True))],
action="students__release",
)

def test_get_permissions__partial_update__teacher(self):
"""Only admin-teachers can update a teacher."""
self.assert_get_permissions(
Expand All @@ -99,6 +109,14 @@ def test_get_permissions__destroy(self):
action="destroy",
)

# test: get serializer class

def test_get_serializer_class(self):
"""The action for releasing students has a dedicated serializer."""
self.assert_get_serializer_class(
ReleaseStudentUserSerializer, action="students__release"
)

# test: get queryset

def _test_get_queryset__student_users(
Expand Down Expand Up @@ -137,6 +155,12 @@ def test_get_queryset__students__reset_password(self):
action="students__reset_password", request_method="patch"
)

def test_get_queryset__students__release(self):
"""Releasing students can only target student-users."""
self._test_get_queryset__student_users(
action="students__release", request_method="patch"
)

def test_get_queryset__destroy(self):
"""Destroying a user can only target the user making the request."""
return self.assert_get_queryset(
Expand Down Expand Up @@ -358,6 +382,32 @@ def test_students__reset_password(self):
self.client.login_as(student_user, password)
assert student_user.student.login_id == student_login_id

def test_students__release(self):
"""
Admin-teacher or class_teacher can convert their students to independent
learners.
"""
user = self.admin_school_teacher_user
student_users = list(user.teacher.student_users)

self.client.login_as(user)
response = self.client.patch(
self.reverse_action("students--release"),
data={
student_user.id: {"email": f"{student_user.id}@email.com"}
for student_user in student_users
},
)

for student_user, json_model in zip(student_users, response.json()):
student_user.refresh_from_db()
self.assert_serialized_model_equals_json_model(
model=student_user,
json_model=json_model,
action="students__release",
request_method="patch",
)

# test: generic actions

def test_partial_update__teacher(self):
Expand Down
46 changes: 36 additions & 10 deletions backend/api/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from rest_framework.permissions import AllowAny
from rest_framework.response import Response

from ..serializers import UserSerializer
from ..serializers import ReleaseStudentUserSerializer, UserSerializer

# NOTE: type hint to help Intellisense.
default_token_generator: PasswordResetTokenGenerator = default_token_generator
Expand All @@ -34,7 +34,11 @@ class UserViewSet(_UserViewSet):
def get_permissions(self):
if self.action == "destroy":
return [OR(IsTeacher(), IsIndependent())]
if self.action in ["bulk", "students__reset_password"]:
if self.action in [
"bulk",
"students__reset_password",
"students__release",
]:
return [OR(IsTeacher(is_admin=True), IsTeacher(in_class=True))]
if self.action == "partial_update":
if "teacher" in self.request.data:
Expand All @@ -44,13 +48,22 @@ def get_permissions(self):

return super().get_permissions()

def get_queryset(self):
queryset = super().get_queryset()
def get_serializer_class(self):
if self.action == "students__release":
return ReleaseStudentUserSerializer

return super().get_serializer_class()

def get_queryset(self, user_class=User):
queryset = super().get_queryset(user_class)
if self.action == "destroy":
queryset = queryset.filter(pk=self.request.auth_user.pk)
elif (
elif self.action in [
"students__reset_password",
"students__release",
] or (
self.action == "bulk" and self.request.method in ["PATCH", "DELETE"]
) or self.action == "students__reset_password":
):
queryset = queryset.filter(
new_student__isnull=False,
new_student__class_field__isnull=False,
Expand Down Expand Up @@ -190,14 +203,13 @@ def request_password_reset(self, request: Request):
@action(detail=False, methods=["patch"], url_path="students/reset-password")
def students__reset_password(self, request: Request):
"""Bulk reset students' password."""
queryset = self._get_bulk_queryset(request.data)
queryset = self.get_bulk_queryset(request.data, StudentUser)

fields: t.Dict[int, DataDict] = {}
for pk in queryset.values_list("pk", flat=True):
student_user = StudentUser(pk=pk)
for student_user in queryset:
student_user.set_password()

fields[pk] = {
fields[student_user.pk] = {
# pylint: disable-next=protected-access
"password": student_user._password,
"student": {"login_id": student_user.student.login_id},
Expand All @@ -208,3 +220,17 @@ def students__reset_password(self, request: Request):
student_user.student.save(update_fields=["login_id"])

return Response(fields)

@action(detail=False, methods=["patch"], url_path="students/release")
def students__release(self, request: Request):
"""Convert a list of students into independent learners."""
queryset = self.get_bulk_queryset(request.json_dict.keys(), StudentUser)
serializer = self.get_serializer(
queryset,
data=request.data,
many=True,
context=self.get_serializer_context(),
)
serializer.is_valid(raise_exception=True)
serializer.save()
return Response(serializer.data)

0 comments on commit 698e750

Please sign in to comment.