From 963cbbef32c3158d3db5706279ce0e8e89a9e5cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Thu, 8 Aug 2019 23:51:26 +0200 Subject: [PATCH] RemovePostEmotions should return deleted object @mattwr18 I prefer (I believe it's even best practice) that a delete mutation should return the deleted object. If you run the delete mutation again, it should return `null` because there is no object like that anymore. That way the client knows if a delete mutation has changed any state in the database. Also I fixed another bug in the resolver. If your graphql mutation looks like this: ```gql mutation { RemovePostEmotions(to:{ id:"p15"}, data:{emotion: angry}) { from { id name } to { id title } emotion } } ``` Then you get errors because your resolver does not return the name for the user or the title for the post anymore. Just use spread operator... and it's fixed. --- backend/src/schema/resolvers/posts.js | 20 +++++++++------- backend/src/schema/resolvers/posts.spec.js | 26 +++++++++++++++++---- backend/src/schema/types/type/Post.gql | 2 +- webapp/components/Emotions/Emotions.spec.js | 11 +++++---- webapp/components/Emotions/Emotions.vue | 1 - webapp/graphql/PostMutations.js | 10 +++++++- 6 files changed, 50 insertions(+), 20 deletions(-) diff --git a/backend/src/schema/resolvers/posts.js b/backend/src/schema/resolvers/posts.js index 5306c18d7..336f816f5 100644 --- a/backend/src/schema/resolvers/posts.js +++ b/backend/src/schema/resolvers/posts.js @@ -84,9 +84,9 @@ export default { session.close() const [emoted] = transactionRes.records.map(record => { return { - from: { id: record.get('userFrom').properties.id }, - to: { id: record.get('postTo').properties.id }, - emotion: record.get('emotedRelation').properties.emotion, + from: { ...record.get('userFrom').properties }, + to: { ...record.get('postTo').properties }, + ...record.get('emotedRelation').properties, } }) return emoted @@ -98,16 +98,18 @@ export default { const transactionRes = await session.run( `MATCH (userFrom:User {id: $from})-[emotedRelation:EMOTED {emotion: $data.emotion}]->(postTo:Post {id: $to.id}) DELETE emotedRelation - RETURN userFrom - `, + RETURN userFrom, postTo`, { from, to, data }, ) session.close() - const [userFrom] = transactionRes.records.map(record => { - return record.get('userFrom') + const [emoted] = transactionRes.records.map(record => { + return { + from: { ...record.get('userFrom').properties }, + to: { ...record.get('postTo').properties }, + emotion: data.emotion, + } }) - if (!userFrom) throw new Error('Not Authorised!') - return Boolean(userFrom) + return emoted }, }, Query: { diff --git a/backend/src/schema/resolvers/posts.spec.js b/backend/src/schema/resolvers/posts.spec.js index 083283689..b48be16db 100644 --- a/backend/src/schema/resolvers/posts.spec.js +++ b/backend/src/schema/resolvers/posts.spec.js @@ -588,7 +588,15 @@ describe('emotions', () => { let removePostEmotionsVariables, postsEmotionsQueryVariables const removePostEmotionsMutation = gql` mutation($to: _PostInput!, $data: _EMOTEDInput!) { - RemovePostEmotions(to: $to, data: $data) + RemovePostEmotions(to: $to, data: $data) { + from { + id + } + to { + id + } + emotion + } } ` beforeEach(async () => { @@ -616,21 +624,31 @@ describe('emotions', () => { describe('authenticated', () => { describe('but not the emoter', () => { - it('throws an authorization error', async () => { + it('returns null if the emotion could not be found', async () => { user = someUser const removePostEmotions = await postMutationAction( user, removePostEmotionsMutation, removePostEmotionsVariables, ) - expect(removePostEmotions.errors[0]).toHaveProperty('message', 'Not Authorised!') + expect(removePostEmotions).toEqual( + expect.objectContaining({ data: { RemovePostEmotions: null } }), + ) }) }) describe('as the emoter', () => { it('removes an emotion from a post', async () => { user = owner - const expected = { data: { RemovePostEmotions: true } } + const expected = { + data: { + RemovePostEmotions: { + to: { id: postToEmote.id }, + from: { id: user.id }, + emotion: 'cry', + }, + }, + } await expect( postMutationAction(user, removePostEmotionsMutation, removePostEmotionsVariables), ).resolves.toEqual(expect.objectContaining(expected)) diff --git a/backend/src/schema/types/type/Post.gql b/backend/src/schema/types/type/Post.gql index 28329ee85..519af14ae 100644 --- a/backend/src/schema/types/type/Post.gql +++ b/backend/src/schema/types/type/Post.gql @@ -92,7 +92,7 @@ type Mutation { categoryIds: [ID] ): Post AddPostEmotions(to: _PostInput!, data: _EMOTEDInput!): EMOTED - RemovePostEmotions(to: _PostInput!, data: _EMOTEDInput!): Boolean! + RemovePostEmotions(to: _PostInput!, data: _EMOTEDInput!): EMOTED } type Query { diff --git a/webapp/components/Emotions/Emotions.spec.js b/webapp/components/Emotions/Emotions.spec.js index 8400b3ff7..ecc7f9e94 100644 --- a/webapp/components/Emotions/Emotions.spec.js +++ b/webapp/components/Emotions/Emotions.spec.js @@ -26,7 +26,6 @@ describe('Emotions.vue', () => { .mockResolvedValueOnce({ data: { AddPostEmotions: { - from: { id: 'u176' }, to: { id: 'p143' }, data: { emotion: 'happy' }, }, @@ -34,7 +33,11 @@ describe('Emotions.vue', () => { }) .mockResolvedValueOnce({ data: { - RemovePostEmotions: true, + RemovePostEmotions: { + from: { id: 'u176' }, + to: { id: 'p143' }, + data: { emotion: 'happy' }, + }, }, }), query: jest.fn().mockResolvedValue({ @@ -85,7 +88,7 @@ describe('Emotions.vue', () => { it('sends the AddPostEmotionsMutation for an emotion when clicked', () => { expectedParams = { mutation: PostMutations().AddPostEmotionsMutation, - variables: { from: { id: 'u176' }, to: { id: 'p143' }, data: { emotion: 'funny' } }, + variables: { to: { id: 'p143' }, data: { emotion: 'funny' } }, } expect(mocks.$apollo.mutate).toHaveBeenCalledWith(expect.objectContaining(expectedParams)) }) @@ -106,7 +109,7 @@ describe('Emotions.vue', () => { it('sends the RemovePostEmotionsMutation when a user clicks on an active emotion', () => { expectedParams = { mutation: PostMutations().RemovePostEmotionsMutation, - variables: { from: { id: 'u176' }, to: { id: 'p143' }, data: { emotion: 'funny' } }, + variables: { to: { id: 'p143' }, data: { emotion: 'funny' } }, } expect(mocks.$apollo.mutate).toHaveBeenCalledWith(expect.objectContaining(expectedParams)) }) diff --git a/webapp/components/Emotions/Emotions.vue b/webapp/components/Emotions/Emotions.vue index be8fc9f23..3be7ee790 100644 --- a/webapp/components/Emotions/Emotions.vue +++ b/webapp/components/Emotions/Emotions.vue @@ -56,7 +56,6 @@ export default { ? PostMutations().RemovePostEmotionsMutation : PostMutations().AddPostEmotionsMutation, variables: { - from: { id: this.currentUser.id }, to: { id: this.post.id }, data: { emotion }, }, diff --git a/webapp/graphql/PostMutations.js b/webapp/graphql/PostMutations.js index 3b94e809c..fc672c40d 100644 --- a/webapp/graphql/PostMutations.js +++ b/webapp/graphql/PostMutations.js @@ -75,7 +75,15 @@ export default () => { `, RemovePostEmotionsMutation: gql` mutation($to: _PostInput!, $data: _EMOTEDInput!) { - RemovePostEmotions(to: $to, data: $data) + RemovePostEmotions(to: $to, data: $data) { + emotion + from { + id + } + to { + id + } + } } `, }