diff --git a/server/api/schema.py b/server/api/schema.py index 3cc57896..ca6ff5aa 100644 --- a/server/api/schema.py +++ b/server/api/schema.py @@ -26,7 +26,8 @@ from users.schema import AllUsersQuery, UsersQuery, ProfileMutations class Query(UsersQuery, AllUsersQuery, ModuleRoomsQuery, RoomsQuery, ObjectivesQuery, BookQuery, AssignmentsQuery, - StudentSubmissionQuery, InstrumentQuery, PortfolioQuery, SurveysQuery, AllNewsTeasersQuery, graphene.ObjectType): + StudentSubmissionQuery, InstrumentQuery, PortfolioQuery, SurveysQuery, AllNewsTeasersQuery, + graphene.ObjectType): node = relay.Node.Field() if settings.DEBUG: @@ -34,8 +35,8 @@ class Query(UsersQuery, AllUsersQuery, ModuleRoomsQuery, RoomsQuery, ObjectivesQ class Mutation(BookMutations, RoomMutations, AssignmentMutations, ObjectiveMutations, OauthMutations, - PortfolioMutations, ProfileMutations, SurveyMutations, NoteMutations, SpellCheckMutations, - graphene.ObjectType): + PortfolioMutations, ProfileMutations, SurveyMutations, NoteMutations, SpellCheckMutations, + graphene.ObjectType): if settings.DEBUG: debug = graphene.Field(DjangoDebug, name='_debug') diff --git a/server/assignments/models.py b/server/assignments/models.py index 317d8726..1705c936 100644 --- a/server/assignments/models.py +++ b/server/assignments/models.py @@ -52,6 +52,7 @@ class StudentSubmission(TimeStampedModel): class SubmissionFeedback(TimeStampedModel): text = models.TextField(blank=True) + # teacher who created the feedback teacher = models.ForeignKey(get_user_model(), on_delete=models.CASCADE, related_name='feedbacks') student_submission = models.OneToOneField(StudentSubmission, on_delete=models.CASCADE, primary_key=True, related_name='submission_feedback') diff --git a/server/assignments/schema/mutations.py b/server/assignments/schema/mutations.py index bf2f40f4..33566bef 100644 --- a/server/assignments/schema/mutations.py +++ b/server/assignments/schema/mutations.py @@ -5,7 +5,7 @@ from graphql_relay import from_global_id from rest_framework.exceptions import PermissionDenied from api.utils import get_object -from assignments.models import Assignment, SubmissionFeedback +from assignments.models import Assignment, StudentSubmission, SubmissionFeedback from assignments.schema.types import AssignmentNode, StudentSubmissionNode, SubmissionFeedbackNode from .inputs import AssignmentInput, SubmissionFeedbackInput @@ -57,19 +57,25 @@ class UpdateSubmissionFeedback(relay.ClientIDMutation): submission_feedback_data = kwargs.get('submission_feedback') user = info.context.user student_submission_id = from_global_id(submission_feedback_data['student_submission'])[1] + student_submission = StudentSubmission.objects.get(id=student_submission_id) - if not user.has_perm('users.can_manage_school_class_content'): + if not user.is_teacher() or not student_submission.student in user.users_in_active_school_class(): raise PermissionDenied('Missing permissions') # todo: replace with middleware if user.read_only: raise PermissionError('No valid license') - (submission_feedback, created) = SubmissionFeedback.objects.get_or_create(teacher=user, - student_submission_id=student_submission_id) + (submission_feedback, created) = SubmissionFeedback.objects.get_or_create( + student_submission_id=student_submission_id, + defaults={ + 'teacher': user + }) - final = submission_feedback_data.get('final') if 'final' in submission_feedback_data else submission_feedback.final + final = submission_feedback_data.get( + 'final') if 'final' in submission_feedback_data else submission_feedback.final + submission_feedback.teacher = user submission_feedback.final = final submission_feedback.text = submission_feedback_data.get('text') submission_feedback.save() diff --git a/server/assignments/schema/queries.py b/server/assignments/schema/queries.py index 4e5bb12d..56e1fe0e 100644 --- a/server/assignments/schema/queries.py +++ b/server/assignments/schema/queries.py @@ -1,6 +1,8 @@ import graphene from graphene import relay +from api.utils import get_object +from assignments.models import StudentSubmission from assignments.schema.types import AssignmentNode, StudentSubmissionNode @@ -9,5 +11,14 @@ class AssignmentsQuery(object): assignments = graphene.List(AssignmentNode) -class StudentSubmissionQuery(object): - student_submission = relay.Node.Field(StudentSubmissionNode) + +class StudentSubmissionQuery: + student_submission = graphene.Field(StudentSubmissionNode, id=graphene.ID(required=True)) + + def resolve_student_submission(root, info, *args, **kwargs): + user = info.context.user + id = kwargs.get('id') + submission = get_object(StudentSubmission, id) + if user == submission.student or (user.is_teacher() and submission.student in user.users_in_active_school_class()): + return root + return None diff --git a/server/assignments/schema/types.py b/server/assignments/schema/types.py index 453050af..2ac7814d 100644 --- a/server/assignments/schema/types.py +++ b/server/assignments/schema/types.py @@ -22,24 +22,20 @@ class StudentSubmissionNode(DjangoObjectType): filter_fields = [] interfaces = (relay.Node,) - def resolve_submission_feedback(self, info, **kwargs): - + @staticmethod + def resolve_submission_feedback(root: StudentSubmission, info, **kwargs): user = info.context.user - if not hasattr(self, 'submission_feedback'): + if not hasattr(root, 'submission_feedback'): return None # teacher path - if user.has_perm('users.can_manage_school_class_content'): - if self.submission_feedback.teacher == user: - return self.submission_feedback - else: - raise PermissionDenied('Missing permissions') + if user.is_teacher() and root.student in user.users_in_active_school_class(): + return root.submission_feedback # student path - - if self.submission_feedback.final: - return self.submission_feedback + if root.submission_feedback.final: + return root.submission_feedback return None diff --git a/server/assignments/tests/test_feedback.py b/server/assignments/tests/test_feedback.py index b5f510a8..62f1d875 100644 --- a/server/assignments/tests/test_feedback.py +++ b/server/assignments/tests/test_feedback.py @@ -33,7 +33,7 @@ class SubmissionFeedbackTestCase(SkillboxTestCase): self.student_submission_id = to_global_id('StudentSubmissionNode', self.student_submission.pk) school_class = SchoolClassFactory() - for user in [self.student1, self.teacher, self.teacher2]: + for user in [self.student1, self.teacher]: SchoolClassMember.objects.create( user=user, school_class=school_class @@ -94,6 +94,24 @@ class SubmissionFeedbackTestCase(SkillboxTestCase): 'id': self.assignment_id }) + def _fetch_submission_teacher(self, user): + client = self.get_client(user) + query = ''' + query StudentSubmission($id: ID!) { + studentSubmission(id: $id) { + id + text + document + submissionFeedback { + text + } + } + } + ''' + return client.execute(query, variables={ + 'id': self.student_submission_id + }) + def _fetch_submission_feedback(self, user): client = create_client(user) query = ''' @@ -125,6 +143,8 @@ class SubmissionFeedbackTestCase(SkillboxTestCase): def test_student_cannot_create_feedback(self): result = self._create_submission_feedback(self.student1, False, 'Balalal', self.student_submission_id) self.assertIsNotNone(result.errors) + self.assertEqual(len(result.errors), 1) + self.assertEqual(result.errors[0].get('message'), 'Missing permissions') def test_teacher_can_update_feedback(self): assignment = AssignmentFactory( @@ -146,7 +166,7 @@ class SubmissionFeedbackTestCase(SkillboxTestCase): self.assertTrue(submission_feedback_response.get('final')) self.assertEqual(submission_feedback_response.get('text'), 'Some') - def test_rogue_teacher_cannot_update_feedback(self): + def test_external_teacher_cannot_update_feedback(self): assignment = AssignmentFactory( owner=self.teacher ) @@ -183,11 +203,21 @@ class SubmissionFeedbackTestCase(SkillboxTestCase): self.assertEqual(result.data.get('assignment').get('submissions')[0].get('submissionFeedback') .get('text'), submission_feedback.text) - def test_rogue_teacher_cannot_see_feedback(self): + def test_external_teacher_cannot_see_assignment_with_feedback(self): SubmissionFeedbackFactory(teacher=self.teacher, final=False, student_submission=self.student_submission) self.student_submission.final = True self.student_submission.save() result = self._fetch_assignment_teacher(self.teacher2) - self.assertIsNone(result.data.get('assignment').get('submissions')[0].get('submissionFeedback')) + self.assertEqual(result.data.get('assignment').get('submissions'), []) + + def test_external_teacher_cannot_see_feedback(self): + SubmissionFeedbackFactory(teacher=self.teacher, final=False, + student_submission=self.student_submission) + self.student_submission.final = True + self.student_submission.save() + + result = self._fetch_submission_teacher(self.teacher2) + self.assertIsNone(result.errors) + self.assertIsNone(result.data.get('studentSubmission'))