From 55d62558742b0c39680d19f154c9cb31c3df838b Mon Sep 17 00:00:00 2001 From: janmax <j.michal@stud.uni-goettingen.de> Date: Sun, 17 Dec 2017 14:20:08 +0100 Subject: [PATCH] Started with a new mechanism to assign work to tutors * The mechanism proposed should work as follows: * Tutors can subscribe to certain submission categries (currently this includes exam, student or type specific submissions). If the set of submissions to corrent is small (student) all submissions of that category are reserved for that tutor. * A reviewer should also be able to subscribe other users (delegation) * A subscription contains assignments or creates them: * Only one assignment per user may be active. * No new assignments can be added to a subscription after it was created while another assignment is present for that subscription. * An assignment delegates a submission to a tutor. * An active assignment indicates that the tutor is working on that assignment * After an assignment was finished it is deleted (or archived). * Upgraded to Django 2.0 * Closes #66, #53. --- .gitlab-ci.yml | 2 +- core/migrations/0001_initial.py | 56 ++- core/migrations/0002_auto_20171110_1612.py | 79 ---- core/migrations/0002_auto_20171222_1116.py | 18 + core/migrations/0003_student_matrikel_no.py | 22 -- core/models.py | 370 +++++++++--------- core/permissions.py | 2 +- core/serializers.py | 64 ++- core/tests/test_factory_and_feedback.py | 28 +- core/tests/test_functional_views.py | 1 + .../test_subscription_assignment_service.py | 78 ++++ core/urls.py | 34 +- core/views.py | 46 ++- grady/settings/default.py | 1 - grady/urls.py | 26 +- requirements.txt | 4 +- util/factories.py | 44 ++- 17 files changed, 491 insertions(+), 384 deletions(-) delete mode 100644 core/migrations/0002_auto_20171110_1612.py create mode 100644 core/migrations/0002_auto_20171222_1116.py delete mode 100644 core/migrations/0003_student_matrikel_no.py create mode 100644 core/tests/test_subscription_assignment_service.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 79e0ef5c..a896dbc0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -40,7 +40,7 @@ test_pytest: services: - postgres:9.5 script: - - DJANGO_SETTINGS_MODULE=grady.settings pytest --cov + - pytest --cov --ds=grady.settings core/tests artifacts: paths: - .coverage diff --git a/core/migrations/0001_initial.py b/core/migrations/0001_initial.py index 5f840961..e28dd8e6 100644 --- a/core/migrations/0001_initial.py +++ b/core/migrations/0001_initial.py @@ -1,10 +1,13 @@ # -*- coding: utf-8 -*- -# Generated by Django 1.11.7 on 2017-11-04 19:10 +# Generated by Django 1.11.8 on 2017-12-22 10:51 from __future__ import unicode_literals -from typing import List, Text +import uuid +import django.contrib.auth.models +import django.contrib.auth.validators import django.db.models.deletion +import django.utils.timezone from django.conf import settings from django.db import migrations, models @@ -15,7 +18,9 @@ class Migration(migrations.Migration): initial = True - dependencies: List[Text] = [] + dependencies = [ + ('auth', '0008_alter_user_username_max_length'), + ] operations = [ migrations.CreateModel( @@ -24,16 +29,27 @@ class Migration(migrations.Migration): ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('password', models.CharField(max_length=128, verbose_name='password')), ('last_login', models.DateTimeField(blank=True, null=True, verbose_name='last login')), - ('username', models.CharField(error_messages={'unique': 'A user with that username already exists.'}, max_length=150, unique=True)), + ('is_superuser', models.BooleanField(default=False, help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status')), + ('username', models.CharField(error_messages={'unique': 'A user with that username already exists.'}, help_text='Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.', max_length=150, unique=True, validators=[django.contrib.auth.validators.UnicodeUsernameValidator()], verbose_name='username')), + ('first_name', models.CharField(blank=True, max_length=30, verbose_name='first name')), + ('last_name', models.CharField(blank=True, max_length=30, verbose_name='last name')), + ('email', models.EmailField(blank=True, max_length=254, verbose_name='email address')), + ('is_staff', models.BooleanField(default=False, help_text='Designates whether the user can log into this admin site.', verbose_name='staff status')), + ('is_active', models.BooleanField(default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active')), + ('date_joined', models.DateTimeField(default=django.utils.timezone.now, verbose_name='date joined')), ('fullname', models.CharField(blank=True, max_length=70, verbose_name='full name')), - ('is_staff', models.BooleanField(default=False, verbose_name='staff status')), ('is_admin', models.BooleanField(default=False)), - ('is_superuser', models.BooleanField(default=False)), - ('is_active', models.BooleanField(default=True, verbose_name='active')), + ('groups', models.ManyToManyField(blank=True, help_text='The groups this user belongs to. A user will get all permissions granted to each of their groups.', related_name='user_set', related_query_name='user', to='auth.Group', verbose_name='groups')), + ('user_permissions', models.ManyToManyField(blank=True, help_text='Specific permissions for this user.', related_name='user_set', related_query_name='user', to='auth.Permission', verbose_name='user permissions')), ], options={ + 'verbose_name': 'user', + 'verbose_name_plural': 'users', 'abstract': False, }, + managers=[ + ('objects', django.contrib.auth.models.UserManager()), + ], ), migrations.CreateModel( name='ExamType', @@ -57,7 +73,6 @@ class Migration(migrations.Migration): ('score', models.PositiveIntegerField(default=0)), ('created', models.DateTimeField(auto_now_add=True)), ('modified', models.DateTimeField(auto_now=True)), - ('status', models.IntegerField(choices=[(0, 'editable'), (1, 'request reassignment'), (2, 'request review'), (3, 'accepted')], default=0)), ('origin', models.IntegerField(choices=[(0, 'was empty'), (1, 'passed unittests'), (2, 'did not compile'), (3, 'could not link'), (4, 'created by a human. yak!')], default=4)), ], options={ @@ -65,6 +80,15 @@ class Migration(migrations.Migration): 'verbose_name_plural': 'Feedback Set', }, ), + migrations.CreateModel( + name='GeneralTaskSubscription', + fields=[ + ('subscription_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('query_key', models.CharField(blank=True, max_length=75)), + ('query_type', models.CharField(choices=[('random', 'Query for any submission'), ('student', 'Query for submissions of student'), ('exam', 'Query for submissions of exam type'), ('submission_type', 'Query for submissions of submissions_type')], default='random', max_length=75)), + ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='susbscriptions', to=settings.AUTH_USER_MODEL)), + ], + ), migrations.CreateModel( name='Reviewer', fields=[ @@ -77,6 +101,7 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('has_logged_in', models.BooleanField(default=False)), + ('matrikel_no', models.CharField(default=core.models.random_matrikel_no, max_length=8, unique=True)), ('exam', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='students', to='core.ExamType')), ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='student', to=settings.AUTH_USER_MODEL)), ], @@ -88,7 +113,7 @@ class Migration(migrations.Migration): migrations.CreateModel( name='Submission', fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('submission_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), ('seen_by_student', models.BooleanField(default=False)), ('text', models.TextField(blank=True)), ('student', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='submissions', to='core.Student')), @@ -134,6 +159,15 @@ class Migration(migrations.Migration): ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='tutor', to=settings.AUTH_USER_MODEL)), ], ), + migrations.CreateModel( + name='TutorSubmissionAssignment', + fields=[ + ('assignment_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('active', models.BooleanField(default=False)), + ('submission', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='assignment', to='core.Submission')), + ('subscription', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='assignments', to='core.GeneralTaskSubscription')), + ], + ), migrations.AddField( model_name='submission', name='type', @@ -162,4 +196,8 @@ class Migration(migrations.Migration): name='submission', unique_together=set([('type', 'student')]), ), + migrations.AlterUniqueTogether( + name='generaltasksubscription', + unique_together=set([('owner', 'query_key', 'query_type')]), + ), ] diff --git a/core/migrations/0002_auto_20171110_1612.py b/core/migrations/0002_auto_20171110_1612.py deleted file mode 100644 index d7eba8e7..00000000 --- a/core/migrations/0002_auto_20171110_1612.py +++ /dev/null @@ -1,79 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.11.7 on 2017-11-10 16:12 -from __future__ import unicode_literals - -import django.contrib.auth.models -import django.contrib.auth.validators -import django.utils.timezone -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('auth', '0008_alter_user_username_max_length'), - ('core', '0001_initial'), - ] - - operations = [ - migrations.AlterModelOptions( - name='useraccount', - options={'verbose_name': 'user', 'verbose_name_plural': 'users'}, - ), - migrations.AlterModelManagers( - name='useraccount', - managers=[ - ('objects', django.contrib.auth.models.UserManager()), - ], - ), - migrations.AddField( - model_name='useraccount', - name='date_joined', - field=models.DateTimeField(default=django.utils.timezone.now, verbose_name='date joined'), - ), - migrations.AddField( - model_name='useraccount', - name='email', - field=models.EmailField(blank=True, max_length=254, verbose_name='email address'), - ), - migrations.AddField( - model_name='useraccount', - name='first_name', - field=models.CharField(blank=True, max_length=30, verbose_name='first name'), - ), - migrations.AddField( - model_name='useraccount', - name='groups', - field=models.ManyToManyField(blank=True, help_text='The groups this user belongs to. A user will get all permissions granted to each of their groups.', related_name='user_set', related_query_name='user', to='auth.Group', verbose_name='groups'), - ), - migrations.AddField( - model_name='useraccount', - name='last_name', - field=models.CharField(blank=True, max_length=30, verbose_name='last name'), - ), - migrations.AddField( - model_name='useraccount', - name='user_permissions', - field=models.ManyToManyField(blank=True, help_text='Specific permissions for this user.', related_name='user_set', related_query_name='user', to='auth.Permission', verbose_name='user permissions'), - ), - migrations.AlterField( - model_name='useraccount', - name='is_active', - field=models.BooleanField(default=True, help_text='Designates whether this user should be treated as active. Unselect this instead of deleting accounts.', verbose_name='active'), - ), - migrations.AlterField( - model_name='useraccount', - name='is_staff', - field=models.BooleanField(default=False, help_text='Designates whether the user can log into this admin site.', verbose_name='staff status'), - ), - migrations.AlterField( - model_name='useraccount', - name='is_superuser', - field=models.BooleanField(default=False, help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status'), - ), - migrations.AlterField( - model_name='useraccount', - name='username', - field=models.CharField(error_messages={'unique': 'A user with that username already exists.'}, help_text='Required. 150 characters or fewer. Letters, digits and @/./+/-/_ only.', max_length=150, unique=True, validators=[django.contrib.auth.validators.UnicodeUsernameValidator()], verbose_name='username'), - ), - ] diff --git a/core/migrations/0002_auto_20171222_1116.py b/core/migrations/0002_auto_20171222_1116.py new file mode 100644 index 00000000..47abe7a7 --- /dev/null +++ b/core/migrations/0002_auto_20171222_1116.py @@ -0,0 +1,18 @@ +# Generated by Django 2.0 on 2017-12-22 11:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0001_initial'), + ] + + operations = [ + migrations.AlterField( + model_name='useraccount', + name='last_name', + field=models.CharField(blank=True, max_length=150, verbose_name='last name'), + ), + ] diff --git a/core/migrations/0003_student_matrikel_no.py b/core/migrations/0003_student_matrikel_no.py deleted file mode 100644 index 82da5d1a..00000000 --- a/core/migrations/0003_student_matrikel_no.py +++ /dev/null @@ -1,22 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.11.7 on 2017-11-10 21:46 -from __future__ import unicode_literals - -from django.db import migrations, models - -import core.models - - -class Migration(migrations.Migration): - - dependencies = [ - ('core', '0002_auto_20171110_1612'), - ] - - operations = [ - migrations.AddField( - model_name='student', - name='matrikel_no', - field=models.CharField(default=core.models.random_matrikel_no, max_length=8, unique=True), - ), - ] diff --git a/core/models.py b/core/models.py index 966b647d..df10d6e5 100644 --- a/core/models.py +++ b/core/models.py @@ -6,17 +6,21 @@ See docstring of the individual models for information on the setup of the database. ''' +import logging +import uuid from collections import OrderedDict from random import randrange -from typing import Dict, Union +from typing import Dict from django.contrib.auth import get_user_model from django.contrib.auth.models import AbstractUser -from django.db import models +from django.db import models, transaction from django.db.models import (BooleanField, Case, Count, F, IntegerField, Q, QuerySet, Sum, Value, When) from django.db.models.functions import Coalesce +log = logging.getLogger(__name__) + def random_matrikel_no() -> str: """Use as a default value for student's matriculation number. @@ -152,20 +156,29 @@ class UserAccount(AbstractUser): (hasattr(self, 'reviewer') and self.reviewer) or \ (hasattr(self, 'tutor') and self.tutor) + def is_reviewer(self): + return hasattr(self, 'reviewer') + + def is_tutor(self): + return hasattr(self, 'tutor') + + def is_student(self): + return hasattr(self, 'student') + class Tutor(models.Model): - user = models.OneToOneField( - get_user_model(), unique=True, - on_delete=models.CASCADE, related_name='tutor') + user = models.OneToOneField(get_user_model(), + on_delete=models.CASCADE, + related_name='tutor') def get_feedback_count(self) -> int: return self.feedback_list.count() class Reviewer(models.Model): - user = models.OneToOneField( - get_user_model(), unique=True, - on_delete=models.CASCADE, related_name='reviewer') + user = models.OneToOneField(get_user_model(), + on_delete=models.CASCADE, + related_name='reviewer') class Student(models.Model): @@ -188,14 +201,16 @@ class Student(models.Model): The matriculation number of the student """ has_logged_in = models.BooleanField(default=False) - matrikel_no = models.CharField( - unique=True, max_length=8, default=random_matrikel_no) - exam = models.ForeignKey( - 'ExamType', on_delete=models.SET_NULL, - related_name='students', null=True) - user = models.OneToOneField( - get_user_model(), unique=True, - on_delete=models.CASCADE, related_name='student') + matrikel_no = models.CharField(unique=True, + max_length=8, + default=random_matrikel_no) + exam = models.ForeignKey('ExamType', + on_delete=models.SET_NULL, + related_name='students', + null=True) + user = models.OneToOneField(get_user_model(), + on_delete=models.CASCADE, + related_name='student') def score_per_submission(self) -> Dict[str, int]: """ TODO: get rid of it and use an annotation. @@ -273,11 +288,9 @@ class Test(models.Model): name = models.CharField(max_length=30) label = models.CharField(max_length=50) annotation = models.TextField() - submission = models.ForeignKey( - 'submission', - related_name='tests', - on_delete=models.CASCADE, - ) + submission = models.ForeignKey('submission', + related_name='tests', + on_delete=models.CASCADE,) class Meta: verbose_name = "Test" @@ -308,6 +321,9 @@ class Submission(models.Model): type : OneToOneField Relation to the type containing meta information """ + submission_id = models.UUIDField(primary_key=True, + default=uuid.uuid4, + editable=False) seen_by_student = models.BooleanField(default=False) text = models.TextField(blank=True) type = models.ForeignKey( @@ -331,63 +347,6 @@ class Submission(models.Model): self.student ) - @classmethod - def assign_tutor(cls, tutor: Tutor, slug: str=None) -> bool: - """Assigns a tutor to a submission - - A submission is not assigned to the specified tutor in the case - 1. the tutor already has a feedback in progress - 2. there is no more feedback to give - - Parameters - ---------- - tutor : User object - The tutor that a submission should be assigned to. - slug : None, optional - If a slug for a submission is given the belonging Feedback is - assigned to the tutor. If this submission had feedback before - the tutor that worked on it, is unassigned. - - Returns - ------- - bool - Returns True only if feedback was actually assigned otherwise False - - """ - - # Get a submission from the submission set - unfinished = Feedback.tutor_unfinished_feedback(tutor) - if unfinished: - return False - - candidates = cls.objects.filter( - ( - Q(feedback__isnull=True) | - Q(feedback__origin=Feedback.DID_NOT_COMPILE) | - Q(feedback__origin=Feedback.COULD_NOT_LINK) | - Q(feedback__origin=Feedback.FAILED_UNIT_TESTS) - ) & - ~Q(feedback__of_tutor=tutor) - ) - - # we want a submission of a specific type - if slug: - candidates = candidates.filter(type__slug=slug) - - # we couldn't find any submission to correct - if not candidates: - return False - - submission = candidates[0] - feedback = submission.feedback if hasattr( - submission, 'feedback') else Feedback() - feedback.origin = Feedback.MANUAL - feedback.status = Feedback.EDITABLE - feedback.of_tutor = tutor - feedback.of_submission = submission - feedback.save() - return True - class Feedback(models.Model): """ @@ -404,25 +363,15 @@ class Feedback(models.Model): points a student receives for his submission. of_tutor : ForeignKey The tutor/reviewer how last edited the feedback - ORIGIN : TYPE - Description origin : IntegerField Of whom was this feedback originally created. She below for the choices score : PositiveIntegerField A score that has been assigned to he submission. Is final if it was accepted. - STATUS : The status determines - Description - status : PositiveIntegerField - The status roughly determines in which state a feedback is in. A just - initiated submission is editable. Based on the status feedback is - presented to different types of users. Students may see feedback only - if it has been accepted, while reviewers have access at any time. text : TextField Detailed description by the tutor about what went wrong. Every line in the feedback should correspond with a line in the students submission, maybe with additional comments appended. - """ text = models.TextField() score = models.PositiveIntegerField(default=0) @@ -432,10 +381,7 @@ class Feedback(models.Model): of_submission = models.OneToOneField( Submission, on_delete=models.CASCADE, - related_name='feedback', - unique=True, - blank=False, - null=False) + related_name='feedback') of_tutor = models.ForeignKey( Tutor, on_delete=models.SET_NULL, @@ -449,24 +395,6 @@ class Feedback(models.Model): blank=True, null=True) - # what is the current status of our feedback - ( - EDITABLE, - OPEN, - NEEDS_REVIEW, - ACCEPTED, - ) = range(4) # this order matters - STATUS = ( - (EDITABLE, 'editable'), - (OPEN, 'request reassignment'), - (NEEDS_REVIEW, 'request review'), - (ACCEPTED, 'accepted'), - ) - status = models.IntegerField( - choices=STATUS, - default=EDITABLE, - ) - # how was this feedback created ( WAS_EMPTY, @@ -500,87 +428,169 @@ class Feedback(models.Model): def get_full_score(self) -> int: return self.of_submission.type.full_score - @classmethod - def get_open_feedback(cls, user: Union[Tutor, Reviewer]) -> QuerySet: - """For a user, returns the feedback that is up for reassignment that - does not belong to the user. - Parameters - ---------- - user : User object - The user for which feedback should not be returned. Often the user - that is currently searching for a task someone else does not want - to do. +class SubscriptionEnded(Exception): + pass - Returns - ------- - QuerySet - All feedback objects that are open for reassignment that do not - belong to the user - """ - return cls.objects.filter( - Q(status=Feedback.OPEN) & - ~Q(of_tutor=user) # you shall not request your own feedback + +class AssignmentError(Exception): + pass + # message = ( + # f'User {self.owner} cannot create new assignment for' + # f'subscription {self} while a active submission exists') + + +class GeneralTaskSubscription(models.Model): + + RANDOM = 'random' + STUDENT_QUERY = 'student' + EXAM_TYPE_QUERY = 'exam' + SUBMISSION_TYPE_QUERY = 'submission_type' + + type_query_mapper = { + RANDOM: '__any', + STUDENT_QUERY: 'student__user__username', + EXAM_TYPE_QUERY: 'student__examtype__module_reference', + SUBMISSION_TYPE_QUERY: 'type__title', + } + + QUERY_CHOICE = ( + (RANDOM, 'Query for any submission'), + (STUDENT_QUERY, 'Query for submissions of student'), + (EXAM_TYPE_QUERY, 'Query for submissions of exam type'), + (SUBMISSION_TYPE_QUERY, 'Query for submissions of submissions_type'), + ) + + subscription_id = models.UUIDField(primary_key=True, + default=uuid.uuid4, + editable=False) + owner = models.ForeignKey(get_user_model(), + on_delete=models.CASCADE, + related_name='susbscriptions') + query_key = models.CharField(max_length=75, blank=True) + query_type = models.CharField(max_length=75, + choices=QUERY_CHOICE, + default=RANDOM) + + class Meta: + unique_together = ('owner', 'query_key', 'query_type') + + def _get_submission_base_query(self) -> QuerySet: + if self.query_type == self.RANDOM: + return Submission.objects.all() + + return Submission.objects.filter( + **{self.type_query_mapper[self.query_type]: self.query_key}) + + def _find_submission_with_active_assignment(self): + active_workassignment = self._get_submission_base_query().filter( + Q(assignment__isnull=False) & + Q(assignment__subscription__owner=self.owner) & + Q(assignment__active=True) ) - @classmethod - def tutor_unfinished_feedback(cls, user: Union[Tutor, Reviewer]): - """Gets only the feedback that is assigned and not accepted. A tutor - should have only one feedback assigned that is not accepted + log.debug(f'found active assignment: {active_workassignment})') - Parameters - ---------- - user : User object - The tutor who formed the request + return active_workassignment - Returns - ------- - The feedback or none if no feedback was assigned - """ - tutor_feedback = cls.objects.filter( - Q(of_tutor=user), Q(status=Feedback.EDITABLE), + def _find_reserved_submissions_of_user(self): + reserved_workassignments = self._get_submission_base_query().filter( + Q(assignment__isnull=False) & + Q(assignment__subscription__owner=self.owner) & + Q(feedback__isnull=True) ) - return tutor_feedback[0] if tutor_feedback else None - @classmethod - def tutor_assigned_feedback(cls, user: Union[Tutor, Reviewer]): - """Gets all feedback that is assigned to the tutor including - all status cases. + log.debug('reserved assignment %s', reserved_workassignments) - Returns - ------- - a QuerySet of tasks that have been assigned to this tutor + return reserved_workassignments - Parameters - ---------- - user : User object - The user for which the feedback should be returned - """ - tutor_feedback = cls.objects.filter(of_tutor=user) - return tutor_feedback + def _find_unassigned_non_final_submissions(self): + unassigned_non_final_submissions = \ + self._get_submission_base_query().filter( + Q(assignment__isnull=True) & + Q(feedback__isnull=True) + ) - def finalize_feedback(self, user: Union[Tutor, Reviewer]): - """Used to mark feedback as accepted (reviewed). + log.debug('unassigned notfinal submissions %s', + unassigned_non_final_submissions) - Parameters - ---------- - user : User object - The tutor/reviewer that marks some feedback as accepted - """ - self.status = Feedback.ACCEPTED - self.of_reviewer = user - self.save() + return unassigned_non_final_submissions - def reassign_to_tutor(self, user: Union[Tutor, Reviewer]): - """When a tutor does not want to correct some task they can pass it - along to another tutor who will accept the request. + def _find_unassigned_generated_non_final_submissions(self): + raise NotImplementedError - Parameters - ---------- - User object - The user to which to feedback should be assigned to - """ - assert self.status == Feedback.OPEN - self.of_tutor = user - self.status = Feedback.EDITABLE - self.save() + generated_not_final_submissions = \ + self._get_submission_base_query().filter( + Q(feedback__isnull=False) + ).annotate( + final=False + ) + + return generated_not_final_submissions + + def _get_next_assignment_in_subscription(self): + assignment_priority = ( + self._find_submission_with_active_assignment, + self._find_reserved_submissions_of_user, + self._find_unassigned_non_final_submissions, + self._find_unassigned_generated_non_final_submissions + ) + + lazy_queries = (query_set() for query_set in assignment_priority) + for query in (q for q in lazy_queries if len(q) > 0): + return query.first() + + raise SubscriptionEnded( + f'The task which user {self.owner} subscribed to is done') + + @transaction.atomic + def get_or_create_work_assignment(self): + task = self._get_next_assignment_in_subscription() + + return TutorSubmissionAssignment.objects.get_or_create( + subscription=self, + submission=task) + + +class TutorSubmissionAssignment(models.Model): + + assignment_id = models.UUIDField(primary_key=True, + default=uuid.uuid4, + editable=False) + submission = models.OneToOneField(Submission, + on_delete=models.CASCADE, + related_name='assignment') + subscription = models.ForeignKey(GeneralTaskSubscription, + on_delete=models.CASCADE, + related_name='assignments') + active = models.BooleanField(default=False) + + @transaction.atomic + def save(self, *args, **kwargs): + if self.active: + TutorSubmissionAssignment.objects \ + .filter(subscription__owner=self.subscription.owner, + active=True) \ + .update(active=False) + super().save(*args, **kwargs) + + @transaction.atomic + def start(self): + if not hasattr(self.submission, 'feedback'): + self.submission.feedback = Feedback() + self.submission.feedback.save() + + self.active = True + + @transaction.atomic + def finish(self): + # TODO think of constraints that forbin finishing + self.delete() + + @transaction.atomic + def hold(self): + self.active = False + + def __str__(self): + return (f'{self.assignee} assigned to {self.submission}' + f' (active={self.active})') diff --git a/core/permissions.py b/core/permissions.py index bb478558..211ed2fb 100644 --- a/core/permissions.py +++ b/core/permissions.py @@ -22,7 +22,7 @@ class IsUserGenericPermission(permissions.BasePermission): ) user = request.user - is_authorized = user.is_authenticated() and any(isinstance( + is_authorized = user.is_authenticated and any(isinstance( user.get_associated_user(), models) for models in self.models) if not is_authorized: diff --git a/core/serializers.py b/core/serializers.py index 44b91717..a33ba5cf 100644 --- a/core/serializers.py +++ b/core/serializers.py @@ -1,10 +1,12 @@ import logging +from django.contrib.auth import get_user_model from drf_dynamic_fields import DynamicFieldsMixin from rest_framework import serializers -from core.models import (ExamType, Feedback, Student, Submission, - SubmissionType, Tutor) +from core.models import (ExamType, Feedback, GeneralTaskSubscription, Student, + Submission, SubmissionType, Tutor, + TutorSubmissionAssignment) from util.factories import GradyUserFactory log = logging.getLogger(__name__) @@ -96,3 +98,61 @@ class TutorSerializer(DynamicFieldsModelSerializer): class Meta: model = Tutor fields = ('username', 'feedback_count') + + +class AssignmentSerializer(DynamicFieldsModelSerializer): + subscription_id = serializers.CharField() + active = serializers.ReadOnlyField() + + def create(self, validated_data): + subscription = GeneralTaskSubscription.objects.get( + subscription_id=validated_data['subscription_id']) + assignment, _ = subscription.get_or_create_work_assignment() + return assignment + + class Meta: + model = TutorSubmissionAssignment + fields = ('assignment_id', 'subscription_id', 'submission', 'active') + read_only_fields = ('submission',) + + +class SubscriptionSerializer(DynamicFieldsModelSerializer): + owner = serializers.CharField(source='owner.username') + query_key = serializers.CharField(required=False) + assignments = AssignmentSerializer(read_only=True, many=True) + + def validate(self, data): + if 'query_key' in data != \ + data['query_type'] == GeneralTaskSubscription.RANDOM: + raise serializers.ValidationError( + f'The {data["query_type"]} query_type does not work with the' + f'provided key') + + # Todo more validators especially regarding unique or + # mandatory key field + + return data + + def create(self, validated_data) -> GeneralTaskSubscription: + validated_data['owner'] = \ + get_user_model().objects.get( + username=validated_data['owner']['username']) + + return GeneralTaskSubscription.objects.create( + **validated_data) + + class Meta: + model = GeneralTaskSubscription + fields = ( + 'subscription_id', + 'owner', + 'query_type', + 'query_key', + 'assignments') + + # validators = [ + # UniqueTogetherValidator( + # queryset=model.objects.all(), + # fields=('owner', 'query_type', 'query_key') + # ) + # ] diff --git a/core/tests/test_factory_and_feedback.py b/core/tests/test_factory_and_feedback.py index 408ebb75..42ed4a4e 100644 --- a/core/tests/test_factory_and_feedback.py +++ b/core/tests/test_factory_and_feedback.py @@ -1,35 +1,9 @@ from django.test import TestCase -from core.models import (Feedback, Reviewer, Student, Submission, - SubmissionType, Tutor) +from core.models import Reviewer, Student, Tutor from util.factories import GradyUserFactory -class FeedbackTestCase(TestCase): - - factory = GradyUserFactory() - - def setUp(self): - self.tutor = self.factory.make_tutor() - self.student = self.factory.make_student() - - submission_type = SubmissionType.objects.create( - name='Cooking some crystal with Jesse') - Submission.objects.create(student=self.student, type=submission_type) - Submission.assign_tutor(self.tutor) - - def test_can_assign_tutor(self): - self.assertEqual(self.tutor.feedback_list.count(), 1) - - def test_feedback_origin_is_manual(self): - feedback = self.tutor.feedback_list.all()[0] - self.assertEqual(feedback.origin, Feedback.MANUAL) - - def test_feedback_status_is_editable(self): - feedback = self.tutor.feedback_list.all()[0] - self.assertEqual(feedback.status, Feedback.EDITABLE) - - class FactoryTestCase(TestCase): factory = GradyUserFactory() diff --git a/core/tests/test_functional_views.py b/core/tests/test_functional_views.py index f48645b2..5a99c8b9 100644 --- a/core/tests/test_functional_views.py +++ b/core/tests/test_functional_views.py @@ -1,6 +1,7 @@ from django.urls import reverse from rest_framework.test import (APIRequestFactory, APITestCase, force_authenticate) + from core.views import get_user_role from util.factories import GradyUserFactory diff --git a/core/tests/test_subscription_assignment_service.py b/core/tests/test_subscription_assignment_service.py new file mode 100644 index 00000000..0a713c0a --- /dev/null +++ b/core/tests/test_subscription_assignment_service.py @@ -0,0 +1,78 @@ + +from django.test import TestCase + +from core.models import (GeneralTaskSubscription, Submission, SubmissionType, + TutorSubmissionAssignment) +from util.factories import GradyUserFactory + + +class GeneralTaskSubscriptionRandomTest(TestCase): + + @classmethod + def setUpTestData(cls): + cls.user_factory = GradyUserFactory() + + def setUp(self): + self.t = self.user_factory.make_tutor() + self.s1 = self.user_factory.make_student() + self.s2 = self.user_factory.make_student() + self.subscription = GeneralTaskSubscription.objects.create( + owner=self.t.user, query_type=GeneralTaskSubscription.RANDOM) + + self.submission_type = SubmissionType.objects.create( + name='submission_01', full_score=14 + ) + + self.submission_01 = Submission.objects.create( + type=self.submission_type, student=self.s1, text='I really failed') + self.submission_02 = Submission.objects.create( + type=self.submission_type, student=self.s2, text='I like apples') + + self.work, _ = self.subscription.get_or_create_work_assignment() + + def test_first_work_assignment_was_created_inactive(self): + self.assertFalse(self.work.active) + + def test_cannot_get_new_assignment_when_previous_unfinished(self): + _, created = self.subscription.get_or_create_work_assignment() + self.assertFalse(created) + + +class AssignmentTest(TestCase): + + @classmethod + def setUpTestData(cls): + cls.user_factory = GradyUserFactory() + + def setUp(self): + self.t = self.user_factory.make_tutor() + self.s = self.user_factory.make_student() + + self.subscription = GeneralTaskSubscription.objects.create( + owner=self.t.user, query_type=GeneralTaskSubscription.RANDOM) + + self.submission_type = SubmissionType.objects.create( + name='submission_01', full_score=14 + ) + self.submission_01 = Submission.objects.create( + type=self.submission_type, student=self.s, text='I really failed') + + self.work, _ = self.subscription.get_or_create_work_assignment() + + def test_can_start_assignment(self): + self.work.start() + self.assertTrue(self.work.active) + + def test_feedback_was_created(self): + self.work.start() + self.assertTrue(hasattr(self.work.submission, 'feedback')) + + def test_stop(self): + self.work.start() + self.work.finish() + self.assertEqual(0, TutorSubmissionAssignment.objects.count()) + + def test_hold(self): + self.work.start() + self.work.hold() + self.assertFalse(self.work.active) diff --git a/core/urls.py b/core/urls.py index cb66e7d2..ab37362d 100644 --- a/core/urls.py +++ b/core/urls.py @@ -1,5 +1,5 @@ -from django.conf.urls import include, url from django.contrib.staticfiles.urls import staticfiles_urlpatterns +from django.urls import include, path from django.views.generic.base import TemplateView from rest_framework.routers import DefaultRouter from rest_framework_jwt.views import obtain_jwt_token, refresh_jwt_token @@ -8,26 +8,30 @@ from core import views # Create a router and register our viewsets with it. router = DefaultRouter() -router.register(r'student', views.StudentReviewerApiViewSet) -router.register(r'examtype', views.ExamApiViewSet) -router.register(r'submissiontype', views.SubmissionTypeApiView) -router.register(r'tutor', views.TutorApiViewSet) +router.register('student', views.StudentReviewerApiViewSet) +router.register('examtype', views.ExamApiViewSet) +router.register('submissiontype', views.SubmissionTypeApiView) +router.register('tutor', views.TutorApiViewSet) +router.register('subscription', views.SubscriptionApiViewSet) +router.register('assignment', views.AssignmentApiViewSet) # regular views that are not viewsets regular_views_urlpatterns = [ - url(r'student-page', views.StudentSelfApiView.as_view(), - name='student-page'), - url(r'user-role', views.get_user_role, name='user-role'), - url(r'jwt-time-delta', views.get_jwt_expiration_delta, - name='jwt-time-delta') + path('student-page', + views.StudentSelfApiView.as_view(), + name='student-page'), + path('user-role', views.get_user_role, name='user-role'), + path('jwt-time-delta', + views.get_jwt_expiration_delta, + name='jwt-time-delta') ] urlpatterns = [ - url(r'^api/', include(router.urls)), - url(r'^api/', include(regular_views_urlpatterns)), - url(r'^api-token-auth/', obtain_jwt_token), - url(r'^api-token-refresh', refresh_jwt_token), - url(r'^$', TemplateView.as_view(template_name='index.html')), + path('api/', include(router.urls)), + path('api/', include(regular_views_urlpatterns)), + path('api-token-auth/', obtain_jwt_token), + path('api-token-refresh/', refresh_jwt_token), + path('', TemplateView.as_view(template_name='index.html')), ] urlpatterns += staticfiles_urlpatterns() diff --git a/core/views.py b/core/views.py index 6d5a9595..7adbf78a 100644 --- a/core/views.py +++ b/core/views.py @@ -2,15 +2,17 @@ can be categorized by the permissions they require. All views require a user to be authenticated and most are only accessible by one user group """ from django.conf import settings -from rest_framework import mixins, viewsets, generics +from rest_framework import generics, mixins, viewsets from rest_framework.decorators import api_view from rest_framework.response import Response -from core.models import ExamType, Student, SubmissionType, Tutor +from core.models import (ExamType, GeneralTaskSubscription, Student, + SubmissionType, Tutor, TutorSubmissionAssignment) from core.permissions import IsReviewer, IsStudent -from core.serializers import (ExamSerializer, StudentSerializer, - StudentSerializerForListView, - SubmissionTypeSerializer, TutorSerializer) +from core.serializers import (AssignmentSerializer, ExamSerializer, + StudentSerializer, StudentSerializerForListView, + SubmissionTypeSerializer, SubscriptionSerializer, + TutorSerializer) @api_view() @@ -42,11 +44,12 @@ class ExamApiViewSet(viewsets.ReadOnlyModelViewSet): serializer_class = ExamSerializer -class TutorApiViewSet(mixins.RetrieveModelMixin, - mixins.CreateModelMixin, - mixins.DestroyModelMixin, - mixins.ListModelMixin, - viewsets.GenericViewSet): +class TutorApiViewSet( + mixins.RetrieveModelMixin, + mixins.CreateModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin, + viewsets.GenericViewSet): """ Api endpoint for creating, listing, viewing or deleteing tutors """ permission_classes = (IsReviewer,) queryset = Tutor.objects.all() @@ -70,3 +73,26 @@ class SubmissionTypeApiView(viewsets.ReadOnlyModelViewSet): """ Gets a list or a detail view of a single SubmissionType """ queryset = SubmissionType.objects.all() serializer_class = SubmissionTypeSerializer + + +class SubscriptionApiViewSet( + mixins.RetrieveModelMixin, + mixins.CreateModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin, + viewsets.GenericViewSet): + permission_classes = (IsReviewer,) + queryset = GeneralTaskSubscription.objects.all() + serializer_class = SubscriptionSerializer + + +class AssignmentApiViewSet( + mixins.CreateModelMixin, + mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, + mixins.DestroyModelMixin, + mixins.ListModelMixin, + viewsets.GenericViewSet): + permission_classes = (IsReviewer,) + queryset = TutorSubmissionAssignment.objects.all() + serializer_class = AssignmentSerializer diff --git a/grady/settings/default.py b/grady/settings/default.py index 066af8ec..3be7ad73 100644 --- a/grady/settings/default.py +++ b/grady/settings/default.py @@ -37,7 +37,6 @@ INSTALLED_APPS = [ 'django.contrib.sessions', 'django.contrib.messages', 'django.contrib.staticfiles', - 'django_extensions', 'rest_framework', 'corsheaders', 'drf_dynamic_fields', diff --git a/grady/urls.py b/grady/urls.py index 5a75e52c..6b2b980f 100644 --- a/grady/urls.py +++ b/grady/urls.py @@ -1,25 +1,9 @@ -"""grady URL Configuration - -The `urlpatterns` list routes URLs to views. For more information please see: - https://docs.djangoproject.com/en/1.10/topics/http/urls/ -Examples: -Function views - 1. Add an import: from my_app import views - 2. Add a URL to urlpatterns: url(r'^$', views.home, name='home') -Class-based views - 1. Add an import: from other_app.views import Home - 2. Add a URL to urlpatterns: url(r'^$', Home.as_view(), name='home') -Including another URLconf - 1. Import the include() function: from django.conf.urls import url, include - 2. Add a URL to urlpatterns: url(r'^blog/', include('blog.urls')) -""" -from django.conf.urls import include, url from django.contrib import admin +from django.urls import include, path urlpatterns = [ - url(r'^admin/', admin.site.urls), - url(r'^', include('core.urls')), - - url(r'^api-auth/', include('rest_framework.urls', - namespace='rest_framework')), + path('admin/', admin.site.urls), + path('api-auth/', include('rest_framework.urls', + namespace='rest_framework')), + path('', include('core.urls')), ] diff --git a/requirements.txt b/requirements.txt index 8c712f88..c6db510e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,9 +1,9 @@ django-cors-headers~=2.1.0 django-extensions~=1.7.7 djangorestframework-jwt~=1.11.0 -djangorestframework~=3.6.3 +djangorestframework~=3.7.7 drf-dynamic-fields~=0.2.0 -Django~=1.11.8 +Django~=2.0 gevent~=1.2.2 gunicorn~=19.7.0 psycopg2~=2.7.1 diff --git a/util/factories.py b/util/factories.py index 5b6f7157..6c9a2eb9 100644 --- a/util/factories.py +++ b/util/factories.py @@ -2,9 +2,9 @@ import configparser import secrets import string -from core.models import (Reviewer, Student, Tutor, ExamType, - SubmissionType, Submission, Feedback) from core.models import UserAccount as User +from core.models import (ExamType, Feedback, Reviewer, Student, Submission, + SubmissionType, Tutor) STUDENTS = 'students' TUTORS = 'tutors' @@ -35,22 +35,22 @@ def store_password(username, groupname, password): class GradyUserFactory: def __init__(self, - password_generator_func=get_random_password, + make_password=get_random_password, password_storge=store_password, *args, **kwargs): - self.password_generator_func = password_generator_func + self.make_password = make_password self.password_storge = password_storge - @staticmethod - def _get_random_name(prefix='', suffix='', k=1): - return ''.join((prefix, get_random_password(k), suffix)) + def _get_random_name(self, prefix='', suffix='', k=4): + return ''.join((prefix, self.make_password(k), suffix)) - def _make_base_user(self, username, groupname, password=None, store_pw=False, **kwargs): + def _make_base_user(self, username, groupname, password=None, + store_pw=False, **kwargs): """ This is a specific wrapper for the django update_or_create method of objects. * A new user is created and password and group are set accordingly - * If the user was there before password is NOT change but group is. A - user must only have one group. + * If the user was there before password is NOT change but group is. + * A user must only have one group. Returns: (User object, str): The user object that was added to the group and @@ -63,7 +63,7 @@ class GradyUserFactory: defaults=kwargs) if created: - password = self.password_generator_func() if password is None else password + password = self.make_password() if password is None else password user.set_password(password) user.save() @@ -94,7 +94,8 @@ class GradyUserFactory: return generic_user - def make_student(self, username=None, matrikel_no=None, exam=None, **kwargs): + def make_student(self, username=None, matrikel_no=None, + exam=None, **kwargs): """ Creates a student. Defaults can be passed via kwargs like in relation managers objects.update method. """ user = self._make_user_generic(username, STUDENTS, **kwargs) @@ -123,7 +124,7 @@ def make_exams(exams=[], **kwargs): def make_submission_types(submission_types=[], **kwargs): return [SubmissionType.objects.get_or_create( name=submission_type['name'], defaults=submission_type)[0] - for submission_type in submission_types] + for submission_type in submission_types] def make_students(students=[], **kwargs): @@ -216,12 +217,17 @@ def init_test_instance(): 'students': [{ 'username': 'student01', 'exam': 'Test Exam 01', + }, + { + 'username': 'student02', + 'exam': 'Test Exam 01', }], 'tutors': [{ 'username': 'tutor01' }], 'reviewers': [{ - 'username': 'reviewer01' + 'username': 'reviewer01', + 'password': 'pass' }], 'submissions': [ { @@ -269,5 +275,15 @@ def init_test_instance(): 'of_tutor': 'tutor01', }, }, + { + 'text': 'function blabl\n' + ' on multi lines\n' + ' for blabla in bla:\n' + ' arrrgh\n' + ' asasxasx\n' + ' lorem ipsum und so\n', + 'type': '03. This one exists for the sole purpose to test', + 'user': 'student02', + }, ]} ) -- GitLab