From b87f2a52cfc1ae73463254682c70774c61ef2134 Mon Sep 17 00:00:00 2001 From: aonomike Date: Sat, 12 Oct 2019 15:04:43 +0300 Subject: [PATCH 1/5] Refactor rewards spec - Refactor unauthorised path --- backend/src/schema/resolvers/rewards.spec.js | 48 ++++++++++++++------ 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/backend/src/schema/resolvers/rewards.spec.js b/backend/src/schema/resolvers/rewards.spec.js index 3b94e93aa..ea6f737b0 100644 --- a/backend/src/schema/resolvers/rewards.spec.js +++ b/backend/src/schema/resolvers/rewards.spec.js @@ -1,10 +1,14 @@ -import { GraphQLClient } from 'graphql-request' +import { createTestClient } from 'apollo-server-testing' import Factory from '../../seed/factories' -import { host, login, gql } from '../../jest/helpers' +import { gql } from '../../jest/helpers' +import { neode as getNeode, getDriver } from '../../bootstrap/neo4j' +import createServer from '../../server' const factory = Factory() -let user -let badge +const driver = getDriver() +const instance = getNeode() + +let authenticatedUser, regularUser, badge, query, mutate, variables describe('rewards', () => { const variables = { @@ -12,20 +16,35 @@ describe('rewards', () => { to: 'u1', } + beforeAll(async()=>{ + const { server } = createServer({ + context: () => { + return { + driver, + neode: instance, + user: authenticatedUser, + } + }, + }) + query = createTestClient(server).query + mutate = createTestClient(server).mutate + }) + beforeEach(async () => { - user = await factory.create('User', { - id: 'u1', + + regularUser = await factory.create('User', { + id: 'regular-user-id', role: 'user', email: 'user@example.org', password: '1234', }) await factory.create('User', { - id: 'u2', + id: 'moderator-id', role: 'moderator', email: 'moderator@example.org', }) await factory.create('User', { - id: 'u3', + id: 'admin-id', role: 'admin', email: 'admin@example.org', }) @@ -54,11 +73,14 @@ describe('rewards', () => { ` describe('unauthenticated', () => { - let client - - it('throws authorization error', async () => { - client = new GraphQLClient(host) - await expect(client.request(mutation, variables)).rejects.toThrow('Not Authorised') + it.only('throws authorization error', async () => { + const variables = { + from: 'indiegogo_en_rhino', + to: 'regular-user-id', + } + authenticatedUser = null + await expect(mutate({mutation, variables})).resolves.toMatchObject({ data: {reward: null}, + errors: [{ message: 'Not Authorised!' }]}) }) }) From bd834824650af01df5c5ed8ba35a0de4cd03f564 Mon Sep 17 00:00:00 2001 From: aonomike Date: Sat, 12 Oct 2019 16:50:58 +0300 Subject: [PATCH 2/5] Complete refactor of logged in administrator spec path --- backend/src/schema/resolvers/rewards.spec.js | 144 +++++++++++++------ 1 file changed, 98 insertions(+), 46 deletions(-) diff --git a/backend/src/schema/resolvers/rewards.spec.js b/backend/src/schema/resolvers/rewards.spec.js index ea6f737b0..4965f3fcf 100644 --- a/backend/src/schema/resolvers/rewards.spec.js +++ b/backend/src/schema/resolvers/rewards.spec.js @@ -2,13 +2,14 @@ import { createTestClient } from 'apollo-server-testing' import Factory from '../../seed/factories' import { gql } from '../../jest/helpers' import { neode as getNeode, getDriver } from '../../bootstrap/neo4j' -import createServer from '../../server' +import createServer from '../../server' +import { addMinutes } from 'date-fns' const factory = Factory() const driver = getDriver() const instance = getNeode() -let authenticatedUser, regularUser, badge, query, mutate, variables +let authenticatedUser, regularUser, administrator, moderator, badge, query, mutate, variables describe('rewards', () => { const variables = { @@ -16,7 +17,7 @@ describe('rewards', () => { to: 'u1', } - beforeAll(async()=>{ + beforeAll(async () => { const { server } = createServer({ context: () => { return { @@ -31,19 +32,18 @@ describe('rewards', () => { }) beforeEach(async () => { - regularUser = await factory.create('User', { id: 'regular-user-id', role: 'user', email: 'user@example.org', password: '1234', }) - await factory.create('User', { + moderator = await factory.create('User', { id: 'moderator-id', role: 'moderator', email: 'moderator@example.org', }) - await factory.create('User', { + administrator = await factory.create('User', { id: 'admin-id', role: 'admin', email: 'admin@example.org', @@ -73,54 +73,62 @@ describe('rewards', () => { ` describe('unauthenticated', () => { - it.only('throws authorization error', async () => { + it('throws authorization error', async () => { const variables = { from: 'indiegogo_en_rhino', to: 'regular-user-id', } authenticatedUser = null - await expect(mutate({mutation, variables})).resolves.toMatchObject({ data: {reward: null}, - errors: [{ message: 'Not Authorised!' }]}) + await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + data: { reward: null }, + errors: [{ message: 'Not Authorised!' }], + }) }) }) describe('authenticated admin', () => { - let client beforeEach(async () => { - const headers = await login({ email: 'admin@example.org', password: '1234' }) - client = new GraphQLClient(host, { headers }) + authenticatedUser = await administrator.toJson() }) describe('badge for id does not exist', () => { it('rejects with a telling error message', async () => { await expect( - client.request(mutation, { - ...variables, - from: 'bullshit', - }), - ).rejects.toThrow("Couldn't find a badge with that id") + mutate({ mutation, variables: { to: 'regular-user-id', from: 'ndiegogo_en_rhino' } }), + ).resolves.toMatchObject({ + data: { reward: null }, + errors: [{ message: "Couldn't find a badge with that id" }], + }) }) }) describe('user for id does not exist', () => { it('rejects with a telling error message', async () => { await expect( - client.request(mutation, { - ...variables, - to: 'bullshit', + mutate({ + mutation, + variables: { to: 'non-existent-user-id', from: 'non-existent-badge' }, }), - ).rejects.toThrow("Couldn't find a user with that id") + ).resolves.toMatchObject({ + data: { reward: null }, + errors: [{ message: "Couldn't find a user with that id" }], + }) }) }) it('rewards a badge to user', async () => { const expected = { - reward: { - id: 'u1', - badges: [{ id: 'indiegogo_en_rhino' }], + data: { + reward: { + id: 'regular-user-id', + badges: [{ id: 'indiegogo_en_rhino' }], + }, }, + errors: undefined, } - await expect(client.request(mutation, variables)).resolves.toEqual(expected) + await expect( + mutate({ mutation, variables: { to: 'regular-user-id', from: 'indiegogo_en_rhino' } }), + ).resolves.toMatchObject(expected) }) it('rewards a second different badge to same user', async () => { @@ -130,42 +138,83 @@ describe('rewards', () => { }) const badges = [{ id: 'indiegogo_en_racoon' }, { id: 'indiegogo_en_rhino' }] const expected = { - reward: { - id: 'u1', - badges: expect.arrayContaining(badges), + data: { + reward: { + id: 'regular-user-id', + badges: expect.arrayContaining(badges), + }, }, + errors: undefined, } - await client.request(mutation, variables) + await mutate({ + mutation, + variables: { + to: 'regular-user-id', + from: 'indiegogo_en_rhino', + }, + }) await expect( - client.request(mutation, { - ...variables, - from: 'indiegogo_en_racoon', + mutate({ + mutation, + variables: { + to: 'regular-user-id', + from: 'indiegogo_en_racoon', + }, }), - ).resolves.toEqual(expected) + ).resolves.toMatchObject(expected) }) it('rewards the same badge as well to another user', async () => { const expected = { - reward: { - id: 'u2', - badges: [{ id: 'indiegogo_en_rhino' }], + data: { + reward: { + id: 'regular-user-2-id', + badges: [{ id: 'indiegogo_en_rhino' }], + }, }, + errors: undefined, } + await factory.create('User', { + id: 'regular-user-2-id', + email: 'regular2@email.com', + }) + await mutate({ + mutation, + variables: { + to: 'regular-user-id', + from: 'indiegogo_en_rhino', + }, + }) await expect( - client.request(mutation, { - ...variables, - to: 'u2', + mutate({ + mutation, + variables: { + to: 'regular-user-2-id', + from: 'indiegogo_en_rhino', + }, }), - ).resolves.toEqual(expected) + ).resolves.toMatchObject(expected) }) it('creates no duplicate reward relationships', async () => { - await client.request(mutation, variables) - await client.request(mutation, variables) + await mutate({ + mutation, + variables: { + to: 'regular-user-id', + from: 'indiegogo_en_rhino', + }, + }) + await mutate({ + mutation, + variables: { + to: 'regular-user-id', + from: 'indiegogo_en_rhino', + }, + }) - const query = gql` + const userQuery = gql` { - User(id: "u1") { + User(id: "regular-user-id") { badgesCount badges { id @@ -173,9 +222,12 @@ describe('rewards', () => { } } ` - const expected = { User: [{ badgesCount: 1, badges: [{ id: 'indiegogo_en_rhino' }] }] } + const expected = { + data: { User: [{ badgesCount: 1, badges: [{ id: 'indiegogo_en_rhino' }] }] }, + errors: undefined, + } - await expect(client.request(query)).resolves.toEqual(expected) + await expect(query({ query: userQuery })).resolves.toMatchObject(expected) }) }) From dcb40c2c33ef89fda0f04f18c8dd3abe8ad4a377 Mon Sep 17 00:00:00 2001 From: aonomike Date: Sat, 12 Oct 2019 17:00:36 +0300 Subject: [PATCH 3/5] refactor moderator user path --- backend/src/schema/resolvers/rewards.spec.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/src/schema/resolvers/rewards.spec.js b/backend/src/schema/resolvers/rewards.spec.js index 4965f3fcf..2acadfb8c 100644 --- a/backend/src/schema/resolvers/rewards.spec.js +++ b/backend/src/schema/resolvers/rewards.spec.js @@ -232,15 +232,15 @@ describe('rewards', () => { }) describe('authenticated moderator', () => { - let client beforeEach(async () => { - const headers = await login({ email: 'moderator@example.org', password: '1234' }) - client = new GraphQLClient(host, { headers }) + authenticatedUser = moderator.toJson() }) - describe('rewards bage to user', () => { + describe('rewards badge to user', () => { it('throws authorization error', async () => { - await expect(client.request(mutation, variables)).rejects.toThrow('Not Authorised') + await expect( + mutate({ mutation, variables: { to: 'regular-user-id', from: 'indiegogo_en_rhino' } }), + ).resolves.toMatchObject({data: {reward: null}, errors: [{message: 'Not Authorised!'}]}) }) }) }) From 0fe94a5d82ce08eae89b0c569bd193d4d37e71cb Mon Sep 17 00:00:00 2001 From: aonomike Date: Sat, 12 Oct 2019 17:39:29 +0300 Subject: [PATCH 4/5] Complete refactor rewards spec - Refactor unreward path - refactor to use variables constant --- backend/src/schema/resolvers/rewards.spec.js | 83 +++++++++----------- 1 file changed, 37 insertions(+), 46 deletions(-) diff --git a/backend/src/schema/resolvers/rewards.spec.js b/backend/src/schema/resolvers/rewards.spec.js index 2acadfb8c..2af16094d 100644 --- a/backend/src/schema/resolvers/rewards.spec.js +++ b/backend/src/schema/resolvers/rewards.spec.js @@ -3,18 +3,17 @@ import Factory from '../../seed/factories' import { gql } from '../../jest/helpers' import { neode as getNeode, getDriver } from '../../bootstrap/neo4j' import createServer from '../../server' -import { addMinutes } from 'date-fns' const factory = Factory() const driver = getDriver() const instance = getNeode() -let authenticatedUser, regularUser, administrator, moderator, badge, query, mutate, variables +let authenticatedUser, regularUser, administrator, moderator, badge, query, mutate describe('rewards', () => { const variables = { from: 'indiegogo_en_rhino', - to: 'u1', + to: 'regular-user-id', } beforeAll(async () => { @@ -74,10 +73,6 @@ describe('rewards', () => { describe('unauthenticated', () => { it('throws authorization error', async () => { - const variables = { - from: 'indiegogo_en_rhino', - to: 'regular-user-id', - } authenticatedUser = null await expect(mutate({ mutation, variables })).resolves.toMatchObject({ data: { reward: null }, @@ -126,9 +121,7 @@ describe('rewards', () => { }, errors: undefined, } - await expect( - mutate({ mutation, variables: { to: 'regular-user-id', from: 'indiegogo_en_rhino' } }), - ).resolves.toMatchObject(expected) + await expect(mutate({ mutation, variables })).resolves.toMatchObject(expected) }) it('rewards a second different badge to same user', async () => { @@ -180,10 +173,7 @@ describe('rewards', () => { }) await mutate({ mutation, - variables: { - to: 'regular-user-id', - from: 'indiegogo_en_rhino', - }, + variables, }) await expect( mutate({ @@ -199,17 +189,11 @@ describe('rewards', () => { it('creates no duplicate reward relationships', async () => { await mutate({ mutation, - variables: { - to: 'regular-user-id', - from: 'indiegogo_en_rhino', - }, + variables, }) await mutate({ mutation, - variables: { - to: 'regular-user-id', - from: 'indiegogo_en_rhino', - }, + variables, }) const userQuery = gql` @@ -238,9 +222,10 @@ describe('rewards', () => { describe('rewards badge to user', () => { it('throws authorization error', async () => { - await expect( - mutate({ mutation, variables: { to: 'regular-user-id', from: 'indiegogo_en_rhino' } }), - ).resolves.toMatchObject({data: {reward: null}, errors: [{message: 'Not Authorised!'}]}) + await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + data: { reward: null }, + errors: [{ message: 'Not Authorised!' }], + }) }) }) }) @@ -248,9 +233,12 @@ describe('rewards', () => { describe('unreward', () => { beforeEach(async () => { - await user.relateTo(badge, 'rewarded') + await regularUser.relateTo(badge, 'rewarded') }) - const expected = { unreward: { id: 'u1', badges: [] } } + const expected = { + data: { unreward: { id: 'regular-user-id', badges: [] } }, + errors: undefined, + } const mutation = gql` mutation($from: ID!, $to: ID!) { @@ -265,9 +253,10 @@ describe('rewards', () => { describe('check test setup', () => { it('user has one badge', async () => { - const query = gql` + authenticatedUser = regularUser.toJson() + const userQuery = gql` { - User(id: "u1") { + User(id: "regular-user-id") { badgesCount badges { id @@ -275,48 +264,50 @@ describe('rewards', () => { } } ` - const expected = { User: [{ badgesCount: 1, badges: [{ id: 'indiegogo_en_rhino' }] }] } - const client = new GraphQLClient(host) - await expect(client.request(query)).resolves.toEqual(expected) + const expected = { + data: { User: [{ badgesCount: 1, badges: [{ id: 'indiegogo_en_rhino' }] }] }, + errors: undefined, + } + await expect(query({ query: userQuery })).resolves.toMatchObject(expected) }) }) describe('unauthenticated', () => { - let client - it('throws authorization error', async () => { - client = new GraphQLClient(host) - await expect(client.request(mutation, variables)).rejects.toThrow('Not Authorised') + authenticatedUser = null + await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + data: { unreward: null }, + errors: [{ message: 'Not Authorised!' }], + }) }) }) describe('authenticated admin', () => { - let client beforeEach(async () => { - const headers = await login({ email: 'admin@example.org', password: '1234' }) - client = new GraphQLClient(host, { headers }) + authenticatedUser = await administrator.toJson() }) it('removes a badge from user', async () => { - await expect(client.request(mutation, variables)).resolves.toEqual(expected) + await expect(mutate({ mutation, variables })).resolves.toMatchObject(expected) }) it('does not crash when unrewarding multiple times', async () => { - await client.request(mutation, variables) - await expect(client.request(mutation, variables)).resolves.toEqual(expected) + await mutate({ mutation, variables }) + await expect(mutate({ mutation, variables })).resolves.toMatchObject(expected) }) }) describe('authenticated moderator', () => { - let client beforeEach(async () => { - const headers = await login({ email: 'moderator@example.org', password: '1234' }) - client = new GraphQLClient(host, { headers }) + authenticatedUser = await moderator.toJson() }) describe('removes bage from user', () => { it('throws authorization error', async () => { - await expect(client.request(mutation, variables)).rejects.toThrow('Not Authorised') + await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + data: { unreward: null }, + errors: [{ message: 'Not Authorised!' }], + }) }) }) }) From 47a2a21fc0db732d8b52992048f8fb9076f0bc5d Mon Sep 17 00:00:00 2001 From: aonomike Date: Mon, 14 Oct 2019 16:46:37 +0300 Subject: [PATCH 5/5] Resolve review comments --- backend/src/schema/resolvers/rewards.spec.js | 51 ++++++++++++-------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/backend/src/schema/resolvers/rewards.spec.js b/backend/src/schema/resolvers/rewards.spec.js index 2af16094d..a81133373 100644 --- a/backend/src/schema/resolvers/rewards.spec.js +++ b/backend/src/schema/resolvers/rewards.spec.js @@ -60,7 +60,7 @@ describe('rewards', () => { }) describe('reward', () => { - const mutation = gql` + const rewardMutation = gql` mutation($from: ID!, $to: ID!) { reward(badgeKey: $from, userId: $to) { id @@ -74,7 +74,7 @@ describe('rewards', () => { describe('unauthenticated', () => { it('throws authorization error', async () => { authenticatedUser = null - await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + await expect(mutate({ mutation: rewardMutation, variables })).resolves.toMatchObject({ data: { reward: null }, errors: [{ message: 'Not Authorised!' }], }) @@ -87,9 +87,12 @@ describe('rewards', () => { }) describe('badge for id does not exist', () => { - it('rejects with a telling error message', async () => { + it('rejects with an informative error message', async () => { await expect( - mutate({ mutation, variables: { to: 'regular-user-id', from: 'ndiegogo_en_rhino' } }), + mutate({ + mutation: rewardMutation, + variables: { to: 'regular-user-id', from: 'non-existent-badge-id' }, + }), ).resolves.toMatchObject({ data: { reward: null }, errors: [{ message: "Couldn't find a badge with that id" }], @@ -97,12 +100,12 @@ describe('rewards', () => { }) }) - describe('user for id does not exist', () => { + describe('non-existent user', () => { it('rejects with a telling error message', async () => { await expect( mutate({ - mutation, - variables: { to: 'non-existent-user-id', from: 'non-existent-badge' }, + mutation: rewardMutation, + variables: { to: 'non-existent-user-id', from: 'indiegogo_en_rhino' }, }), ).resolves.toMatchObject({ data: { reward: null }, @@ -121,7 +124,9 @@ describe('rewards', () => { }, errors: undefined, } - await expect(mutate({ mutation, variables })).resolves.toMatchObject(expected) + await expect(mutate({ mutation: rewardMutation, variables })).resolves.toMatchObject( + expected, + ) }) it('rewards a second different badge to same user', async () => { @@ -140,7 +145,7 @@ describe('rewards', () => { errors: undefined, } await mutate({ - mutation, + mutation: rewardMutation, variables: { to: 'regular-user-id', from: 'indiegogo_en_rhino', @@ -148,7 +153,7 @@ describe('rewards', () => { }) await expect( mutate({ - mutation, + mutation: rewardMutation, variables: { to: 'regular-user-id', from: 'indiegogo_en_racoon', @@ -172,12 +177,12 @@ describe('rewards', () => { email: 'regular2@email.com', }) await mutate({ - mutation, + mutation: rewardMutation, variables, }) await expect( mutate({ - mutation, + mutation: rewardMutation, variables: { to: 'regular-user-2-id', from: 'indiegogo_en_rhino', @@ -188,11 +193,11 @@ describe('rewards', () => { it('creates no duplicate reward relationships', async () => { await mutate({ - mutation, + mutation: rewardMutation, variables, }) await mutate({ - mutation, + mutation: rewardMutation, variables, }) @@ -222,7 +227,7 @@ describe('rewards', () => { describe('rewards badge to user', () => { it('throws authorization error', async () => { - await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + await expect(mutate({ mutation: rewardMutation, variables })).resolves.toMatchObject({ data: { reward: null }, errors: [{ message: 'Not Authorised!' }], }) @@ -240,7 +245,7 @@ describe('rewards', () => { errors: undefined, } - const mutation = gql` + const unrewardMutation = gql` mutation($from: ID!, $to: ID!) { unreward(badgeKey: $from, userId: $to) { id @@ -275,7 +280,7 @@ describe('rewards', () => { describe('unauthenticated', () => { it('throws authorization error', async () => { authenticatedUser = null - await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + await expect(mutate({ mutation: unrewardMutation, variables })).resolves.toMatchObject({ data: { unreward: null }, errors: [{ message: 'Not Authorised!' }], }) @@ -288,12 +293,16 @@ describe('rewards', () => { }) it('removes a badge from user', async () => { - await expect(mutate({ mutation, variables })).resolves.toMatchObject(expected) + await expect(mutate({ mutation: unrewardMutation, variables })).resolves.toMatchObject( + expected, + ) }) it('does not crash when unrewarding multiple times', async () => { - await mutate({ mutation, variables }) - await expect(mutate({ mutation, variables })).resolves.toMatchObject(expected) + await mutate({ mutation: unrewardMutation, variables }) + await expect(mutate({ mutation: unrewardMutation, variables })).resolves.toMatchObject( + expected, + ) }) }) @@ -304,7 +313,7 @@ describe('rewards', () => { describe('removes bage from user', () => { it('throws authorization error', async () => { - await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + await expect(mutate({ mutation: unrewardMutation, variables })).resolves.toMatchObject({ data: { unreward: null }, errors: [{ message: 'Not Authorised!' }], })