From 8dc5d7dfaf0247e17c204a2d1b3dc9f2b6671a1d Mon Sep 17 00:00:00 2001 From: Ramon Wenger Date: Mon, 15 Oct 2018 20:08:22 +0200 Subject: [PATCH 1/3] Add initial tests for assignment --- server/assignments/factories.py | 16 ++++ server/assignments/tests.py | 3 - server/assignments/tests/__init__.py | 0 .../tests/test_assignment_permissions.py | 86 +++++++++++++++++++ server/users/managers.py | 9 +- server/users/services.py | 18 +++- 6 files changed, 120 insertions(+), 12 deletions(-) create mode 100644 server/assignments/factories.py delete mode 100644 server/assignments/tests.py create mode 100644 server/assignments/tests/__init__.py create mode 100644 server/assignments/tests/test_assignment_permissions.py diff --git a/server/assignments/factories.py b/server/assignments/factories.py new file mode 100644 index 00000000..2c0638e1 --- /dev/null +++ b/server/assignments/factories.py @@ -0,0 +1,16 @@ +import random + +import factory + +from books.factories import ModuleFactory +from .models import Assignment + +from core.factories import fake + +class AssignmentFactory(factory.django.DjangoModelFactory): + class Meta: + model = Assignment + + title = factory.LazyAttribute(lambda x: fake.sentence(nb_words=random.randint(4, 8))) + assignment = factory.LazyAttribute(lambda x: fake.sentence(nb_words=random.randint(4, 8))) + module = factory.SubFactory(ModuleFactory) diff --git a/server/assignments/tests.py b/server/assignments/tests.py deleted file mode 100644 index 7ce503c2..00000000 --- a/server/assignments/tests.py +++ /dev/null @@ -1,3 +0,0 @@ -from django.test import TestCase - -# Create your tests here. diff --git a/server/assignments/tests/__init__.py b/server/assignments/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/server/assignments/tests/test_assignment_permissions.py b/server/assignments/tests/test_assignment_permissions.py new file mode 100644 index 00000000..755c9dcf --- /dev/null +++ b/server/assignments/tests/test_assignment_permissions.py @@ -0,0 +1,86 @@ +from unittest import TestCase + +from django.test import RequestFactory +from graphene.test import Client +from graphql_relay import to_global_id + +from api.utils import get_graphql_mutation +from assignments.models import Assignment, StudentSubmission +from books.factories import ModuleFactory +from ..factories import AssignmentFactory +from users.models import User +from users.services import create_users +from api.schema import schema + + +class AssignmentPermissionsTestCase(TestCase): + def setUp(self): + create_users() + + self.teacher = User.objects.get(username='teacher') + self.teacher2 = User.objects.get(username='teacher2') + self.student1 = User.objects.get(username='student1') + self.student2 = User.objects.get(username='student2') + self.assignment = AssignmentFactory( + owner=self.teacher + ) + + request = RequestFactory().get('/') + request.user = self.student1 + self.client = Client(schema=schema, context_value=request) + + """ + to test: + create assignment + should be visible to teachers + should be visible to students + student1 submits result + teacher1 should see result + student1 should see result + student2 should not see result + teacher2 should not see result + """ + + def test_count(self): + self.assertEqual(Assignment.objects.count(), 1) + + def test_submit_submission(self): + """ + id = graphene.ID(required=True) + answer = graphene.String(required=True) + document = graphene.String() + final = graphene.Boolean() + """ + + id = to_global_id('Assignment', self.assignment.pk) + + mutation = ''' + mutation UpdateAssignment($input: UpdateAssignmentInput!) { + updateAssignment(input: $input){ + updatedAssignment { + id + title + assignment + submission { + id + text + final + document + } + } + } + } + + ''' + + result = self.client.execute(mutation, variables={ + 'input': { + "assignment": { + "id": id, + "answer": 'Halo', + "final": True + } + } + }) + self.assertIsNone(result.get('errors')) + self.assertEqual(StudentSubmission.objects.count(), 1) diff --git a/server/users/managers.py b/server/users/managers.py index 90afec13..8bd35110 100644 --- a/server/users/managers.py +++ b/server/users/managers.py @@ -33,20 +33,13 @@ class RoleManager(models.Manager): def create_default_roles(self): for key, value in self.DEFAULT_ROLES.items(): - role = self.create(name=value, key=key) - role.save() + role, created = self.get_or_create(name=value, key=key) can_manage_school_class_content, = self._create_default_permissions() if key == "teacher": role.role_permission.add(can_manage_school_class_content.id) - # elif key == "school_admin": - # role.role_permission.add() - # - # elif key == "student": - # role.role_permission.add() - def get_default_teacher_role(self): return self._get_default_role(self.TEACHER_KEY) diff --git a/server/users/services.py b/server/users/services.py index 85eb2ee7..2c91049e 100644 --- a/server/users/services.py +++ b/server/users/services.py @@ -13,10 +13,26 @@ def create_users(data=None): teacher = UserFactory(username='teacher') UserRole.objects.create(user=teacher, role=teacher_role) + students = [] for i in range(1, 7): student = UserFactory(username='student{}'.format(i)) UserRole.objects.create(user=student, role=student_role) - SchoolClassFactory(users=[teacher, student]) + students.append(student) + + SchoolClassFactory( + users=[teacher] + students, + year='2018', + name='skillbox' + ) + teacher2 = UserFactory(username='teacher2') + SchoolClassFactory( + users=[teacher2], + year='2018', + name='second_class' + ) + + + else: for school_class in data: From 5218a3a8673747fdb050d6ce19bfd8952415d101 Mon Sep 17 00:00:00 2001 From: Ramon Wenger Date: Tue, 16 Oct 2018 13:36:32 +0200 Subject: [PATCH 2/3] Limit submissions to teacher of student's class --- server/assignments/schema/types.py | 5 +- .../tests/test_assignment_permissions.py | 79 +++++++++++++++---- server/users/models.py | 5 +- server/users/services.py | 1 + 4 files changed, 70 insertions(+), 20 deletions(-) diff --git a/server/assignments/schema/types.py b/server/assignments/schema/types.py index 53f52636..eed00bee 100644 --- a/server/assignments/schema/types.py +++ b/server/assignments/schema/types.py @@ -25,4 +25,7 @@ class AssignmentNode(DjangoObjectType): #todo: restrict for students def resolve_submissions(self, info, **kwargs): - return self.submissions.all() + user = info.context.user + if user.has_perm('users.can_manage_school_class_content'): + return self.submissions.filter(student__in=user.users_in_same_school_class()) + return [] diff --git a/server/assignments/tests/test_assignment_permissions.py b/server/assignments/tests/test_assignment_permissions.py index 755c9dcf..fcd3f74f 100644 --- a/server/assignments/tests/test_assignment_permissions.py +++ b/server/assignments/tests/test_assignment_permissions.py @@ -25,9 +25,8 @@ class AssignmentPermissionsTestCase(TestCase): owner=self.teacher ) - request = RequestFactory().get('/') - request.user = self.student1 - self.client = Client(schema=schema, context_value=request) + self.assignment_id = to_global_id('AssignmentNode', self.assignment.pk) + self.module_id = to_global_id('ModuleNode', self.assignment.module.pk) """ to test: @@ -41,19 +40,12 @@ class AssignmentPermissionsTestCase(TestCase): teacher2 should not see result """ - def test_count(self): - self.assertEqual(Assignment.objects.count(), 1) - - def test_submit_submission(self): - """ - id = graphene.ID(required=True) - answer = graphene.String(required=True) - document = graphene.String() - final = graphene.Boolean() - """ - - id = to_global_id('Assignment', self.assignment.pk) + def _create_client(self, user): + request = RequestFactory().get('/') + request.user = user + return Client(schema=schema, context_value=request) + def _submit_submission(self): mutation = ''' mutation UpdateAssignment($input: UpdateAssignmentInput!) { updateAssignment(input: $input){ @@ -73,14 +65,67 @@ class AssignmentPermissionsTestCase(TestCase): ''' - result = self.client.execute(mutation, variables={ + client = self._create_client(self.student1) + + return client.execute(mutation, variables={ 'input': { "assignment": { - "id": id, + "id": self.assignment_id, "answer": 'Halo', "final": True } } }) + + def test_permissions(self): + self.assertTrue(self.teacher.has_perm('users.can_manage_school_class_content')) + self.assertTrue(self.teacher2.has_perm('users.can_manage_school_class_content')) + self.assertFalse(self.student1.has_perm('users.can_manage_school_class_content')) + self.assertFalse(self.student2.has_perm('users.can_manage_school_class_content')) + + def test_count(self): + self.assertEqual(Assignment.objects.count(), 1) + + def test_submit_submission(self): + result = self._submit_submission() self.assertIsNone(result.get('errors')) self.assertEqual(StudentSubmission.objects.count(), 1) + + def _test_visibility(self, user, count): + self._submit_submission() + client = self._create_client(user) + query = ''' + query AssignmentWithSubmissions($id: ID!) { + assignment(id: $id) { + title + submissions { + id + text + document + student { + firstName + lastName + } + } + } + } + ''' + result = client.execute(query, variables={ + 'id': self.assignment_id + }) + + self.assertIsNone(result.get('errors')) + self.assertEqual(len(result.get('data').get('assignment').get('submissions')), count) + + + def test_visible_for_teacher(self): + self._test_visibility(self.teacher, 1) + + def test_visible_for_teacher2(self): + self._test_visibility(self.teacher2, 0) + + def test_visible_for_student1(self): + self._test_visibility(self.student1, 0) + + def test_visible_for_student2(self): + self._test_visibility(self.student2, 0) diff --git a/server/users/models.py b/server/users/models.py index 764d8e96..1079f5fc 100644 --- a/server/users/models.py +++ b/server/users/models.py @@ -31,6 +31,9 @@ class User(AbstractUser): def has_perm(self, perm, obj=None): return super(User, self).has_perm(perm, obj) or perm in self.get_all_permissions(obj) + def users_in_same_school_class(self): + return User.objects.filter(school_classes__users=self.pk) + class SchoolClass(models.Model): name = models.CharField(max_length=100, blank=False, null=False) @@ -43,7 +46,6 @@ class SchoolClass(models.Model): return 'SchoolClass {}-{}-{}'.format(self.id, self.name, self.year) - class Role(models.Model): key = models.CharField(_('Key'), max_length=100, blank=False, null=False, unique=True) name = models.CharField(_('Name'), max_length=100, blank=False, null=False) @@ -79,7 +81,6 @@ class Role(models.Model): ) - class UserRole(models.Model): user = models.ForeignKey(User, blank=False, null=False, on_delete=models.CASCADE, related_name='user_roles') role = models.ForeignKey(Role, blank=False, null=False, on_delete=models.CASCADE, related_name='user_roles') diff --git a/server/users/services.py b/server/users/services.py index 2c91049e..0c4d43c0 100644 --- a/server/users/services.py +++ b/server/users/services.py @@ -25,6 +25,7 @@ def create_users(data=None): name='skillbox' ) teacher2 = UserFactory(username='teacher2') + UserRole.objects.create(user=teacher2, role=teacher_role) SchoolClassFactory( users=[teacher2], year='2018', From a4479727ce578b008e11ed8d2274304c91e5091b Mon Sep 17 00:00:00 2001 From: Ramon Wenger Date: Tue, 16 Oct 2018 13:52:40 +0200 Subject: [PATCH 3/3] Add another unit test --- .../tests/test_assignment_permissions.py | 36 +++++++++---------- server/users/services.py | 4 ++- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/server/assignments/tests/test_assignment_permissions.py b/server/assignments/tests/test_assignment_permissions.py index fcd3f74f..f45fff12 100644 --- a/server/assignments/tests/test_assignment_permissions.py +++ b/server/assignments/tests/test_assignment_permissions.py @@ -1,6 +1,4 @@ -from unittest import TestCase - -from django.test import RequestFactory +from django.test import RequestFactory, TestCase from graphene.test import Client from graphql_relay import to_global_id @@ -21,6 +19,7 @@ class AssignmentPermissionsTestCase(TestCase): self.teacher2 = User.objects.get(username='teacher2') self.student1 = User.objects.get(username='student1') self.student2 = User.objects.get(username='student2') + self.student_second_class = User.objects.get(username='student_second_class') self.assignment = AssignmentFactory( owner=self.teacher ) @@ -28,24 +27,12 @@ class AssignmentPermissionsTestCase(TestCase): self.assignment_id = to_global_id('AssignmentNode', self.assignment.pk) self.module_id = to_global_id('ModuleNode', self.assignment.module.pk) - """ - to test: - create assignment - should be visible to teachers - should be visible to students - student1 submits result - teacher1 should see result - student1 should see result - student2 should not see result - teacher2 should not see result - """ - def _create_client(self, user): request = RequestFactory().get('/') request.user = user return Client(schema=schema, context_value=request) - def _submit_submission(self): + def _submit_submission(self, user=None): mutation = ''' mutation UpdateAssignment($input: UpdateAssignmentInput!) { updateAssignment(input: $input){ @@ -65,7 +52,10 @@ class AssignmentPermissionsTestCase(TestCase): ''' - client = self._create_client(self.student1) + if user is None: + client = self._create_client(self.student1) + else: + client = self._create_client(user) return client.execute(mutation, variables={ 'input': { @@ -92,7 +82,6 @@ class AssignmentPermissionsTestCase(TestCase): self.assertEqual(StudentSubmission.objects.count(), 1) def _test_visibility(self, user, count): - self._submit_submission() client = self._create_client(user) query = ''' query AssignmentWithSubmissions($id: ID!) { @@ -117,15 +106,24 @@ class AssignmentPermissionsTestCase(TestCase): self.assertIsNone(result.get('errors')) self.assertEqual(len(result.get('data').get('assignment').get('submissions')), count) - def test_visible_for_teacher(self): + self._submit_submission() self._test_visibility(self.teacher, 1) def test_visible_for_teacher2(self): + self._submit_submission() self._test_visibility(self.teacher2, 0) def test_visible_for_student1(self): + self._submit_submission() self._test_visibility(self.student1, 0) def test_visible_for_student2(self): + self._submit_submission() self._test_visibility(self.student2, 0) + + def test_advanced_visibility(self): + self._submit_submission(self.student1) + self._submit_submission(self.student_second_class) + self._test_visibility(self.teacher, 1) + self._test_visibility(self.teacher2, 1) diff --git a/server/users/services.py b/server/users/services.py index 0c4d43c0..60b796fc 100644 --- a/server/users/services.py +++ b/server/users/services.py @@ -26,8 +26,10 @@ def create_users(data=None): ) teacher2 = UserFactory(username='teacher2') UserRole.objects.create(user=teacher2, role=teacher_role) + student_second_class = UserFactory(username='student_second_class') + UserRole.objects.create(user=student_second_class, role=student_role) SchoolClassFactory( - users=[teacher2], + users=[teacher2, student_second_class], year='2018', name='second_class' )