From 9b970dae15442ca65939b8e4bece1e4651308a04 Mon Sep 17 00:00:00 2001 From: Vasily Belolapotkov Date: Mon, 16 Sep 2019 13:54:00 +0300 Subject: [PATCH 1/4] refactor follow resolver spec --- backend/src/schema/resolvers/follow.spec.js | 153 +++++++++++++------- 1 file changed, 101 insertions(+), 52 deletions(-) diff --git a/backend/src/schema/resolvers/follow.spec.js b/backend/src/schema/resolvers/follow.spec.js index 66be20841..3cf14a902 100644 --- a/backend/src/schema/resolvers/follow.spec.js +++ b/backend/src/schema/resolvers/follow.spec.js @@ -1,10 +1,17 @@ -import { GraphQLClient } from 'graphql-request' +import { createTestClient } from 'apollo-server-testing' import Factory from '../../seed/factories' -import { host, login } from '../../jest/helpers' +import { getDriver } from '../../bootstrap/neo4j' +import createServer from '../../server' -const factory = Factory() -let clientUser1 -let headersUser1 +let factory +let driver + +let query +let mutate +let authenticatedUser + +let user1 +let user2 const mutationFollowUser = id => ` mutation { @@ -17,20 +24,42 @@ const mutationUnfollowUser = id => ` } ` -beforeEach(async () => { - await factory.create('User', { - id: 'u1', - email: 'test@example.org', - password: '1234', - }) - await factory.create('User', { - id: 'u2', - email: 'test2@example.org', - password: '1234', +beforeAll(() => { + factory = Factory() + driver = getDriver() + + const { server } = createServer({ + context: () => ({ + driver, + user: authenticatedUser, + cypherParams: { + currentUserId: authenticatedUser ? authenticatedUser.id : null, + }, + }), }) - headersUser1 = await login({ email: 'test@example.org', password: '1234' }) - clientUser1 = new GraphQLClient(host, { headers: headersUser1 }) + const testClient = createTestClient(server) + query = testClient.query + mutate = testClient.mutate +}) + +beforeEach(async () => { + user1 = await factory + .create('User', { + id: 'u1', + email: 'test@example.org', + password: '1234', + }) + .then(user => user.toJson()) + user2 = await factory + .create('User', { + id: 'u2', + email: 'test2@example.org', + password: '1234', + }) + .then(user => user.toJson()) + + authenticatedUser = user1 }) afterEach(async () => { @@ -41,44 +70,57 @@ describe('follow', () => { describe('follow user', () => { describe('unauthenticated follow', () => { it('throws authorization error', async () => { - const client = new GraphQLClient(host) - await expect(client.request(mutationFollowUser('u2'))).rejects.toThrow('Not Authorised') + authenticatedUser = null + const { errors } = await mutate({ + mutation: mutationFollowUser('u2'), + }) + expect(errors[0]).toHaveProperty('message', 'Not Authorised!') }) }) it('I can follow another user', async () => { - const res = await clientUser1.request(mutationFollowUser('u2')) - const expected = { - follow: true, - } - expect(res).toMatchObject(expected) + const { data: result } = await mutate({ + mutation: mutationFollowUser(user2.id), + }) + + const expected = { follow: true } + expect(result).toMatchObject(expected) + + const { + data: { User }, + } = await query({ + query: `{ + User(id: "${user2.id}") { + followedBy { id } + followedByCurrentUser + } + }`, + }) - const { User } = await clientUser1.request(`{ - User(id: "u2") { - followedBy { id } - followedByCurrentUser - } - }`) const expected2 = { - followedBy: [{ id: 'u1' }], + followedBy: [{ id: user1.id }], followedByCurrentUser: true, } expect(User[0]).toMatchObject(expected2) }) it('I can`t follow myself', async () => { - const res = await clientUser1.request(mutationFollowUser('u1')) - const expected = { - follow: false, - } - expect(res).toMatchObject(expected) + const { data: result } = await mutate({ + mutation: mutationFollowUser(user1.id), + }) + const expected = { follow: false } + expect(result).toMatchObject(expected) - const { User } = await clientUser1.request(`{ - User(id: "u1") { + const { + data: { User }, + } = await query({ + query: `{ + User(id: "${user1.id}") { followedBy { id } followedByCurrentUser } - }`) + }`, + }) const expected2 = { followedBy: [], followedByCurrentUser: false, @@ -87,32 +129,39 @@ describe('follow', () => { }) }) describe('unfollow user', () => { + beforeEach(async () => { + await mutate({ mutation: mutationFollowUser(user2.id) }) + }) + describe('unauthenticated follow', () => { it('throws authorization error', async () => { - // follow - await clientUser1.request(mutationFollowUser('u2')) - // unfollow - const client = new GraphQLClient(host) - await expect(client.request(mutationUnfollowUser('u2'))).rejects.toThrow('Not Authorised') + authenticatedUser = null + const { errors } = await mutate({ mutation: mutationUnfollowUser(user2.id) }) + expect(errors[0]).toHaveProperty('message', 'Not Authorised!') }) }) it('I can unfollow a user', async () => { - // follow - await clientUser1.request(mutationFollowUser('u2')) - // unfollow + const { data: result } = await mutate({ + mutation: mutationUnfollowUser(user2.id), + }) + const expected = { unfollow: true, } - const res = await clientUser1.request(mutationUnfollowUser('u2')) - expect(res).toMatchObject(expected) - const { User } = await clientUser1.request(`{ - User(id: "u2") { + expect(result).toMatchObject(expected) + + const { + data: { User }, + } = await query({ + query: `{ + User(id: "${user2.id}") { followedBy { id } followedByCurrentUser } - }`) + }`, + }) const expected2 = { followedBy: [], followedByCurrentUser: false, From eeee5f8e9b424459ee06c9a47930e7644d2c824b Mon Sep 17 00:00:00 2001 From: Vasily Belolapotkov Date: Tue, 17 Sep 2019 10:50:03 +0300 Subject: [PATCH 2/4] refactor graphQL queries in follow spec --- backend/src/schema/resolvers/follow.spec.js | 125 +++++++++----------- 1 file changed, 58 insertions(+), 67 deletions(-) diff --git a/backend/src/schema/resolvers/follow.spec.js b/backend/src/schema/resolvers/follow.spec.js index 3cf14a902..2f3f1f4ad 100644 --- a/backend/src/schema/resolvers/follow.spec.js +++ b/backend/src/schema/resolvers/follow.spec.js @@ -2,9 +2,10 @@ import { createTestClient } from 'apollo-server-testing' import Factory from '../../seed/factories' import { getDriver } from '../../bootstrap/neo4j' import createServer from '../../server' +import { gql } from '../../jest/helpers' -let factory -let driver +const factory = Factory() +const driver = getDriver() let query let mutate @@ -12,22 +13,32 @@ let authenticatedUser let user1 let user2 +let variables -const mutationFollowUser = id => ` - mutation { - follow(id: "${id}", type: User) +const mutationFollowUser = gql` + mutation($id: ID!, $type: FollowTypeEnum) { + follow(id: $id, type: $type) } ` -const mutationUnfollowUser = id => ` - mutation { - unfollow(id: "${id}", type: User) + +const mutationUnfollowUser = gql` + mutation($id: ID!, $type: FollowTypeEnum) { + unfollow(id: $id, type: $type) + } +` + +const userQuery = gql` + query($id: ID) { + User(id: $id) { + followedBy { + id + } + followedByCurrentUser + } } ` beforeAll(() => { - factory = Factory() - driver = getDriver() - const { server } = createServer({ context: () => ({ driver, @@ -60,6 +71,7 @@ beforeEach(async () => { .then(user => user.toJson()) authenticatedUser = user1 + variables = { id: user2.id, type: 'User' } }) afterEach(async () => { @@ -72,7 +84,8 @@ describe('follow', () => { it('throws authorization error', async () => { authenticatedUser = null const { errors } = await mutate({ - mutation: mutationFollowUser('u2'), + mutation: mutationFollowUser, + variables, }) expect(errors[0]).toHaveProperty('message', 'Not Authorised!') }) @@ -80,93 +93,71 @@ describe('follow', () => { it('I can follow another user', async () => { const { data: result } = await mutate({ - mutation: mutationFollowUser(user2.id), + mutation: mutationFollowUser, + variables, }) + const expectedResult = { follow: true } + expect(result).toMatchObject(expectedResult) - const expected = { follow: true } - expect(result).toMatchObject(expected) - - const { - data: { User }, - } = await query({ - query: `{ - User(id: "${user2.id}") { - followedBy { id } - followedByCurrentUser - } - }`, + const { data } = await query({ + query: userQuery, + variables: { id: user2.id }, }) - - const expected2 = { + const expectedUser = { followedBy: [{ id: user1.id }], followedByCurrentUser: true, } - expect(User[0]).toMatchObject(expected2) + expect(data).toMatchObject({ User: [expectedUser] }) }) it('I can`t follow myself', async () => { - const { data: result } = await mutate({ - mutation: mutationFollowUser(user1.id), - }) - const expected = { follow: false } - expect(result).toMatchObject(expected) + variables.id = user1.id + const { data: result } = await mutate({ mutation: mutationFollowUser, variables }) + const expectedResult = { follow: false } + expect(result).toMatchObject(expectedResult) - const { - data: { User }, - } = await query({ - query: `{ - User(id: "${user1.id}") { - followedBy { id } - followedByCurrentUser - } - }`, + const { data } = await query({ + query: userQuery, + variables: { id: user1.id }, }) - const expected2 = { + const expectedUser = { followedBy: [], followedByCurrentUser: false, } - expect(User[0]).toMatchObject(expected2) + expect(data).toMatchObject({ User: [expectedUser] }) }) }) describe('unfollow user', () => { beforeEach(async () => { - await mutate({ mutation: mutationFollowUser(user2.id) }) + variables = { + id: user2.id, + type: 'User', + } + await mutate({ mutation: mutationFollowUser, variables }) }) describe('unauthenticated follow', () => { it('throws authorization error', async () => { authenticatedUser = null - const { errors } = await mutate({ mutation: mutationUnfollowUser(user2.id) }) + const { errors } = await mutate({ mutation: mutationUnfollowUser, variables }) expect(errors[0]).toHaveProperty('message', 'Not Authorised!') }) }) it('I can unfollow a user', async () => { - const { data: result } = await mutate({ - mutation: mutationUnfollowUser(user2.id), + const { data: result } = await mutate({ mutation: mutationUnfollowUser, variables }) + const expectedResult = { unfollow: true } + expect(result).toMatchObject(expectedResult) + + const { data } = await query({ + query: userQuery, + variables: { id: user2.id }, }) - - const expected = { - unfollow: true, - } - - expect(result).toMatchObject(expected) - - const { - data: { User }, - } = await query({ - query: `{ - User(id: "${user2.id}") { - followedBy { id } - followedByCurrentUser - } - }`, - }) - const expected2 = { + const expectedUser = { followedBy: [], followedByCurrentUser: false, } - expect(User[0]).toMatchObject(expected2) + expect(data).toMatchObject({ User: [expectedUser] }) }) }) }) From 7d048b029d97108a5096009587f67c918e1f3179 Mon Sep 17 00:00:00 2001 From: Vasily Belolapotkov Date: Wed, 18 Sep 2019 09:49:59 +0300 Subject: [PATCH 3/4] return user from follow/unfollow mutation --- backend/src/schema/resolvers/follow.js | 61 +++++++++------------ backend/src/schema/resolvers/follow.spec.js | 58 +++++++++++--------- backend/src/schema/types/schema.gql | 4 +- 3 files changed, 61 insertions(+), 62 deletions(-) diff --git a/backend/src/schema/resolvers/follow.js b/backend/src/schema/resolvers/follow.js index 730a66cfd..f0a98e1f7 100644 --- a/backend/src/schema/resolvers/follow.js +++ b/backend/src/schema/resolvers/follow.js @@ -1,51 +1,44 @@ +import { neode as getNeode } from '../../bootstrap/neo4j' + +const neode = getNeode() + export default { Mutation: { follow: async (_object, params, context, _resolveInfo) => { - const { id, type } = params + const { id: followedId, type } = params + const { user: currentUser } = context - const session = context.driver.session() - const transactionRes = await session.run( - `MATCH (node {id: $id}), (user:User {id: $userId}) - WHERE $type IN labels(node) AND NOT $id = $userId - MERGE (user)-[relation:FOLLOWS]->(node) - RETURN COUNT(relation) > 0 as isFollowed`, - { - id, - type, - userId: context.user.id, - }, - ) + if (type === 'User' && currentUser.id === followedId) { + return null + } - const [isFollowed] = transactionRes.records.map(record => { - return record.get('isFollowed') - }) - - session.close() - - return isFollowed + const [user, followedNode] = await Promise.all([ + neode.find('User', currentUser.id), + neode.find(type, followedId), + ]) + await user.relateTo(followedNode, 'following') + return followedNode.toJson() }, unfollow: async (_object, params, context, _resolveInfo) => { - const { id, type } = params - const session = context.driver.session() + const { id: followedId, type } = params + const { user: currentUser } = context - const transactionRes = await session.run( - `MATCH (user:User {id: $userId})-[relation:FOLLOWS]->(node {id: $id}) + /* + * Note: Neode doesn't provide an easy method for retrieving or removing relationships. + * It's suggested to use query builder feature (https://github.com/adam-cowley/neode/issues/67) + * However, pure cypher query looks cleaner IMO + */ + await neode.cypher( + `MATCH (user:User {id: $currentUser.id})-[relation:FOLLOWS]->(node {id: $followedId}) WHERE $type IN labels(node) DELETE relation RETURN COUNT(relation) > 0 as isFollowed`, - { - id, - type, - userId: context.user.id, - }, + { followedId, type, currentUser }, ) - const [isFollowed] = transactionRes.records.map(record => { - return record.get('isFollowed') - }) - session.close() - return isFollowed + const followedNode = await neode.find(type, followedId) + return followedNode.toJson() }, }, } diff --git a/backend/src/schema/resolvers/follow.spec.js b/backend/src/schema/resolvers/follow.spec.js index 2f3f1f4ad..1aa146075 100644 --- a/backend/src/schema/resolvers/follow.spec.js +++ b/backend/src/schema/resolvers/follow.spec.js @@ -17,13 +17,27 @@ let variables const mutationFollowUser = gql` mutation($id: ID!, $type: FollowTypeEnum) { - follow(id: $id, type: $type) + follow(id: $id, type: $type) { + name + followedBy { + id + name + } + followedByCurrentUser + } } ` const mutationUnfollowUser = gql` mutation($id: ID!, $type: FollowTypeEnum) { - unfollow(id: $id, type: $type) + unfollow(id: $id, type: $type) { + name + followedBy { + id + name + } + followedByCurrentUser + } } ` @@ -58,6 +72,7 @@ beforeEach(async () => { user1 = await factory .create('User', { id: 'u1', + name: 'user1', email: 'test@example.org', password: '1234', }) @@ -65,6 +80,7 @@ beforeEach(async () => { user2 = await factory .create('User', { id: 'u2', + name: 'user2', email: 'test2@example.org', password: '1234', }) @@ -81,39 +97,34 @@ afterEach(async () => { describe('follow', () => { describe('follow user', () => { describe('unauthenticated follow', () => { - it('throws authorization error', async () => { + test('throws authorization error', async () => { authenticatedUser = null - const { errors } = await mutate({ + const { errors, data } = await mutate({ mutation: mutationFollowUser, variables, }) expect(errors[0]).toHaveProperty('message', 'Not Authorised!') + expect(data).toMatchObject({ follow: null }) }) }) - it('I can follow another user', async () => { + test('I can follow another user', async () => { const { data: result } = await mutate({ mutation: mutationFollowUser, variables, }) - const expectedResult = { follow: true } - expect(result).toMatchObject(expectedResult) - - const { data } = await query({ - query: userQuery, - variables: { id: user2.id }, - }) const expectedUser = { - followedBy: [{ id: user1.id }], + name: user2.name, + followedBy: [{ id: user1.id, name: user1.name }], followedByCurrentUser: true, } - expect(data).toMatchObject({ User: [expectedUser] }) + expect(result).toMatchObject({ follow: expectedUser }) }) - it('I can`t follow myself', async () => { + test('I can`t follow myself', async () => { variables.id = user1.id const { data: result } = await mutate({ mutation: mutationFollowUser, variables }) - const expectedResult = { follow: false } + const expectedResult = { follow: null } expect(result).toMatchObject(expectedResult) const { data } = await query({ @@ -137,27 +148,22 @@ describe('follow', () => { }) describe('unauthenticated follow', () => { - it('throws authorization error', async () => { + test('throws authorization error', async () => { authenticatedUser = null - const { errors } = await mutate({ mutation: mutationUnfollowUser, variables }) + const { errors, data } = await mutate({ mutation: mutationUnfollowUser, variables }) expect(errors[0]).toHaveProperty('message', 'Not Authorised!') + expect(data).toMatchObject({ unfollow: null }) }) }) it('I can unfollow a user', async () => { const { data: result } = await mutate({ mutation: mutationUnfollowUser, variables }) - const expectedResult = { unfollow: true } - expect(result).toMatchObject(expectedResult) - - const { data } = await query({ - query: userQuery, - variables: { id: user2.id }, - }) const expectedUser = { + name: user2.name, followedBy: [], followedByCurrentUser: false, } - expect(data).toMatchObject({ User: [expectedUser] }) + expect(result).toMatchObject({ unfollow: expectedUser }) }) }) }) diff --git a/backend/src/schema/types/schema.gql b/backend/src/schema/types/schema.gql index c641763f0..620039b28 100644 --- a/backend/src/schema/types/schema.gql +++ b/backend/src/schema/types/schema.gql @@ -32,9 +32,9 @@ type Mutation { # Unshout the given Type and ID unshout(id: ID!, type: ShoutTypeEnum): Boolean! # Follow the given Type and ID - follow(id: ID!, type: FollowTypeEnum): Boolean! + follow(id: ID!, type: FollowTypeEnum): User # Unfollow the given Type and ID - unfollow(id: ID!, type: FollowTypeEnum): Boolean! + unfollow(id: ID!, type: FollowTypeEnum): User } type Report { From 39db954ec68802b979da275cb6671c00e4260053 Mon Sep 17 00:00:00 2001 From: Vasily Belolapotkov Date: Thu, 19 Sep 2019 12:10:59 +0300 Subject: [PATCH 4/4] improve follow/unfollow mutation usage --- webapp/components/FollowButton.vue | 25 ++++---- webapp/components/User/User.vue | 11 ++-- webapp/graphql/User.js | 93 +++++++++++++----------------- webapp/pages/profile/_id/_slug.vue | 36 +++++++++--- 4 files changed, 87 insertions(+), 78 deletions(-) diff --git a/webapp/components/FollowButton.vue b/webapp/components/FollowButton.vue index cc4662c85..5275035e5 100644 --- a/webapp/components/FollowButton.vue +++ b/webapp/components/FollowButton.vue @@ -15,7 +15,7 @@