diff --git a/server/basicknowledge/queries.py b/server/basicknowledge/queries.py index 38bf75a8..9faf1726 100644 --- a/server/basicknowledge/queries.py +++ b/server/basicknowledge/queries.py @@ -64,4 +64,4 @@ class InstrumentQuery(object): return BasicKnowledge.objects.all().live() def resolve_instrument_types(self, info, **kwargs): - return InstrumentType.objects.filter(instruments__isnull=False).distinct() + return InstrumentType.objects.filter(instruments__isnull=False).order_by('name').distinct() diff --git a/server/core/mixins.py b/server/core/mixins.py index f444085e..4fb87c0b 100644 --- a/server/core/mixins.py +++ b/server/core/mixins.py @@ -1,4 +1,5 @@ import graphene +from graphql_relay import to_global_id class HiddenForMixin: @@ -18,3 +19,12 @@ class VisibleForMixin: class HiddenAndVisibleForMixin(HiddenForMixin, VisibleForMixin): pass + + +class GraphqlNodeMixin: + def default_node_name(self): + return f'{self.__class__.__name__}Node' + + @property + def graphql_id(self): + return to_global_id(self.default_node_name(), self.id) diff --git a/server/portfolio/models.py b/server/portfolio/models.py index 7cae2516..54565588 100644 --- a/server/portfolio/models.py +++ b/server/portfolio/models.py @@ -1,19 +1,11 @@ from django.contrib.auth import get_user_model from django.db import models from django_extensions.db.models import TitleSlugDescriptionModel -from graphql_relay import to_global_id +from core.mixins import GraphqlNodeMixin from users.models import User -class GraphqlNodeMixin: - def default_node_name(self): - return f'{self.__class__.__name__}Node' - - @property - def graphql_id(self): - return to_global_id(self.default_node_name(), self.id) - class Project(TitleSlugDescriptionModel, GraphqlNodeMixin): objectives = models.TextField(blank=True) appearance = models.CharField(blank=True, null=False, max_length=255) @@ -29,6 +21,7 @@ class Project(TitleSlugDescriptionModel, GraphqlNodeMixin): self.final and self.student.get_teacher().id == user.id ) + class ProjectEntry(models.Model): activity = models.TextField(blank=True) reflection = models.TextField(blank=True) diff --git a/server/portfolio/mutations.py b/server/portfolio/mutations.py index 66d29fba..8ee1cdfd 100644 --- a/server/portfolio/mutations.py +++ b/server/portfolio/mutations.py @@ -8,6 +8,7 @@ from portfolio.inputs import AddProjectArgument, AddProjectEntryArgument, Update from portfolio.models import Project, ProjectEntry from portfolio.schema import ProjectEntryNode, ProjectNode from portfolio.serializers import ProjectEntrySerializer, ProjectSerializer +from users.models import UserSetting def check_owner(user, project): @@ -24,23 +25,11 @@ class MutateProject(relay.ClientIDMutation): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): - data = kwargs.get('project') - data['student'] = info.context.user.id + raise Exception('Must be subclassed') - if data.get('id') is not None: - entity = get_object(Project, data['id']) - serializer = ProjectSerializer(entity, data=data) - else: - serializer = ProjectSerializer(data=data) - if serializer.is_valid(): - serializer.save() - props = { - 'project': serializer.instance, - 'errors': None - } - return cls(**props) - - return cls(errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) + @classmethod + def create_error_response(cls, serializer): + return cls(room=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) class AddProject(MutateProject): @@ -52,19 +41,45 @@ class AddProject(MutateProject): def mutate_and_get_payload(cls, root, info, **kwargs): data = kwargs.get('project') data['student'] = info.context.user.id + data['school_class'] = info.context.user.selected_class.id serializer = ProjectSerializer(data=data) if serializer.is_valid(): serializer.save() return cls(project=serializer.instance) - return cls(room=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) + return cls.create_error_response(serializer) class UpdateProject(MutateProject): class Input: project = graphene.Argument(UpdateProjectArgument) + @classmethod + def mutate_and_get_payload(cls, root, info, **kwargs): + data = kwargs.get('project') + + cls.user_is_owner(data, info.context.user) + data['student'] = info.context.user.id + + entity = get_object(Project, data['id']) + serializer = ProjectSerializer(entity, data=data) + if serializer.is_valid(): + serializer.save() + props = { + 'project': serializer.instance, + 'errors': None + } + return cls(**props) + + return cls.create_error_response(serializer) + + @classmethod + def user_is_owner(cls, data, user): + project = get_object(Project, data['id']) + if not project or not project.student == user: + raise PermissionDenied('not allowed') + class MutateProjectEntry(relay.ClientIDMutation): errors = graphene.List(graphene.String) @@ -73,7 +88,6 @@ class MutateProjectEntry(relay.ClientIDMutation): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): data = kwargs.get('project_entry') - project = None if data.get('project') is not None: project = get_object(Project, data.get('project')) diff --git a/server/portfolio/schema.py b/server/portfolio/schema.py index c647b510..a578e41f 100644 --- a/server/portfolio/schema.py +++ b/server/portfolio/schema.py @@ -6,6 +6,7 @@ from graphene_django import DjangoObjectType from api.utils import get_by_id_or_slug from portfolio.models import Project, ProjectEntry from users.models import Role, UserRole, User +from users.schema import PublicUserNode class ProjectEntryNode(DjangoObjectType): @@ -19,6 +20,7 @@ class ProjectNode(DjangoObjectType): pk = graphene.Int() entries_count = graphene.Int() entries = graphene.List(ProjectEntryNode) + owner = graphene.Field(PublicUserNode) class Meta: model = Project @@ -52,8 +54,8 @@ class PortfolioQuery(object): return Project.objects.filter(student=user) def resolve_project(self, info, **kwargs): - user = info.context.user # type: User - project = get_by_id_or_slug(Project, **kwargs) #type: Project + user = info.context.user # type: User + project = get_by_id_or_slug(Project, **kwargs) # type: Project if project.is_viewable_by(user): return project diff --git a/server/portfolio/tests/test_project_mutations.py b/server/portfolio/tests/test_project_mutations.py index ddcd3638..7008a521 100644 --- a/server/portfolio/tests/test_project_mutations.py +++ b/server/portfolio/tests/test_project_mutations.py @@ -3,6 +3,7 @@ from graphene.test import Client from graphql_relay import to_global_id from api.schema import schema +from core.tests.base_test import SkillboxTestCase from portfolio.factories import ProjectFactory from users.factories import SchoolClassFactory from users.models import User @@ -10,7 +11,7 @@ from users.services import create_users from api.test_utils import create_client, DefaultUserTestCase from portfolio.models import Project -class ProjectQuery(TestCase): +class ProjectQuery(SkillboxTestCase): def setUp(self): create_users() self.teacher = User.objects.get(username='teacher') @@ -49,7 +50,6 @@ class ProjectQuery(TestCase): self.assertEqual(Project.objects.count(), 0) def test_should_not_be_able_to_delete_other_projects(self): - self.assertEqual(Project.objects.count(), 1) request = RequestFactory().get('/') request.user = self.student2 @@ -58,6 +58,68 @@ class ProjectQuery(TestCase): result = self.client.execute(self.mutation, variables=self.variables) self.assertEqual(result.get('errors')[0]['message'], 'Permission denied: Incorrect project') + def test_should_not_be_able_to_edit_other_projects(self): + self.assertEqual(Project.objects.count(), 1) + request = RequestFactory().get('/') + request.user = self.student2 + self.client = Client(schema=schema, context_value=request) + mutation = ''' +mutation UpdateProjectMutation($input: UpdateProjectInput!){ + updateProject(input: $input) { + project { + id + } + } +} +''' + # project: + # { + # title: String + # description: String + # objectives: String + # appearance: String + # id: ID! + # final: Boolean + # } + input = { + 'project': { + 'id': self.project1.graphql_id, + 'title': 'BAD! THIS IS BAD!' + } + } + result = self.get_client(self.student2).get_result(mutation, variables={ + 'input': input + }) + self.assertIsNotNone(result.errors) + self.assertTrue('message' in result.errors[0]) + self.assertEqual(result.errors[0]['message'], 'not allowed') + + def test_owner_can_edit(self): + self.assertEqual(Project.objects.count(), 1) + request = RequestFactory().get('/') + request.user = self.student + self.client = Client(schema=schema, context_value=request) + mutation = ''' +mutation UpdateProjectMutation($input: UpdateProjectInput!){ + updateProject(input: $input) { + project { + id + } + } +} +''' + + input = { + 'project': { + 'id': self.project1.graphql_id, + 'title': 'Good! THIS IS good!' + } + } + result = self.get_client(self.student).get_result(mutation, variables={ + 'input': input + }) + self.assertIsNone(result.errors) + class ProjectMutationsTestCase(DefaultUserTestCase): def test_add_project(self): diff --git a/server/portfolio/tests/test_project_query.py b/server/portfolio/tests/test_project_query.py index b49c6452..9c66de0c 100644 --- a/server/portfolio/tests/test_project_query.py +++ b/server/portfolio/tests/test_project_query.py @@ -1,3 +1,5 @@ +from graphql_relay import to_global_id + from core.tests.base_test import SkillboxTestCase from portfolio.factories import ProjectFactory from portfolio.models import Project @@ -13,7 +15,7 @@ query ProjectQuery($id: ID!) { """ -class ProjectQueryTestCaswe(SkillboxTestCase): +class ProjectQueryTestCase(SkillboxTestCase): def _test_direct_project_access(self, user: User, should_have_access: bool): result = self.get_client(user).get_result(project_query, variables={ 'id': self.project1.graphql_id @@ -30,6 +32,7 @@ class ProjectQueryTestCaswe(SkillboxTestCase): school_class2 = SchoolClassFactory(users=[self.teacher2, self.student2]) self.project1 = ProjectFactory(student=self.student1) + self.project1_id = to_global_id('ProjectNode', self.project1.id) self.query = ''' query ProjectsQuery { projects { @@ -113,3 +116,19 @@ class ProjectQueryTestCaswe(SkillboxTestCase): self._test_direct_project_access(self.teacher2, False) # non-owner can't access project self._test_direct_project_access(self.student2, False) + + def test_project_owner_can_view(self): + query = """ +query ProjectQuery($id: ID!) { + project(id: $id) { + id + student { + email + } + } +} + """ + result = self.get_client(self.student1).get_result(query, variables={ + 'id': self.project1.graphql_id + }) + self.assertEqual(result.data['project']['student']['email'], self.student1.email) diff --git a/server/rooms/models.py b/server/rooms/models.py index d8d7d0e8..68f8c902 100644 --- a/server/rooms/models.py +++ b/server/rooms/models.py @@ -5,10 +5,11 @@ from wagtail.core.fields import StreamField from books.blocks import DocumentBlock, ImageUrlBlock, LinkBlock, VideoBlock from books.models import TextBlock +from core.mixins import GraphqlNodeMixin from users.models import SchoolClass -class Room(TitleSlugDescriptionModel): +class Room(TitleSlugDescriptionModel, GraphqlNodeMixin): class Meta: verbose_name = 'Raum' verbose_name_plural = 'Räume' diff --git a/server/rooms/mutations.py b/server/rooms/mutations.py index 91401fc5..fa25f783 100644 --- a/server/rooms/mutations.py +++ b/server/rooms/mutations.py @@ -39,7 +39,7 @@ class AddRoom(MutateRoom): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): user = info.context.user - if not user.has_perm('users.can_manage_school_class_content'): + if not user.is_teacher(): return cls(room=None, errors=['not allowed']) room_data = kwargs.get('room') @@ -85,12 +85,26 @@ class MutateRoomEntry(relay.ClientIDMutation): @classmethod def mutate_and_get_payload(cls, root, info, **kwargs): room_entry_data = kwargs.get('room_entry') + room = None if room_entry_data.get('room') is not None: - room_entry_data['room'] = get_object(Room, room_entry_data.get('room')).id + room = get_object(Room, room_entry_data.get('room')) + room_entry_data['room'] = room.id if room_entry_data.get('id') is not None: - # update path + serializer = cls.update_room_entry(info, room_entry_data) + else: + serializer = cls.add_room_entry(info, room_entry_data, room) + + if serializer.is_valid(): + serializer.save() + + return cls(room_entry=serializer.instance) + + return cls(room_entry=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) + + @classmethod + def update_room_entry(cls, info, room_entry_data): instance = get_object(RoomEntry, room_entry_data.get('id')) if not instance.room.school_class.is_user_in_schoolclass(info.context.user): @@ -99,18 +113,16 @@ class MutateRoomEntry(relay.ClientIDMutation): if instance.author.pk != info.context.user.pk: raise Exception('You are not the author') - serializer = RoomEntrySerializer(instance, data=room_entry_data, partial=True) - else: - # add path - room_entry_data['author'] = info.context.user.pk - serializer = RoomEntrySerializer(data=room_entry_data) + return RoomEntrySerializer(instance, data=room_entry_data, partial=True) - if serializer.is_valid(): - serializer.save() + @classmethod + def add_room_entry(cls, info, room_entry_data, room): - return cls(room_entry=serializer.instance) + if not room or not room.school_class.is_user_in_schoolclass(info.context.user): + raise PermissionDenied('You are in the wrong class') - return cls(room_entry=None, errors=['{}: {}'.format(key, value) for key, value in serializer.errors.items()]) + room_entry_data['author'] = info.context.user.pk + return RoomEntrySerializer(data=room_entry_data) class AddRoomEntry(MutateRoomEntry): @@ -165,7 +177,6 @@ class UpdateRoomVisibility(relay.ClientIDMutation): return cls(success=True, room=room) - class AddComment(relay.ClientIDMutation): class Input: comment = graphene.String(required=True) diff --git a/server/rooms/tests/test_new_room_mutation.py b/server/rooms/tests/test_new_room_mutation.py index 8dcfb9c1..7ed22dba 100644 --- a/server/rooms/tests/test_new_room_mutation.py +++ b/server/rooms/tests/test_new_room_mutation.py @@ -2,6 +2,8 @@ from graphql_relay import from_global_id from core.tests.base_test import SkillboxTestCase from core.tests.helpers import GQLResult +from rooms.models import Room + class GQLRoom: def __init__(self, room_data): @@ -30,29 +32,30 @@ class AddRoomResult: class NewRoomMutationTestCase(SkillboxTestCase): def setUp(self) -> None: self.createDefault() + self.mutation = """ + mutation AddRoom($input: AddRoomInput!){ + addRoom(input: $input) { + room { + id + slug + title + entryCount + appearance + description + schoolClass { + id + name + } + } + } + } + """ def test_create_new_room(self): - mutation = """ -mutation AddRoom($input: AddRoomInput!){ - addRoom(input: $input) { - room { - id - slug - title - entryCount - appearance - description - schoolClass { - id - name - } - } - } -} -""" + self.assertEqual(Room.objects.count(), 0) title = 'some title' - appearance='blue' - res = self.get_client().execute(mutation, variables={ + appearance = 'blue' + res = self.get_client().execute(self.mutation, variables={ 'input': { 'room': { 'title': title, @@ -68,5 +71,25 @@ mutation AddRoom($input: AddRoomInput!){ self.assertEqual(room.appearance, appearance) self.assertIsNone(room.description) self.assertEqual(int(from_global_id(room.school_class.get('id'))[1]), self.teacher.selected_class.id) + self.assertEqual(Room.objects.count(), 1) + def test_create_new_room_for_other_school_class(self): + self.assertEqual(Room.objects.count(), 0) + result = self.get_client(self.teacher2).get_result(self.mutation, variables={ + 'input': { + 'room': { + 'title': 'BIG NO NO!', + # description + 'schoolClass': { + 'id': self.school_class.graphql_id + }, + 'appearance': 'red' + } + } + }) + + self.assertEqual(Room.objects.count(), 1) + + room = Room.objects.all()[0] + self.assertEqual(room.school_class.id, self.teacher2.selected_class.id) diff --git a/server/rooms/tests/test_room_entry_mutations.py b/server/rooms/tests/test_room_entry_mutations.py index d20f5c0a..76128b68 100644 --- a/server/rooms/tests/test_room_entry_mutations.py +++ b/server/rooms/tests/test_room_entry_mutations.py @@ -4,12 +4,13 @@ from graphql_relay import to_global_id from api.schema import schema from core.factories import UserFactory +from core.tests.base_test import SkillboxTestCase from rooms.factories import RoomEntryFactory, RoomFactory from rooms.models import RoomEntry from users.factories import SchoolClassFactory -class RoomEntryMutationsTestCase(TestCase): +class RoomEntryMutationsTestCase(SkillboxTestCase): def setUp(self): self.user = UserFactory(username='aschi') self.another_user = UserFactory(username='pesche') @@ -17,6 +18,8 @@ class RoomEntryMutationsTestCase(TestCase): s = SchoolClassFactory(users=[self.user, self.another_user]) s2 = SchoolClassFactory(users=[self.yet_another_user]) self.room_entry = RoomEntryFactory(author=self.user, room=RoomFactory(school_class=s)) + self.room = self.room_entry.room + self.first_school_class = s request = RequestFactory().get('/') request.user = self.user @@ -135,3 +138,48 @@ class RoomEntryMutationsTestCase(TestCase): entry = RoomEntry.objects.get(pk=self.room_entry.pk) self.assertIsNotNone(result.get('errors')) self.assertEqual(entry.title, self.room_entry.title) + + def test_add_room_entry_not_owner_from_other_class(self): + self.assertEqual(RoomEntry.objects.count(), 1) + mutation = """ +fragment RoomEntryParts on RoomEntryNode { + id + slug + title + contents + author { + id + firstName + lastName + avatarUrl + } +} + + +mutation AddRoomEntry($input: AddRoomEntryInput!){ + addRoomEntry(input: $input) { + roomEntry { + ...RoomEntryParts + } + errors + + } +} + """ + # input: + # title = graphene.String(required=True) + # contents = graphene.List(ContentElementInput) + # room = graphene.ID(required=True) + room_entry = { + 'title': 'Bad Actor!', + 'room': self.room.graphql_id + } + + result = self.get_client(self.yet_another_user).get_result(mutation, variables={ + 'input': { + 'roomEntry': room_entry + } + }) + self.assertIsNotNone(result.errors) + self.assertTrue('message' in result.errors[0]) + self.assertEqual(result.errors[0]['message'], 'You are in the wrong class') diff --git a/server/users/models.py b/server/users/models.py index 50a748ba..e87b88dd 100644 --- a/server/users/models.py +++ b/server/users/models.py @@ -14,6 +14,7 @@ from django.utils.functional import cached_property from django.utils.timezone import is_aware, make_aware from django.utils.translation import ugettext_lazy as _ +from core.mixins import GraphqlNodeMixin from users.licenses import MYSKILLBOX_LICENSES from users.managers import LicenseManager, RoleManager, UserManager, UserRoleManager @@ -176,7 +177,7 @@ class Team(GroupWithCode): return self.name -class SchoolClass(GroupWithCode): +class SchoolClass(GroupWithCode, GraphqlNodeMixin): users = models.ManyToManyField(get_user_model(), related_name='school_classes', blank=True, through='users.SchoolClassMember')