Skip to content
Snippets Groups Projects
Verified Commit 0d238a43 authored by Jan Maximilian Michal's avatar Jan Maximilian Michal
Browse files

Some API changes 

* assignments are always implicitly checked if feedback is created
* deactivated subscriptions are now visible
* creating subscription does not have side effects
* using the assignment endpoint to create subscriptions instead of
subscription endpoint
* (subscription refactorings)

Closes #93.
parent fcdccce0
No related branches found
No related tags found
No related merge requests found
Pipeline #
Hide whitespace changes
Inline Side-by-side
core/models.py
+ 23
− 31
View file @ 0d238a43
...@@ -424,6 +424,10 @@ class SubscriptionTemporarilyEnded(Exception): ...@@ -424,6 +424,10 @@ class SubscriptionTemporarilyEnded(Exception):
pass pass
class NotMoreThanTwoOpenAssignmentsAllowed(Exception):
pass
class SubmissionSubscription(models.Model): class SubmissionSubscription(models.Model):
RANDOM = 'random' RANDOM = 'random'
...@@ -502,7 +506,7 @@ class SubmissionSubscription(models.Model): ...@@ -502,7 +506,7 @@ class SubmissionSubscription(models.Model):
) )
) )
def _get_next_submission_in_subscription(self): def _get_available_submissions_in_subscription_stage(self):
candidates = self._get_submissions_that_do_not_have_final_feedback() candidates = self._get_submissions_that_do_not_have_final_feedback()
if candidates.count() == 0: if candidates.count() == 0:
...@@ -519,48 +523,36 @@ class SubmissionSubscription(models.Model): ...@@ -519,48 +523,36 @@ class SubmissionSubscription(models.Model):
'Currently unavailabe. Please check for more soon. ' 'Currently unavailabe. Please check for more soon. '
'Submissions remaining: %s' % stage_candiates.count()) 'Submissions remaining: %s' % stage_candiates.count())
return stage_candiates.first() return stage_candiates
@transaction.atomic @transaction.atomic
def get_or_create_work_assignment(self): def get_or_create_work_assignment(self):
task = self._get_next_submission_in_subscription() task = self._get_available_submissions_in_subscription_stage().first()
if self.assignments.filter(is_done=False).count() >= 2:
raise NotMoreThanTwoOpenAssignmentsAllowed(
'Not more than 2 active assignments allowed.')
return TutorSubmissionAssignment.objects.get_or_create( return TutorSubmissionAssignment.objects.get_or_create(
subscription=self, subscription=self,
submission=task)[0] submission=task)[0]
@transaction.atomic
def reserve_all_assignments_for_a_student(self): def reserve_all_assignments_for_a_student(self):
assert self.query_type == self.STUDENT_QUERY assert self.query_type == self.STUDENT_QUERY
try: submissions = self._get_submissions_that_do_not_have_final_feedback()
while True:
self.get_or_create_work_assignment()
except SubscriptionEnded as err:
log.info(f'Loaded all subscriptions of student {self.query_key}')
def _create_new_assignment_if_subscription_empty(self):
if self.assignments.filter(is_done=False).count() < 1:
self.get_or_create_work_assignment()
def _eagerly_reserve_the_next_assignment(self):
if self.assignments.filter(is_done=False).count() < 2:
self.get_or_create_work_assignment()
def get_oldest_unfinished_assignment(self):
self._create_new_assignment_if_subscription_empty()
return self.assignments \
.filter(is_done=False) \
.order_by('created') \
.first()
def get_youngest_unfinished_assignment(self):
self._create_new_assignment_if_subscription_empty()
self._eagerly_reserve_the_next_assignment()
return self.assignments \
.filter(is_done=False) \
.order_by('-created') \
.first()
for submission in submissions:
if hasattr(submission, 'assignments'):
submission.assignments.filter(is_done=False).delete()
TutorSubmissionAssignment.objects.create(
subscription=self,
submission=submission
)
log.info(f'Loaded all subscriptions of student {self.query_key}')
@transaction.atomic
def delete(self): def delete(self):
self.assignments.filter(is_done=False).delete() self.assignments.filter(is_done=False).delete()
if self.assignments.count() == 0: if self.assignments.count() == 0:
......
core/serializers/feedback.py
+ 23
− 19
View file @ 0d238a43
import logging import logging
from collections import defaultdict from collections import defaultdict
from django.core.exceptions import ObjectDoesNotExist
from django.db import transaction from django.db import transaction
from django.db.models.manager import Manager from django.db.models.manager import Manager
from rest_framework import serializers from rest_framework import serializers
from rest_framework.utils import html from rest_framework.utils import html
from core import models from core import models
from core.models import Feedback, TutorSubmissionAssignment from core.models import Feedback
from util.factories import GradyUserFactory from util.factories import GradyUserFactory
from .generic import DynamicFieldsModelSerializer from .generic import DynamicFieldsModelSerializer
...@@ -82,16 +81,16 @@ class FeedbackCommentSerializer(serializers.ModelSerializer): ...@@ -82,16 +81,16 @@ class FeedbackCommentSerializer(serializers.ModelSerializer):
class FeedbackSerializer(DynamicFieldsModelSerializer): class FeedbackSerializer(DynamicFieldsModelSerializer):
pk = serializers.ReadOnlyField(source='of_submission.pk')
assignment_pk = serializers.UUIDField(write_only=True)
feedback_lines = FeedbackCommentSerializer(many=True, required=False) feedback_lines = FeedbackCommentSerializer(many=True, required=False)
@transaction.atomic @transaction.atomic
def create(self, validated_data) -> Feedback: def create(self, validated_data) -> Feedback:
assignment = validated_data.pop('assignment_pk') submission = validated_data.pop('of_submission')
assignment = submission.assignments.get(
subscription__owner=self.context['request'].user)
feedback_lines = validated_data.pop('feedback_lines', []) feedback_lines = validated_data.pop('feedback_lines', [])
feedback = Feedback.objects.create(of_submission=assignment.submission, feedback = Feedback.objects.create(of_submission=submission,
**validated_data) **validated_data)
for comment in feedback_lines: for comment in feedback_lines:
...@@ -102,7 +101,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer): ...@@ -102,7 +101,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
) )
assignment.set_done() assignment.set_done()
return Feedback.objects.get(of_submission=assignment.submission) return Feedback.objects.get(of_submission=submission)
@transaction.atomic @transaction.atomic
def update(self, instance, validated_data): def update(self, instance, validated_data):
...@@ -115,21 +114,26 @@ class FeedbackSerializer(DynamicFieldsModelSerializer): ...@@ -115,21 +114,26 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
return super().update(instance, validated_data) return super().update(instance, validated_data)
def validate_assignment_pk(self, assignment_pk): def validate_of_submission(self, submission):
try: feedback = self.instance
assignment = TutorSubmissionAssignment.objects.get( if feedback is not None and feedback.submission is not submission:
pk=assignment_pk) raise serializers.ValidationError(
except ObjectDoesNotExist as err: 'It is not allowed to update this field.')
raise serializers.ValidationError('No assignment for given id.')
return assignment return submission
def validate(self, data): def validate(self, data):
log.debug("Validate feedback data: %s", data) if self.instance:
score = data.get('score') score = data.get('score', self.instance.score)
assignment = data.get('assignment_pk') submission = data.get('of_submission', self.instance.of_submission)
else:
try:
score = data.get('score')
submission = data.get('of_submission')
except KeyError as err:
raise serializers.ValidationError(
'You need a score and a submission.')
submission = assignment.submission
if not 0 <= score <= submission.type.full_score: if not 0 <= score <= submission.type.full_score:
raise serializers.ValidationError( raise serializers.ValidationError(
f'Score has to be in range [0..{submission.type.full_score}].') f'Score has to be in range [0..{submission.type.full_score}].')
...@@ -157,4 +161,4 @@ class FeedbackSerializer(DynamicFieldsModelSerializer): ...@@ -157,4 +161,4 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
class Meta: class Meta:
model = Feedback model = Feedback
fields = ('pk', 'assignment_pk', 'is_final', 'score', 'feedback_lines') fields = ('pk', 'of_submission', 'is_final', 'score', 'feedback_lines')
core/serializers/subscription.py
+ 12
− 3
View file @ 0d238a43
...@@ -6,12 +6,19 @@ from core.serializers import DynamicFieldsModelSerializer, FeedbackSerializer ...@@ -6,12 +6,19 @@ from core.serializers import DynamicFieldsModelSerializer, FeedbackSerializer
class AssignmentSerializer(DynamicFieldsModelSerializer): class AssignmentSerializer(DynamicFieldsModelSerializer):
submission_pk = serializers.ReadOnlyField( submission_pk = serializers.ReadOnlyField(source='submission.pk')
source='submission.pk')
class Meta: class Meta:
model = TutorSubmissionAssignment model = TutorSubmissionAssignment
fields = ('pk', 'submission_pk', 'is_done',) fields = ('pk', 'submission_pk', 'is_done', 'subscription',)
read_only_fields = ('is_done',)
extra_kwargs = {
'subscription': {'write_only': True},
}
def create(self, validated_data):
subscription = validated_data.get('subscription')
return subscription.get_or_create_work_assignment()
class SubmissionAssignmentSerializer(DynamicFieldsModelSerializer): class SubmissionAssignmentSerializer(DynamicFieldsModelSerializer):
...@@ -64,4 +71,6 @@ class SubscriptionSerializer(DynamicFieldsModelSerializer): ...@@ -64,4 +71,6 @@ class SubscriptionSerializer(DynamicFieldsModelSerializer):
'query_type', 'query_type',
'query_key', 'query_key',
'feedback_stage', 'feedback_stage',
'deactivated',
'assignments') 'assignments')
read_only_fields = ('deactivated',)
core/tests/test_feedback.py
+ 15
− 14
View file @ 0d238a43
...@@ -123,7 +123,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -123,7 +123,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': 10, 'score': 10,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.pk 'of_submission': self.assignment.submission.pk
} }
self.assertEqual(Feedback.objects.count(), 0) self.assertEqual(Feedback.objects.count(), 0)
...@@ -135,7 +135,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -135,7 +135,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': 101, 'score': 101,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.pk 'of_submission': self.assignment.submission.pk
} }
self.assertEqual(Feedback.objects.count(), 0) self.assertEqual(Feedback.objects.count(), 0)
response = self.client.post(self.url, data, format='json') response = self.client.post(self.url, data, format='json')
...@@ -146,7 +146,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -146,7 +146,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': 100, 'score': 100,
'is_final': True, 'is_final': True,
'assignment_pk': self.assignment.assignment_id 'of_submission': self.assignment.submission.pk
} }
response = self.client.post(self.url, data, format='json') response = self.client.post(self.url, data, format='json')
self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code) self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
...@@ -156,7 +156,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -156,7 +156,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': 50, 'score': 50,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.assignment_id 'of_submission': self.assignment.submission.pk
} }
response = self.client.post(self.url, data, format='json') response = self.client.post(self.url, data, format='json')
self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code) self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code)
...@@ -166,7 +166,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -166,7 +166,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': -1, 'score': -1,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.pk 'of_submission': self.assignment.submission.pk
} }
self.assertEqual(Feedback.objects.count(), 0) self.assertEqual(Feedback.objects.count(), 0)
response = self.client.post(self.url, data, format='json') response = self.client.post(self.url, data, format='json')
...@@ -177,7 +177,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -177,7 +177,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': 5, 'score': 5,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.pk, 'of_submission': self.assignment.submission.pk,
'feedback_lines': { 'feedback_lines': {
'4': { '4': {
'text': 'Why you no learn how to code, man?' 'text': 'Why you no learn how to code, man?'
...@@ -192,7 +192,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -192,7 +192,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': 0, 'score': 0,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.pk, 'of_submission': self.assignment.submission.pk,
'feedback_lines': { 'feedback_lines': {
'4': { '4': {
'text': 'Nice meth!' 'text': 'Nice meth!'
...@@ -208,7 +208,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -208,7 +208,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': 0, 'score': 0,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.pk, 'of_submission': self.assignment.submission.pk,
'feedback_lines': { 'feedback_lines': {
'3': { '3': {
'text': 'Nice meth!' 'text': 'Nice meth!'
...@@ -226,7 +226,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -226,7 +226,7 @@ class FeedbackCreateTestCase(APITestCase):
def test_cannot_create_without_assignment(self): def test_cannot_create_without_assignment(self):
data = { data = {
'score': 0, 'score': 0,
'assignment_pk': self.assignment.assignment_id, 'of_submission': self.assignment.submission.pk,
'feedback_lines': { 'feedback_lines': {
'2': { '2': {
'text': 'Well, at least you tried.' 'text': 'Well, at least you tried.'
...@@ -235,12 +235,12 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -235,12 +235,12 @@ class FeedbackCreateTestCase(APITestCase):
} }
self.assignment.delete() self.assignment.delete()
response = self.client.post(self.url, data, format='json') response = self.client.post(self.url, data, format='json')
self.assertEqual(status.HTTP_400_BAD_REQUEST, response.status_code) self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
def test_cannot_create_with_someoneelses_assignment(self): def test_cannot_create_with_someoneelses_assignment(self):
data = { data = {
'score': 0, 'score': 0,
'assignment_pk': self.assignment.assignment_id, 'of_submission': self.assignment.submission.pk,
'feedback_lines': { 'feedback_lines': {
'1': { '1': {
'text': 'Well, at least you tried.' 'text': 'Well, at least you tried.'
...@@ -256,7 +256,7 @@ class FeedbackCreateTestCase(APITestCase): ...@@ -256,7 +256,7 @@ class FeedbackCreateTestCase(APITestCase):
data = { data = {
'score': 0, 'score': 0,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.pk, 'of_submission': self.assignment.submission.pk,
'feedback_lines': { 'feedback_lines': {
'1': { '1': {
'text': 'Nice meth!' 'text': 'Nice meth!'
...@@ -325,13 +325,14 @@ class FeedbackPatchTestCase(APITestCase): ...@@ -325,13 +325,14 @@ class FeedbackPatchTestCase(APITestCase):
data = { data = {
'score': 35, 'score': 35,
'is_final': False, 'is_final': False,
'assignment_pk': self.assignment.assignment_id, 'of_submission': self.assignment.submission.pk,
'feedback_lines': { 'feedback_lines': {
'2': {'text': 'Very good.'}, '2': {'text': 'Very good.'},
} }
} }
response = self.client.post(self.burl, data, format='json') response = self.client.post(self.burl, data, format='json')
self.feedback = Feedback.objects.get(of_submission=response.data['pk']) self.feedback = Feedback.objects.get(
of_submission=response.data['of_submission'])
self.url = f'{self.burl}{self.feedback.of_submission.submission_id}/' self.url = f'{self.burl}{self.feedback.of_submission.submission_id}/'
def test_can_patch_onto_the_own_feedback(self): def test_can_patch_onto_the_own_feedback(self):
......
core/tests/test_subscription_assignment_service.py
+ 40
− 34
View file @ 0d238a43
...@@ -31,47 +31,42 @@ class SubmissionSubscriptionRandomTest(APITestCase): ...@@ -31,47 +31,42 @@ class SubmissionSubscriptionRandomTest(APITestCase):
owner=self.t, query_type=SubmissionSubscription.RANDOM) owner=self.t, query_type=SubmissionSubscription.RANDOM)
def test_subscription_gets_an_assignment(self): def test_subscription_gets_an_assignment(self):
self.subscription._create_new_assignment_if_subscription_empty() self.subscription.get_or_create_work_assignment()
self.assertEqual(1, self.subscription.assignments.count()) self.assertEqual(1, self.subscription.assignments.count())
def test_first_work_assignment_was_created_unfinished(self): def test_first_work_assignment_was_created_unfinished(self):
self.subscription._create_new_assignment_if_subscription_empty() self.subscription.get_or_create_work_assignment()
self.assertFalse(self.subscription.assignments.first().is_done) self.assertFalse(self.subscription.assignments.first().is_done)
def test_subscription_raises_error_when_depleted(self): def test_subscription_raises_error_when_depleted(self):
self.submission_01.delete() self.submission_01.delete()
self.submission_02.delete() self.submission_02.delete()
try: try:
self.subscription._create_new_assignment_if_subscription_empty() self.subscription.get_or_create_work_assignment()
except SubscriptionEnded as err: except SubscriptionEnded as err:
self.assertFalse(False) self.assertFalse(False)
else: else:
self.assertTrue(False) self.assertTrue(False)
def test_can_prefetch(self): def test_can_prefetch(self):
self.subscription._create_new_assignment_if_subscription_empty() self.subscription.get_or_create_work_assignment()
self.subscription._eagerly_reserve_the_next_assignment() self.subscription.get_or_create_work_assignment()
self.assertEqual(2, self.subscription.assignments.count()) self.assertEqual(2, self.subscription.assignments.count())
def test_oldest_assignment_is_current(self):
assignment = self.subscription.get_oldest_unfinished_assignment()
self.assertEqual(assignment,
self.subscription.get_oldest_unfinished_assignment())
def test_new_subscription_is_temporarily_unavailabe(self): def test_new_subscription_is_temporarily_unavailabe(self):
validation = SubmissionSubscription.objects.create( validation = SubmissionSubscription.objects.create(
owner=self.t, query_type=SubmissionSubscription.RANDOM, owner=self.t, query_type=SubmissionSubscription.RANDOM,
feedback_stage=SubmissionSubscription.FEEDBACK_VALIDATION) feedback_stage=SubmissionSubscription.FEEDBACK_VALIDATION)
try: try:
validation._create_new_assignment_if_subscription_empty() validation.get_or_create_work_assignment()
except SubscriptionTemporarilyEnded as err: except SubscriptionTemporarilyEnded as err:
self.assertTrue(True) self.assertTrue(True)
else: else:
self.assertTrue(False) self.assertTrue(False)
def test_delete_with_done_assignments_subscription_remains(self): def test_delete_with_done_assignments_subscription_remains(self):
first = self.subscription.get_oldest_unfinished_assignment() first = self.subscription.get_or_create_work_assignment()
self.subscription.get_youngest_unfinished_assignment() self.subscription.get_or_create_work_assignment()
self.assertEqual(2, self.subscription.assignments.count()) self.assertEqual(2, self.subscription.assignments.count())
first.is_done = True first.is_done = True
...@@ -84,7 +79,7 @@ class SubmissionSubscriptionRandomTest(APITestCase): ...@@ -84,7 +79,7 @@ class SubmissionSubscriptionRandomTest(APITestCase):
self.assertEqual(0, models.SubmissionSubscription.objects.count()) self.assertEqual(0, models.SubmissionSubscription.objects.count())
def test_assignment_delete_of_done_not_permitted(self): def test_assignment_delete_of_done_not_permitted(self):
first = self.subscription.get_oldest_unfinished_assignment() first = self.subscription.get_or_create_work_assignment()
first.is_done = True first.is_done = True
first.save() first.save()
...@@ -92,7 +87,7 @@ class SubmissionSubscriptionRandomTest(APITestCase): ...@@ -92,7 +87,7 @@ class SubmissionSubscriptionRandomTest(APITestCase):
first.delete) first.delete)
def test_assignment_delete_undone_permitted(self): def test_assignment_delete_undone_permitted(self):
first = self.subscription.get_oldest_unfinished_assignment() first = self.subscription.get_or_create_work_assignment()
first.delete() first.delete()
self.assertEqual(0, self.subscription.assignments.count()) self.assertEqual(0, self.subscription.assignments.count())
...@@ -196,24 +191,31 @@ class TestApiEndpoints(APITestCase): ...@@ -196,24 +191,31 @@ class TestApiEndpoints(APITestCase):
client = APIClient() client = APIClient()
client.force_authenticate(user=self.data['tutors'][0]) client.force_authenticate(user=self.data['tutors'][0])
response_subs = client.post( response_subscription_create = client.post(
'/api/subscription/', {'query_type': 'random'}) '/api/subscription/', {'query_type': 'random'})
subscription_id = response_subs.data['pk'] subscription_pk = response_subscription_create.data['pk']
subscription_pk = response_subscription_create.data['pk']
response_assignment = client.post(
f'/api/assignment/', {
'subscription': subscription_pk
})
response_assignment = client.get(
f'/api/subscription/{subscription_id}/assignments/current/')
assignment_pk = response_assignment.data['pk'] assignment_pk = response_assignment.data['pk']
response_subscription = client.get( response_subscription = client.get(
f'/api/subscription/{subscription_id}/') f'/api/subscription/{subscription_pk}/')
self.assertEqual(1, len(response_subscription.data['assignments'])) self.assertEqual(1, len(response_subscription.data['assignments']))
self.assertEqual(response_assignment.data['pk'], self.assertEqual(response_assignment.data['pk'],
response_subscription.data['assignments'][0]['pk']) response_subscription.data['assignments'][0]['pk'])
response_next = client.get( subscription_pk = response_subscription.data['pk']
f'/api/subscription/{subscription_id}/assignments/next/') response_next = client.post(
f'/api/assignment/', {
'subscription': subscription_pk
})
response_detail_subs = \ response_detail_subs = \
client.get(f'/api/subscription/{subscription_id}/') client.get(f'/api/subscription/{subscription_pk}/')
self.assertEqual(2, len(response_detail_subs.data['assignments'])) self.assertEqual(2, len(response_detail_subs.data['assignments']))
self.assertNotEqual(assignment_pk, response_next.data['pk']) self.assertNotEqual(assignment_pk, response_next.data['pk'])
...@@ -245,15 +247,17 @@ class TestApiEndpoints(APITestCase): ...@@ -245,15 +247,17 @@ class TestApiEndpoints(APITestCase):
'feedback_stage': 'feedback-creation' 'feedback_stage': 'feedback-creation'
}) })
self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(status.HTTP_201_CREATED, response.status_code)
response = client.get( subscription_pk = response.data['pk']
f'/api/subscription/{response.data["pk"]}/assignments/current/') response = client.post(
assignment_pk = response.data['pk'] f'/api/assignment/', {
'subscription': subscription_pk
})
response = client.post( response = client.post(
f'/api/feedback/', { f'/api/feedback/', {
"score": 23, "score": 23,
"assignment_pk": assignment_pk, "of_submission": response.data['submission']['pk'],
"feedback_lines": { "feedback_lines": {
2: {"text": "< some string >"}, 2: {"text": "< some string >"},
3: {"text": "< some string >"} 3: {"text": "< some string >"}
...@@ -261,7 +265,7 @@ class TestApiEndpoints(APITestCase): ...@@ -261,7 +265,7 @@ class TestApiEndpoints(APITestCase):
} }
) )
print(response, response.data) print(response, response.data)
self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(status.HTTP_201_CREATED, response.status_code)
# some other tutor reviews it # some other tutor reviews it
client.force_authenticate(user=self.data['tutors'][1]) client.force_authenticate(user=self.data['tutors'][1])
...@@ -272,13 +276,15 @@ class TestApiEndpoints(APITestCase): ...@@ -272,13 +276,15 @@ class TestApiEndpoints(APITestCase):
'feedback_stage': 'feedback-validation' 'feedback_stage': 'feedback-validation'
}) })
self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(status.HTTP_201_CREATED, response.status_code)
subscription_id = response.data['pk'] subscription_pk = response.data['pk']
response = client.get( response = client.post(
f'/api/subscription/{subscription_id}/assignments/current/') f'/api/assignment/', {
'subscription': subscription_pk
})
self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(status.HTTP_201_CREATED, response.status_code)
submission_id_in_database = models.Feedback.objects.filter( submission_id_in_database = models.Feedback.objects.filter(
is_final=False).first().of_submission.submission_id is_final=False).first().of_submission.submission_id
submission_id_in_response = \ submission_id_in_response = \
......
core/views/feedback.py
+ 24
− 21
View file @ 0d238a43
...@@ -24,9 +24,19 @@ class FeedbackApiView( ...@@ -24,9 +24,19 @@ class FeedbackApiView(
user_is_tutor = self.request.user.role == models.UserAccount.TUTOR user_is_tutor = self.request.user.role == models.UserAccount.TUTOR
return feedback_is_final and user_is_tutor 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): def _request_user_does_not_own_assignment(self, serializer):
assignment = serializer.validated_data['assignment_pk'] try:
return assignment.subscription.owner != self.request.user 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
def _tutor_attempts_to_set_first_feedback_final(self, serializer): def _tutor_attempts_to_set_first_feedback_final(self, serializer):
is_final_set = serializer.validated_data.get('is_final', False) is_final_set = serializer.validated_data.get('is_final', False)
...@@ -34,17 +44,19 @@ class FeedbackApiView( ...@@ -34,17 +44,19 @@ class FeedbackApiView(
return is_final_set and user_is_tutor return is_final_set and user_is_tutor
def _tutor_is_allowed_to_change_own_feedback(self, serializer): def _tutor_is_allowed_to_change_own_feedback(self, serializer):
assignment = serializer.validated_data['assignment_pk'] submission = self.get_object().of_submission
youngest = models.TutorSubmissionAssignment.objects\ assignment = self._get_implicit_assignment_for_user(submission)
.filter(submission=assignment.submission)\ youngest = models.TutorSubmissionAssignment.objects \
.order_by('-created')\ .filter(submission=submission) \
.order_by('-created') \
.first() .first()
return assignment == youngest return assignment == youngest
def _tutor_attempts_to_patch_first_feedback_final(self, serializer): def _tutor_attempts_to_patch_first_feedback_final(self, serializer):
is_final_set = serializer.validated_data.get('is_final', False) is_final_set = serializer.validated_data.get('is_final', False)
assignment = serializer.validated_data.get('assignment_pk') submission = self.get_object().of_submission
assignment = self._get_implicit_assignment_for_user(submission)
in_creation = assignment.subscription.feedback_stage == models.SubmissionSubscription.FEEDBACK_CREATION # noqa in_creation = assignment.subscription.feedback_stage == models.SubmissionSubscription.FEEDBACK_CREATION # noqa
return is_final_set and in_creation return is_final_set and in_creation
...@@ -54,12 +66,12 @@ class FeedbackApiView( ...@@ -54,12 +66,12 @@ class FeedbackApiView(
if self._tutor_attempts_to_set_first_feedback_final(serializer): if self._tutor_attempts_to_set_first_feedback_final(serializer):
return Response( return Response(
{'It is not allowed to create feedback final for tutors'}, {'For tutors it is not allowed to create feedback final.'},
status=status.HTTP_403_FORBIDDEN) status=status.HTTP_403_FORBIDDEN)
if self._request_user_does_not_own_assignment(serializer): if self._request_user_does_not_own_assignment(serializer):
return Response( return Response(
{'This user has not permission to create this feedback'}, {'This user has no permission to create this feedback'},
status=status.HTTP_403_FORBIDDEN) status=status.HTTP_403_FORBIDDEN)
self.perform_create(serializer) self.perform_create(serializer)
...@@ -68,22 +80,13 @@ class FeedbackApiView( ...@@ -68,22 +80,13 @@ class FeedbackApiView(
def partial_update(self, request, **kwargs): def partial_update(self, request, **kwargs):
feedback = self.get_object() feedback = self.get_object()
assignment = models.TutorSubmissionAssignment.objects.get( serializer = self.get_serializer(feedback, data=request.data,
subscription__owner=request.user, partial=True)
submission=feedback.of_submission
)
serializer = self.get_serializer(
feedback,
data={'assignment_pk': assignment.pk,
'score': feedback.score,
**request.data},
partial=True)
serializer.is_valid(raise_exception=True) serializer.is_valid(raise_exception=True)
if self._tutor_attempts_to_change_final_feedback(serializer): if self._tutor_attempts_to_change_final_feedback(serializer):
return Response( return Response(
{'Cannot set the first feedback final unless user reviewer'}, {"Changing final feedback is not allowed"},
status=status.HTTP_403_FORBIDDEN) status=status.HTTP_403_FORBIDDEN)
if self._tutor_attempts_to_patch_first_feedback_final(serializer): if self._tutor_attempts_to_patch_first_feedback_final(serializer):
......
core/views/subscription.py
+ 22
− 29
View file @ 0d238a43
...@@ -2,13 +2,12 @@ import logging ...@@ -2,13 +2,12 @@ import logging
from django.core.exceptions import ObjectDoesNotExist from django.core.exceptions import ObjectDoesNotExist
from rest_framework import mixins, status, viewsets from rest_framework import mixins, status, viewsets
from rest_framework.decorators import detail_route
from rest_framework.response import Response from rest_framework.response import Response
from core import models, permissions, serializers from core import models, permissions, serializers
from core.models import TutorSubmissionAssignment from core.models import TutorSubmissionAssignment
from core.permissions import IsTutorOrReviewer from core.permissions import IsTutorOrReviewer
from core.serializers import AssignmentSerializer from core.serializers import AssignmentDetailSerializer, AssignmentSerializer
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -22,35 +21,9 @@ class SubscriptionApiViewSet( ...@@ -22,35 +21,9 @@ class SubscriptionApiViewSet(
permission_classes = (permissions.IsTutorOrReviewer,) permission_classes = (permissions.IsTutorOrReviewer,)
serializer_class = serializers.SubscriptionSerializer serializer_class = serializers.SubscriptionSerializer
def fetch_assignment(self, request, assignment_fetcher):
try:
assignment = assignment_fetcher()
except models.SubscriptionEnded as err:
return Response({'Error': str(err)},
status=status.HTTP_410_GONE)
except models.SubscriptionTemporarilyEnded as err:
return Response({'Error': str(err)},
status=status.HTTP_404_NOT_FOUND)
serializer = serializers.AssignmentDetailSerializer(assignment)
return Response(serializer.data)
@detail_route(methods=['get'], url_path='assignments/current')
def current_assignment(self, request, pk=None):
subscription = self.get_object()
return self.fetch_assignment(
request, subscription.get_oldest_unfinished_assignment)
@detail_route(methods=['get'], url_path='assignments/next')
def next_assignment(self, request, pk=None):
subscription = self.get_object()
return self.fetch_assignment(
request, subscription.get_youngest_unfinished_assignment)
def get_queryset(self): def get_queryset(self):
return models.SubmissionSubscription.objects.filter( return models.SubmissionSubscription.objects.filter(
owner=self.request.user, deactivated=False) owner=self.request.user)
def _get_subscription_if_type_exists(self, data): def _get_subscription_if_type_exists(self, data):
try: try:
...@@ -96,6 +69,21 @@ class AssignmentApiViewSet( ...@@ -96,6 +69,21 @@ class AssignmentApiViewSet(
queryset = TutorSubmissionAssignment.objects.all() queryset = TutorSubmissionAssignment.objects.all()
serializer_class = AssignmentSerializer serializer_class = AssignmentSerializer
def _fetch_assignment(self, serializer):
try:
serializer.save()
except models.SubscriptionEnded as err:
return Response({'Error': str(err)},
status=status.HTTP_410_GONE)
except models.SubscriptionTemporarilyEnded as err:
return Response({'Error': str(err)},
status=status.HTTP_404_NOT_FOUND)
except models.NotMoreThanTwoOpenAssignmentsAllowed as err:
return Response({'Error': str(err)},
status=status.HTTP_403_FORBIDDEN)
serializer = AssignmentDetailSerializer(serializer.instance)
return Response(serializer.data, status=status.HTTP_201_CREATED)
def get_queryset(self): def get_queryset(self):
""" Get only assignments of that user """ """ Get only assignments of that user """
return TutorSubmissionAssignment.objects.filter( return TutorSubmissionAssignment.objects.filter(
...@@ -110,3 +98,8 @@ class AssignmentApiViewSet( ...@@ -110,3 +98,8 @@ class AssignmentApiViewSet(
instance.delete() instance.delete()
return Response(status=status.HTTP_204_NO_CONTENT) return Response(status=status.HTTP_204_NO_CONTENT)
def create(self, request, *args, **kwargs):
serializer = self.get_serializer(data=request.data)
serializer.is_valid(raise_exception=True)
return self._fetch_assignment(serializer)
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment