From d62e564c235449c7f820b7b07a3419a32db299db Mon Sep 17 00:00:00 2001
From: janmax <j.michal@stud.uni-goettingen.de>
Date: Sat, 10 Feb 2018 18:40:50 +0100
Subject: [PATCH] Migrated everything to UUID fields for primary keys
* 'submission_pk' -> 'submission' on AssignmentSerializer
* subscription now uniformly use the private key of a model
that they want to receive submissions from
* introduced remaining and available fields on subscription
* query key and type are now checked
---
core/migrations/0001_initial.py | 13 ++---
...ctivated.py => 0002_auto_20180210_1727.py} | 8 +--
core/models.py | 34 +++++++++---
core/permissions.py | 5 +-
core/serializers/feedback.py | 2 +-
core/serializers/generic.py | 2 +-
core/serializers/subscription.py | 54 ++++++++++++-------
core/tests/test_feedback.py | 4 +-
core/tests/test_student_page.py | 4 +-
core/tests/test_submissiontypeview.py | 2 +-
.../test_subscription_assignment_service.py | 18 +++++++
core/views/feedback.py | 27 ++++------
core/views/subscription.py | 2 +-
13 files changed, 114 insertions(+), 61 deletions(-)
rename core/migrations/{0002_submissionsubscription_deactivated.py => 0002_auto_20180210_1727.py} (58%)
diff --git a/core/migrations/0001_initial.py b/core/migrations/0001_initial.py
index 7f984f22..1f103389 100644
--- a/core/migrations/0001_initial.py
+++ b/core/migrations/0001_initial.py
@@ -1,4 +1,4 @@
-# Generated by Django 2.0.2 on 2018-02-08 15:11
+# Generated by Django 2.0.2 on 2018-02-10 17:00
import uuid
@@ -24,7 +24,6 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name='UserAccount',
fields=[
- ('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')),
('is_superuser', models.BooleanField(default=False, help_text='Designates that this user has all permissions without explicitly assigning them.', verbose_name='superuser status')),
@@ -35,6 +34,7 @@ class Migration(migrations.Migration):
('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')),
+ ('user_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
('fullname', models.CharField(blank=True, max_length=70, verbose_name='full name')),
('is_admin', models.BooleanField(default=False)),
('role', models.CharField(choices=[('Student', 'student'), ('Tutor', 'tutor'), ('Reviewer', 'reviewer')], max_length=50)),
@@ -53,7 +53,7 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name='ExamType',
fields=[
- ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+ ('exam_type_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
('module_reference', models.CharField(max_length=50, unique=True)),
('total_score', models.PositiveIntegerField()),
('pass_score', models.PositiveIntegerField()),
@@ -127,8 +127,9 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name='SubmissionSubscription',
fields=[
+ ('deactivated', models.BooleanField(default=False)),
('subscription_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
- ('query_key', models.CharField(blank=True, max_length=75)),
+ ('query_key', models.UUIDField(blank=True)),
('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)),
('feedback_stage', models.CharField(choices=[('feedback-creation', 'No feedback was ever assigned'), ('feedback-validation', 'Feedback exists but is not validated'), ('feedback-conflict-resolution', 'Previous correctors disagree')], default='feedback-creation', max_length=40)),
('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='subscriptions', to=settings.AUTH_USER_MODEL)),
@@ -137,7 +138,7 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name='SubmissionType',
fields=[
- ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+ ('submission_type_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
('name', models.CharField(max_length=50, unique=True)),
('full_score', models.PositiveIntegerField(default=0)),
('description', models.TextField()),
@@ -151,7 +152,7 @@ class Migration(migrations.Migration):
migrations.CreateModel(
name='Test',
fields=[
- ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+ ('test_id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)),
('name', models.CharField(max_length=30)),
('label', models.CharField(max_length=50)),
('annotation', models.TextField()),
diff --git a/core/migrations/0002_submissionsubscription_deactivated.py b/core/migrations/0002_auto_20180210_1727.py
similarity index 58%
rename from core/migrations/0002_submissionsubscription_deactivated.py
rename to core/migrations/0002_auto_20180210_1727.py
index d10cbe69..6efb9d34 100644
--- a/core/migrations/0002_submissionsubscription_deactivated.py
+++ b/core/migrations/0002_auto_20180210_1727.py
@@ -1,4 +1,4 @@
-# Generated by Django 2.0.2 on 2018-02-08 15:50
+# Generated by Django 2.0.2 on 2018-02-10 17:27
from django.db import migrations, models
@@ -10,9 +10,9 @@ class Migration(migrations.Migration):
]
operations = [
- migrations.AddField(
+ migrations.AlterField(
model_name='submissionsubscription',
- name='deactivated',
- field=models.BooleanField(default=False),
+ name='query_key',
+ field=models.UUIDField(null=True),
),
]
diff --git a/core/models.py b/core/models.py
index 77f8fcc0..d10cc0d2 100644
--- a/core/models.py
+++ b/core/models.py
@@ -70,6 +70,9 @@ class ExamType(models.Model):
def __str__(self) -> str:
return self.module_reference
+ exam_type_id = models.UUIDField(primary_key=True,
+ default=uuid.uuid4,
+ editable=False)
module_reference = models.CharField(max_length=50, unique=True)
total_score = models.PositiveIntegerField()
pass_score = models.PositiveIntegerField()
@@ -95,6 +98,9 @@ class SubmissionType(models.Model):
solution : TextField
A sample solution or a correction guideline
"""
+ submission_type_id = models.UUIDField(primary_key=True,
+ default=uuid.uuid4,
+ editable=False)
name = models.CharField(max_length=50, unique=True)
full_score = models.PositiveIntegerField(default=0)
description = models.TextField()
@@ -146,6 +152,10 @@ class UserAccount(AbstractUser):
Username and password are required. Other fields are optional.
"""
+ user_id = models.UUIDField(primary_key=True,
+ default=uuid.uuid4,
+ editable=False)
+
fullname = models.CharField('full name', max_length=70, blank=True)
is_admin = models.BooleanField(default=False)
@@ -295,6 +305,10 @@ class Test(models.Model):
submission : ForeignKey
The submission the tests where unapproved on
"""
+ test_id = models.UUIDField(primary_key=True,
+ default=uuid.uuid4,
+ editable=False)
+
name = models.CharField(max_length=30)
label = models.CharField(max_length=50)
annotation = models.TextField()
@@ -437,9 +451,9 @@ class SubmissionSubscription(models.Model):
type_query_mapper = {
RANDOM: '__any',
- STUDENT_QUERY: 'student__student_id',
- EXAM_TYPE_QUERY: 'student__exam__module_reference',
- SUBMISSION_TYPE_QUERY: 'type__name',
+ STUDENT_QUERY: 'student__pk',
+ EXAM_TYPE_QUERY: 'student__exam__pk',
+ SUBMISSION_TYPE_QUERY: 'type__pk',
}
QUERY_CHOICE = (
@@ -472,7 +486,7 @@ class SubmissionSubscription(models.Model):
owner = models.ForeignKey(get_user_model(),
on_delete=models.CASCADE,
related_name='subscriptions')
- query_key = models.CharField(max_length=75, blank=True)
+ query_key = models.UUIDField(null=True)
query_type = models.CharField(max_length=75,
choices=QUERY_CHOICE,
default=RANDOM)
@@ -495,8 +509,7 @@ class SubmissionSubscription(models.Model):
def _get_submissions_that_do_not_have_final_feedback(self):
return self._get_submission_base_query().exclude(
- Q(feedback__isnull=False),
- Q(feedback__is_final=True)
+ Q(feedback__isnull=False) & Q(feedback__is_final=True)
).exclude(
Q(assignments__subscription__owner=self.owner)
).annotate(
@@ -525,6 +538,15 @@ class SubmissionSubscription(models.Model):
return stage_candiates
+ def get_remaining_not_final(self):
+ return self._get_submissions_that_do_not_have_final_feedback().count()
+
+ def get_available_in_stage(self):
+ try:
+ return self._get_available_submissions_in_subscription_stage().count() # noqa
+ except (SubscriptionTemporarilyEnded, SubscriptionEnded) as err:
+ return 0
+
@transaction.atomic
def get_or_create_work_assignment(self):
task = self._get_available_submissions_in_subscription_stage().first()
diff --git a/core/permissions.py b/core/permissions.py
index 21299ccc..e52f68a2 100644
--- a/core/permissions.py
+++ b/core/permissions.py
@@ -20,13 +20,14 @@ class IsUserRoleGenericPermission(permissions.BasePermission):
)
user = request.user
- is_authorized = user.is_authenticated and user.role in self.roles
+ is_authorized = user.is_superuser or (user.is_authenticated and
+ user.role in self.roles)
if not is_authorized:
log.warn('User "%s" has no permission to view %s',
user.username, view.__class__.__name__)
- return is_authorized or user.is_superuser
+ return is_authorized
class IsStudent(IsUserRoleGenericPermission):
diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py
index b57b21a6..28e3eec7 100644
--- a/core/serializers/feedback.py
+++ b/core/serializers/feedback.py
@@ -116,7 +116,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
def validate_of_submission(self, submission):
feedback = self.instance
- if feedback is not None and feedback.submission is not submission:
+ if feedback is not None and feedback.of_submission.pk != submission.pk:
raise serializers.ValidationError(
'It is not allowed to update this field.')
diff --git a/core/serializers/generic.py b/core/serializers/generic.py
index 20edc3c6..ce377fdd 100644
--- a/core/serializers/generic.py
+++ b/core/serializers/generic.py
@@ -15,6 +15,6 @@ class DynamicFieldsModelSerializer(DynamicFieldsMixin,
if fields is not None:
# Drop any fields that are not specified in the `fields` argument.
allowed = set(fields)
- existing = set(self.fields.keys())
+ existing = set(self.fields)
for field_name in existing - allowed:
self.fields.pop(field_name)
diff --git a/core/serializers/subscription.py b/core/serializers/subscription.py
index ef9a63c8..8bb83418 100644
--- a/core/serializers/subscription.py
+++ b/core/serializers/subscription.py
@@ -5,13 +5,22 @@ from core.models import (Submission, SubmissionSubscription,
from core.serializers import DynamicFieldsModelSerializer, FeedbackSerializer
+class SubmissionAssignmentSerializer(DynamicFieldsModelSerializer):
+ text = serializers.ReadOnlyField()
+ type_pk = serializers.ReadOnlyField(source='type.pk')
+ full_score = serializers.ReadOnlyField(source='type.full_score')
+
+ class Meta:
+ model = Submission
+ fields = ('pk', 'type_pk', 'text', 'full_score')
+
+
class AssignmentSerializer(DynamicFieldsModelSerializer):
- submission_pk = serializers.ReadOnlyField(source='submission.pk')
class Meta:
model = TutorSubmissionAssignment
- fields = ('pk', 'submission_pk', 'is_done', 'subscription',)
- read_only_fields = ('is_done',)
+ fields = ('pk', 'submission', 'is_done', 'subscription',)
+ read_only_fields = ('is_done', 'submission')
extra_kwargs = {
'subscription': {'write_only': True},
}
@@ -21,29 +30,27 @@ class AssignmentSerializer(DynamicFieldsModelSerializer):
return subscription.get_or_create_work_assignment()
-class SubmissionAssignmentSerializer(DynamicFieldsModelSerializer):
- text = serializers.ReadOnlyField()
- type_pk = serializers.ReadOnlyField(source='type.pk')
- full_score = serializers.ReadOnlyField(source='type.full_score')
-
- class Meta:
- model = Submission
- fields = ('pk', 'type_pk', 'text', 'full_score')
-
-
-class AssignmentDetailSerializer(DynamicFieldsModelSerializer):
- submission = SubmissionAssignmentSerializer()
+class AssignmentDetailSerializer(AssignmentSerializer):
feedback = FeedbackSerializer(source='submission.feedback')
+ submission = SubmissionAssignmentSerializer()
class Meta:
model = TutorSubmissionAssignment
- fields = ('pk', 'feedback', 'submission', 'is_done',)
+ fields = ('pk', 'submission', 'feedback', 'is_done',)
class SubscriptionSerializer(DynamicFieldsModelSerializer):
owner = serializers.ReadOnlyField(source='owner.username')
- query_key = serializers.CharField(required=False)
+ query_key = serializers.UUIDField(required=False)
assignments = AssignmentSerializer(read_only=True, many=True)
+ remaining = serializers.SerializerMethodField()
+ available = serializers.SerializerMethodField()
+
+ def get_remaining(self, subscription):
+ return subscription.get_remaining_not_final()
+
+ def get_available(self, subscription):
+ return subscription.get_available_in_stage()
def validate(self, data):
data['owner'] = self.context['request'].user
@@ -54,6 +61,15 @@ class SubscriptionSerializer(DynamicFieldsModelSerializer):
f'The {data["query_type"]} query_type does not work with the'
f'provided key')
+ elif 'query_key' in data:
+ query_type = data.get('query_type')
+ query_key = data.get('query_key')
+
+ select = SubmissionSubscription.type_query_mapper[query_type]
+ if Submission.objects.filter(**{select: query_key}).count() == 0:
+ raise serializers.ValidationError(
+ 'Seems no submissions exist for given query and key')
+
return data
def create(self, validated_data) -> SubmissionSubscription:
@@ -72,5 +88,7 @@ class SubscriptionSerializer(DynamicFieldsModelSerializer):
'query_key',
'feedback_stage',
'deactivated',
- 'assignments')
+ 'assignments',
+ 'remaining',
+ 'available')
read_only_fields = ('deactivated',)
diff --git a/core/tests/test_feedback.py b/core/tests/test_feedback.py
index 8397faf8..8b377de2 100644
--- a/core/tests/test_feedback.py
+++ b/core/tests/test_feedback.py
@@ -111,8 +111,7 @@ class FeedbackCreateTestCase(APITestCase):
self.client.force_authenticate(user=self.tutor)
self.subscription = models.SubmissionSubscription.objects.create(
owner=self.tutor,
- query_type='random',
- query_key=''
+ query_type='random'
)
self.assignment = self.subscription.get_or_create_work_assignment()
@@ -319,7 +318,6 @@ class FeedbackPatchTestCase(APITestCase):
self.subscription = models.SubmissionSubscription.objects.create(
owner=self.tutor01,
query_type='random',
- query_key=''
)
self.assignment = self.subscription.get_or_create_work_assignment()
data = {
diff --git a/core/tests/test_student_page.py b/core/tests/test_student_page.py
index 52d09daf..c8a1365b 100644
--- a/core/tests/test_student_page.py
+++ b/core/tests/test_student_page.py
@@ -105,7 +105,7 @@ class StudentPageTests(APITestCase):
def test_a_student_submissions_contains_type_id(self):
self.assertEqual(
self.submission_list_first_entry['type']['pk'],
- self.student_info.submissions.first().type.id)
+ str(self.student_info.submissions.first().type.pk))
def test_submission_data_contains_full_score(self):
self.assertEqual(
@@ -183,7 +183,7 @@ class StudentSelfSubmissionsTests(APITestCase):
def test_a_student_submissions_contains_type_id(self):
self.assertEqual(
self.submission_list_first_entry['type']['pk'],
- self.student_info.submissions.first().type.pk)
+ str(self.student_info.submissions.first().type.pk))
def test_submission_data_contains_full_score(self):
self.assertEqual(
diff --git a/core/tests/test_submissiontypeview.py b/core/tests/test_submissiontypeview.py
index cbb68f07..3973d7de 100644
--- a/core/tests/test_submissiontypeview.py
+++ b/core/tests/test_submissiontypeview.py
@@ -62,7 +62,7 @@ class SubmissionTypeViewTestRetrieve(APITestCase):
self.assertEqual(self.response.status_code, status.HTTP_200_OK)
def test_get_id(self):
- self.assertEqual(self.pk, self.response.data['pk'])
+ self.assertEqual(str(self.pk), self.response.data['pk'])
def test_get_sumbission_type_name(self):
self.assertEqual('Hard question', self.response.data['name'])
diff --git a/core/tests/test_subscription_assignment_service.py b/core/tests/test_subscription_assignment_service.py
index 24721706..1f4b2651 100644
--- a/core/tests/test_subscription_assignment_service.py
+++ b/core/tests/test_subscription_assignment_service.py
@@ -171,6 +171,24 @@ class TestApiEndpoints(APITestCase):
]}
)
+ def test_remaining_submissions(self):
+ client = APIClient()
+ client.force_authenticate(user=self.data['tutors'][0])
+
+ response = client.post('/api/subscription/', {'query_type': 'random'})
+
+ self.assertEqual(3, response.data['remaining'])
+
+ def test_available_submissions_in_stage(self):
+ client = APIClient()
+ client.force_authenticate(user=self.data['tutors'][0])
+
+ response = client.post('/api/subscription/',
+ {'query_type': 'random',
+ 'feedback_stage': 'feedback-validation'})
+
+ self.assertEqual(0, response.data['available'])
+
def test_can_create_a_subscription(self):
client = APIClient()
client.force_authenticate(user=self.data['tutors'][0])
diff --git a/core/views/feedback.py b/core/views/feedback.py
index f0af99b1..2b1f4b94 100644
--- a/core/views/feedback.py
+++ b/core/views/feedback.py
@@ -1,6 +1,7 @@
import logging
from rest_framework import mixins, status, viewsets
+from rest_framework.exceptions import PermissionDenied
from rest_framework.response import Response
from core import models, permissions, serializers
@@ -25,18 +26,14 @@ class FeedbackApiView(
return feedback_is_final and user_is_tutor
def _get_implicit_assignment_for_user(self, submission):
- return models.TutorSubmissionAssignment.objects.get(
- subscription__owner=self.request.user,
- submission=submission
- )
-
- def _request_user_does_not_own_assignment(self, serializer):
try:
- submission = serializer.validated_data['of_submission']
- assignment = self._get_implicit_assignment_for_user(submission)
- return assignment.subscription.owner != self.request.user
- except models.TutorSubmissionAssignment.DoesNotExist as err:
- return True
+ return models.TutorSubmissionAssignment.objects.get(
+ subscription__owner=self.request.user,
+ submission=submission
+ )
+ except models.TutorSubmissionAssignment.DoesNotExist:
+ raise PermissionDenied(
+ detail='This user has no permission to create this feedback')
def _tutor_attempts_to_set_first_feedback_final(self, serializer):
is_final_set = serializer.validated_data.get('is_final', False)
@@ -64,16 +61,14 @@ class FeedbackApiView(
serializer = self.get_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
+ self._get_implicit_assignment_for_user(
+ serializer.validated_data['of_submission'])
+
if self._tutor_attempts_to_set_first_feedback_final(serializer):
return Response(
{'For tutors it is not allowed to create feedback final.'},
status=status.HTTP_403_FORBIDDEN)
- if self._request_user_does_not_own_assignment(serializer):
- return Response(
- {'This user has no permission to create this feedback'},
- status=status.HTTP_403_FORBIDDEN)
-
self.perform_create(serializer)
return Response(serializer.data,
status=status.HTTP_201_CREATED)
diff --git a/core/views/subscription.py b/core/views/subscription.py
index 66908d69..0797e7e4 100644
--- a/core/views/subscription.py
+++ b/core/views/subscription.py
@@ -30,7 +30,7 @@ class SubscriptionApiViewSet(
return models.SubmissionSubscription.objects.get(
owner=self.request.user,
query_type=data.get('query_type', ''),
- query_key=data.get('query_key', ''),
+ query_key=data.get('query_key'),
feedback_stage=data.get(
'feedback_stage',
models.SubmissionSubscription.FEEDBACK_CREATION)
--
GitLab