From 75d984073d44101998d329d758bc7ada0646b566 Mon Sep 17 00:00:00 2001 From: Matt Rider Date: Mon, 26 Aug 2019 16:19:42 +0200 Subject: [PATCH 01/42] Check there are ids in the badIds array - if there are no ids, we shouldn't add an empty array since it adds unneccessarily to our auto-generated post query and affects greatly performance. see issue #1390 --- backend/src/schema/resolvers/posts.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/backend/src/schema/resolvers/posts.js b/backend/src/schema/resolvers/posts.js index 46d7c414f..ea715f676 100644 --- a/backend/src/schema/resolvers/posts.js +++ b/backend/src/schema/resolvers/posts.js @@ -11,17 +11,19 @@ const filterForBlockedUsers = async (params, context) => { getBlockedByUsers(context), ]) const badIds = [...blockedByUsers.map(b => b.id), ...blockedUsers.map(b => b.id)] - params.filter = mergeWith( - params.filter, - { - author_not: { id_in: badIds }, - }, - (objValue, srcValue) => { - if (isArray(objValue)) { - return objValue.concat(srcValue) - } - }, - ) + if (badIds.length) { + params.filter = mergeWith( + params.filter, + { + author_not: { id_in: badIds }, + }, + (objValue, srcValue) => { + if (isArray(objValue)) { + return objValue.concat(srcValue) + } + }, + ) + } return params } From 9f6757f409642046866fd45bdb36995dc21da4df Mon Sep 17 00:00:00 2001 From: roschaefer Date: Mon, 26 Aug 2019 16:53:22 +0200 Subject: [PATCH 02/42] Use guard clauses if possible --- backend/src/schema/resolvers/posts.js | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/backend/src/schema/resolvers/posts.js b/backend/src/schema/resolvers/posts.js index ea715f676..a265c28f0 100644 --- a/backend/src/schema/resolvers/posts.js +++ b/backend/src/schema/resolvers/posts.js @@ -11,19 +11,19 @@ const filterForBlockedUsers = async (params, context) => { getBlockedByUsers(context), ]) const badIds = [...blockedByUsers.map(b => b.id), ...blockedUsers.map(b => b.id)] - if (badIds.length) { - params.filter = mergeWith( - params.filter, - { - author_not: { id_in: badIds }, - }, - (objValue, srcValue) => { - if (isArray(objValue)) { - return objValue.concat(srcValue) - } - }, - ) - } + if (!badIds.length) return params + + params.filter = mergeWith( + params.filter, + { + author_not: { id_in: badIds }, + }, + (objValue, srcValue) => { + if (isArray(objValue)) { + return objValue.concat(srcValue) + } + }, + ) return params } From b990a1055b48515866c31c131c9aa0553577fa34 Mon Sep 17 00:00:00 2001 From: Matt Rider Date: Mon, 26 Aug 2019 17:51:44 +0200 Subject: [PATCH 03/42] Fix search functionality - remove unused line that allows for an empty array of author_not.id_in --- backend/src/schema/resolvers/posts.js | 2 +- backend/src/schema/types/schema.gql | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/backend/src/schema/resolvers/posts.js b/backend/src/schema/resolvers/posts.js index a265c28f0..17107c241 100644 --- a/backend/src/schema/resolvers/posts.js +++ b/backend/src/schema/resolvers/posts.js @@ -35,7 +35,7 @@ export default { }, findPosts: async (object, params, context, resolveInfo) => { params = await filterForBlockedUsers(params, context) - return neo4jgraphql(object, params, context, resolveInfo, false) + return neo4jgraphql(object, params, context, resolveInfo, true) }, PostsEmotionsCountByEmotion: async (object, params, context, resolveInfo) => { const session = context.driver.session() diff --git a/backend/src/schema/types/schema.gql b/backend/src/schema/types/schema.gql index e0a2c328b..d4959889f 100644 --- a/backend/src/schema/types/schema.gql +++ b/backend/src/schema/types/schema.gql @@ -13,7 +13,6 @@ type Query { WHERE score >= 0.2 AND NOT user.deleted = true AND NOT user.disabled = true AND NOT post.deleted = true AND NOT post.disabled = true - AND NOT user.id in COALESCE($filter.author_not.id_in, []) RETURN post LIMIT $limit """ From 6de602fd5885b0c39fb7ee06ffc0f7f40b8c9218 Mon Sep 17 00:00:00 2001 From: Matt Rider Date: Mon, 26 Aug 2019 21:37:36 +0200 Subject: [PATCH 04/42] Revert @roschaefer's hacky fix to Search.feature - now that we are checking to see if there are any badIds, it was breaking the search functionality --- backend/src/schema/resolvers/posts.js | 2 +- backend/src/schema/types/schema.gql | 4 ++-- webapp/store/search.js | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/backend/src/schema/resolvers/posts.js b/backend/src/schema/resolvers/posts.js index 17107c241..a265c28f0 100644 --- a/backend/src/schema/resolvers/posts.js +++ b/backend/src/schema/resolvers/posts.js @@ -35,7 +35,7 @@ export default { }, findPosts: async (object, params, context, resolveInfo) => { params = await filterForBlockedUsers(params, context) - return neo4jgraphql(object, params, context, resolveInfo, true) + return neo4jgraphql(object, params, context, resolveInfo, false) }, PostsEmotionsCountByEmotion: async (object, params, context, resolveInfo) => { const session = context.driver.session() diff --git a/backend/src/schema/types/schema.gql b/backend/src/schema/types/schema.gql index d4959889f..6d9ec99db 100644 --- a/backend/src/schema/types/schema.gql +++ b/backend/src/schema/types/schema.gql @@ -4,10 +4,10 @@ type Query { currentUser: User # Get the latest Network Statistics statistics: Statistics! - findPosts(query: String!, limit: Int = 10): [Post]! + findPosts(filter: String!, limit: Int = 10): [Post]! @cypher( statement: """ - CALL db.index.fulltext.queryNodes('full_text_search', $query) + CALL db.index.fulltext.queryNodes('full_text_search', $filter) YIELD node as post, score MATCH (post)<-[:WROTE]-(user:User) WHERE score >= 0.2 diff --git a/webapp/store/search.js b/webapp/store/search.js index 0d0172a98..103cb2493 100644 --- a/webapp/store/search.js +++ b/webapp/store/search.js @@ -46,8 +46,8 @@ export const actions = { await this.app.apolloProvider.defaultClient .query({ query: gql` - query findPosts($query: String!) { - findPosts(query: $query, limit: 10) { + query findPosts($filter: String!) { + findPosts(filter: $filter, limit: 10) { id slug label: title @@ -63,7 +63,7 @@ export const actions = { } `, variables: { - query: value.replace(/\s/g, '~ ') + '~', + filter: value.replace(/\s/g, '~ ') + '~', }, }) .then(res => { From e4d57f80aabc8833beb892957a2d0049c9e9233c Mon Sep 17 00:00:00 2001 From: roschaefer Date: Fri, 23 Aug 2019 01:27:53 +0200 Subject: [PATCH 05/42] Fix #1333 Ok, so here are multiple issues: 1. In cypher, `NOT NULL` will return `NULL` not `FALSE`. If we want `FALSE` to be set in the database import, we should use `COAELESCE` to find the first not-null value. See: https://neo4j.com/docs/cypher-manual/current/syntax/working-with-null/ https://markhneedham.com/blog/2017/02/22/neo4j-null-values-even-work/ 2. I removed the `disabled` and `deleted` checks on the commented counter. With `neo4j-graphql-js` it is not possible to filter on the join models (at least not without a lot of complexity) for disabled or deleted items. Let's live with the fact that the list of commented posts will include those posts, where the user has deleted his comment or where the user's comment was disabled. It's being displayed as "not available" so I think this is OK for now. 3. De-couple the pagination counters from the "commented", "shouted" etc. counters. It might be that the list of posts is different for different users. E.g. if the user has blocked you, the "posts" list will be empty. The "shouted" or "commented" list will not have the posts of the author. If you are a moderator, the list will include disabled posts. So the counters are not in sync with the actual list coming from the backend. Therefore I implemented "fetch and check if resultSet < pageSize" instead of a global counter. --- .../src/schema/resolvers/helpers/Resolver.js | 1 - backend/src/schema/resolvers/users.js | 9 ++- backend/src/schema/types/type/User.gql | 2 +- .../neo4j/contributions/contributions.cql | 4 +- webapp/pages/index.vue | 3 - webapp/pages/profile/_id/_slug.spec.js | 12 +-- webapp/pages/profile/_id/_slug.vue | 76 +++++++------------ 7 files changed, 39 insertions(+), 68 deletions(-) diff --git a/backend/src/schema/resolvers/helpers/Resolver.js b/backend/src/schema/resolvers/helpers/Resolver.js index fd41205a3..9a6f77513 100644 --- a/backend/src/schema/resolvers/helpers/Resolver.js +++ b/backend/src/schema/resolvers/helpers/Resolver.js @@ -61,7 +61,6 @@ export default function Resolver(type, options = {}) { const id = parent[idAttribute] const statement = ` MATCH(u:${type} {${idAttribute}: {id}})${connection} - WHERE NOT related.deleted = true AND NOT related.disabled = true RETURN COUNT(DISTINCT(related)) as count ` const result = await instance.cypher(statement, { id }) diff --git a/backend/src/schema/resolvers/users.js b/backend/src/schema/resolvers/users.js index 4710942b6..2ac1d6043 100644 --- a/backend/src/schema/resolvers/users.js +++ b/backend/src/schema/resolvers/users.js @@ -147,12 +147,15 @@ export default { 'MATCH (this)<-[:BLOCKED]-(u:User {id: $cypherParams.currentUserId}) RETURN COUNT(u) >= 1', }, count: { - contributionsCount: '-[:WROTE]->(related:Post)', + contributionsCount: + '-[:WROTE]->(related:Post) WHERE NOT related.disabled = true AND NOT related.deleted = true', friendsCount: '<-[:FRIENDS]->(related:User)', followingCount: '-[:FOLLOWS]->(related:User)', followedByCount: '<-[:FOLLOWS]-(related:User)', - commentedCount: '-[:WROTE]->(:Comment)-[:COMMENTS]->(related:Post)', - shoutedCount: '-[:SHOUTED]->(related:Post)', + commentedCount: + '-[:WROTE]->(c:Comment)-[:COMMENTS]->(related:Post) WHERE NOT related.disabled = true AND NOT related.deleted = true', + shoutedCount: + '-[:SHOUTED]->(related:Post) WHERE NOT related.disabled = true AND NOT related.deleted = true', badgesCount: '<-[:REWARDED]-(related:Badge)', }, hasOne: { diff --git a/backend/src/schema/types/type/User.gql b/backend/src/schema/types/type/User.gql index 46e699410..49592e8ba 100644 --- a/backend/src/schema/types/type/User.gql +++ b/backend/src/schema/types/type/User.gql @@ -64,7 +64,7 @@ type User { ) comments: [Comment]! @relation(name: "WROTE", direction: "OUT") - commentedCount: Int! @cypher(statement: "MATCH (this)-[:WROTE]->(r:Comment)-[:COMMENTS]->(p:Post) WHERE NOT r.deleted = true AND NOT r.disabled = true AND NOT p.deleted = true AND NOT p.disabled = true RETURN COUNT(DISTINCT(p))") + commentedCount: Int! @cypher(statement: "MATCH (this)-[:WROTE]->(:Comment)-[:COMMENTS]->(p:Post) WHERE NOT p.deleted = true AND NOT p.disabled = true RETURN COUNT(DISTINCT(p))") shouted: [Post]! @relation(name: "SHOUTED", direction: "OUT") shoutedCount: Int! @cypher(statement: "MATCH (this)-[:SHOUTED]->(r:Post) WHERE NOT r.deleted = true AND NOT r.disabled = true RETURN COUNT(DISTINCT r)") diff --git a/deployment/legacy-migration/maintenance-worker/migration/neo4j/contributions/contributions.cql b/deployment/legacy-migration/maintenance-worker/migration/neo4j/contributions/contributions.cql index af81528f2..6fad4218d 100644 --- a/deployment/legacy-migration/maintenance-worker/migration/neo4j/contributions/contributions.cql +++ b/deployment/legacy-migration/maintenance-worker/migration/neo4j/contributions/contributions.cql @@ -137,8 +137,8 @@ p.contentExcerpt = post.contentExcerpt, p.visibility = toLower(post.visibility), p.createdAt = post.createdAt.`$date`, p.updatedAt = post.updatedAt.`$date`, -p.deleted = COALESCE(post.deleted,false), -p.disabled = NOT post.isEnabled +p.deleted = COALESCE(post.deleted, false), +p.disabled = COALESCE(NOT post.isEnabled, false) WITH p, post MATCH (u:User {id: post.userId}) MERGE (u)-[:WROTE]->(p) diff --git a/webapp/pages/index.vue b/webapp/pages/index.vue index afc566940..1783007db 100644 --- a/webapp/pages/index.vue +++ b/webapp/pages/index.vue @@ -185,9 +185,6 @@ export default { return result }, update({ Post }) { - // TODO: find out why `update` gets called twice initially. - // We have to filter for uniq posts only because we get the same - // result set twice. this.hasMore = Post.length >= this.pageSize const posts = uniqBy([...this.posts, ...Post], 'id') this.posts = posts diff --git a/webapp/pages/profile/_id/_slug.spec.js b/webapp/pages/profile/_id/_slug.spec.js index 16b4776b4..60b7ea4c7 100644 --- a/webapp/pages/profile/_id/_slug.spec.js +++ b/webapp/pages/profile/_id/_slug.spec.js @@ -104,9 +104,7 @@ describe('ProfileSlug', () => { describe('currently no posts available (e.g. after tab switching)', () => { beforeEach(() => { - wrapper.setData({ - Post: null, - }) + wrapper.setData({ posts: [], hasMore: false }) }) it('displays no "load more" button', () => { @@ -137,9 +135,7 @@ describe('ProfileSlug', () => { } }) - wrapper.setData({ - Post: posts, - }) + wrapper.setData({ posts, hasMore: true }) }) it('displays a "load more" button', () => { @@ -170,9 +166,7 @@ describe('ProfileSlug', () => { } }) - wrapper.setData({ - Post: posts, - }) + wrapper.setData({ posts, hasMore: false }) }) it('displays no "load more" button', () => { diff --git a/webapp/pages/profile/_id/_slug.vue b/webapp/pages/profile/_id/_slug.vue index d2fe497c7..4a94e5fc4 100644 --- a/webapp/pages/profile/_id/_slug.vue +++ b/webapp/pages/profile/_id/_slug.vue @@ -219,8 +219,8 @@ -