From 39a94a8aacf6046e1fef8729a47d3cc962d34e9e Mon Sep 17 00:00:00 2001 From: Matt Rider Date: Wed, 7 Aug 2019 14:55:32 +0200 Subject: [PATCH] Refactor - follow PR review by @roschaefer --- .../src/middleware/permissionsMiddleware.js | 12 +- backend/src/models/Post.js | 34 ++++ backend/src/models/User.js | 15 ++ backend/src/models/index.js | 1 + backend/src/schema/resolvers/posts.js | 29 ++-- backend/src/schema/resolvers/posts.spec.js | 145 ++++++++---------- backend/src/schema/types/type/Post.gql | 7 +- .../Emotions.spec.js} | 26 ++-- .../Emotions.vue} | 72 +++------ .../EmotionsButton/EmotionsButton.vue | 50 ++++++ webapp/graphql/PostMutations.js | 8 +- webapp/graphql/PostQuery.js | 6 +- webapp/pages/post/_id/_slug/index.vue | 6 +- 13 files changed, 230 insertions(+), 181 deletions(-) create mode 100644 backend/src/models/Post.js rename webapp/components/{EmotionsButtons/EmotionsButtons.spec.js => Emotions/Emotions.spec.js} (78%) rename webapp/components/{EmotionsButtons/EmotionsButtons.vue => Emotions/Emotions.vue} (52%) create mode 100644 webapp/components/EmotionsButton/EmotionsButton.vue diff --git a/backend/src/middleware/permissionsMiddleware.js b/backend/src/middleware/permissionsMiddleware.js index bbcfe667e..78f833c23 100644 --- a/backend/src/middleware/permissionsMiddleware.js +++ b/backend/src/middleware/permissionsMiddleware.js @@ -27,12 +27,6 @@ const onlyYourself = rule({ return context.user.id === args.id }) -const isMyEmotion = rule({ - cache: 'no_cache', -})(async (parent, args, context, info) => { - return context.user.id === args.from.id -}) - const isMyOwn = rule({ cache: 'no_cache', })(async (parent, args, context, info) => { @@ -163,8 +157,8 @@ const permissions = shield( User: or(noEmailFilter, isAdmin), isLoggedIn: allow, Badge: allow, - postsEmotionsCountByEmotion: allow, - postsEmotionsCountByCurrentUser: allow, + PostsEmotionsCountByEmotion: allow, + PostsEmotionsByCurrentUser: allow, }, Mutation: { '*': deny, @@ -200,7 +194,7 @@ const permissions = shield( requestPasswordReset: allow, resetPassword: allow, AddPostEmotions: isAuthenticated, - RemovePostEmotions: isMyEmotion, + RemovePostEmotions: isAuthenticated, }, User: { email: isMyOwn, diff --git a/backend/src/models/Post.js b/backend/src/models/Post.js new file mode 100644 index 000000000..7dd62287d --- /dev/null +++ b/backend/src/models/Post.js @@ -0,0 +1,34 @@ +import uuid from 'uuid/v4' + +module.exports = { + id: { type: 'string', primary: true, default: uuid }, + activityId: { type: 'string', allow: [null] }, + objectId: { type: 'string', allow: [null] }, + author: { + type: 'relationship', + relationship: 'WROTE', + target: 'User', + direction: 'in', + }, + title: { type: 'string', disallow: [null], min: 3 }, + slug: { type: 'string', allow: [null] }, + content: { type: 'string', disallow: [null], min: 3 }, + contentExcerpt: { type: 'string', allow: [null] }, + image: { type: 'string', allow: [null] }, + deleted: { type: 'boolean', default: false }, + disabled: { type: 'boolean', default: false }, + disabledBy: { + type: 'relationship', + relationship: 'DISABLED', + target: 'User', + direction: 'in', + }, + createdAt: { type: 'string', isoDate: true, default: () => new Date().toISOString() }, + updatedAt: { + type: 'string', + isoDate: true, + required: true, + default: () => new Date().toISOString(), + }, + language: { type: 'string', allow: [null] }, +} diff --git a/backend/src/models/User.js b/backend/src/models/User.js index bcd9fcf35..2c1575423 100644 --- a/backend/src/models/User.js +++ b/backend/src/models/User.js @@ -56,4 +56,19 @@ module.exports = { required: true, default: () => new Date().toISOString(), }, + emoted: { + type: 'relationships', + relationship: 'EMOTED', + target: 'Post', + direction: 'out', + properties: { + emotion: { + type: 'string', + valid: ['happy', 'cry', 'surprised', 'angry', 'funny'], + invalid: [null], + }, + }, + eager: true, + cascade: true, + }, } diff --git a/backend/src/models/index.js b/backend/src/models/index.js index b468dedf2..b8dc451d7 100644 --- a/backend/src/models/index.js +++ b/backend/src/models/index.js @@ -6,4 +6,5 @@ export default { InvitationCode: require('./InvitationCode.js'), EmailAddress: require('./EmailAddress.js'), SocialMedia: require('./SocialMedia.js'), + Post: require('./Post.js'), } diff --git a/backend/src/schema/resolvers/posts.js b/backend/src/schema/resolvers/posts.js index 930dd8875..4ecbf5116 100644 --- a/backend/src/schema/resolvers/posts.js +++ b/backend/src/schema/resolvers/posts.js @@ -92,23 +92,25 @@ export default { }, RemovePostEmotions: async (object, params, context, resolveInfo) => { const session = context.driver.session() - const { from, to, data } = params + const { to, data } = params + const { id: from } = context.user const transactionRes = await session.run( - `MATCH (userFrom:User {id: $from.id})-[emotedRelation:EMOTED {emotion: $data.emotion}]->(postTo:Post {id: $to.id}) + `MATCH (userFrom:User {id: $from})-[emotedRelation:EMOTED {emotion: $data.emotion}]->(postTo:Post {id: $to.id}) DELETE emotedRelation + RETURN userFrom `, { from, to, data }, ) session.close() - const [emoted] = transactionRes.records.map(record => { - return record.get('emotedRelation').properties.emotion + const [userFrom] = transactionRes.records.map(record => { + return record.get('userFrom') }) - - return !emoted + if (!userFrom) throw new Error('Not Authorised!') + return Boolean(userFrom) }, }, Query: { - postsEmotionsCountByEmotion: async (object, params, context, resolveInfo) => { + PostsEmotionsCountByEmotion: async (object, params, context, resolveInfo) => { const session = context.driver.session() const { postId, data } = params const transactionRes = await session.run( @@ -125,22 +127,21 @@ export default { return emotionsCount }, - postsEmotionsCountByCurrentUser: async (object, params, context, resolveInfo) => { + PostsEmotionsByCurrentUser: async (object, params, context, resolveInfo) => { const session = context.driver.session() const { postId } = params const transactionRes = await session.run( `MATCH (user:User {id: $userId})-[emoted:EMOTED]->(post:Post {id: $postId}) - RETURN emoted.emotion as emotion`, + RETURN collect(emoted.emotion) as emotion`, { userId: context.user.id, postId }, ) session.close() - const emotionsArray = [] - transactionRes.records.map(record => { - emotionsArray.push(record.get('emotion')) - }) - return emotionsArray + const [emotions] = transactionRes.records.map(record => { + return record.get('emotion') + }) + return emotions }, }, } diff --git a/backend/src/schema/resolvers/posts.spec.js b/backend/src/schema/resolvers/posts.spec.js index f34711aff..997291264 100644 --- a/backend/src/schema/resolvers/posts.spec.js +++ b/backend/src/schema/resolvers/posts.spec.js @@ -406,15 +406,17 @@ describe('emotions', () => { owner, postMutationAction, user, - postQueryAction - const postEmotionsCountQuery = ` + postQueryAction, + postToEmote, + postToEmoteNode + const PostsEmotionsCountQuery = ` query($id: ID!) { Post(id: $id) { emotionsCount } } ` - const postEmotionsQuery = ` + const PostsEmotionsQuery = ` query($id: ID!) { Post(id: $id) { emotions { @@ -425,14 +427,27 @@ describe('emotions', () => { } } } +` + const addPostEmotionsMutation = ` + mutation($from: _UserInput!, $to: _PostInput!, $data: _EMOTEDInput!) { + AddPostEmotions(from: $from, to: $to, data: $data) { + from { id } + to { id } + emotion + } + } ` beforeEach(async () => { userParams.id = 'u1987' authorParams.id = 'u257' + createPostVariables.id = 'p1376' const someUserNode = await instance.create('User', userParams) someUser = await someUserNode.toJson() ownerNode = await instance.create('User', authorParams) owner = await ownerNode.toJson() + postToEmoteNode = await instance.create('Post', createPostVariables) + postToEmote = await postToEmoteNode.toJson() + await postToEmoteNode.relateTo(ownerNode, 'author') postMutationAction = async (user, mutation, variables) => { const { server } = createServer({ @@ -462,42 +477,29 @@ describe('emotions', () => { const { query } = createTestClient(server) return query({ query: postQuery, variables }) } + addPostEmotionsVariables = { + from: { id: authorParams.id }, + to: { id: postToEmote.id }, + data: { emotion: 'happy' }, + } }) describe('AddPostEmotions', () => { - let postEmotionsQueryVariables + let postsEmotionsQueryVariables beforeEach(async () => { - user = owner - const { - data: { CreatePost }, - } = await postMutationAction(user, createPostMutation, createPostVariables) - addPostEmotionsVariables = { - from: { id: authorParams.id }, - to: { id: CreatePost.id }, - data: { emotion: 'happy' }, - } - postEmotionsQueryVariables = { id: CreatePost.id } + postsEmotionsQueryVariables = { id: postToEmote.id } }) - const addPostEmotionsMutation = ` - mutation($from: _UserInput!, $to: _PostInput!, $data: _EMOTEDInput!) { - AddPostEmotions(from: $from, to: $to, data: $data) { - from { id } - to { id } - emotion - } - } - ` describe('unauthenticated', () => { it('throws authorization error', async () => { user = null - const result = await postMutationAction( + const addPostEmotions = await postMutationAction( user, addPostEmotionsMutation, addPostEmotionsVariables, ) - expect(result.errors[0]).toHaveProperty('message', 'Not Authorised!') + expect(addPostEmotions.errors[0]).toHaveProperty('message', 'Not Authorised!') }) }) @@ -535,7 +537,7 @@ describe('emotions', () => { await postMutationAction(user, addPostEmotionsMutation, addPostEmotionsVariables) await postMutationAction(user, addPostEmotionsMutation, addPostEmotionsVariables) await expect( - postQueryAction(postEmotionsCountQuery, postEmotionsQueryVariables), + postQueryAction(PostsEmotionsCountQuery, postsEmotionsQueryVariables), ).resolves.toEqual(expect.objectContaining(expected)) }) @@ -551,7 +553,7 @@ describe('emotions', () => { addPostEmotionsVariables.data.emotion = 'surprised' await postMutationAction(user, addPostEmotionsMutation, addPostEmotionsVariables) await expect( - postQueryAction(postEmotionsQuery, postEmotionsQueryVariables), + postQueryAction(PostsEmotionsQuery, postsEmotionsQueryVariables), ).resolves.toEqual(expect.objectContaining(expectedResponse)) }) }) @@ -579,16 +581,19 @@ describe('emotions', () => { }) describe('RemovePostEmotions', () => { - let removePostEmotionsVariables, postEmotionsQueryVariables + let removePostEmotionsVariables, postsEmotionsQueryVariables const removePostEmotionsMutation = ` - mutation($from: _UserInput!, $to: _PostInput!, $data: _EMOTEDInput!) { - RemovePostEmotions(from: $from, to: $to, data: $data) + mutation($to: _PostInput!, $data: _EMOTEDInput!) { + RemovePostEmotions(to: $to, data: $data) } ` - beforeEach(() => { + beforeEach(async () => { + await ownerNode.relateTo(postToEmoteNode, 'emoted', { emotion: 'cry' }) + await postMutationAction(user, addPostEmotionsMutation, addPostEmotionsVariables) + + postsEmotionsQueryVariables = { id: postToEmote.id } removePostEmotionsVariables = { - from: { id: authorParams.id }, - to: { id: 'p1376' }, + to: { id: postToEmote.id }, data: { emotion: 'cry' }, } }) @@ -596,45 +601,25 @@ describe('emotions', () => { describe('unauthenticated', () => { it('throws authorization error', async () => { user = null - const result = await postMutationAction( + const removePostEmotions = await postMutationAction( user, removePostEmotionsMutation, removePostEmotionsVariables, ) - - expect(result.errors[0]).toHaveProperty('message', 'Not Authorised!') + expect(removePostEmotions.errors[0]).toHaveProperty('message', 'Not Authorised!') }) }) describe('authenticated', () => { - beforeEach(async () => { - user = owner - const { - data: { CreatePost }, - } = await postMutationAction(user, createPostMutation, createPostVariables) - await factory.emote({ - from: authorParams.id, - to: CreatePost.id, - data: 'cry', - }) - await factory.emote({ - from: authorParams.id, - to: CreatePost.id, - data: 'happy', - }) - postEmotionsQueryVariables = { id: CreatePost.id } - }) - describe('but not the emoter', () => { it('throws an authorization error', async () => { user = someUser - const result = await postMutationAction( + const removePostEmotions = await postMutationAction( user, removePostEmotionsMutation, removePostEmotionsVariables, ) - - expect(result.errors[0]).toHaveProperty('message', 'Not Authorised!') + expect(removePostEmotions.errors[0]).toHaveProperty('message', 'Not Authorised!') }) }) @@ -654,7 +639,7 @@ describe('emotions', () => { } await postMutationAction(user, removePostEmotionsMutation, removePostEmotionsVariables) await expect( - postQueryAction(postEmotionsQuery, postEmotionsQueryVariables), + postQueryAction(PostsEmotionsQuery, postsEmotionsQueryVariables), ).resolves.toEqual(expect.objectContaining(expectedResponse)) }) }) @@ -662,54 +647,44 @@ describe('emotions', () => { }) describe('posts emotions count', () => { - let postsEmotionsCountByEmotionVariables - let postsEmotionsCountByCurrentUserVariables + let PostsEmotionsCountByEmotionVariables + let PostsEmotionsByCurrentUserVariables - const postsEmotionsCountByEmotionQuery = ` + const PostsEmotionsCountByEmotionQuery = ` query($postId: ID!, $data: _EMOTEDInput!) { - postsEmotionsCountByEmotion(postId: $postId, data: $data) + PostsEmotionsCountByEmotion(postId: $postId, data: $data) } ` - const postsEmotionsCountByCurrentUserQuery = ` + const PostsEmotionsByCurrentUserQuery = ` query($postId: ID!) { - postsEmotionsCountByCurrentUser(postId: $postId) + PostsEmotionsByCurrentUser(postId: $postId) } ` beforeEach(async () => { - user = owner - const { - data: { CreatePost }, - } = await postMutationAction(user, createPostMutation, createPostVariables) - await factory.emote({ - from: authorParams.id, - to: CreatePost.id, - data: 'cry', - }) - postsEmotionsCountByEmotionVariables = { - postId: CreatePost.id, + await ownerNode.relateTo(postToEmoteNode, 'emoted', { emotion: 'cry' }) + + PostsEmotionsCountByEmotionVariables = { + postId: postToEmote.id, data: { emotion: 'cry' }, } - postsEmotionsCountByCurrentUserVariables = { postId: CreatePost.id } + PostsEmotionsByCurrentUserVariables = { postId: postToEmote.id } }) - describe('postsEmotionsCountByEmotion', () => { + describe('PostsEmotionsCountByEmotion', () => { it("returns a post's emotions count", async () => { - const expectedResponse = { data: { postsEmotionsCountByEmotion: 1 } } + const expectedResponse = { data: { PostsEmotionsCountByEmotion: 1 } } await expect( - postQueryAction(postsEmotionsCountByEmotionQuery, postsEmotionsCountByEmotionVariables), + postQueryAction(PostsEmotionsCountByEmotionQuery, PostsEmotionsCountByEmotionVariables), ).resolves.toEqual(expect.objectContaining(expectedResponse)) }) }) - describe('postsEmotionsCountByEmotion', () => { + describe('PostsEmotionsCountByEmotion', () => { it("returns a currentUser's emotions on a post", async () => { - const expectedResponse = { data: { postsEmotionsCountByCurrentUser: ['cry'] } } + const expectedResponse = { data: { PostsEmotionsByCurrentUser: ['cry'] } } await expect( - postQueryAction( - postsEmotionsCountByCurrentUserQuery, - postsEmotionsCountByCurrentUserVariables, - ), + postQueryAction(PostsEmotionsByCurrentUserQuery, PostsEmotionsByCurrentUserVariables), ).resolves.toEqual(expect.objectContaining(expectedResponse)) }) }) diff --git a/backend/src/schema/types/type/Post.gql b/backend/src/schema/types/type/Post.gql index ba2b6ceef..b5265203d 100644 --- a/backend/src/schema/types/type/Post.gql +++ b/backend/src/schema/types/type/Post.gql @@ -91,10 +91,11 @@ type Mutation { language: String categoryIds: [ID] ): Post - RemovePostEmotions(from: _UserInput!, to: _PostInput!, data: _EMOTEDInput!): Boolean! + AddPostEmotions(from: _UserInput!, to: _PostInput!, data: _EMOTEDInput!): EMOTED + RemovePostEmotions(to: _PostInput!, data: _EMOTEDInput!): Boolean! } type Query { - postsEmotionsCountByEmotion(postId: ID!, data: _EMOTEDInput!): Int! - postsEmotionsCountByCurrentUser(postId: ID!): [String] + PostsEmotionsCountByEmotion(postId: ID!, data: _EMOTEDInput!): Int! + PostsEmotionsByCurrentUser(postId: ID!): [String] } diff --git a/webapp/components/EmotionsButtons/EmotionsButtons.spec.js b/webapp/components/Emotions/Emotions.spec.js similarity index 78% rename from webapp/components/EmotionsButtons/EmotionsButtons.spec.js rename to webapp/components/Emotions/Emotions.spec.js index 625ec13de..8400b3ff7 100644 --- a/webapp/components/EmotionsButtons/EmotionsButtons.spec.js +++ b/webapp/components/Emotions/Emotions.spec.js @@ -1,5 +1,5 @@ import { mount, createLocalVue } from '@vue/test-utils' -import EmotionsButtons from './EmotionsButtons.vue' +import Emotions from './Emotions.vue' import Styleguide from '@human-connection/styleguide' import Vuex from 'vuex' import PostMutations from '~/graphql/PostMutations.js' @@ -9,7 +9,7 @@ const localVue = createLocalVue() localVue.use(Styleguide) localVue.use(Vuex) -describe('EmotionsButtons.vue', () => { +describe('Emotions.vue', () => { let wrapper let mocks let propsData @@ -39,7 +39,7 @@ describe('EmotionsButtons.vue', () => { }), query: jest.fn().mockResolvedValue({ data: { - postsEmotionsCountByEmotion: 1, + PostsEmotionsCountByEmotion: 1, }, }), }, @@ -59,7 +59,7 @@ describe('EmotionsButtons.vue', () => { const store = new Vuex.Store({ getters, }) - return mount(EmotionsButtons, { mocks, propsData, store, localVue }) + return mount(Emotions, { mocks, propsData, store, localVue }) } beforeEach(() => { wrapper = Wrapper() @@ -72,7 +72,7 @@ describe('EmotionsButtons.vue', () => { describe('adding emotions', () => { let expectedParams beforeEach(() => { - wrapper.vm.postsEmotionsCountByEmotion.funny = 0 + wrapper.vm.PostsEmotionsCountByEmotion.funny = 0 funnyButton = wrapper.findAll('button').at(0) funnyButton.trigger('click') }) @@ -82,16 +82,16 @@ describe('EmotionsButtons.vue', () => { expect(funnyImage.attributes().src).toEqual(funnyImageSrc) }) - it('sends the addPostEmotionsMutation for an emotion when clicked', () => { + it('sends the AddPostEmotionsMutation for an emotion when clicked', () => { expectedParams = { - mutation: PostMutations().addPostEmotionsMutation, + mutation: PostMutations().AddPostEmotionsMutation, variables: { from: { id: 'u176' }, to: { id: 'p143' }, data: { emotion: 'funny' } }, } expect(mocks.$apollo.mutate).toHaveBeenCalledWith(expect.objectContaining(expectedParams)) }) - it('increases the postEmotionsCountByEmotion for the emotion clicked', () => { - expect(wrapper.vm.postsEmotionsCountByEmotion.funny).toEqual(1) + it('increases the PostsEmotionsCountByEmotion for the emotion clicked', () => { + expect(wrapper.vm.PostsEmotionsCountByEmotion.funny).toEqual(1) }) it('adds an emotion to selectedEmotions to show the colored image when the button is active', () => { @@ -103,16 +103,16 @@ describe('EmotionsButtons.vue', () => { funnyButton.trigger('click') }) - it('sends the removePostEmotionsMutation when a user clicks on an active emotion', () => { + it('sends the RemovePostEmotionsMutation when a user clicks on an active emotion', () => { expectedParams = { - mutation: PostMutations().removePostEmotionsMutation, + mutation: PostMutations().RemovePostEmotionsMutation, variables: { from: { id: 'u176' }, to: { id: 'p143' }, data: { emotion: 'funny' } }, } expect(mocks.$apollo.mutate).toHaveBeenCalledWith(expect.objectContaining(expectedParams)) }) - it('decreases the postEmotionsCountByEmotion for the emotion clicked', async () => { - expect(wrapper.vm.postsEmotionsCountByEmotion.funny).toEqual(0) + it('decreases the PostsEmotionsCountByEmotion for the emotion clicked', async () => { + expect(wrapper.vm.PostsEmotionsCountByEmotion.funny).toEqual(0) }) it('removes an emotion from selectedEmotions to show the default image', async () => { diff --git a/webapp/components/EmotionsButtons/EmotionsButtons.vue b/webapp/components/Emotions/Emotions.vue similarity index 52% rename from webapp/components/EmotionsButtons/EmotionsButtons.vue rename to webapp/components/Emotions/Emotions.vue index 50aefe9b4..be8fc9f23 100644 --- a/webapp/components/EmotionsButtons/EmotionsButtons.vue +++ b/webapp/components/Emotions/Emotions.vue @@ -1,18 +1,13 @@