Commit 7895c7f6 authored by Richard Glosner's avatar Richard Glosner Committed by Martin Juhás
Browse files

feat: forbid removing all instructors from exercise by an instructor

No API changes

Closes #305
parent 1e40b3e0
Loading
Loading
Loading
Loading
+3 −1
Original line number Diff line number Diff line
@@ -164,7 +164,9 @@ class RemoveInstructorsFromExerciseMutation(graphene.Mutation):
    ) -> graphene.Mutation:
        exercise_access(info.context, int(exercise_id))
        exercise = get_model(Exercise, id=exercise_id)
        users = validate_instructor_removing(user_ids, exercise)
        users = validate_instructor_removing(
            user_ids, exercise, user_from_context(info.context)
        )
        InstructorOfExercise.objects.filter(
            user__in=users, exercise=exercise
        ).delete()
+10 −1
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ from django.db.models import Model
from django.db.models import QuerySet
from rest_framework.exceptions import PermissionDenied

from common_lib.exceptions import UserOperationException
from exercise.models import (
    Team,
    Exercise,
@@ -176,7 +177,7 @@ def validate_instructor_assigning(


def validate_instructor_removing(
    user_ids: List[str], exercise: Exercise
    user_ids: List[str], exercise: Exercise, user: User
) -> List[User]:
    users = _check_nonexistent_users(user_ids)
    _check_presence(
@@ -186,6 +187,14 @@ def validate_instructor_removing(
        user_id__in=user_ids,
        exercise_id=exercise.id,
    )
    if (
        user.group != User.AuthGroup.ADMIN
        and len(users) >= exercise.instructors.count()
    ):
        raise UserOperationException(
            "Only admin can remove all instructors from exercise. "
            "At least one instructor must remain."
        )
    return users


+44 −1
Original line number Diff line number Diff line
@@ -5,7 +5,7 @@ import uuid
from django.conf import settings
from django.test import TestCase, override_settings

from common_lib.exceptions import ModelNotFoundException
from common_lib.exceptions import ModelNotFoundException, UserOperationException
from common_lib.test_utils import (
    internal_create_exercise,
    internal_upload_definition,
@@ -17,6 +17,7 @@ from user.schema.validators import (
    _check_nonexistent_users,
    _check_group,
    _check_presence,
    validate_instructor_removing,
)

TEST_DATA_STORAGE = "user_tests_test_data"
@@ -49,6 +50,9 @@ class UserMutationValidatorsTest(TestCase):
        cls.instructor2 = User.objects.create_staffuser(
            username="i2@mail.com", password="i2"
        )
        cls.admin = User.objects.create_superuser(
            username="admin@mail.com", password="a"
        )
        cls.definition = internal_upload_definition("base_definition")
        cls.exercise = internal_create_exercise(cls.definition.id, 3)
        cls.team = cls.exercise.teams.all().first()
@@ -124,3 +128,42 @@ class UserMutationValidatorsTest(TestCase):
                user_id__in=[self.instructor2.id],
                exercise_id=self.exercise.id,
            )

    def test_invalid_instructor_removing(self):
        InstructorOfExercise.objects.create(
            user=self.instructor, exercise=self.exercise
        )
        # instructor can't remove last instructor
        with self.assertRaises(UserOperationException):
            validate_instructor_removing(
                [self.instructor.id], self.exercise, self.instructor
            )

    def test_valid_instructor_removing(self):
        InstructorOfExercise.objects.create(
            user=self.instructor, exercise=self.exercise
        )
        InstructorOfExercise.objects.create(
            user=self.instructor2, exercise=self.exercise
        )
        self.assertIsNotNone(
            validate_instructor_removing(
                [self.instructor2.id], self.exercise, self.instructor
            )
        )

    def test_admin_instructor_removing(self):
        InstructorOfExercise.objects.create(
            user=self.instructor, exercise=self.exercise
        )
        InstructorOfExercise.objects.create(
            user=self.instructor2, exercise=self.exercise
        )
        # admin can remove all instructors
        self.assertIsNotNone(
            validate_instructor_removing(
                [self.instructor.id, self.instructor2.id],
                self.exercise,
                self.admin,
            )
        )