From 4b97131225df60f68d5a085cb7a59a4e2ce08cf6 Mon Sep 17 00:00:00 2001 From: ogerly Date: Thu, 12 Sep 2019 16:37:30 +0200 Subject: [PATCH 1/5] old stand --- backend/src/models/User.js | 5 +- backend/src/schema/resolvers/registration.js | 1 + .../src/schema/resolvers/registration.spec.js | 6 ++- backend/src/schema/resolvers/users.js | 4 +- backend/src/schema/resolvers/users.spec.js | 52 ++++++++++++++----- .../src/schema/types/type/EmailAddress.gql | 1 + backend/src/schema/types/type/User.gql | 2 + backend/src/seed/factories/users.js | 2 +- 8 files changed, 53 insertions(+), 20 deletions(-) diff --git a/backend/src/models/User.js b/backend/src/models/User.js index c2749ce6a..465abfb65 100644 --- a/backend/src/models/User.js +++ b/backend/src/models/User.js @@ -87,12 +87,11 @@ module.exports = { type: 'string', allow: [null], }, - /* termsAndConditionsAgreedAt: { + termsAndConditionsAgreedAt: { type: 'string', isoDate: true, allow: [null], - // required: true, TODO - }, */ + }, shouted: { type: 'relationship', relationship: 'SHOUTED', diff --git a/backend/src/schema/resolvers/registration.js b/backend/src/schema/resolvers/registration.js index f96767006..fa7262f43 100644 --- a/backend/src/schema/resolvers/registration.js +++ b/backend/src/schema/resolvers/registration.js @@ -82,6 +82,7 @@ export default { if (!regEx.test(termsAndConditionsAgreedVersion)) { throw new UserInputError('Invalid version format!') } + args.termsAndConditionsAgreedAt = new Date().toISOString() let { nonce, email } = args email = email.toLowerCase() diff --git a/backend/src/schema/resolvers/registration.spec.js b/backend/src/schema/resolvers/registration.spec.js index ae8cc16f6..41800b0c5 100644 --- a/backend/src/schema/resolvers/registration.spec.js +++ b/backend/src/schema/resolvers/registration.spec.js @@ -340,6 +340,7 @@ describe('SignupVerification', () => { ) { id termsAndConditionsAgreedVersion + termsAndConditionsAgreedAt } } ` @@ -352,6 +353,7 @@ describe('SignupVerification', () => { password: '123', email: 'john@example.org', termsAndConditionsAgreedVersion: '0.0.1', + termsAndConditionsAgreedAt: null, } }) @@ -444,11 +446,11 @@ describe('SignupVerification', () => { expect(emails).toHaveLength(1) }) - it('is version of terms and conditions saved correctly', async () => { + it('if a current date of the General Terms and Conditions is available', async () => { await expect(mutate({ mutation, variables })).resolves.toMatchObject({ data: { SignupVerification: expect.objectContaining({ - termsAndConditionsAgreedVersion: '0.0.1', + termsAndConditionsAgreedAt: expect.any(String), }), }, }) diff --git a/backend/src/schema/resolvers/users.js b/backend/src/schema/resolvers/users.js index c7afee7c8..635a284f0 100644 --- a/backend/src/schema/resolvers/users.js +++ b/backend/src/schema/resolvers/users.js @@ -94,6 +94,7 @@ export default { if (!regEx.test(termsAndConditionsAgreedVersion)) { throw new ForbiddenError('Invalid version format!') } + args.termsAndConditionsAgreedAt = new Date().toISOString() } args = await fileUpload(args, { file: 'avatarUpload', url: 'avatar' }) try { @@ -165,7 +166,6 @@ export default { }, ...Resolver('User', { undefinedToNull: [ - 'termsAndConditionsAgreedVersion', 'actorId', 'avatar', 'coverImg', @@ -174,7 +174,7 @@ export default { 'locationName', 'about', 'termsAndConditionsAgreedVersion', - // TODO: 'termsAndConditionsAgreedAt', + 'termsAndConditionsAgreedAt', ], boolean: { followedByCurrentUser: diff --git a/backend/src/schema/resolvers/users.spec.js b/backend/src/schema/resolvers/users.spec.js index 50f413157..784a48c06 100644 --- a/backend/src/schema/resolvers/users.spec.js +++ b/backend/src/schema/resolvers/users.spec.js @@ -84,6 +84,8 @@ describe('UpdateUser', () => { password: '1234', id: 'u47', name: 'John Doe', + termsAndConditionsAgreedVersion: null, + termsAndConditionsAgreedAt: null, } variables = { @@ -102,6 +104,7 @@ describe('UpdateUser', () => { id name termsAndConditionsAgreedVersion + termsAndConditionsAgreedAt } } ` @@ -171,19 +174,44 @@ describe('UpdateUser', () => { ) }) - it('given a new agreed version of terms and conditions', async () => { - variables = { ...variables, termsAndConditionsAgreedVersion: '0.0.2' } - const expected = { - data: { - UpdateUser: expect.objectContaining({ - termsAndConditionsAgreedVersion: '0.0.2', - }), - }, - } + describe('given a new agreed version of terms and conditions', () => { + beforeEach(async () => { + variables = { ...variables, termsAndConditionsAgreedVersion: '0.0.2' } + }) + it('update termsAndConditionsAgreedVersion', async () => { + const expected = { + data: { + UpdateUser: expect.objectContaining({ + termsAndConditionsAgreedVersion: '0.0.2', + termsAndConditionsAgreedAt: expect.any(String), + }), + }, + } - await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject( - expected, - ) + await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject( + expected, + ) + }) + }) + + describe('given any attribute other than termsAndConditionsAgreedVersion', () => { + beforeEach(async () => { + variables = { ...variables, name: 'any name' } + }) + it('update termsAndConditionsAgreedVersion', async () => { + const expected = { + data: { + UpdateUser: expect.objectContaining({ + termsAndConditionsAgreedVersion: null, + termsAndConditionsAgreedAt: null, + }), + }, + } + + await expect(mutate({ mutation: updateUserMutation, variables })).resolves.toMatchObject( + expected, + ) + }) }) it('rejects if version of terms and conditions has wrong format', async () => { diff --git a/backend/src/schema/types/type/EmailAddress.gql b/backend/src/schema/types/type/EmailAddress.gql index fe7e4cffb..7501a3e8c 100644 --- a/backend/src/schema/types/type/EmailAddress.gql +++ b/backend/src/schema/types/type/EmailAddress.gql @@ -19,5 +19,6 @@ type Mutation { locationName: String about: String termsAndConditionsAgreedVersion: String + termsAndConditionsAgreedAt: String ): User } diff --git a/backend/src/schema/types/type/User.gql b/backend/src/schema/types/type/User.gql index 1c1b9041d..f1c38b8d6 100644 --- a/backend/src/schema/types/type/User.gql +++ b/backend/src/schema/types/type/User.gql @@ -25,6 +25,7 @@ type User { updatedAt: String termsAndConditionsAgreedVersion: String + termsAndConditionsAgreedAt: String friends: [User]! @relation(name: "FRIENDS", direction: "BOTH") friendsCount: Int! @cypher(statement: "MATCH (this)<-[: FRIENDS]->(r: User) RETURN COUNT(DISTINCT r)") @@ -164,6 +165,7 @@ type Mutation { locationName: String about: String termsAndConditionsAgreedVersion: String + termsAndConditionsAgreedAt: String ): User DeleteUser(id: ID!, resource: [Deletable]): User diff --git a/backend/src/seed/factories/users.js b/backend/src/seed/factories/users.js index 5cb7f8842..e0ec04fc0 100644 --- a/backend/src/seed/factories/users.js +++ b/backend/src/seed/factories/users.js @@ -14,8 +14,8 @@ export default function create() { role: 'user', avatar: faker.internet.avatar(), about: faker.lorem.paragraph(), - // termsAndConditionsAgreedAt: new Date().toISOString(), termsAndConditionsAgreedVersion: '0.0.1', + termsAndConditionsAgreedAt: '2019-08-01T10:47:19.212Z', } defaults.slug = slugify(defaults.name, { lower: true }) args = { From aa234edc0aab3ccc7431630a0d24aad0b91189b8 Mon Sep 17 00:00:00 2001 From: Alexander Friedland Date: Thu, 12 Sep 2019 18:00:02 +0200 Subject: [PATCH 2/5] Update backend/src/schema/resolvers/registration.spec.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Robert Schäfer --- backend/src/schema/resolvers/registration.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/schema/resolvers/registration.spec.js b/backend/src/schema/resolvers/registration.spec.js index 41800b0c5..20619181a 100644 --- a/backend/src/schema/resolvers/registration.spec.js +++ b/backend/src/schema/resolvers/registration.spec.js @@ -352,7 +352,7 @@ describe('SignupVerification', () => { name: 'John Doe', password: '123', email: 'john@example.org', - termsAndConditionsAgreedVersion: '0.0.1', + termsAndConditionsAgreedVersion: null, termsAndConditionsAgreedAt: null, } }) From 0bef3a729c21e5bd5acf615a226a945bb8bcb554 Mon Sep 17 00:00:00 2001 From: Alexander Friedland Date: Thu, 12 Sep 2019 18:00:14 +0200 Subject: [PATCH 3/5] Update backend/src/schema/resolvers/registration.spec.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Robert Schäfer --- backend/src/schema/resolvers/registration.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/schema/resolvers/registration.spec.js b/backend/src/schema/resolvers/registration.spec.js index 20619181a..5b1bc4a17 100644 --- a/backend/src/schema/resolvers/registration.spec.js +++ b/backend/src/schema/resolvers/registration.spec.js @@ -446,7 +446,7 @@ describe('SignupVerification', () => { expect(emails).toHaveLength(1) }) - it('if a current date of the General Terms and Conditions is available', async () => { + it('updates `termsAndConditionsAgreedAt`'', async () => { await expect(mutate({ mutation, variables })).resolves.toMatchObject({ data: { SignupVerification: expect.objectContaining({ From 8fa42ea6ead23715781a67ac720996bb775e63f7 Mon Sep 17 00:00:00 2001 From: Alexander Friedland Date: Thu, 12 Sep 2019 18:51:41 +0200 Subject: [PATCH 4/5] update --- backend/src/schema/resolvers/registration.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/src/schema/resolvers/registration.spec.js b/backend/src/schema/resolvers/registration.spec.js index 5b1bc4a17..e71fbffa3 100644 --- a/backend/src/schema/resolvers/registration.spec.js +++ b/backend/src/schema/resolvers/registration.spec.js @@ -446,7 +446,7 @@ describe('SignupVerification', () => { expect(emails).toHaveLength(1) }) - it('updates `termsAndConditionsAgreedAt`'', async () => { + it('updates termsAndConditionsAgreedAt', async () => { await expect(mutate({ mutation, variables })).resolves.toMatchObject({ data: { SignupVerification: expect.objectContaining({ From f3c31aed28dcd4d33e0e6ecbe72ff084b74bdb7e Mon Sep 17 00:00:00 2001 From: roschaefer Date: Fri, 13 Sep 2019 01:11:30 +0200 Subject: [PATCH 5/5] Don't expose termsAndConditionsAgreedAt 1. Don't expose `termsAndConditionsAgreedAt` as input param, because of ..why? 2. Make the `termsAndConditionsAgreedVersion` a *required* input param for `SignupVerification`. If new users register, they have to confirm the terms and conditions. I added another test to check what happens if the user sends `null`. 3. Sorry @ogerly for confusing you with my review here: https://github.com/Human-Connection/Human-Connection/pull/1556#pullrequestreview-287516516 What I meant is that we want to simulate a user with no `termsAndConditionsAgreedVersion`. But of course the `variables` must have it set when you run the mutations. Now we have the exclamation mark in the input param, see point 1 :point_up: --- .../src/schema/resolvers/registration.spec.js | 27 ++++++++++++++++--- .../src/schema/types/type/EmailAddress.gql | 3 +-- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/backend/src/schema/resolvers/registration.spec.js b/backend/src/schema/resolvers/registration.spec.js index e71fbffa3..ca19f03c4 100644 --- a/backend/src/schema/resolvers/registration.spec.js +++ b/backend/src/schema/resolvers/registration.spec.js @@ -62,7 +62,7 @@ describe('CreateInvitationCode', () => { name: 'Inviter', email: 'inviter@example.org', password: '1234', - termsAndConditionsAgreedVersion: '0.0.1', + termsAndConditionsAgreedVersion: null, }) authenticatedUser = await user.toJson() }) @@ -352,8 +352,7 @@ describe('SignupVerification', () => { name: 'John Doe', password: '123', email: 'john@example.org', - termsAndConditionsAgreedVersion: null, - termsAndConditionsAgreedAt: null, + termsAndConditionsAgreedVersion: '0.1.0', } }) @@ -446,6 +445,16 @@ describe('SignupVerification', () => { expect(emails).toHaveLength(1) }) + it('updates termsAndConditionsAgreedVersion', async () => { + await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + data: { + SignupVerification: expect.objectContaining({ + termsAndConditionsAgreedVersion: '0.1.0', + }), + }, + }) + }) + it('updates termsAndConditionsAgreedAt', async () => { await expect(mutate({ mutation, variables })).resolves.toMatchObject({ data: { @@ -456,6 +465,18 @@ describe('SignupVerification', () => { }) }) + it('rejects if version of terms and conditions is missing', async () => { + variables = { ...variables, termsAndConditionsAgreedVersion: null } + await expect(mutate({ mutation, variables })).resolves.toMatchObject({ + errors: [ + { + message: + 'Variable "$termsAndConditionsAgreedVersion" of non-null type "String!" must not be null.', + }, + ], + }) + }) + it('rejects if version of terms and conditions has wrong format', async () => { variables = { ...variables, termsAndConditionsAgreedVersion: 'invalid version format' } await expect(mutate({ mutation, variables })).resolves.toMatchObject({ diff --git a/backend/src/schema/types/type/EmailAddress.gql b/backend/src/schema/types/type/EmailAddress.gql index 7501a3e8c..4bf8ff724 100644 --- a/backend/src/schema/types/type/EmailAddress.gql +++ b/backend/src/schema/types/type/EmailAddress.gql @@ -18,7 +18,6 @@ type Mutation { avatarUpload: Upload locationName: String about: String - termsAndConditionsAgreedVersion: String - termsAndConditionsAgreedAt: String + termsAndConditionsAgreedVersion: String! ): User }