From b3d216eadeb787b9a3e5c557b4b18e1a1cd35a4a Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Tue, 6 Aug 2019 15:43:27 +0200
Subject: [PATCH 1/8] Closes #155
---
core/models/assignment.py | 18 +++++
core/models/submission.py | 2 +
core/serializers/feedback.py | 3 +-
core/signals.py | 10 ---
core/tests/test_custom_subscription_filter.py | 5 ++
core/tests/test_feedback.py | 35 +++++-----
.../test_subscription_assignment_service.py | 7 +-
core/tests/test_tutor_api_endpoints.py | 1 +
core/urls.py | 1 +
core/views/feedback.py | 68 +++++--------------
core/views/subscription.py | 43 ++++++++++--
core/views/util.py | 33 +++++++++
frontend/src/api.ts | 18 +++--
.../src/store/modules/submission-notes.ts | 17 +++--
functional_tests/test_export_modal.py | 2 +-
15 files changed, 160 insertions(+), 103 deletions(-)
create mode 100644 core/views/util.py
diff --git a/core/models/assignment.py b/core/models/assignment.py
index fc8cfa17..77967461 100644
--- a/core/models/assignment.py
+++ b/core/models/assignment.py
@@ -14,6 +14,10 @@ class DeletionOfDoneAssignmentsNotPermitted(Exception):
pass
+class CanOnlyCallFinishOnUnfinishedAssignments(Exception):
+ pass
+
+
class TutorSubmissionAssignment(models.Model):
assignment_id = models.UUIDField(primary_key=True,
@@ -32,6 +36,20 @@ class TutorSubmissionAssignment(models.Model):
return (f'{self.subscription.owner} assigned to {self.submission}'
f' (done={self.is_done})')
+ def finish(self):
+ self.refresh_from_db()
+ if self.is_done:
+ raise CanOnlyCallFinishOnUnfinishedAssignments()
+
+ meta = self.submission.meta
+ # TODO add reviewer final
+ meta.feedback_authors.add(self.subscription.owner)
+ meta.done_assignments += 1
+ meta.has_active_assignment = False
+ self.is_done = True
+ self.save()
+ meta.save()
+
def delete(self, *args, **kwargs):
if self.is_done:
raise DeletionOfDoneAssignmentsNotPermitted()
diff --git a/core/models/submission.py b/core/models/submission.py
index 3687fb67..f968a657 100644
--- a/core/models/submission.py
+++ b/core/models/submission.py
@@ -63,7 +63,9 @@ class MetaSubmission(models.Model):
done_assignments = models.PositiveIntegerField(default=0)
has_active_assignment = models.BooleanField(default=False)
+ # Managed by signal!
has_feedback = models.BooleanField(default=False)
+ # Managed by signal!
has_final_feedback = models.BooleanField(default=False)
feedback_authors = models.ManyToManyField(get_user_model())
diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py
index fbbc9b32..19a430cc 100644
--- a/core/serializers/feedback.py
+++ b/core/serializers/feedback.py
@@ -205,8 +205,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
raise serializers.ValidationError(
'Sorry, you have to explain why this does not get full score')
- http_method = self.context['request'].method
- if hasattr(submission, 'feedback') and http_method == 'POST':
+ if hasattr(submission, 'feedback') and not self.instance:
raise serializers.ValidationError(
'Feedback for this submission already exists')
diff --git a/core/signals.py b/core/signals.py
index 95cdcc1a..110a15e8 100644
--- a/core/signals.py
+++ b/core/signals.py
@@ -50,16 +50,6 @@ def update_after_feedback_save(sender, instance, created, **kwargs):
meta = instance.of_submission.meta
meta.has_feedback = True
meta.has_final_feedback = instance.is_final
-
- undone_assignment = meta.submission.assignments.filter(is_done=False)
- assert undone_assignment.count() <= 1
- if undone_assignment.count() > 0:
- log.debug('SIGNAL -- Completed: %s' % undone_assignment.first())
- meta.feedback_authors.add(undone_assignment.first().subscription.owner)
- meta.done_assignments += 1
- meta.has_active_assignment = False
- undone_assignment.update(is_done=True)
-
meta.save()
diff --git a/core/tests/test_custom_subscription_filter.py b/core/tests/test_custom_subscription_filter.py
index 64525f58..c6ac1767 100644
--- a/core/tests/test_custom_subscription_filter.py
+++ b/core/tests/test_custom_subscription_filter.py
@@ -140,6 +140,7 @@ class StopOnPass(APITestCase):
self.assertEqual(35, self.data['students'][0].student.total_score)
self.assertTrue(self.data['students'][0].student.passes_exam)
+ # TODO why is this code commented?!?
# def test_submissions_left_after_not_pass_only_student_passed_exam(self):
# Feedback.objects.create(
# of_submission=self.data['submissions'][3], score=20)
@@ -164,8 +165,10 @@ class StopOnPass(APITestCase):
# signals recognize the open assignments
Feedback.objects.create(
of_submission=a1.submission, score=20)
+ a1.finish()
Feedback.objects.create(
of_submission=a2.submission, score=15)
+ a2.finish()
subscription_other_tutor = SubmissionSubscription.objects.create(
owner=self.tutor02,
@@ -188,8 +191,10 @@ class StopOnPass(APITestCase):
# signals recognize the open assignments
Feedback.objects.create(
of_submission=a1.submission, score=20)
+ a1.finish()
Feedback.objects.create(
of_submission=a2.submission, score=15)
+ a2.finish()
subscription_other_tutor = SubmissionSubscription.objects.create(
owner=self.tutor02,
diff --git a/core/tests/test_feedback.py b/core/tests/test_feedback.py
index dea21a72..7217cc86 100644
--- a/core/tests/test_feedback.py
+++ b/core/tests/test_feedback.py
@@ -93,7 +93,7 @@ class FeedbackCreateTestCase(APITestCase):
@classmethod
def setUpTestData(cls):
- cls.url = '/api/feedback/'
+ cls.url = lambda self: f'/api/assignment/{self.assignment.pk}/finish/'
cls.user_factory = GradyUserFactory()
cls.tutor = cls.user_factory.make_tutor(password='p')
cls.exam = make_exams(exams=[{
@@ -138,7 +138,7 @@ class FeedbackCreateTestCase(APITestCase):
}
self.assertEqual(Feedback.objects.count(), 0)
- 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(Feedback.objects.count(), 0)
@@ -149,7 +149,7 @@ class FeedbackCreateTestCase(APITestCase):
'of_submission': self.assignment.submission.pk
}
self.assertEqual(Feedback.objects.count(), 0)
- response = self.client.post(self.url, data, format='json')
+ response = self.client.post(self.url(), data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(Feedback.objects.count(), 0)
@@ -159,7 +159,7 @@ class FeedbackCreateTestCase(APITestCase):
'is_final': True,
'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(Feedback.objects.count(), 0)
@@ -169,7 +169,7 @@ class FeedbackCreateTestCase(APITestCase):
'is_final': False,
'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(Feedback.objects.count(), 0)
@@ -180,7 +180,7 @@ class FeedbackCreateTestCase(APITestCase):
'of_submission': self.assignment.submission.pk
}
self.assertEqual(Feedback.objects.count(), 0)
- response = self.client.post(self.url, data, format='json')
+ response = self.client.post(self.url(), data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(Feedback.objects.count(), 0)
@@ -191,7 +191,7 @@ class FeedbackCreateTestCase(APITestCase):
'of_submission': self.assignment.submission.pk
}
self.assertEqual(Feedback.objects.count(), 0)
- response = self.client.post(self.url, data, format='json')
+ response = self.client.post(self.url(), data, format='json')
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(Feedback.objects.count(), 0)
@@ -209,7 +209,7 @@ class FeedbackCreateTestCase(APITestCase):
}
}
self.assertEqual(self.fst_label.feedback.count(), 0)
- response = self.client.post(self.url, data, format='json')
+ response = self.client.post(self.url(), data, format='json')
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
self.fst_label.refresh_from_db()
self.snd_label.refresh_from_db()
@@ -229,7 +229,7 @@ class FeedbackCreateTestCase(APITestCase):
}
}
}
- self.client.post(self.url, data, format='json')
+ self.client.post(self.url(), data, format='json')
object_score = self.sub.feedback.score
self.assertEqual(object_score, 0.5)
@@ -245,7 +245,7 @@ class FeedbackCreateTestCase(APITestCase):
}
}
}
- self.client.post(self.url, data, format='json')
+ self.client.post(self.url(), data, format='json')
object_score = self.sub.feedback.score
self.assertEqual(object_score, 5)
@@ -262,7 +262,7 @@ class FeedbackCreateTestCase(APITestCase):
}
}
self.assertEqual(FeedbackComment.objects.count(), 0)
- response = self.client.post(self.url, data, format='json')
+ response = self.client.post(self.url(), data, format='json')
self.assertEqual(response.status_code, status.HTTP_201_CREATED)
self.assertEqual(FeedbackComment.objects.count(), 1)
@@ -278,7 +278,7 @@ class FeedbackCreateTestCase(APITestCase):
}
}
}
- self.client.post(self.url, data, format='json')
+ self.client.post(self.url(), data, format='json')
comment = FeedbackComment.objects.first()
self.assertEqual(comment.of_tutor, self.tutor)
self.assertEqual(comment.text, 'Nice meth!')
@@ -298,8 +298,8 @@ class FeedbackCreateTestCase(APITestCase):
}
}
self.assignment.delete()
- response = self.client.post(self.url, data, format='json')
- self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
+ response = self.client.post(self.url(), data, format='json')
+ self.assertEqual(status.HTTP_404_NOT_FOUND, response.status_code)
def test_cannot_create_with_someoneelses_assignment(self):
data = {
@@ -314,8 +314,9 @@ class FeedbackCreateTestCase(APITestCase):
}
other_tutor = self.user_factory.make_tutor('Berta')
self.client.force_authenticate(other_tutor)
- response = self.client.post(self.url, data, format='json')
- self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
+ response = self.client.post(self.url(), data, format='json')
+ # returns 404 since the other users assignment is not visible to this one
+ self.assertEqual(status.HTTP_404_NOT_FOUND, response.status_code)
def test_can_create_multiple_feedback_comments(self):
data = {
@@ -333,7 +334,7 @@ class FeedbackCreateTestCase(APITestCase):
}
}
}
- self.client.post(self.url, data, format='json')
+ self.client.post(self.url(), data, format='json')
first_comment = FeedbackComment.objects.get(text='Nice meth!')
self.assertEqual(first_comment.of_tutor, self.tutor)
self.assertIsNotNone(first_comment.created)
diff --git a/core/tests/test_subscription_assignment_service.py b/core/tests/test_subscription_assignment_service.py
index 79f0f14a..c4522609 100644
--- a/core/tests/test_subscription_assignment_service.py
+++ b/core/tests/test_subscription_assignment_service.py
@@ -235,7 +235,6 @@ class TestApiEndpoints(APITestCase):
response_subscription_create = self.client.post(
'/api/subscription/', {'query_type': 'random'})
- subscription_pk = response_subscription_create.data['pk']
subscription_pk = response_subscription_create.data['pk']
response_assignment = self.client.post(
@@ -374,7 +373,7 @@ class TestApiEndpoints(APITestCase):
})
self.assertEqual(status.HTTP_201_CREATED, response.status_code)
response = self.client.post(
- f'/api/feedback/', {
+ f'/api/assignment/{response.data["pk"]}/finish/', {
"score": 23,
"of_submission": response.data['submission']['pk'],
"feedback_lines": {
@@ -415,8 +414,8 @@ class TestApiEndpoints(APITestCase):
assignment = models.TutorSubmissionAssignment.objects.get(
pk=response.data['pk'])
self.assertFalse(assignment.is_done)
- response = self.client.patch(
- '/api/feedback/%s/' % submission_id_in_response, {
+ response = self.client.post(
+ f'/api/assignment/{assignment.pk}/finish/', {
"score": 20,
"is_final": True,
"feedback_lines": {
diff --git a/core/tests/test_tutor_api_endpoints.py b/core/tests/test_tutor_api_endpoints.py
index 7d0f0fc7..343b649d 100644
--- a/core/tests/test_tutor_api_endpoints.py
+++ b/core/tests/test_tutor_api_endpoints.py
@@ -107,6 +107,7 @@ class TutorListTests(APITestCase):
Feedback.objects.update_or_create(
of_submission=assignment.submission,
score=35)
+ assignment.finish()
tutor01 = data['tutors'][0]
tutor02 = data['tutors'][1]
diff --git a/core/urls.py b/core/urls.py
index b0e100b4..aee6e268 100644
--- a/core/urls.py
+++ b/core/urls.py
@@ -7,6 +7,7 @@ from rest_framework.permissions import AllowAny
from core import views
# Create a router and register our viewsets with it.
+
router = DefaultRouter()
router.register('student', views.StudentReviewerApiViewSet,
basename='student')
diff --git a/core/views/feedback.py b/core/views/feedback.py
index e7ccfb22..b034edd1 100644
--- a/core/views/feedback.py
+++ b/core/views/feedback.py
@@ -4,8 +4,11 @@ from multiprocessing import Lock
from rest_framework import mixins, status, viewsets
from rest_framework.exceptions import PermissionDenied
from rest_framework.response import Response
+from rest_framework import decorators
from core import models, permissions, serializers
+from core.views.util import tutor_attempts_to_patch_first_feedback_final, \
+ get_implicit_assignment_for_user
log = logging.getLogger(__name__)
@@ -17,11 +20,11 @@ class FeedbackApiView(
viewsets.GenericViewSet):
""" Gets a list of an individual exam by Id if provided """
permission_classes = (permissions.IsTutorOrReviewer,)
- queryset = models.Feedback.objects\
- .select_related('of_submission')\
- .select_related('of_submission__type')\
- .select_related('of_submission__student')\
- .select_related('of_submission__student__user')\
+ queryset = models.Feedback.objects \
+ .select_related('of_submission') \
+ .select_related('of_submission__type') \
+ .select_related('of_submission__student') \
+ .select_related('of_submission__student__user') \
.all()
serializer_class = serializers.FeedbackSerializer
lookup_field = 'of_submission__pk'
@@ -32,44 +35,11 @@ class FeedbackApiView(
user_is_tutor = self.request.user.role == models.UserAccount.TUTOR
return feedback_final_by_reviewer and user_is_tutor
- def _get_implicit_assignment_for_user(self, submission):
- """ Check for tutor if it exists. Not relevant for reviewer """
- try:
- return models.TutorSubmissionAssignment.objects.get(
- subscription__owner=self.request.user,
- submission=submission
- )
- except models.TutorSubmissionAssignment.DoesNotExist:
- if self.request.user.role == models.UserAccount.REVIEWER:
- return None
- 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)
user_is_tutor = self.request.user.role == models.UserAccount.TUTOR
return is_final_set and user_is_tutor
- # unused
- def _tutor_is_allowed_to_change_own_feedback(self, serializer):
- submission = self.get_object().of_submission
- assignment = self._get_implicit_assignment_for_user(submission)
- youngest = models.TutorSubmissionAssignment.objects \
- .filter(submission=submission) \
- .order_by('-created') \
- .first()
-
- return assignment == youngest
-
- def _tutor_attempts_to_patch_first_feedback_final(self, serializer):
- if self.request.user.role == models.UserAccount.REVIEWER:
- return False
- is_final_set = serializer.validated_data.get('is_final', False)
- 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
- return is_final_set and in_creation
-
def get_queryset(self):
if self.request.user.is_reviewer():
return self.queryset \
@@ -81,19 +51,16 @@ class FeedbackApiView(
of_submission__assignments__subscription__owner=self.request.user
)
+ @decorators.permission_classes((permissions.IsReviewer,))
def create(self, request, *args, **kwargs):
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)
-
self.perform_create(serializer)
+
+ # update MetaSubmission information
+ meta = serializer.validated_data.get('of_submission').meta
+ # TODO add reviewer final
+ meta.feedback_authors.add(self.request.user)
return Response(serializer.data,
status=status.HTTP_201_CREATED)
@@ -103,16 +70,17 @@ class FeedbackApiView(
partial=True)
serializer.is_valid(raise_exception=True)
- self._get_implicit_assignment_for_user(feedback.of_submission)
+ assignment = get_implicit_assignment_for_user(feedback.of_submission, self.request.user)
+ # TODO change this to check reviewer final
if self._tutor_attempts_to_change_final_feedback_of_reviewer(serializer): # noqa
raise PermissionDenied(
detail="Changing final feedback is not allowed.")
- if self._tutor_attempts_to_patch_first_feedback_final(serializer):
+ if tutor_attempts_to_patch_first_feedback_final(serializer, self.request.user, assignment):
raise PermissionDenied(
detail='Cannot set the first feedback final.')
-
+ # TODO add reviewer final
serializer.save()
return Response(serializer.data)
diff --git a/core/views/subscription.py b/core/views/subscription.py
index bf77f467..2ea0b0c0 100644
--- a/core/views/subscription.py
+++ b/core/views/subscription.py
@@ -2,7 +2,8 @@ import logging
from django.core.exceptions import ObjectDoesNotExist
from rest_framework import mixins, status, viewsets
-from rest_framework.decorators import action, permission_classes
+from rest_framework import decorators
+from rest_framework.exceptions import PermissionDenied
from rest_framework.response import Response
from core import models, permissions, serializers
@@ -12,6 +13,8 @@ from core.serializers import AssignmentDetailSerializer, AssignmentSerializer
from multiprocessing import Lock
+from core.views.util import tutor_attempts_to_patch_first_feedback_final
+
log = logging.getLogger(__name__)
@@ -76,6 +79,7 @@ class AssignmentApiViewSet(
queryset = TutorSubmissionAssignment.objects\
.select_related('subscription').all()
serializer_class = AssignmentSerializer
+ permission_classes = (IsTutorOrReviewer, )
def get_queryset(self):
if self.action in ['list', 'active', 'destroy']:
@@ -97,11 +101,11 @@ class AssignmentApiViewSet(
status=status.HTTP_403_FORBIDDEN)
return Response(serializer.data, status=status.HTTP_201_CREATED)
- @permission_classes((IsReviewer,))
+ @decorators.permission_classes((IsReviewer,))
def list(self, *args, **kwargs):
return super().list(*args, **kwargs)
- @action(detail=False, permission_classes=(IsReviewer,), methods=['get', 'delete'])
+ @decorators.action(detail=False, permission_classes=(IsReviewer,), methods=['get', 'delete'])
def active(self, request):
if request.method == 'GET':
queryset = self.get_queryset().filter(is_done=False)
@@ -111,7 +115,36 @@ class AssignmentApiViewSet(
self.get_queryset().filter(is_done=False).delete()
return Response(status=status.HTTP_204_NO_CONTENT)
- @permission_classes((IsTutorOrReviewer,))
+ @decorators.action(detail=True, methods=['post'])
+ def finish(self, request, *args, **kwargs):
+ context = self.get_serializer_context()
+ instance = self.get_object()
+ if instance.is_done or (instance.subscription.owner != request.user):
+ return Response(status=status.HTTP_403_FORBIDDEN)
+ try:
+ orig_feedback = instance.submission.feedback
+ serializer = serializers.FeedbackSerializer(
+ orig_feedback,
+ data=request.data,
+ context=context,
+ partial=True)
+ except models.Feedback.DoesNotExist:
+ serializer = serializers.FeedbackSerializer(
+ data=request.data,
+ context=context)
+
+ serializer.is_valid(raise_exception=True)
+ if tutor_attempts_to_patch_first_feedback_final(serializer, self.request.user, instance):
+ raise PermissionDenied(
+ detail='Cannot set the first feedback final.')
+ # TODO check if is reviewer final
+ serializer.save()
+ instance.finish()
+ response_status = status.HTTP_201_CREATED if \
+ instance.subscription.feedback_stage == \
+ models.SubmissionSubscription.FEEDBACK_CREATION else status.HTTP_200_OK
+ return Response(serializer.data, status=response_status)
+
def destroy(self, request, pk=None):
""" Stop working on the assignment before it is finished """
instance = self.get_object()
@@ -123,7 +156,6 @@ class AssignmentApiViewSet(
instance.delete()
return Response(status=status.HTTP_204_NO_CONTENT)
- @permission_classes((IsTutorOrReviewer,))
def create(self, request, *args, **kwargs):
with Lock():
context = self.get_serializer_context()
@@ -133,7 +165,6 @@ class AssignmentApiViewSet(
assignment = self._fetch_assignment(serializer)
return assignment
- @permission_classes((IsTutorOrReviewer,))
def retrieve(self, request, *args, **kwargs):
assignment = self.get_object()
if assignment.subscription.owner != request.user:
diff --git a/core/views/util.py b/core/views/util.py
new file mode 100644
index 00000000..b0301873
--- /dev/null
+++ b/core/views/util.py
@@ -0,0 +1,33 @@
+from rest_framework.exceptions import PermissionDenied
+
+from core import models
+
+
+class NoAssignmentForTutor(Exception):
+ pass
+
+
+def tutor_attempts_to_patch_first_feedback_final(feedback_serializer,
+ user,
+ assignment=None):
+ if user.role == models.UserAccount.REVIEWER:
+ return False
+ if user.role == models.UserAccount.TUTOR and assignment is None:
+ raise NoAssignmentForTutor()
+ is_final_set = feedback_serializer.validated_data.get('is_final', False)
+ in_creation = assignment.subscription.feedback_stage == models.SubmissionSubscription.FEEDBACK_CREATION # noqa
+ return is_final_set and in_creation
+
+
+def get_implicit_assignment_for_user(submission, user):
+ """ Check for tutor if it exists. Not relevant for reviewer """
+ try:
+ return models.TutorSubmissionAssignment.objects.get(
+ subscription__owner=user,
+ submission=submission
+ )
+ except models.TutorSubmissionAssignment.DoesNotExist:
+ if user.role == models.UserAccount.REVIEWER:
+ return None
+ raise PermissionDenied(
+ detail='This user has no permission to create this feedback')
diff --git a/frontend/src/api.ts b/frontend/src/api.ts
index d6a1d838..e1c7f83b 100644
--- a/frontend/src/api.ts
+++ b/frontend/src/api.ts
@@ -115,7 +115,7 @@ export async function fetchStatistics (): Promise<Statistics> {
return (await ax.get(url)).data
}
-export async function fetchLabelStatistics (): Promise<LabelStatisticsForSubType []> {
+export async function fetchLabelStatistics (): Promise<LabelStatisticsForSubType[]> {
const url = '/api/label-statistics'
return (await ax.get(url)).data
}
@@ -152,16 +152,20 @@ export async function createAssignment (
return (await ax.post(`/api/assignment/`, data)).data
}
-export async function submitFeedbackForAssignment ({ feedback }:
- { feedback: Partial<CreateUpdateFeedback>}): Promise<CreateUpdateFeedback> {
- return (await ax.post('/api/feedback/', feedback)).data
+export async function submitFeedbackForAssignment ({ feedback, assignment }:
+ { feedback: Partial<CreateUpdateFeedback>, assignment: Assignment}): Promise<CreateUpdateFeedback> {
+ return (await ax.post(`/api/assignment/${assignment.pk}/finish`, feedback)).data
}
-export async function submitUpdatedFeedback ({ feedback }:
+export async function submitUpdatedFeedback ({ feedback }:
{feedback: CreateUpdateFeedback}): Promise<CreateUpdateFeedback> {
return (await ax.patch(`/api/feedback/${feedback.ofSubmission}/`, feedback)).data
}
+export async function submitFeedback ({ feedback }: {feedback: CreateUpdateFeedback}): Promise<Feedback> {
+ return (await ax.post(`/api/feedback/`, feedback)).data
+}
+
export async function fetchSubmissionTypes (): Promise<Array<SubmissionType>> {
const url = '/api/submissiontype/'
return (await ax.get(url)).data
@@ -221,7 +225,7 @@ export async function createLabel (payload: Partial<FeedbackLabel>) {
}
export async function updateLabel (payload: FeedbackLabel) {
- return (await ax.put('/api/label/' + payload.pk + '/', payload))
+ return (await ax.put('/api/label/' + payload.pk + '/', payload)).data
}
export interface StudentExportOptions { setPasswords?: boolean }
@@ -250,6 +254,6 @@ export async function fetchInstanceExportData (): Promise<InstanceExportData> {
return (await ax.get('/api/instance/export')).data
}
-ax.interceptors.response.use(undefined, errorInterceptor);
+ax.interceptors.response.use(undefined, errorInterceptor)
export default ax
diff --git a/frontend/src/store/modules/submission-notes.ts b/frontend/src/store/modules/submission-notes.ts
index 8247114b..50fa49b0 100644
--- a/frontend/src/store/modules/submission-notes.ts
+++ b/frontend/src/store/modules/submission-notes.ts
@@ -4,8 +4,9 @@ import * as api from '@/api'
import { Feedback, FeedbackComment, SubmissionNoType, CreateUpdateFeedback } from '@/models'
import { RootState } from '@/store/store'
import { getStoreBuilder, BareActionContext } from 'vuex-typex'
-import { syntaxPostProcess } from '@/util/helpers';
-import { AxiosResponse } from 'axios';
+import { syntaxPostProcess } from '@/util/helpers'
+import { AxiosResponse } from 'axios'
+import { Subscriptions } from './subscriptions';
export interface SubmissionNotesState {
submission: SubmissionNoType
@@ -21,7 +22,7 @@ export interface SubmissionNotesState {
changedLabels: boolean
}
-function initialState(): SubmissionNotesState {
+function initialState (): SubmissionNotesState {
return {
submission: {
text: '',
@@ -194,11 +195,15 @@ Promise<AxiosResponse<void>[]> {
} else if (feedback.score! < SubmissionNotes.submissionType.fullScore! && !state.hasOrigFeedback) {
throw new Error('You need to add or change a comment when setting a non full score.')
}
- if (!state.hasOrigFeedback) {
- await api.submitFeedbackForAssignment({ feedback })
- } else {
+
+ const assignment = Subscriptions.state.currentAssignment
+ if (assignment) {
+ await api.submitFeedbackForAssignment({ feedback , assignment})
+ } else if (state.origFeedback) {
feedback.pk = state.origFeedback.pk
await api.submitUpdatedFeedback(<{ feedback: CreateUpdateFeedback }>{ feedback })
+ } else {
+ await api.submitFeedback(<{ feedback: CreateUpdateFeedback }>{feedback})
}
// delete those comments that have been marked for deletion
return SubmissionNotes.deleteComments()
diff --git a/functional_tests/test_export_modal.py b/functional_tests/test_export_modal.py
index e6af2003..6fa6aa16 100644
--- a/functional_tests/test_export_modal.py
+++ b/functional_tests/test_export_modal.py
@@ -117,8 +117,8 @@ class ExportTestModal(LiveServerTestCase):
os.remove(JSON_EXPORT_FILE)
def test_export_instance(self):
- self._login()
fact.SubmissionFactory()
+ self._login()
self.browser.find_element_by_id('export-btn').click()
self.browser.find_element_by_id('export-list1').click()
instance_export_modal = self.browser.find_element_by_id('instance-export-modal')
--
GitLab
From 93eb267c01ce22d33ed8c6f5d3a88a5c820c006f Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Wed, 14 Aug 2019 15:10:29 +0200
Subject: [PATCH 2/8] Added test case
---
functional_tests/test_feedback_creation.py | 9 +--
functional_tests/test_feedback_update.py | 85 ++++++++++++++++++++++
functional_tests/util.py | 12 +++
3 files changed, 99 insertions(+), 7 deletions(-)
create mode 100644 functional_tests/test_feedback_update.py
diff --git a/functional_tests/test_feedback_creation.py b/functional_tests/test_feedback_creation.py
index c85c84be..1cf9caa3 100644
--- a/functional_tests/test_feedback_creation.py
+++ b/functional_tests/test_feedback_creation.py
@@ -8,6 +8,7 @@ from selenium.webdriver.common.by import By
from core.models import UserAccount, Submission, FeedbackComment
from functional_tests.util import (login, create_browser, reset_browser_after_test,
go_to_subscription, wait_until_code_changes,
+ correct_some_submission,
reconstruct_submission_code, wait_until_element_count_equals)
from util import factory_boys as fact
@@ -95,13 +96,7 @@ class UntestedParent:
def test_can_give_max_score(self):
self._login()
go_to_subscription(self)
- code = reconstruct_submission_code(self)
- self.browser.find_element_by_id('score-full').click()
- submit_btn = self.browser.find_element_by_id('submit-feedback')
- submit_btn.click()
- WebDriverWait(self.browser, 10).until(
- wait_until_code_changes(self, code)
- )
+ code = correct_some_submission()
submission_for_code = Submission.objects.get(text=code)
self.assertEqual(self.sub_type.full_score, submission_for_code.feedback.score)
diff --git a/functional_tests/test_feedback_update.py b/functional_tests/test_feedback_update.py
new file mode 100644
index 00000000..f2848b88
--- /dev/null
+++ b/functional_tests/test_feedback_update.py
@@ -0,0 +1,85 @@
+from django.test import LiveServerTestCase
+from selenium import webdriver
+from selenium.webdriver import ActionChains
+from selenium.webdriver.common.by import By
+from selenium.webdriver.common.keys import Keys
+from selenium.webdriver.support.ui import WebDriverWait
+from selenium.webdriver.support import expected_conditions as ec
+
+from core.models import FeedbackLabel
+from functional_tests.util import (login, create_browser, reset_browser_after_test,
+ query_returns_object, go_to_subscription,
+ reconstruct_submission_code, wait_until_code_changes,
+ correct_some_submission)
+from util import factory_boys as fact
+
+import time
+
+class TestFeedbackUpdate(LiveServerTestCase):
+ browser: webdriver.Firefox = None
+ username = None
+ password = None
+ role = None
+
+ @classmethod
+ def setUpClass(cls):
+ super().setUpClass()
+ cls.browser = create_browser()
+
+ @classmethod
+ def tearDownClass(cls):
+ super().tearDownClass()
+ cls.browser.quit()
+
+ def setUp(self):
+ super().setUp()
+ self.username = 'tut'
+ self.password = 'p'
+ fact.UserAccountFactory(
+ username=self.username,
+ password=self.password,
+ )
+ self.sub_type = fact.SubmissionTypeFactory.create()
+ fact.SubmissionFactory.create_batch(2, type=self.sub_type)
+
+ def _login(self):
+ login(self.browser, self.live_server_url, self.username, self.password)
+
+ def test_updating_own_feedback_doesnt_invalidate_other_tutors_assignment(self):
+ self._login()
+ code = correct_some_submission(self)
+ first_tab = self.browser.current_window_handle
+
+ # body = self.browser.find_element_by_tag_name('body')
+ # body.send_keys(Keys.LEFT_CONTROL + 't')
+ self.browser.execute_script('window.open()')
+ self.browser.switch_to.window(self.browser.window_handles[-1])
+ second_tab = self.browser.current_window_handle
+ self.browser.execute_script("window.sessionStorage.clear()")
+ self.browser.get(self.live_server_url)
+ username = 'other_tut'
+ password = 'p'
+ fact.UserAccountFactory(
+ username=username,
+ password=password
+ )
+ login(self.browser, self.live_server_url, username, password)
+ go_to_subscription(self, stage='validate')
+ other_code = reconstruct_submission_code(self)
+ self.assertEqual(
+ code, other_code,
+ "Code for validation submissions is different than initial")
+
+ self.browser.switch_to.window(first_tab)
+ self.browser.find_element_by_partial_link_text('Feedback History').click()
+ # time.sleep(3)
+ feedback_entry = self.browser.find_element_by_class_name('feedback-row')
+ ActionChains(self.browser).move_to_element(feedback_entry).click().perform()
+ self.browser.find_element_by_id('submit-feedback').click()
+
+ self.browser.switch_to.window(second_tab)
+ self.browser.find_element_by_id('submit-feedback').click()
+ WebDriverWait(self.browser, 10).until(
+ ec.url_contains('ended'),
+ 'Browser is not on Subscription ended site, therefore Feedback could not be submitted'
+ )
diff --git a/functional_tests/util.py b/functional_tests/util.py
index 728542e2..7d10f0dc 100644
--- a/functional_tests/util.py
+++ b/functional_tests/util.py
@@ -106,6 +106,18 @@ def go_to_subscription(test_class_instance, stage='initial', sub_type=None):
WebDriverWait(test_class_instance.browser, 10).until(ec.url_contains('subscription'))
+def correct_some_submission(test_class_instance):
+ go_to_subscription(test_class_instance)
+ code = reconstruct_submission_code(test_class_instance)
+ test_class_instance.browser.find_element_by_id('score-full').click()
+ submit_btn = test_class_instance.browser.find_element_by_id('submit-feedback')
+ submit_btn.click()
+ WebDriverWait(test_class_instance.browser, 10).until(
+ wait_until_code_changes(test_class_instance, code)
+ )
+ return code
+
+
def reconstruct_submission_code(test_class_instance):
sub_table = test_class_instance.browser.find_element_by_class_name('submission-table')
lines = sub_table.find_elements_by_tag_name('tr')
--
GitLab
From dacfbeb39093a5cce904f3c9bb374c4fc97ba58a Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Wed, 14 Aug 2019 18:00:50 +0200
Subject: [PATCH 3/8] Fix after rebase
---
core/models/assignment.py | 1 -
core/serializers/feedback.py | 8 ++++----
core/signals.py | 2 +-
core/views/feedback.py | 5 +----
core/views/subscription.py | 5 ++++-
functional_tests/test_feedback_creation.py | 2 +-
functional_tests/test_feedback_update.py | 20 ++++++++------------
7 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/core/models/assignment.py b/core/models/assignment.py
index 77967461..2f9151bc 100644
--- a/core/models/assignment.py
+++ b/core/models/assignment.py
@@ -42,7 +42,6 @@ class TutorSubmissionAssignment(models.Model):
raise CanOnlyCallFinishOnUnfinishedAssignments()
meta = self.submission.meta
- # TODO add reviewer final
meta.feedback_authors.add(self.subscription.owner)
meta.done_assignments += 1
meta.has_active_assignment = False
diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py
index 19a430cc..8e9f6e69 100644
--- a/core/serializers/feedback.py
+++ b/core/serializers/feedback.py
@@ -124,11 +124,11 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
feedback_lines = validated_data.pop('feedback_lines', [])
labels = validated_data.pop('labels', [])
user = self.context['request'].user
+ final_by_reviewer = validated_data.pop('is_final', False) and \
+ user.role == UserAccount.REVIEWER
feedback = Feedback.objects.create(of_submission=submission,
+ final_by_reviewer=final_by_reviewer,
**validated_data)
- if user.role == UserAccount.REVIEWER:
- feedback.final_by_reviewer = self.context['request'].data['is_final']
-
for label in labels:
feedback.labels.add(label)
@@ -143,7 +143,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
)
comment_instance.labels.set(labels)
- return Feedback.objects.get(of_submission=submission)
+ return feedback
@transaction.atomic
def update(self, feedback, validated_data):
diff --git a/core/signals.py b/core/signals.py
index 110a15e8..754dd966 100644
--- a/core/signals.py
+++ b/core/signals.py
@@ -49,7 +49,7 @@ def update_after_feedback_save(sender, instance, created, **kwargs):
log.debug('SIGNAL -- update_after_feedback_save')
meta = instance.of_submission.meta
meta.has_feedback = True
- meta.has_final_feedback = instance.is_final
+ meta.has_final_feedback = instance.is_final or instance.final_by_reviewer
meta.save()
diff --git a/core/views/feedback.py b/core/views/feedback.py
index b034edd1..a1107208 100644
--- a/core/views/feedback.py
+++ b/core/views/feedback.py
@@ -59,7 +59,6 @@ class FeedbackApiView(
# update MetaSubmission information
meta = serializer.validated_data.get('of_submission').meta
- # TODO add reviewer final
meta.feedback_authors.add(self.request.user)
return Response(serializer.data,
status=status.HTTP_201_CREATED)
@@ -72,15 +71,13 @@ class FeedbackApiView(
assignment = get_implicit_assignment_for_user(feedback.of_submission, self.request.user)
- # TODO change this to check reviewer final
if self._tutor_attempts_to_change_final_feedback_of_reviewer(serializer): # noqa
raise PermissionDenied(
- detail="Changing final feedback is not allowed.")
+ detail="Changing feedback set to final by a reviewer is not allowed.")
if tutor_attempts_to_patch_first_feedback_final(serializer, self.request.user, assignment):
raise PermissionDenied(
detail='Cannot set the first feedback final.')
- # TODO add reviewer final
serializer.save()
return Response(serializer.data)
diff --git a/core/views/subscription.py b/core/views/subscription.py
index 2ea0b0c0..ec052529 100644
--- a/core/views/subscription.py
+++ b/core/views/subscription.py
@@ -128,6 +128,10 @@ class AssignmentApiViewSet(
data=request.data,
context=context,
partial=True)
+ if orig_feedback.final_by_reviewer and request.user.role == models.UserAccount.TUTOR:
+ raise PermissionDenied(detail="Unfortunately you won't be able to finish this"
+ "assignment since a reviewer has marked it as "
+ "final while you were assigned.")
except models.Feedback.DoesNotExist:
serializer = serializers.FeedbackSerializer(
data=request.data,
@@ -137,7 +141,6 @@ class AssignmentApiViewSet(
if tutor_attempts_to_patch_first_feedback_final(serializer, self.request.user, instance):
raise PermissionDenied(
detail='Cannot set the first feedback final.')
- # TODO check if is reviewer final
serializer.save()
instance.finish()
response_status = status.HTTP_201_CREATED if \
diff --git a/functional_tests/test_feedback_creation.py b/functional_tests/test_feedback_creation.py
index 1cf9caa3..bc81158a 100644
--- a/functional_tests/test_feedback_creation.py
+++ b/functional_tests/test_feedback_creation.py
@@ -96,7 +96,7 @@ class UntestedParent:
def test_can_give_max_score(self):
self._login()
go_to_subscription(self)
- code = correct_some_submission()
+ code = correct_some_submission(self)
submission_for_code = Submission.objects.get(text=code)
self.assertEqual(self.sub_type.full_score, submission_for_code.feedback.score)
diff --git a/functional_tests/test_feedback_update.py b/functional_tests/test_feedback_update.py
index f2848b88..30bf1a2d 100644
--- a/functional_tests/test_feedback_update.py
+++ b/functional_tests/test_feedback_update.py
@@ -1,19 +1,13 @@
from django.test import LiveServerTestCase
from selenium import webdriver
from selenium.webdriver import ActionChains
-from selenium.webdriver.common.by import By
-from selenium.webdriver.common.keys import Keys
-from selenium.webdriver.support.ui import WebDriverWait
from selenium.webdriver.support import expected_conditions as ec
+from selenium.webdriver.support.ui import WebDriverWait
-from core.models import FeedbackLabel
-from functional_tests.util import (login, create_browser, reset_browser_after_test,
- query_returns_object, go_to_subscription,
- reconstruct_submission_code, wait_until_code_changes,
- correct_some_submission)
+from functional_tests.util import (login, create_browser, go_to_subscription,
+ reconstruct_submission_code, correct_some_submission)
from util import factory_boys as fact
-import time
class TestFeedbackUpdate(LiveServerTestCase):
browser: webdriver.Firefox = None
@@ -46,12 +40,12 @@ class TestFeedbackUpdate(LiveServerTestCase):
login(self.browser, self.live_server_url, self.username, self.password)
def test_updating_own_feedback_doesnt_invalidate_other_tutors_assignment(self):
+ # First correct some submission as the first tutor
self._login()
code = correct_some_submission(self)
first_tab = self.browser.current_window_handle
- # body = self.browser.find_element_by_tag_name('body')
- # body.send_keys(Keys.LEFT_CONTROL + 't')
+ # open a new tab and go to the validation page of the just corrected submission
self.browser.execute_script('window.open()')
self.browser.switch_to.window(self.browser.window_handles[-1])
second_tab = self.browser.current_window_handle
@@ -66,17 +60,19 @@ class TestFeedbackUpdate(LiveServerTestCase):
login(self.browser, self.live_server_url, username, password)
go_to_subscription(self, stage='validate')
other_code = reconstruct_submission_code(self)
+ # The submission to be validated should be the same as the one that has been corrected
self.assertEqual(
code, other_code,
"Code for validation submissions is different than initial")
+ # Go to first tab and update the submission as the first tutor via the Feedback History page
self.browser.switch_to.window(first_tab)
self.browser.find_element_by_partial_link_text('Feedback History').click()
- # time.sleep(3)
feedback_entry = self.browser.find_element_by_class_name('feedback-row')
ActionChains(self.browser).move_to_element(feedback_entry).click().perform()
self.browser.find_element_by_id('submit-feedback').click()
+ # as the second tutor, submit the validated feedback
self.browser.switch_to.window(second_tab)
self.browser.find_element_by_id('submit-feedback').click()
WebDriverWait(self.browser, 10).until(
--
GitLab
From e1d1d9ff496ac00d0b0ede18189d69d6edd8175a Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Wed, 14 Aug 2019 18:45:09 +0200
Subject: [PATCH 4/8] Added screenshot on fail for export test
---
.gitignore | 1 +
.gitlab-ci.yml | 8 ++++++--
functional_tests/test_export_modal.py | 9 ++++++++-
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/.gitignore b/.gitignore
index 391b49d5..efc87898 100644
--- a/.gitignore
+++ b/.gitignore
@@ -38,6 +38,7 @@ coverage_html/
anon-export/
public/
geckodriver.log
+.screenshots
# node
node_modules
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 9c6ce22d..be215ebd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -25,7 +25,7 @@ build_test_env:
artifacts:
paths:
- .venv/
- expire_in: 20 minutes
+ expire_in: 1 days
cache:
key: "$CI_JOB_NAME"
paths:
@@ -44,7 +44,7 @@ build_frontend:
paths:
- frontend/dist
- frontend/node_modules/
- expire_in: 20 minutes
+ expire_in: 1 days
cache:
key: "$CI_JOB_NAME"
paths:
@@ -106,6 +106,10 @@ test_frontend:
- python util/format_index.py
- python manage.py collectstatic --no-input
- HEADLESS_TESTS=True pytest --ds=grady.settings.test functional_tests
+ artifacts:
+ paths:
+ - .screenshots
+ expire_in: 30 days
test_frontend_unit:
image: node:carbon
diff --git a/functional_tests/test_export_modal.py b/functional_tests/test_export_modal.py
index 6fa6aa16..afa74d6e 100644
--- a/functional_tests/test_export_modal.py
+++ b/functional_tests/test_export_modal.py
@@ -51,6 +51,13 @@ class ExportTestModal(LiveServerTestCase):
)
def tearDown(self):
+ try:
+ os.mkdir('.screenshots')
+ except FileExistsError:
+ pass
+ for method, error in self._outcome.errors:
+ if error:
+ self.browser.get_screenshot_as_file(".screenshots/" + self.id() + ".png")
reset_browser_after_test(self.browser, self.live_server_url)
def _login(self):
@@ -97,8 +104,8 @@ class ExportTestModal(LiveServerTestCase):
list_elements[1].find_element_by_tag_name('a').text)
def test_export_student_scores_as_json(self):
- self._login()
fact.StudentInfoFactory()
+ self._login()
export_btn = self.browser.find_element_by_id('export-btn')
export_btn.click()
export_scores = self.browser.find_element_by_id('export-list0')
--
GitLab
From 4cd83e41560629a5b12f8f975b0419c049710bc9 Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Wed, 14 Aug 2019 19:13:42 +0200
Subject: [PATCH 5/8] Parralelized tests
---
.gitlab-ci.yml | 4 ++--
Makefile | 6 +++---
requirements.dev.txt | 2 ++
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index be215ebd..37677087 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -70,7 +70,7 @@ test_pytest:
services:
- postgres:9.6
script:
- - pytest --cov --ds=grady.settings.test core/tests
+ - pytest -n 4 --cov --ds=grady.settings.test core/tests
artifacts:
paths:
- .coverage
@@ -105,7 +105,7 @@ test_frontend:
- cp frontend/dist/index.html core/templates
- python util/format_index.py
- python manage.py collectstatic --no-input
- - HEADLESS_TESTS=True pytest --ds=grady.settings.test functional_tests
+ - HEADLESS_TESTS=True pytest -n 4 --ds=grady.settings.test functional_tests
artifacts:
paths:
- .screenshots
diff --git a/Makefile b/Makefile
index b20a9c5d..24452859 100644
--- a/Makefile
+++ b/Makefile
@@ -28,13 +28,13 @@ install:
pip install -Ur requirements.dev.txt
test:
- DJANGO_SETTINGS_MODULE=grady.settings pytest
+ pytest -n 4 --ds=grady.settings core/tests
teste2e:
- cd frontend && yarn build && cp dist/index.html ../core/templates && cd .. && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest --ds=grady.settings $(path); git checkout core/templates/index.html
+ cd frontend && yarn build && cp dist/index.html ../core/templates && cd .. && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest -n 4 --ds=grady.settings $(path); git checkout core/templates/index.html
teste2e-nc:
- cp frontend/dist/index.html ./core/templates && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest --ds=grady.settings $(path); git checkout core/templates/index.html
+ cp frontend/dist/index.html ./core/templates && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest -n 4 --ds=grady.settings $(path); git checkout core/templates/index.html
coverage:
diff --git a/requirements.dev.txt b/requirements.dev.txt
index d883c351..a48c0a16 100644
--- a/requirements.dev.txt
+++ b/requirements.dev.txt
@@ -1,6 +1,8 @@
flake8~=3.6.0
pre-commit~=1.13.0
+pytest~=4.4
pytest-cov~=2.6.0
+pytest-xdist~=1.29
pytest-django~=3.5.0
selenium~=3.141.0
factory-boy~=2.11.0
--
GitLab
From 9dd3962d8448287f97e5a9d87c08024af4cff6fd Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Thu, 15 Aug 2019 14:56:54 +0200
Subject: [PATCH 6/8] Removed paralleization
---
.gitlab-ci.yml | 4 +--
Makefile | 4 +--
functional_tests/test_export_modal.py | 46 +++++++++++++++++----------
3 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 37677087..be215ebd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -70,7 +70,7 @@ test_pytest:
services:
- postgres:9.6
script:
- - pytest -n 4 --cov --ds=grady.settings.test core/tests
+ - pytest --cov --ds=grady.settings.test core/tests
artifacts:
paths:
- .coverage
@@ -105,7 +105,7 @@ test_frontend:
- cp frontend/dist/index.html core/templates
- python util/format_index.py
- python manage.py collectstatic --no-input
- - HEADLESS_TESTS=True pytest -n 4 --ds=grady.settings.test functional_tests
+ - HEADLESS_TESTS=True pytest --ds=grady.settings.test functional_tests
artifacts:
paths:
- .screenshots
diff --git a/Makefile b/Makefile
index 24452859..4610651a 100644
--- a/Makefile
+++ b/Makefile
@@ -28,10 +28,10 @@ install:
pip install -Ur requirements.dev.txt
test:
- pytest -n 4 --ds=grady.settings core/tests
+ pytest --ds=grady.settings core/tests
teste2e:
- cd frontend && yarn build && cp dist/index.html ../core/templates && cd .. && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest -n 4 --ds=grady.settings $(path); git checkout core/templates/index.html
+ cd frontend && yarn build && cp dist/index.html ../core/templates && cd .. && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest --ds=grady.settings $(path); git checkout core/templates/index.html
teste2e-nc:
cp frontend/dist/index.html ./core/templates && python util/format_index.py && python manage.py collectstatic --no-input && HEADLESS_TESTS=$(headless) pytest -n 4 --ds=grady.settings $(path); git checkout core/templates/index.html
diff --git a/functional_tests/test_export_modal.py b/functional_tests/test_export_modal.py
index afa74d6e..4754e72a 100644
--- a/functional_tests/test_export_modal.py
+++ b/functional_tests/test_export_modal.py
@@ -1,5 +1,6 @@
import json
import os
+from pathlib import Path
from django.test import LiveServerTestCase
from selenium import webdriver
@@ -10,14 +11,17 @@ from functional_tests.util import login, create_browser, reset_browser_after_tes
from util import factory_boys as fact
-def expect_file_to_be_present(path):
+def expect_file_to_be_downloaded(path):
+ """
+ Checks if a file has finished downloading by checking if a file exists at the path and
+ no accompanying `<path>.part` file.
+ :param path: path to check
+ :return:
+ """
def condition(*args):
- try:
- with open(path):
- pass
- except FileNotFoundError:
- return False
- return True
+ file_present = Path(path).is_file()
+ partial_file_present = Path(os.path.join(path, '.part')).is_file()
+ return file_present and not partial_file_present
return condition
@@ -117,11 +121,15 @@ class ExportTestModal(LiveServerTestCase):
export_type_json.click()
data_export_btn = data_export_modal.find_element_by_id('export-data-download-btn')
data_export_btn.click()
- WebDriverWait(self.browser, 10).until(expect_file_to_be_present(JSON_EXPORT_FILE))
- with open(JSON_EXPORT_FILE) as f:
- data = json.load(f)
- self.assertEqual('B.Inf.4242 Test Module', data[0]['Exam'])
- os.remove(JSON_EXPORT_FILE)
+ WebDriverWait(self.browser, 10).until(expect_file_to_be_downloaded(JSON_EXPORT_FILE))
+ try:
+ with open(JSON_EXPORT_FILE) as f:
+ data = json.load(f)
+ self.assertEqual('B.Inf.4242 Test Module', data[0]['Exam'])
+ except Exception as e:
+ raise e
+ finally:
+ os.remove(JSON_EXPORT_FILE)
def test_export_instance(self):
fact.SubmissionFactory()
@@ -130,8 +138,12 @@ class ExportTestModal(LiveServerTestCase):
self.browser.find_element_by_id('export-list1').click()
instance_export_modal = self.browser.find_element_by_id('instance-export-modal')
instance_export_modal.find_element_by_id('instance-export-dl').click()
- WebDriverWait(self.browser, 10).until(expect_file_to_be_present(JSON_EXPORT_FILE))
- with open(JSON_EXPORT_FILE) as f:
- data = json.load(f)
- self.assertEqual('B.Inf.4242 Test Module', data['examTypes'][0]['moduleReference'])
- os.remove(JSON_EXPORT_FILE)
+ WebDriverWait(self.browser, 10).until(expect_file_to_be_downloaded(JSON_EXPORT_FILE))
+ try:
+ with open(JSON_EXPORT_FILE) as f:
+ data = json.load(f)
+ self.assertEqual('B.Inf.4242 Test Module', data['examTypes'][0]['moduleReference'])
+ except Exception as e:
+ raise e
+ finally:
+ os.remove(JSON_EXPORT_FILE)
--
GitLab
From 08ba8ad944b9c888cec53e4b51022d9414588b6f Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Thu, 15 Aug 2019 17:14:49 +0200
Subject: [PATCH 7/8] Some fixes
---
.gitlab-ci.yml | 2 +-
core/serializers/feedback.py | 2 +-
frontend/src/api.ts | 2 +-
.../components/submission_notes/SubmissionCorrection.vue | 7 +++----
frontend/src/store/modules/submission-notes.ts | 2 +-
5 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index be215ebd..f4982209 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -108,7 +108,7 @@ test_frontend:
- HEADLESS_TESTS=True pytest --ds=grady.settings.test functional_tests
artifacts:
paths:
- - .screenshots
+ - .screenshots/
expire_in: 30 days
test_frontend_unit:
diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py
index 8e9f6e69..cccfa16d 100644
--- a/core/serializers/feedback.py
+++ b/core/serializers/feedback.py
@@ -124,7 +124,7 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
feedback_lines = validated_data.pop('feedback_lines', [])
labels = validated_data.pop('labels', [])
user = self.context['request'].user
- final_by_reviewer = validated_data.pop('is_final', False) and \
+ final_by_reviewer = validated_data.get('is_final', False) and \
user.role == UserAccount.REVIEWER
feedback = Feedback.objects.create(of_submission=submission,
final_by_reviewer=final_by_reviewer,
diff --git a/frontend/src/api.ts b/frontend/src/api.ts
index e1c7f83b..7d575c89 100644
--- a/frontend/src/api.ts
+++ b/frontend/src/api.ts
@@ -154,7 +154,7 @@ export async function createAssignment (
export async function submitFeedbackForAssignment ({ feedback, assignment }:
{ feedback: Partial<CreateUpdateFeedback>, assignment: Assignment}): Promise<CreateUpdateFeedback> {
- return (await ax.post(`/api/assignment/${assignment.pk}/finish`, feedback)).data
+ return (await ax.post(`/api/assignment/${assignment.pk}/finish/`, feedback)).data
}
export async function submitUpdatedFeedback ({ feedback }:
diff --git a/frontend/src/components/submission_notes/SubmissionCorrection.vue b/frontend/src/components/submission_notes/SubmissionCorrection.vue
index 69be8828..150e8929 100644
--- a/frontend/src/components/submission_notes/SubmissionCorrection.vue
+++ b/frontend/src/components/submission_notes/SubmissionCorrection.vue
@@ -141,10 +141,11 @@ export default {
this.loading = true
SubmissionNotes.submitFeedback({
isFinal: isFinal
- }).then(feedback => {
+ }).then(_ => {
SubmissionNotes.RESET_UPDATED_FEEDBACK()
- SubmissionNotes.SET_ORIG_FEEDBACK(feedback)
this.$emit('feedbackCreated')
+ this.$emit('feedbackChanged')
+ SubmissionNotes.RESET_MARKED_COMMENTS_FOR_DELETE()
}).catch(err => {
// ignore trivial errors as those are handled
// by an interceptor
@@ -157,8 +158,6 @@ export default {
duration: -1
})
}).finally(() => {
- this.$emit('feedbackChanged')
- SubmissionNotes.RESET_MARKED_COMMENTS_FOR_DELETE()
this.loading = false
})
},
diff --git a/frontend/src/store/modules/submission-notes.ts b/frontend/src/store/modules/submission-notes.ts
index 50fa49b0..e65f6794 100644
--- a/frontend/src/store/modules/submission-notes.ts
+++ b/frontend/src/store/modules/submission-notes.ts
@@ -199,7 +199,7 @@ Promise<AxiosResponse<void>[]> {
const assignment = Subscriptions.state.currentAssignment
if (assignment) {
await api.submitFeedbackForAssignment({ feedback , assignment})
- } else if (state.origFeedback) {
+ } else if (state.hasOrigFeedback) {
feedback.pk = state.origFeedback.pk
await api.submitUpdatedFeedback(<{ feedback: CreateUpdateFeedback }>{ feedback })
} else {
--
GitLab
From 4ef9067a0b0b26f987aea2148839d16390b01047 Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Thu, 15 Aug 2019 17:47:28 +0200
Subject: [PATCH 8/8] Fixed screenshots
---
.gitlab-ci.yml | 3 ++-
functional_tests/.gitignore | 2 +-
functional_tests/test_export_modal.py | 7 +++++--
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index f4982209..ff2b57e4 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -108,7 +108,8 @@ test_frontend:
- HEADLESS_TESTS=True pytest --ds=grady.settings.test functional_tests
artifacts:
paths:
- - .screenshots/
+ - functional_tests/screenshots/
+ when: on_failure
expire_in: 30 days
test_frontend_unit:
diff --git a/functional_tests/.gitignore b/functional_tests/.gitignore
index 8380e4bf..975c04db 100644
--- a/functional_tests/.gitignore
+++ b/functional_tests/.gitignore
@@ -1,3 +1,3 @@
export.json
export.csv
-
+screenshots
diff --git a/functional_tests/test_export_modal.py b/functional_tests/test_export_modal.py
index 4754e72a..92d08bcc 100644
--- a/functional_tests/test_export_modal.py
+++ b/functional_tests/test_export_modal.py
@@ -26,6 +26,7 @@ def expect_file_to_be_downloaded(path):
JSON_EXPORT_FILE = os.path.join(os.path.dirname(__file__), 'export.json')
+SCREENSHOTS = os.path.join(os.path.dirname(__file__), 'screenshots')
class ExportTestModal(LiveServerTestCase):
@@ -56,12 +57,14 @@ class ExportTestModal(LiveServerTestCase):
def tearDown(self):
try:
- os.mkdir('.screenshots')
+ os.mkdir(SCREENSHOTS)
except FileExistsError:
pass
for method, error in self._outcome.errors:
if error:
- self.browser.get_screenshot_as_file(".screenshots/" + self.id() + ".png")
+ self.browser.get_screenshot_as_file(
+ os.path.join(SCREENSHOTS, self.id() + ".png")
+ )
reset_browser_after_test(self.browser, self.live_server_url)
def _login(self):
--
GitLab