From 16230de70cfd8ad4bd72d1c24f890f31fda1f6a7 Mon Sep 17 00:00:00 2001 From: Mateusz Kolpa Date: Wed, 13 Mar 2024 17:07:20 +0100 Subject: [PATCH] `Add user to contest` form doesn't have an option for direct add (#307) * Add some functionality * Finish add user to contest * Fix indentation * Simplify teacher check * Simplify add_user_to_contest view * Add some tests * Gotta improve tests * Works on tests * Some works... * Idk what to do... * Finalize the PR with some cherries on top * Shorten some lines * Shorten some lines --- easy_toolbox.py | 4 +- oioioi/teachers/forms.py | 28 ++++- .../teachers/templates/teachers/members.html | 19 ++++ oioioi/teachers/tests.py | 102 +++++++++++++++++- oioioi/teachers/urls.py | 9 ++ oioioi/teachers/utils.py | 65 +++++++++++ oioioi/teachers/views.py | 44 +++++++- 7 files changed, 265 insertions(+), 6 deletions(-) create mode 100644 oioioi/teachers/utils.py diff --git a/easy_toolbox.py b/easy_toolbox.py index f617208eb..77d196b09 100755 --- a/easy_toolbox.py +++ b/easy_toolbox.py @@ -3,7 +3,7 @@ # pip requirements: # python ^3.6 # inquirer (only for GUI) -# +# # system: # docker # docker-compose @@ -37,7 +37,7 @@ ("test", "Run unit tests.", "{exec} web ../oioioi/test.sh"), ("test-slow", "Run unit tests. (--runslow)", "{exec} web ../oioioi/test.sh --runslow"), ("test-abc", "Run specific test file. (edit the toolbox)", - "{exec} web ../oioioi/test.sh -v oioioi/problems/tests/test_task_archive.py"), + "{exec} web ../oioioi/test.sh -v oioioi/teachers/tests.py"), ("test-coverage", "Run coverage tests.", "{exec} 'web' ../oioioi/test.sh oioioi/problems --cov-report term --cov-report xml:coverage.xml --cov=oioioi"), ("cypress-apply-settings", "Apply settings for CyPress.", diff --git a/oioioi/teachers/forms.py b/oioioi/teachers/forms.py index 179ec764b..ea9a68e09 100644 --- a/oioioi/teachers/forms.py +++ b/oioioi/teachers/forms.py @@ -2,11 +2,14 @@ from django.urls import reverse from django.utils.safestring import mark_safe from django.utils.translation import gettext_lazy as _ +from django.contrib import messages +from django.core.exceptions import ValidationError from oioioi.contests.forms import SimpleContestForm -from oioioi.teachers.models import Teacher +from oioioi.teachers.models import ContestTeacher, Teacher from oioioi.base.utils.user_selection import UserSelectionField - +from oioioi.participants.models import Participant +from oioioi.teachers.utils import validate_can_add_user_to_contest, add_user_to_contest_as class TeacherContestForm(SimpleContestForm): @@ -83,3 +86,24 @@ def __init__(self, *args, **kwargs): def clean_school(self): data = self.cleaned_data['school'] return ' '.join(data.splitlines()) + + +class AddUserToContestForm(forms.Form): + user = UserSelectionField() + + def __init__(self, member_type, contest, *args, **kwargs): + self.member_type = member_type + self.contest = contest + super(AddUserToContestForm, self).__init__(*args, **kwargs) + + def clean(self): + clean_data = super().clean() + + if self.is_valid(): + user = self.cleaned_data['user'] + try: + validate_can_add_user_to_contest(user, self.contest, self.member_type) + except ValidationError as e: + self.add_error('user', e.message) + + return clean_data diff --git a/oioioi/teachers/templates/teachers/members.html b/oioioi/teachers/templates/teachers/members.html index dfc7c0774..9d6468e4b 100644 --- a/oioioi/teachers/templates/teachers/members.html +++ b/oioioi/teachers/templates/teachers/members.html @@ -176,6 +176,25 @@

{% endif %} +
+ {% csrf_token %} + + +
+
+ +
+ + +
+
+ {% if member_type == 'pupil' and usergroups_active and members %}
diff --git a/oioioi/teachers/tests.py b/oioioi/teachers/tests.py index 5e83cb1a6..edf1197aa 100644 --- a/oioioi/teachers/tests.py +++ b/oioioi/teachers/tests.py @@ -6,6 +6,9 @@ from oioioi.contests.tests import make_empty_contest_formset from oioioi.contests.tests.utils import make_user_contest_admin from oioioi.teachers.models import Teacher +from django.core.exceptions import ValidationError + +from oioioi.teachers.utils import add_user_to_contest_as def change_contest_type(contest): @@ -28,7 +31,8 @@ def test_problemset_permissions(self): response = self.client.get(url_add, follow=True) self.assertEqual(response.status_code, 200) - self.assertTrue(self.client.login(username='test_user2')) # test_user2 is not + self.assertTrue( + self.client.login(username='test_user2')) # test_user2 is not response = self.client.get(url_main) self.assertEqual(response.status_code, 200) self.assertNotContains(response, 'Add problem') @@ -181,3 +185,99 @@ def test_teacher_modify(self): self.assertEqual(Teacher.objects.all().count(), 2) mod_teacher = Teacher.objects.get(pk=1001) self.assertEqual(mod_teacher.school, "New School") + + +class TestAddUserToContestForm(TestCase): + fixtures = ['test_users', 'teachers', 'test_contest'] + + def setUp(self): + self.user = User.objects.get(username='test_user') + self.c = Contest.objects.get(id='c') + + # In order to get the required URL for tests, + # we have to first get it as a regular teacher. + self.assertTrue(self.client.login(username='test_user')) + add_user_to_contest_as(self.user, self.c, 'teacher') + self.url_add_teacher = reverse( + 'teachers_add_user_to_contest', + kwargs={'contest_id':self.c.id, 'member_type':'teacher'}) + self.url_add_pupil = reverse( + 'teachers_add_user_to_contest', + kwargs={'contest_id':self.c.id, 'member_type':'pupil'}) + self.assertTrue( + self.c.contestteacher_set + .filter(teacher__user=self.user) + .first().delete()) + self.client.logout() + + def test_add_user_to_contest_as_pupil(self): + self.assertFalse(self.c.participant_set.filter(user=self.user)) + add_user_to_contest_as(self.user, self.c, 'pupil') + self.assertTrue(self.c.participant_set.filter(user=self.user)) + + with self.assertRaisesRegex(ValidationError, 'User is already added'): + add_user_to_contest_as(self.user, self.c, 'pupil') + with self.assertRaisesRegex(ValidationError, 'User is already added'): + add_user_to_contest_as(self.user, self.c, 'teacher') + + def test_add_user_to_contest_as_teacher(self): + self.assertFalse( + self.c.contestteacher_set.filter(teacher__user=self.user)) + add_user_to_contest_as(self.user, self.c, 'teacher') + self.assertTrue( + self.c.contestteacher_set.filter(teacher__user=self.user)) + + with self.assertRaisesRegex(ValidationError, 'User is already added'): + add_user_to_contest_as(self.user, self.c, 'pupil') + with self.assertRaisesRegex(ValidationError, 'User is already added'): + add_user_to_contest_as(self.user, self.c, 'teacher') + + def test_add_non_teacher_as_teacher(self): + not_teacher = User.objects.filter(is_superuser=False, + teacher__isnull=True, + is_active=True).first() + + with self.assertRaisesRegex(ValidationError, 'User is not a teacher'): + add_user_to_contest_as(not_teacher, self.c, 'teacher') + self.assertFalse( + self.c.contestteacher_set.filter(teacher__user=self.user)) + add_user_to_contest_as(not_teacher, self.c, 'pupil') + with self.assertRaisesRegex(ValidationError, 'User is already added'): + add_user_to_contest_as(not_teacher, self.c, 'teacher') + + def test_http_logged_out(self): + post_data = { 'user': 'test_user' } + + for url in [self.url_add_pupil, self.url_add_teacher]: + response = self.client.post(url, post_data, Follow=False) + self.assertEqual(response.status_code, 302) + self.assertIn('/login/?next=', response['Location']) + self.assertTrue(response['Location'].endswith(url)) + + # Check if it has not added any new users. + self.assertFalse( + self.c.contestteacher_set.filter(teacher__user=self.user)) + self.assertFalse( + self.c.participant_set.filter(user=self.user)) + + def test_http_logged_in(self): + self.assertTrue(self.client.login(username='test_user')) + + # Make sure 'test_user' is not a contest teacher. + self.assertFalse( + self.c.contestteacher_set.filter(teacher__user=self.user)) + self.assertFalse( + self.c.participant_set.filter(user=self.user)) + + def try_add(): + post_data = { 'user': 'test_user2' } # Some other user + for url in [self.url_add_pupil, self.url_add_teacher]: + response = self.client.post(url, post_data, Follow=False) + self.assertEqual(response.status_code, 403) + + try_add() + + # Retry as a contest pupil. + add_user_to_contest_as(self.user, self.c, 'pupil') + self.assertTrue(self.c.participant_set.filter(user=self.user)) + try_add() diff --git a/oioioi/teachers/urls.py b/oioioi/teachers/urls.py index cfcf3fb96..1fa49b648 100644 --- a/oioioi/teachers/urls.py +++ b/oioioi/teachers/urls.py @@ -25,6 +25,15 @@ name='teachers_regenerate_key', ), re_path(r'^delete/$', views.delete_members_view, name='teachers_delete_members'), + + re_path( + r'^registration/add_user/$', + views.add_user_to_contest, + name='teachers_add_user_to_contest'), + re_path( + r'^registration/get_appendable_users/$', + views.get_appendable_users_view, + name='teachers_get_appendable_users'), ] contest_patterns = [ diff --git a/oioioi/teachers/utils.py b/oioioi/teachers/utils.py new file mode 100644 index 000000000..a39ebc08e --- /dev/null +++ b/oioioi/teachers/utils.py @@ -0,0 +1,65 @@ +from oioioi.participants.models import Participant +from oioioi.teachers.models import ContestTeacher, Teacher +from django.core.exceptions import ValidationError +from django.utils.translation import gettext_lazy as _ + + +def get_user_teacher_obj(user): + try: + return user.teacher + except Teacher.DoesNotExist: + return None + + +def is_user_already_in_contest(user, contest): + teacher = get_user_teacher_obj(user) + + if user.participant_set.filter(contest=contest) or \ + (teacher and + teacher.contestteacher_set.filter(contest=contest)): + return True + + return False + + +def validate_can_add_user_to_contest(user, contest, member_type): + exists = False + + if not is_user_already_in_contest(user, contest): + if member_type == 'pupil': + exists = len(Participant.objects.filter( + contest=contest, user=user + )) > 0 + elif member_type == 'teacher': + if teacher := get_user_teacher_obj(user): + exists = len(ContestTeacher.objects.filter( + contest=contest, teacher=teacher + )) > 0 + else: + raise ValidationError( + _("User is not a teacher: \'%(user)s\'") + % {"user": user }) + else: + raise ValueError("Invalid member type") + else: + exists = True + + if exists: + raise ValidationError( + _("User is already added: \'%(user)s\'") + % { "user": user }) + + +def add_user_to_contest_as(user, contest, member_type): + validate_can_add_user_to_contest(user, contest, member_type) + created = False + + if member_type == 'pupil': + Participant.objects.get_or_create( + contest=contest, user=user + ) + elif member_type == 'teacher': + teacher = get_user_teacher_obj(user) + ContestTeacher.objects.get_or_create( + contest=contest, teacher=teacher + ) diff --git a/oioioi/teachers/views.py b/oioioi/teachers/views.py index cc45a9c62..b31cb4170 100644 --- a/oioioi/teachers/views.py +++ b/oioioi/teachers/views.py @@ -27,8 +27,11 @@ from oioioi.contests.utils import contest_exists, is_contest_admin from oioioi.participants.models import Participant from oioioi.teachers.controllers import TeacherContestController -from oioioi.teachers.forms import AddTeacherForm +from oioioi.teachers.forms import AddTeacherForm, AddUserToContestForm from oioioi.teachers.models import ContestTeacher, RegistrationConfig, Teacher +from oioioi.teachers.utils import \ + is_user_already_in_contest, get_user_teacher_obj, add_user_to_contest_as +from django.core.exceptions import ValidationError if 'oioioi.usergroups' in settings.INSTALLED_APPS: import oioioi.usergroups.utils as usergroups @@ -398,3 +401,42 @@ def contest_dashboard_redirect(request): kwargs={'contest_id': request.contest.id}, ) ) + + +@enforce_condition(is_teachers_contest & is_contest_admin) +def get_appendable_users_view(request, member_type): + users = User.objects.filter(is_superuser=False, is_active=True) + if member_type == 'teacher': + users = users.filter(teacher__isnull=False) + + return get_user_hints_view(request, 'substr', users) + + +@require_POST +@enforce_condition(contest_exists & is_teachers_contest & is_contest_admin) +def add_user_to_contest(request, member_type): + form = AddUserToContestForm(member_type, request.contest, request.POST) + try: + if form.is_valid(): + user = form.cleaned_data['user'] + + try: + add_user_to_contest_as( + user, + request.contest, + member_type + ) + messages.success( + request, + _('User \'%(user)s\' successfully added as a \'%(member_type)s\'.') + % {'user': user, 'member_type': member_type}) + + except ValidationError as e: + messages.error(request, e.message) + else: + for err in form.errors.as_data()['user']: + messages.error(request, err.message) + except ValidationError as e: + messages.error(e.message) + + return redirect_to_members(request, member_type)