From e4d57f80aabc8833beb892957a2d0049c9e9233c Mon Sep 17 00:00:00 2001 From: roschaefer Date: Fri, 23 Aug 2019 01:27:53 +0200 Subject: [PATCH] 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 @@ -