From 192c6693d27467cb4590655d77fbd3cb325e0805 Mon Sep 17 00:00:00 2001 From: janmax <j.michal@stud.uni-goettingen.de> Date: Tue, 20 Mar 2018 22:56:09 +0100 Subject: [PATCH] Can stop the correction for a student that passes * Set the STOP_ON_PASS setting in the grady/settings/instance.py * Currently uses signals and does not check if the feedback is final or not. * Feedback is validated until the end * Minor fixes in the importer --- ...320_1922.py => 0009_auto_20180320_2335.py} | 12 +- core/models.py | 18 +++ core/serializers/student.py | 18 ++- core/signals.py | 9 ++ core/tests/test_custom_subscription_filter.py | 122 ++++++++++++++++++ core/tests/test_student_reviewer_viewset.py | 9 +- grady/settings/__init__.py | 1 + grady/settings/default.py | 19 +-- grady/settings/instance.py | 5 + grady/settings/live.py | 17 +++ util/importer.py | 3 +- util/processing.py | 4 +- util/testcases.py | 6 - 13 files changed, 206 insertions(+), 37 deletions(-) rename core/migrations/{0009_auto_20180320_1922.py => 0009_auto_20180320_2335.py} (52%) create mode 100644 core/tests/test_custom_subscription_filter.py create mode 100644 grady/settings/instance.py diff --git a/core/migrations/0009_auto_20180320_1922.py b/core/migrations/0009_auto_20180320_2335.py similarity index 52% rename from core/migrations/0009_auto_20180320_1922.py rename to core/migrations/0009_auto_20180320_2335.py index ab2e5c48..692cd7e2 100644 --- a/core/migrations/0009_auto_20180320_1922.py +++ b/core/migrations/0009_auto_20180320_2335.py @@ -1,4 +1,4 @@ -# Generated by Django 2.0.3 on 2018-03-20 19:22 +# Generated by Django 2.0.2 on 2018-03-20 23:35 import core.models from django.db import migrations, models @@ -11,6 +11,16 @@ class Migration(migrations.Migration): ] operations = [ + migrations.AddField( + model_name='studentinfo', + name='passes_exam', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='studentinfo', + name='total_score', + field=models.PositiveIntegerField(default=0), + ), migrations.AlterField( model_name='studentinfo', name='matrikel_no', diff --git a/core/models.py b/core/models.py index 81ab511d..406bffc9 100644 --- a/core/models.py +++ b/core/models.py @@ -16,6 +16,7 @@ from typing import Dict from django.contrib.auth import get_user_model from django.contrib.auth.models import AbstractUser from django.db import models, transaction +from django.conf import settings from django.db.models import (BooleanField, Case, Count, F, IntegerField, Q, QuerySet, Sum, Value, When) from django.db.models.functions import Coalesce @@ -246,6 +247,18 @@ class StudentInfo(models.Model): on_delete=models.CASCADE, related_name='student') + # Managed by signals + total_score = models.PositiveIntegerField(default=0) + passes_exam = models.BooleanField(default=False) + + def update_total_score(self): + ''' This helper is invoked after feedback changes ''' + self.total_score = self.submissions.aggregate( + Sum('feedback__score'))['feedback__score__sum'] or 0 + if self.exam is not None: + self.passes_exam = self.total_score >= self.exam.pass_score + self.save() + def score_per_submission(self) -> Dict[str, int]: """ TODO: get rid of it and use an annotation. @@ -603,6 +616,11 @@ class SubmissionSubscription(models.Model): 'Currently unavailable. Please check for more soon. ' 'Submissions remaining: %s' % stage_candiates.count()) + if (settings.STOP_ON_PASS and + self.feedback_stage == self.FEEDBACK_CREATION): + stage_candiates = stage_candiates.exclude( + submission__student__passes_exam=True) + return stage_candiates @transaction.atomic diff --git a/core/serializers/student.py b/core/serializers/student.py index efa8c537..82ef0bf7 100644 --- a/core/serializers/student.py +++ b/core/serializers/student.py @@ -14,7 +14,13 @@ class StudentInfoSerializer(DynamicFieldsModelSerializer): class Meta: model = StudentInfo - fields = ('pk', 'name', 'user', 'matrikel_no', 'exam', 'submissions') + fields = ('pk', + 'name', + 'user', + 'matrikel_no', + 'exam', + 'submissions', + 'passes_exam') class StudentInfoSerializerForListView(DynamicFieldsModelSerializer): @@ -26,5 +32,11 @@ class StudentInfoSerializerForListView(DynamicFieldsModelSerializer): class Meta: model = StudentInfo - fields = ('pk', 'name', 'user', 'exam', 'submissions', - 'matrikel_no', 'is_active') + fields = ('pk', + 'name', + 'user', + 'exam', + 'submissions', + 'matrikel_no', + 'passes_exam', + 'is_active') diff --git a/core/signals.py b/core/signals.py index f74b8f2d..d5d32b8f 100644 --- a/core/signals.py +++ b/core/signals.py @@ -63,6 +63,15 @@ def update_after_feedback_save(sender, instance, created, **kwargs): meta.save() +@receiver(post_save, sender=Feedback) +def update_student_score(sender, instance, **kwargs): + student = instance.of_submission.student + student.update_total_score() + log.debug('SIGNAL -- Score of student %s was updated %s)', + student, + student.total_score) + + @receiver(pre_save, sender=FeedbackComment) def set_comment_visibility_after_conflict(sender, instance, **kwargs): log.debug('SIGNAL -- set_comment_visibility_after_conflict') diff --git a/core/tests/test_custom_subscription_filter.py b/core/tests/test_custom_subscription_filter.py new file mode 100644 index 00000000..0731815c --- /dev/null +++ b/core/tests/test_custom_subscription_filter.py @@ -0,0 +1,122 @@ +from rest_framework.test import APITestCase + +from util.factories import make_test_data + +from core.models import SubmissionSubscription, Feedback +from django.conf import settings + + +class StopAfterFiftyPercent(APITestCase): + + @classmethod + def setUpTestData(cls): + settings.STOP_ON_PASS = True + + def setUp(self): + self.data = make_test_data(data_dict={ + 'exams': [{ + 'module_reference': 'Test Exam 01', + 'total_score': 50, + 'pass_score': 25, + }], + 'submission_types': [ + { + 'name': '01. Sort this or that', + 'full_score': 20, + 'description': 'Very complicated', + 'solution': 'Trivial!' + }, + { + 'name': '02. Merge this or that or maybe even this', + 'full_score': 15, + 'description': 'Very complicated', + 'solution': 'Trivial!' + }, + { + 'name': '03. Super simple task', + 'full_score': 15, + 'description': 'Very complicated', + 'solution': 'Trivial!' + }, + ], + 'students': [ + {'username': 'student01', 'exam': 'Test Exam 01'}, + ], + 'tutors': [ + {'username': 'tutor01'}, + {'username': 'tutor02'} + ], + 'reviewers': [ + {'username': 'reviewer'} + ], + 'submissions': [ + { + 'text': 'function blabl\n' + ' on multi lines\n' + ' for blabla in bla:\n' + ' lorem ipsum und so\n', + 'type': '01. Sort this or that', + 'user': 'student01', + }, + { + 'text': 'function blabl\n' + ' asasxasx\n' + ' lorem ipsum und so\n', + 'type': '02. Merge this or that or maybe even this', + 'user': 'student01' + }, + { + 'text': 'function blabl\n' + ' on multi lines\n' + ' asasxasx\n' + ' lorem ipsum und so\n', + 'type': '03. Super simple task', + 'user': 'student01' + } + ]} + ) + self.tutor01 = self.data['tutors'][0] + self.tutor02 = self.data['tutors'][1] + self.subscription = SubmissionSubscription.objects.create( + owner=self.tutor01, + feedback_stage=SubmissionSubscription.FEEDBACK_CREATION) + + def test_all_feedback_is_available(self): + self.assertEqual(3, self.subscription.get_available_in_stage()) + + def test_all_is_available_when_score_is_too_low(self): + Feedback.objects.create( + of_submission=self.data['submissions'][0], score=20, is_final=True) + self.assertEqual(2, self.subscription.get_available_in_stage()) + + def test_default_does_not_pass_exam(self): + self.assertFalse(self.data['students'][0].student.passes_exam) + + def test_no_more_submissions_after_student_passed_exam(self): + Feedback.objects.create( + of_submission=self.data['submissions'][0], score=20) + Feedback.objects.create( + of_submission=self.data['submissions'][1], score=15) + + self.data['students'][0].student.refresh_from_db() + self.assertEqual(0, self.subscription.get_available_in_stage()) + self.assertEqual(35, self.data['students'][0].student.total_score) + self.assertTrue(self.data['students'][0].student.passes_exam) + + def test_validation_still_allowed(self): + a1 = self.subscription.get_or_create_work_assignment() + a2 = self.subscription.get_or_create_work_assignment() + + # signals recognize the open assignments + Feedback.objects.create( + of_submission=a1.submission, score=20) + Feedback.objects.create( + of_submission=a2.submission, score=15) + + subscription = SubmissionSubscription.objects.create( + owner=self.tutor02, + feedback_stage=SubmissionSubscription.FEEDBACK_VALIDATION) + + self.assertEqual(0, self.subscription.get_available_in_stage()) + self.assertEqual(2, subscription.get_available_in_stage()) + self.assertEqual(3, subscription.get_remaining_not_final()) diff --git a/core/tests/test_student_reviewer_viewset.py b/core/tests/test_student_reviewer_viewset.py index 9e07425e..588df5bd 100644 --- a/core/tests/test_student_reviewer_viewset.py +++ b/core/tests/test_student_reviewer_viewset.py @@ -32,12 +32,7 @@ class StudentPageTests(APITestCase): 'username': 'user01', 'fullname': 'us er01', 'exam': 'TestExam B.Inf.0042' - }, - { - 'username': 'user02', - 'exam': 'TestExam B.Inf.0042' } - ], 'tutors': [{ 'username': 'tutor' @@ -75,14 +70,14 @@ class StudentPageTests(APITestCase): self.assertEqual(self.response.status_code, status.HTTP_200_OK) def test_can_see_all_students(self): - self.assertEqual(2, len(self.response.data)) + self.assertEqual(1, len(self.response.data)) def test_submissions_score_is_included(self): self.assertEqual(self.student.submissions.first().feedback.score, self.response.data[0]['submissions'][0]['score']) def test_submissions_full_score_is_included(self): - print(self.response.data[0]['submissions'][0]) + self.assertEqual(self.student.submissions.first().type.full_score, self.response.data[0]['submissions'][0]['full_score']) diff --git a/grady/settings/__init__.py b/grady/settings/__init__.py index a8e95f7c..bffd6fae 100644 --- a/grady/settings/__init__.py +++ b/grady/settings/__init__.py @@ -1,5 +1,6 @@ import os from .default import * +from .instance import * # noqa dev = os.environ.get('DJANGO_DEV', False) diff --git a/grady/settings/default.py b/grady/settings/default.py index ac672bd7..8686298f 100644 --- a/grady/settings/default.py +++ b/grady/settings/default.py @@ -12,8 +12,6 @@ https://docs.djangoproject.com/en/1.10/ref/settings/ import datetime import os -import secrets -import string # Build paths inside the project like this: os.path.join(BASE_DIR, ...) BASE_DIR = os.path.dirname(os.path.dirname( @@ -23,21 +21,8 @@ BASE_DIR = os.path.dirname(os.path.dirname( # See https://docs.djangoproject.com/en/1.10/howto/deployment/checklist/ # SECURITY WARNING: keep the secret key used in production secret! -try: - SECRET_KEY -except NameError: - SECRET_FILE = 'secret' - try: - SECRET_KEY = open(SECRET_FILE).read().strip() - except IOError: - try: - SECRET_KEY = ''.join(secrets.choice(string.printable) - for i in range(50)) - with open(SECRET_FILE, 'w') as secret: - secret.write(SECRET_KEY) - except IOError: - Exception('Please create a %s file with random characters \ - to generate your secret key!' % SECRET_FILE) + +SECRET_KEY = '0*h29pqq9n_&5gtcd8qd2mb^uaf8ydo+ck*p72gg18jrrve(ap' # SECURITY WARNING: don't run with debug turned on in production! DEBUG = True diff --git a/grady/settings/instance.py b/grady/settings/instance.py new file mode 100644 index 00000000..0f41f4b5 --- /dev/null +++ b/grady/settings/instance.py @@ -0,0 +1,5 @@ + +# Application specific settings. Mount this file in production to +# set instance specific stuff. In the future this information will +# be stored in the database for dynamic configuration +STOP_ON_PASS = False diff --git a/grady/settings/live.py b/grady/settings/live.py index 50c7887b..ee7ea26a 100644 --- a/grady/settings/live.py +++ b/grady/settings/live.py @@ -1,3 +1,6 @@ +import secrets +import string + """ A live configuration for enhanced security """ CSRF_COOKIE_SECURE = True CSRF_COOKIE_HTTPONLY = True @@ -10,6 +13,20 @@ X_FRAME_OPTIONS = 'DENY' # SECURITY WARNING: don't run with debug turned on in production! DEBUG = False +# Read a new SECRET_KEY or generate a new one +SECRET_FILE = 'secret' +try: + SECRET_KEY = open(SECRET_FILE).read().strip() +except IOError: + try: + SECRET_KEY = ''.join(secrets.choice(string.printable) + for i in range(50)) + with open(SECRET_FILE, 'w') as secret: + secret.write(SECRET_KEY) + except IOError: + Exception('Please create a %s file with random characters \ + to generate your secret key!' % SECRET_FILE) + # adjust this setting to your needs ALLOWED_HOSTS = [ 'localhost', '.grady.janmax.org', 'grady.informatik.uni-goettingen.de' diff --git a/util/importer.py b/util/importer.py index b5d2e173..736591c6 100644 --- a/util/importer.py +++ b/util/importer.py @@ -109,7 +109,8 @@ def add_tests(submission_obj, tests): defaults={'is_active': False} ) - for name, test_data in ((name, tests[name]) for name in TEST_ORDER): + for name in (name for name in TEST_ORDER if name in tests): + test_data = tests[name] test_obj, created = Test.objects.update_or_create( name=test_data['name'], submission=submission_obj, diff --git a/util/processing.py b/util/processing.py index 2ffd2d86..bb80eb82 100644 --- a/util/processing.py +++ b/util/processing.py @@ -200,8 +200,8 @@ def process(descfile, binaries, objects, submissions, header, highest_test): def iterate_submissions(): yield from (obj - for _, data in tqdm(submissions_json.items()) - for obj in data['submissions']) + for student in tqdm(submissions_json['students']) + for obj in student['submissions']) for submission_obj in tqdm(iterate_submissions()): highestTestClass(submission_obj) diff --git a/util/testcases.py b/util/testcases.py index 874d8549..819d871a 100644 --- a/util/testcases.py +++ b/util/testcases.py @@ -66,12 +66,6 @@ def testcases_generator(task, n=10): yield 'NO INPUT' return - yield '' - yield '0' - - for i in range(n // 2): - yield rubbish() - for i in range(n): yield argument_generator(syntax) -- GitLab