Update error handling in mutation for school class creation

client/src/graphql/gql/mutations/createClass.gql

@@ -1,9 +1,14 @@
 mutation CreateSchoolClass($input: CreateSchoolClassInput!) {
   createSchoolClass(input: $input) {
-    success
-    schoolClass {
-      id
-      name
+    result {
+      __typename
+      ...on SchoolClassNode {
+        id
+        name
+      }
+      ...on DuplicateName {
+        reason
+      }
     }
   }
 }

server/api/schema.py

@@ -22,8 +22,7 @@ from surveys.schema import SurveysQuery
 from surveys.mutations import SurveyMutations
 from rooms.mutations import RoomMutations
 from rooms.schema import RoomsQuery, ModuleRoomsQuery
-from users.schema import AllUsersQuery, UsersQuery
-from users.mutations import ProfileMutations
+from users.schema import AllUsersQuery, UsersQuery, ProfileMutations
 
 
 class Query(UsersQuery, AllUsersQuery, ModuleRoomsQuery, RoomsQuery, ObjectivesQuery, BookQuery, AssignmentsQuery,

server/api/schema_public.py

@@ -2,7 +2,7 @@ import graphene
 from django.conf import settings
 from graphene_django.debug import DjangoDebug
 
-from users.mutations_public import UserMutations
+from users.schema import UserMutations
 
 
 class PublicMutation(UserMutations, graphene.ObjectType):

server/basicknowledge/queries.py

@@ -2,6 +2,7 @@ import graphene
 from graphene import relay
 from graphene_django import DjangoObjectType
 
+from api.graphene_wagtail import GenericStreamFieldType
 from api.utils import get_object
 from notes.models import InstrumentBookmark
 from notes.schema import InstrumentBookmarkNode
@@ -25,6 +26,7 @@ class InstrumentTypeNode(DjangoObjectType):
 class InstrumentNode(DjangoObjectType):
     bookmarks = graphene.List(InstrumentBookmarkNode)
     type = graphene.Field(InstrumentTypeNode)
+    contents = GenericStreamFieldType()
 
     class Meta:
         model = BasicKnowledge

server/books/models/module.py

@@ -52,6 +52,7 @@ class Module(StrictHierarchyPage):
     parent_page_types = ['books.Topic']
     subpage_types = ['books.Chapter']
 
+    #todo: isn't this a duplicate definition?
     def get_child_ids(self):
         return self.get_children().values_list('id', flat=True)
 

server/core/tests/base_test.py

@@ -29,5 +29,3 @@ class SkillboxTestCase(TestCase):
             user = self.teacher
         request.user = user
         return GQLClient(schema=schema, context_value=request)
-
-

server/schema.graphql

@@ -181,9 +181,9 @@ input AssignmentInput {
 }
 
 type AssignmentNode implements Node {
+  id: ID!
   created: DateTime!
   modified: DateTime!
-  id: ID!
   title: String!
   assignment: String!
   solution: String
@@ -197,9 +197,9 @@ type AssignmentNode implements Node {
 }
 
 type ChapterBookmarkNode implements Node {
+  id: ID!
   user: PrivateUserNode!
   note: NoteNode
-  id: ID!
   chapter: ChapterNode!
 }
 
@@ -343,11 +343,12 @@ input CreateSchoolClassInput {
 }
 
 type CreateSchoolClassPayload {
-  success: Boolean
-  schoolClass: SchoolClassNode
+  result: CreateSchoolClassResult
   clientMutationId: String
 }
 
+union CreateSchoolClassResult = SchoolClassNode | DuplicateName
+
 input CreateSnapshotInput {
   module: String!
   selectedClass: ID!
@@ -463,6 +464,10 @@ type DjangoDebugSQL {
   encoding: String
 }
 
+type DuplicateName {
+  reason: String
+}
+
 type FieldError {
   code: String
 }
@@ -482,9 +487,9 @@ enum InputTypes {
 }
 
 type InstrumentBookmarkNode implements Node {
+  id: ID!
   user: PrivateUserNode!
   note: NoteNode
-  id: ID!
   uuid: UUID
   instrument: InstrumentNode!
 }
@@ -555,9 +560,9 @@ type Logout {
 }
 
 type ModuleBookmarkNode {
+  id: ID!
   user: PrivateUserNode!
   note: NoteNode
-  id: ID!
   module: ModuleNode!
 }
 
@@ -869,10 +874,10 @@ type Query {
 }
 
 type RoomEntryNode implements Node {
+  id: ID!
   title: String!
   description: String
   slug: String!
-  id: ID!
   room: RoomNode!
   author: PublicUserNode
   contents: GenericStreamFieldType
@@ -891,10 +896,10 @@ type RoomEntryNodeEdge {
 }
 
 type RoomNode implements Node {
+  id: ID!
   title: String!
   description: String
   slug: String!
-  id: ID!
   schoolClass: SchoolClassNode!
   appearance: String!
   userCreated: Boolean!
@@ -1017,9 +1022,9 @@ type SpellCheckStepNode {
 }
 
 type StudentSubmissionNode implements Node {
+  id: ID!
   created: DateTime!
   modified: DateTime!
-  id: ID!
   text: String!
   document: String!
   assignment: AssignmentNode!
@@ -1088,10 +1093,10 @@ type SyncModuleVisibilityPayload {
 }
 
 type TeamNode implements Node {
-  name: String!
-  code: String
   id: ID!
+  name: String!
   isDeleted: Boolean!
+  code: String
   creator: PrivateUserNode
   members: [PublicUserNode]
   pk: Int

server/users/schema/__init__.py

@@ -0,0 +1,4 @@
+from .queries import *
+from .types import *
+from .mutations import *
+from .mutations_public import *

server/users/schema/mutations.py

@@ -1,6 +1,7 @@
 import graphene
 from django.contrib.auth import update_session_auth_hash
 from django.core.exceptions import PermissionDenied
+from django.db import IntegrityError
 from django.db.models import Q
 from graphene import relay
 from graphql_relay import from_global_id
@@ -9,8 +10,9 @@ from api.utils import get_object
 from core.logger import get_logger
 from users.inputs import PasswordUpdateInput
 from users.models import SchoolClass, SchoolClassMember, Team
-from users.schema import SchoolClassNode, TeamNode
-from users.serializers import PasswordSerialzer, AvatarUrlSerializer
+from users.schema import SchoolClassNode, TeamNode, UpdateError, FieldError, CreateSchoolClassResult
+from users.serializers import AvatarUrlSerializer, PasswordSerialzer
+
 
 logger = get_logger(__name__)
 
@@ -19,15 +21,6 @@ class CodeNotFoundException(Exception):
     pass
 
 
-class FieldError(graphene.ObjectType):
-    code = graphene.String()
-
-
-class UpdateError(graphene.ObjectType):
-    field = graphene.String()
-    errors = graphene.List(FieldError)
-
-
 class TeacherOnlyMutation(relay.ClientIDMutation):
     class Meta:
         abstract = True
@@ -204,18 +197,20 @@ class CreateSchoolClass(TeacherOnlyMutation):
     class Input:
         name = graphene.String()
 
-    success = graphene.Boolean()
-    school_class = graphene.Field(SchoolClassNode)
+    result = CreateSchoolClassResult()
 
     @classmethod
     def mutate_and_get_payload(cls, root, info, **kwargs):
         name = kwargs.get('name')
         user = info.context.user
 
-        school_class = SchoolClass.objects.create(name=name)
-        SchoolClassMember.objects.create(school_class=school_class, user=user)
-        user.set_selected_class(school_class)
-        return cls(success=True, school_class=school_class)
+        try:
+            school_class = SchoolClass.objects.create(name=name)
+            SchoolClassMember.objects.create(school_class=school_class, user=user)
+            user.set_selected_class(school_class)
+            return cls(result=school_class)
+        except IntegrityError:
+            return cls(result={"reason": "Name wird bereits verwendet"})
 
 
 class CreateTeam(TeacherOnlyMutation):

server/users/schema/queries.py

@@ -0,0 +1,43 @@
+import graphene
+from graphene_django.filter import DjangoFilterConnectionField
+
+from basicknowledge.models import BasicKnowledge
+from basicknowledge.queries import InstrumentNode
+from books.models import Module
+from books.schema.queries import ModuleNode
+from users.models import User
+
+from .types import PrivateUserNode
+
+
+class UsersQuery(object):
+    me = graphene.Field(PrivateUserNode)
+    all_users = DjangoFilterConnectionField(PrivateUserNode)
+    my_activity = DjangoFilterConnectionField(ModuleNode)
+    my_instrument_activity = DjangoFilterConnectionField(InstrumentNode)
+
+    def resolve_me(self, info, **kwargs):
+        return info.context.user
+
+    def resolve_all_users(self, info, **kwargs):
+        if not info.context.user.is_superuser:
+            return User.objects.none()
+        else:
+            return User.objects.all()
+
+    def resolve_my_activity(self, info, **kwargs):
+        return Module.objects.all()
+
+    def resolve_my_instrument_activity(self, info, **kwargs):
+        return BasicKnowledge.objects.all()
+
+
+class AllUsersQuery(object):
+    me = graphene.Field(PrivateUserNode)
+    all_users = DjangoFilterConnectionField(PrivateUserNode)
+
+    def resolve_all_users(self, info, **kwargs):
+        if not info.context.user.is_superuser:
+            return User.objects.none()
+        else:
+            return User.objects.all()

server/users/schema/types.py

@@ -9,13 +9,23 @@ from graphene_django import DjangoObjectType
 from graphene_django.filter import DjangoFilterConnectionField
 from graphql_relay import to_global_id
 
-from basicknowledge.models import BasicKnowledge
-from basicknowledge.queries import InstrumentNode
 from books.models import Module
 from books.schema.queries import ModuleNode
 from users.models import SchoolClass, SchoolClassMember, Team, User
 
 
+class RecentModuleFilter(FilterSet):
+    class Meta:
+        model = Module
+        fields = ('recent_modules',)
+
+    order_by = OrderingFilter(
+        fields=(
+            ('recent_modules__visited', 'visited'),
+        )
+    )
+
+
 class SchoolClassNode(DjangoObjectType):
     pk = graphene.Int()
     members = graphene.List('users.schema.ClassMemberNode')
@@ -64,18 +74,6 @@ class TeamNode(DjangoObjectType):
         return self.members.all()
 
 
-class RecentModuleFilter(FilterSet):
-    class Meta:
-        model = Module
-        fields = ('recent_modules',)
-
-    order_by = OrderingFilter(
-        fields=(
-            ('recent_modules__visited', 'visited'),
-        )
-    )
-
-
 class PublicUserNode(DjangoObjectType):
     full_name = graphene.String(required=True)
     is_me = graphene.Boolean()
@@ -184,34 +182,25 @@ class ClassMemberNode(ObjectType):
         return info.context.user.pk == parent.user.pk
 
 
-class UsersQuery(object):
-    me = graphene.Field(PrivateUserNode)
-    all_users = DjangoFilterConnectionField(PrivateUserNode)
-    my_activity = DjangoFilterConnectionField(ModuleNode)
-    my_instrument_activity = DjangoFilterConnectionField(InstrumentNode)
-
-    def resolve_me(self, info, **kwargs):
-        return info.context.user
-
-    def resolve_all_users(self, info, **kwargs):
-        if not info.context.user.is_superuser:
-            return User.objects.none()
-        else:
-            return User.objects.all()
-
-    def resolve_my_activity(self, info, **kwargs):
-        return Module.objects.all()
-
-    def resolve_my_instrument_activity(self, info, **kwargs):
-        return BasicKnowledge.objects.all()
+class DuplicateName(graphene.ObjectType):
+    reason = graphene.String()
 
 
-class AllUsersQuery(object):
-    me = graphene.Field(PrivateUserNode)
-    all_users = DjangoFilterConnectionField(PrivateUserNode)
+class CreateSchoolClassResult(graphene.Union):
+    class Meta:
+        types = (SchoolClassNode, DuplicateName)
 
-    def resolve_all_users(self, info, **kwargs):
-        if not info.context.user.is_superuser:
-            return User.objects.none()
-        else:
-            return User.objects.all()
+    @classmethod
+    def resolve_type(cls, instance, info):
+        if type(instance).__name__ == "SchoolClass":
+            return SchoolClassNode
+        return DuplicateName
+
+
+class FieldError(graphene.ObjectType):
+    code = graphene.String()
+
+
+class UpdateError(graphene.ObjectType):
+    field = graphene.String()
+    errors = graphene.List(FieldError)

server/users/tests/test_school_classes.py

@@ -1,11 +1,12 @@
-from django.test import TestCase, RequestFactory
+from django.db import transaction
+from django.test import TestCase
 from graphene import Context
 from graphene.test import Client
 from graphql_relay import to_global_id
 
 from api.schema import schema
 from api.utils import get_graphql_mutation, get_object
-from core.factories import UserFactory, TeacherFactory
+from core.factories import TeacherFactory, UserFactory
 from core.tests.base_test import SkillboxTestCase
 from users.models import SchoolClass, User
 from users.services import create_users
@@ -101,20 +102,41 @@ class ModifySchoolClassTest(SkillboxTestCase):
         self.assertEqual(SchoolClass.objects.count(), 2)
         class_name = 'Moordale'
         mutation = get_graphql_mutation('createClass.gql')
-        result = self.client.execute(mutation, variables={
+        query_result = self.client.execute(mutation, variables={
             'input': {
                 'name': class_name
             }
         })
-        self.assertIsNone(result.get('errors'))
-        id = result.get('data').get('createSchoolClass').get('schoolClass').get('id')
+        self.assertIsNone(query_result.get('errors'))
+        result = query_result.get('data').get('createSchoolClass').get('result')
+        self.assertEqual(result.get('__typename'), 'SchoolClassNode')
+        id = result.get('id')
         self.assertEqual(SchoolClass.objects.count(), 3)
         school_class = get_object(SchoolClass, id)
         self.assertEqual(school_class.name, class_name)
         self.assertEqual(school_class.get_teacher(), self.teacher)
         self.assertEqual(self.teacher.selected_class.name, class_name)
 
-    def test_create_school_class_fail(self):
+    def test_create_school_class_duplicate_name_fail(self):
+        self.assertEqual(SchoolClass.objects.count(), 2)
+        class_name = 'skillbox'
+        mutation = get_graphql_mutation('createClass.gql')
+        # if we don't do this, django wraps the whole test in an atomic operation,
+        # and we trigger an exception so the query later in the test would fail
+        with transaction.atomic():
+            query_result = self.client.execute(mutation, variables={
+                'input': {
+                    'name': class_name
+                }
+            })
+        self.assertIsNone(query_result.get('errors'))
+        result = query_result.get('data').get('createSchoolClass').get('result')
+        self.assertEqual(result.get('__typename'), 'DuplicateName')
+        reason = result.get('reason')
+        self.assertEqual(reason, 'Name wird bereits verwendet')
+        self.assertEqual(SchoolClass.objects.count(), 2)
+
+    def test_create_school_class_fail_permission(self):
         self.assertEqual(SchoolClass.objects.count(), 2)
         mutation = get_graphql_mutation('createClass.gql')
         result = self.student_client.execute(mutation, variables={