Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release students #309

Merged
merged 19 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
Loading