From b9b103b424e9689bc1762352fb79875c2187baa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Mon, 3 Jun 2019 18:56:31 +0200 Subject: [PATCH 1/7] Implement+test replaceParams helper method --- .../middleware/filterBubble/replaceParams.js | 23 +++++ .../filterBubble/replaceParams.spec.js | 99 +++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 backend/src/middleware/filterBubble/replaceParams.js create mode 100644 backend/src/middleware/filterBubble/replaceParams.spec.js diff --git a/backend/src/middleware/filterBubble/replaceParams.js b/backend/src/middleware/filterBubble/replaceParams.js new file mode 100644 index 000000000..a0ced7b15 --- /dev/null +++ b/backend/src/middleware/filterBubble/replaceParams.js @@ -0,0 +1,23 @@ +export default async function replaceParams(args, context) { + const { author = 'all' } = args.filterBubble || {} + + if (author === 'followed') { + const session = context.driver.session() + let { records } = await session.run( + 'MATCH(followed:User)<-[:FOLLOWS]-(u {id: $userId}) RETURN followed.id', + { userId: context.user.id }, + ) + const followedIds = records.map(record => record.get('followed.id')) + + // carefully override `id_in` + args.filter = args.filter || {} + args.filter.author = args.filter.author || {} + args.filter.author.id_in = followedIds + + session.close() + } + + delete args.filterBubble + + return args +} diff --git a/backend/src/middleware/filterBubble/replaceParams.spec.js b/backend/src/middleware/filterBubble/replaceParams.spec.js new file mode 100644 index 000000000..927a362cd --- /dev/null +++ b/backend/src/middleware/filterBubble/replaceParams.spec.js @@ -0,0 +1,99 @@ +import replaceParams from './replaceParams.js' + +describe('replaceParams', () => { + let args + let context + let run + + let action = () => { + return replaceParams(args, context) + } + + beforeEach(() => { + args = {} + run = jest.fn().mockResolvedValue({ + records: [{ get: () => 1 }, { get: () => 2 }, { get: () => 3 }], + }) + context = { + user: { id: 'u4711' }, + driver: { + session: () => { + return { + run, + close: () => {}, + } + }, + }, + } + }) + + describe('given any additional filter args', () => { + describe('merges', () => { + it('empty filter object', async () => { + args = { filter: {}, filterBubble: { author: 'followed' } } + const expected = { filter: { author: { id_in: [1, 2, 3] } } } + await expect(action()).resolves.toEqual(expected) + }) + + it('filter.title', async () => { + args = { filter: { title: 'bla' }, filterBubble: { author: 'followed' } } + const expected = { filter: { title: 'bla', author: { id_in: [1, 2, 3] } } } + await expect(action()).resolves.toEqual(expected) + }) + + it('filter.author', async () => { + args = { filter: { author: { name: 'bla' } }, filterBubble: { author: 'followed' } } + const expected = { filter: { author: { name: 'bla', id_in: [1, 2, 3] } } } + await expect(action()).resolves.toEqual(expected) + }) + }) + }) + + describe('args == ', () => { + describe('{}', () => { + it('does not crash', async () => { + await expect(action()).resolves.toEqual({}) + }) + }) + + describe('{ filterBubble: { author: followed } }', () => { + beforeEach(() => { + args = { filterBubble: { author: 'followed' } } + }) + + it('returns args object with resolved ids of followed users', async () => { + const expected = { filter: { author: { id_in: [1, 2, 3] } } } + await expect(action()).resolves.toEqual(expected) + }) + + it('makes database calls', async () => { + await action() + expect(run).toHaveBeenCalled() + }) + }) + + describe('{ filterBubble: { } }', () => { + it('removes filterBubble param', async () => { + const expected = {} + await expect(action()).resolves.toEqual(expected) + }) + + it('does not make database calls', async () => { + await action() + expect(run).not.toHaveBeenCalled() + }) + }) + + describe('{ filterBubble: { author: all } }', () => { + it('removes filterBubble param', async () => { + const expected = {} + await expect(action()).resolves.toEqual(expected) + }) + + it('does not make database calls', async () => { + await action() + expect(run).not.toHaveBeenCalled() + }) + }) + }) +}) From 8e9b0318da5c811858f92aff94aad6b1a15be46d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Mon, 3 Jun 2019 19:54:02 +0200 Subject: [PATCH 2/7] Fix missing check if user is unauthenticated --- .../middleware/filterBubble/replaceParams.js | 6 + .../filterBubble/replaceParams.spec.js | 124 +++++++++++------- 2 files changed, 83 insertions(+), 47 deletions(-) diff --git a/backend/src/middleware/filterBubble/replaceParams.js b/backend/src/middleware/filterBubble/replaceParams.js index a0ced7b15..82d39d024 100644 --- a/backend/src/middleware/filterBubble/replaceParams.js +++ b/backend/src/middleware/filterBubble/replaceParams.js @@ -1,7 +1,13 @@ +import { UserInputError } from 'apollo-server' + export default async function replaceParams(args, context) { const { author = 'all' } = args.filterBubble || {} + const { user } = context if (author === 'followed') { + if (!user) + throw new UserInputError("You are unauthenticated - I don't know your followed users") + const session = context.driver.session() let { records } = await session.run( 'MATCH(followed:User)<-[:FOLLOWS]-(u {id: $userId}) RETURN followed.id', diff --git a/backend/src/middleware/filterBubble/replaceParams.spec.js b/backend/src/middleware/filterBubble/replaceParams.spec.js index 927a362cd..cd9e0a3da 100644 --- a/backend/src/middleware/filterBubble/replaceParams.spec.js +++ b/backend/src/middleware/filterBubble/replaceParams.spec.js @@ -15,7 +15,6 @@ describe('replaceParams', () => { records: [{ get: () => 1 }, { get: () => 2 }, { get: () => 3 }], }) context = { - user: { id: 'u4711' }, driver: { session: () => { return { @@ -27,28 +26,6 @@ describe('replaceParams', () => { } }) - describe('given any additional filter args', () => { - describe('merges', () => { - it('empty filter object', async () => { - args = { filter: {}, filterBubble: { author: 'followed' } } - const expected = { filter: { author: { id_in: [1, 2, 3] } } } - await expect(action()).resolves.toEqual(expected) - }) - - it('filter.title', async () => { - args = { filter: { title: 'bla' }, filterBubble: { author: 'followed' } } - const expected = { filter: { title: 'bla', author: { id_in: [1, 2, 3] } } } - await expect(action()).resolves.toEqual(expected) - }) - - it('filter.author', async () => { - args = { filter: { author: { name: 'bla' } }, filterBubble: { author: 'followed' } } - const expected = { filter: { author: { name: 'bla', id_in: [1, 2, 3] } } } - await expect(action()).resolves.toEqual(expected) - }) - }) - }) - describe('args == ', () => { describe('{}', () => { it('does not crash', async () => { @@ -56,43 +33,96 @@ describe('replaceParams', () => { }) }) - describe('{ filterBubble: { author: followed } }', () => { + describe('unauthenticated user', () => { beforeEach(() => { - args = { filterBubble: { author: 'followed' } } + context.user = null }) - it('returns args object with resolved ids of followed users', async () => { - const expected = { filter: { author: { id_in: [1, 2, 3] } } } - await expect(action()).resolves.toEqual(expected) + describe('{ filterBubble: { author: followed } }', () => { + it('throws error', async () => { + args = { filterBubble: { author: 'followed' } } + await expect(action()).rejects.toThrow('You are unauthenticated') + }) }) - it('makes database calls', async () => { - await action() - expect(run).toHaveBeenCalled() + describe('{ filterBubble: { author: all } }', () => { + it('removes filterBubble param', async () => { + const expected = {} + await expect(action()).resolves.toEqual(expected) + }) + + it('does not make database calls', async () => { + await action() + expect(run).not.toHaveBeenCalled() + }) }) }) - describe('{ filterBubble: { } }', () => { - it('removes filterBubble param', async () => { - const expected = {} - await expect(action()).resolves.toEqual(expected) + describe('authenticated user', () => { + beforeEach(() => { + context.user = { id: 'u4711' } }) - it('does not make database calls', async () => { - await action() - expect(run).not.toHaveBeenCalled() - }) - }) + describe('{ filterBubble: { author: followed } }', () => { + beforeEach(() => { + args = { filterBubble: { author: 'followed' } } + }) - describe('{ filterBubble: { author: all } }', () => { - it('removes filterBubble param', async () => { - const expected = {} - await expect(action()).resolves.toEqual(expected) + it('returns args object with resolved ids of followed users', async () => { + const expected = { filter: { author: { id_in: [1, 2, 3] } } } + await expect(action()).resolves.toEqual(expected) + }) + + it('makes database calls', async () => { + await action() + expect(run).toHaveBeenCalled() + }) + + describe('given any additional filter args', () => { + describe('merges', () => { + it('empty filter object', async () => { + args.filter = {} + const expected = { filter: { author: { id_in: [1, 2, 3] } } } + await expect(action()).resolves.toEqual(expected) + }) + + it('filter.title', async () => { + args.filter = { title: 'bla' } + const expected = { filter: { title: 'bla', author: { id_in: [1, 2, 3] } } } + await expect(action()).resolves.toEqual(expected) + }) + + it('filter.author', async () => { + args.filter = { author: { name: 'bla' } } + const expected = { filter: { author: { name: 'bla', id_in: [1, 2, 3] } } } + await expect(action()).resolves.toEqual(expected) + }) + }) + }) }) - it('does not make database calls', async () => { - await action() - expect(run).not.toHaveBeenCalled() + describe('{ filterBubble: { } }', () => { + it('removes filterBubble param', async () => { + const expected = {} + await expect(action()).resolves.toEqual(expected) + }) + + it('does not make database calls', async () => { + await action() + expect(run).not.toHaveBeenCalled() + }) + }) + + describe('{ filterBubble: { author: all } }', () => { + it('removes filterBubble param', async () => { + const expected = {} + await expect(action()).resolves.toEqual(expected) + }) + + it('does not make database calls', async () => { + await action() + expect(run).not.toHaveBeenCalled() + }) }) }) }) From ed0c9b775bc4d8d9ccf07e6fe4c7a097641492bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Mon, 3 Jun 2019 19:55:56 +0200 Subject: [PATCH 3/7] Implement+test filterBubble middleware @Tirokk @ulfgebhardt @ogerly @mattwr18 This is interesting because I found out that `neo4j-graphql-js` allows to customize graphql queries. If you define it, then it will merge the other inputs and stuff on top of it. Fair enough! --- .../middleware/filterBubble/filterBubble.js | 12 +++ .../filterBubble/filterBubble.spec.js | 76 +++++++++++++++++++ backend/src/middleware/index.js | 3 + backend/src/schema/types/type/Post.gql | 17 ++++- 4 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 backend/src/middleware/filterBubble/filterBubble.js create mode 100644 backend/src/middleware/filterBubble/filterBubble.spec.js diff --git a/backend/src/middleware/filterBubble/filterBubble.js b/backend/src/middleware/filterBubble/filterBubble.js new file mode 100644 index 000000000..bfdad5e2c --- /dev/null +++ b/backend/src/middleware/filterBubble/filterBubble.js @@ -0,0 +1,12 @@ +import replaceParams from './replaceParams' + +const replaceFilterBubbleParams = async (resolve, root, args, context, resolveInfo) => { + args = await replaceParams(args, context) + return resolve(root, args, context, resolveInfo) +} + +export default { + Query: { + Post: replaceFilterBubbleParams, + }, +} diff --git a/backend/src/middleware/filterBubble/filterBubble.spec.js b/backend/src/middleware/filterBubble/filterBubble.spec.js new file mode 100644 index 000000000..e18654868 --- /dev/null +++ b/backend/src/middleware/filterBubble/filterBubble.spec.js @@ -0,0 +1,76 @@ +import { GraphQLClient } from 'graphql-request' +import { host, login } from '../../jest/helpers' +import Factory from '../../seed/factories' + +const factory = Factory() + +const currentUserParams = { + email: 'you@example.org', + name: 'This is you', + password: '1234', +} +const followedAuthorParams = { + id: 'u2', + email: 'followed@example.org', + name: 'Followed User', + password: '1234', +} +const randomAuthorParams = { + email: 'someone@example.org', + name: 'Someone else', + password: 'else', +} + +beforeEach(async () => { + await Promise.all([ + factory.create('User', currentUserParams), + factory.create('User', followedAuthorParams), + factory.create('User', randomAuthorParams), + ]) + const [asYourself, asFollowedUser, asSomeoneElse] = await Promise.all([ + Factory().authenticateAs(currentUserParams), + Factory().authenticateAs(followedAuthorParams), + Factory().authenticateAs(randomAuthorParams), + ]) + await asYourself.follow({ id: 'u2', type: 'User' }) + await asFollowedUser.create('Post', { title: 'This is the post of a followed user' }) + await asSomeoneElse.create('Post', { title: 'This is some random post' }) +}) + +afterEach(async () => { + await factory.cleanDatabase() +}) + +describe('FilterBubble middleware', () => { + describe('given an authenticated user', () => { + let authenticatedClient + + beforeEach(async () => { + const headers = await login(currentUserParams) + authenticatedClient = new GraphQLClient(host, { headers }) + }) + + describe('no filter bubble', () => { + it('returns all posts', async () => { + const query = '{ Post( filterBubble: {}) { title } }' + const expected = { + Post: [ + { title: 'This is some random post' }, + { title: 'This is the post of a followed user' }, + ], + } + await expect(authenticatedClient.request(query)).resolves.toEqual(expected) + }) + }) + + describe('filtering for posts of followed users only', () => { + it('returns only posts authored by followed users', async () => { + const query = '{ Post( filterBubble: { author: followed }) { title } }' + const expected = { + Post: [{ title: 'This is the post of a followed user' }], + } + await expect(authenticatedClient.request(query)).resolves.toEqual(expected) + }) + }) + }) +}) diff --git a/backend/src/middleware/index.js b/backend/src/middleware/index.js index 75314abc0..6bc7be000 100644 --- a/backend/src/middleware/index.js +++ b/backend/src/middleware/index.js @@ -13,6 +13,7 @@ import includedFields from './includedFieldsMiddleware' import orderBy from './orderByMiddleware' import validation from './validation' import notifications from './notifications' +import filterBubble from './filterBubble/filterBubble' export default schema => { const middlewares = { @@ -30,11 +31,13 @@ export default schema => { user: user, includedFields: includedFields, orderBy: orderBy, + filterBubble: filterBubble, } let order = [ 'permissions', 'activityPub', + 'filterBubble', 'password', 'dateTime', 'validation', diff --git a/backend/src/schema/types/type/Post.gql b/backend/src/schema/types/type/Post.gql index 6e23862ed..f28059366 100644 --- a/backend/src/schema/types/type/Post.gql +++ b/backend/src/schema/types/type/Post.gql @@ -1,3 +1,18 @@ +enum FilterBubbleAuthorEnum { + followed + all +} + +input FilterBubble { + author: FilterBubbleAuthorEnum +} + +type Query { + Post( + filterBubble: FilterBubble + ): [Post] +} + type Post { id: ID! activityId: String @@ -40,4 +55,4 @@ type Post { RETURN COUNT(u) >= 1 """ ) -} \ No newline at end of file +} From 0d8a74809432866056b878b6f340af0471d68db2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Mon, 3 Jun 2019 23:04:43 +0200 Subject: [PATCH 4/7] Apparently I was wrong: types don't get merged --- backend/src/schema/types/type/Post.gql | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/backend/src/schema/types/type/Post.gql b/backend/src/schema/types/type/Post.gql index f28059366..e932bafbf 100644 --- a/backend/src/schema/types/type/Post.gql +++ b/backend/src/schema/types/type/Post.gql @@ -9,6 +9,28 @@ input FilterBubble { type Query { Post( + id: ID + activityId: String + objectId: String + title: String + slug: String + content: String + contentExcerpt: String + image: String + imageUpload: Upload + visibility: Visibility + deleted: Boolean + disabled: Boolean + createdAt: String + updatedAt: String + commentsCount: Int + shoutedCount: Int + shoutedByCurrentUser: Boolean + _id: String + first: Int + offset: Int + orderBy: [_PostOrdering] + filter: _PostFilter filterBubble: FilterBubble ): [Post] } From d59c43330b274d2297ffcffeaedde993073203fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 5 Jun 2019 18:34:02 +0200 Subject: [PATCH 5/7] Rename value of enum type to `following` As discussed in our daily standup with @mattwr18 --- backend/src/middleware/filterBubble/filterBubble.spec.js | 2 +- backend/src/middleware/filterBubble/replaceParams.js | 4 ++-- backend/src/middleware/filterBubble/replaceParams.spec.js | 8 ++++---- backend/src/schema/types/type/Post.gql | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/backend/src/middleware/filterBubble/filterBubble.spec.js b/backend/src/middleware/filterBubble/filterBubble.spec.js index e18654868..afe1df1c9 100644 --- a/backend/src/middleware/filterBubble/filterBubble.spec.js +++ b/backend/src/middleware/filterBubble/filterBubble.spec.js @@ -65,7 +65,7 @@ describe('FilterBubble middleware', () => { describe('filtering for posts of followed users only', () => { it('returns only posts authored by followed users', async () => { - const query = '{ Post( filterBubble: { author: followed }) { title } }' + const query = '{ Post( filterBubble: { author: following }) { title } }' const expected = { Post: [{ title: 'This is the post of a followed user' }], } diff --git a/backend/src/middleware/filterBubble/replaceParams.js b/backend/src/middleware/filterBubble/replaceParams.js index 82d39d024..43877eb43 100644 --- a/backend/src/middleware/filterBubble/replaceParams.js +++ b/backend/src/middleware/filterBubble/replaceParams.js @@ -4,9 +4,9 @@ export default async function replaceParams(args, context) { const { author = 'all' } = args.filterBubble || {} const { user } = context - if (author === 'followed') { + if (author === 'following') { if (!user) - throw new UserInputError("You are unauthenticated - I don't know your followed users") + throw new UserInputError("You are unauthenticated - I don't know any users you are following.") const session = context.driver.session() let { records } = await session.run( diff --git a/backend/src/middleware/filterBubble/replaceParams.spec.js b/backend/src/middleware/filterBubble/replaceParams.spec.js index cd9e0a3da..5566e4a91 100644 --- a/backend/src/middleware/filterBubble/replaceParams.spec.js +++ b/backend/src/middleware/filterBubble/replaceParams.spec.js @@ -38,9 +38,9 @@ describe('replaceParams', () => { context.user = null }) - describe('{ filterBubble: { author: followed } }', () => { + describe('{ filterBubble: { author: following } }', () => { it('throws error', async () => { - args = { filterBubble: { author: 'followed' } } + args = { filterBubble: { author: 'following' } } await expect(action()).rejects.toThrow('You are unauthenticated') }) }) @@ -63,9 +63,9 @@ describe('replaceParams', () => { context.user = { id: 'u4711' } }) - describe('{ filterBubble: { author: followed } }', () => { + describe('{ filterBubble: { author: following } }', () => { beforeEach(() => { - args = { filterBubble: { author: 'followed' } } + args = { filterBubble: { author: 'following' } } }) it('returns args object with resolved ids of followed users', async () => { diff --git a/backend/src/schema/types/type/Post.gql b/backend/src/schema/types/type/Post.gql index e932bafbf..1179c3e20 100644 --- a/backend/src/schema/types/type/Post.gql +++ b/backend/src/schema/types/type/Post.gql @@ -1,5 +1,5 @@ enum FilterBubbleAuthorEnum { - followed + following all } From 16ef304d7051231c1fa462fb473080b262e51f4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 5 Jun 2019 18:37:19 +0200 Subject: [PATCH 6/7] Follow suggestions by @mattwr18 --- backend/src/middleware/filterBubble/replaceParams.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/middleware/filterBubble/replaceParams.spec.js b/backend/src/middleware/filterBubble/replaceParams.spec.js index 5566e4a91..e14fda416 100644 --- a/backend/src/middleware/filterBubble/replaceParams.spec.js +++ b/backend/src/middleware/filterBubble/replaceParams.spec.js @@ -75,7 +75,7 @@ describe('replaceParams', () => { it('makes database calls', async () => { await action() - expect(run).toHaveBeenCalled() + expect(run).toHaveBeenCalledTimes(1) }) describe('given any additional filter args', () => { From 93107bc0f8a568b2b62acfe266c9341b3ed02d0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Sch=C3=A4fer?= Date: Wed, 5 Jun 2019 18:39:11 +0200 Subject: [PATCH 7/7] Fix lint --- backend/src/middleware/filterBubble/replaceParams.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/src/middleware/filterBubble/replaceParams.js b/backend/src/middleware/filterBubble/replaceParams.js index 43877eb43..a10b6c29d 100644 --- a/backend/src/middleware/filterBubble/replaceParams.js +++ b/backend/src/middleware/filterBubble/replaceParams.js @@ -6,7 +6,9 @@ export default async function replaceParams(args, context) { if (author === 'following') { if (!user) - throw new UserInputError("You are unauthenticated - I don't know any users you are following.") + throw new UserInputError( + "You are unauthenticated - I don't know any users you are following.", + ) const session = context.driver.session() let { records } = await session.run(