Feat: Expand User type #55

Bouncey · 2018-04-20T16:40:51Z

This PR contains commits from #48, by mistake.

This PR expands the User type, adjusts the user query's to return a single user and adds a challengeMap Mutation type.

package.json

-    "start": "serverless offline start --skipCacheInvalidation",
-    "test": "snyk test"
+    "start": "DEBUG=fcc:* serverless offline start --skipCacheInvalidation",
+    "test": "jest",


ojongerius · 2018-04-20T21:12:18Z

I'll need a little more time to review this, I can do that on my Monday, your Sunday.

.travis.yml

@@ -12,9 +12,7 @@ env:
    - AWS_REGION=us-east-1
    - SLS_DEBUG=true

-before_install:
-  - npm install -g serverless


dataLayer/mongo/user.js

+    return updated;
+  } catch (err) {
+    log('crashing hard');
+    // TODO(Bouncey): Communicate the problem with the client


dataLayer/mongo/user.js

+
+export async function getUser(email) {
+  const user = await findUserBy({ email });
+  return applyMigrations(user);


graphql/typeDefs/User/query.js

-      name: String
-      email: String
-  ): [User]
+  getSessionUser(email: String!): User


package.json

-    "start": "serverless offline start --skipCacheInvalidation",
-    "test": "snyk test"
+    "start": "DEBUG=fcc:* serverless offline start --skipCacheInvalidation",
+    "test": "jest",


graphql/resolvers/user.js


 export const userResolvers = {
  Query: {
-    users: (_, args) => dbUsers.getUsers(args)
+    // TODO(Bouncey): requires auth protection


Bouncey · 2018-04-23T09:22:48Z

@ojongerius Looking back at this, I think I could make the queries and mutations slicker by using context and not hitting the db on every request to find a user. I am going to re-write some parts of this PR.

It may be that we need auth in place before this PR can/should progress.

Bouncey · 2018-04-24T09:13:09Z

Waiting on #58

ojongerius · 2018-04-29T22:36:42Z

#58 went in (nice work!), what's the status of this one at the moment?

Bouncey · 2018-04-30T06:09:46Z

Almost a rewrite, but it is nearly there.

One issue I am having that ibmeant to ping you for is, if I use mongo in a different test suit in the same way that the users integration test does, I get a race condition between the the afterAll disconnects. If onebsuit is still running then the tests fail because MongoError: Topology was destroyed or something to that effect.

I have tried managing a connection in the global setup/teardown, but the tests would timeout. Any ideas?

ojongerius · 2018-05-01T05:07:57Z

@Bouncey interesting! Does this happen when you run jest with --runInBand too?

Bouncey · 2018-05-01T06:27:10Z

@ojongerius yes it does, I think it is due to the async nature of the setup and teardown, where at times, they overlap. I will push what I have so far so you can see.

I think I'm in a place where I shouldn't be adding any further commits to this PR, else the scope will slip too far.

Bouncey · 2018-05-01T14:05:16Z

@ojongerius I might have fixed the race condition be returning the promise in teardown?

src/dataLayer/mongo/user.js

+    newUser.accountLinkId = accountLinkId;
+  } else {
+    newUser.accountLinkId = uuid();
+    // TODO: appy accountLinkId to app_metadata (auth0)


Bouncey · 2018-05-01T14:14:17Z

I worte all of those test before rebasing, so glad that I did because I made a unholy mess and lost two days with of work with a clumsy git stash pop.

Yay for tests! 🎉

ojongerius · 2018-05-01T21:30:28Z

Sounds good! Sounds like it's review time? Are you keeping the WiP label while finish writing the code to talk to auth0?

Bouncey · 2018-05-01T21:33:04Z

Please review. None of this code should change, all I need to add is an API call to auth0

ojongerius · 2018-05-01T21:36:16Z

👍 will have a look today.

ojongerius

Started, but have not yet finished reviewing, will give this some more attention in a bit.

package.json

@@ -27,7 +27,7 @@
    "snyk-test": "snyk test",
    "start":
      "cross-env DEBUG=fcc:* nodemon node_modules/serverless/bin/serverless offline start --skipCacheInvalidation",
-    "test": "cross-env JWT_CERT=test jest --verbose",
+    "test": "cross-env JWT_CERT=test jest --runInBand  --verbose --silent",


src/dataLayer/model/user.js


 const userSchema = new Schema({
+  accountLinkId: {
+    type: 'string',
+    description: 'A uuid used to link SSO and freeCodeCamp accounts together',


src/dataLayer/model/user.js

  }
 });

-module.exports = mongoose.model('User', userSchema, 'user');


src/dataLayer/model/user.js

    default: false
  },
-  isGithubCool: {


src/dataLayer/mongo/user.js

+  return new Promise(async function getUserPromise(resolve, reject) {
+    if (!isString(email) || !validator.isEmail(email)) {
+      reject(
+        new TypeError(`Expected a valid email, got ${JSON.stringify(email)}`)


src/dataLayer/model/user.js


 const userSchema = new Schema({
+  accountLinkId: {
+    type: 'string',
+    description: 'A uuid used to link SSO and freeCodeCamp accounts together',


src/graphql/typeDefs/User/type.js

+}
+
+type CompletedChallenge {
+  id: String


src/graphql/typeDefs/User/type.js

 type User {
-  _id: ID @isAuthenticatedOnField


test/integration/__snapshots__/userFlow.test.js.snap

+exports[`createUser should not create a second user for the same context: no duplicate users 1`] = `
+Array [
+  Object {
+    "accountLinkId": "[email protected]",


test/integration/userFlow.test.js

+      .catch(done);
+  });
+
+  it('should not create a second user for the same context', done => {


test/integration/userFlow.test.js

+
+    graphql(graphqlSchema, expectedUserQuery, rootValue, validContextCharlie)
+      .then(({ data, errors }) => {
+        expect(data).toMatchSnapshot('user found');


test/utils/test-environment.js

-    const token = jwt.sign({ id: 123, name: 'Charlie' }, JWT_CERT);
+    const token = jwt.sign(
+      {
+        id: 123,


test/utils/test-environment.js

+        name: 'Charlie',
+        email: '[email protected]',
+        [namespace +
+        'accountLinkId']: '[email protected]'


src/graphql/typeDefs/GDPR-User/mutation.js

@@ -0,0 +1,5 @@
+export default `
+type Mutation {
+  deleteGDPRUser(accountLinkId: String!): HTTPStatus! @isAuthenticatedOnQuery


ojongerius · 2018-05-02T08:00:31Z

Finished another pass 😅 This is turned into quite an epic. Awesome work but please don't add more complexity! 😄

FWIW tests are passing but are very noisy for me:

▶ JWT_CERT=test jest --runInBand --verbose
 PASS  ../test/integration/userFlow.test.js
  createUser
    ✓ should return null and an auth error without a token (72ms)
    ✓ should return null and an auth error with an invalid token (3ms)
    ✓ should create a user by query (73ms)
    ✓ should not create a second user for the same context (36ms)
  getUser
    ✓ should return null and an auth error without a token (3ms)
    ✓ should return null and an auth error with an invalid token (1ms)
    ✓ should return a user after one has been created (7ms)
    ✓ should return null if no user has been found (8ms)
    ✓ should return errors for a malformed query (4ms)

  console.error dataLayer/mongo/user.js:60
    Cannot create account, the unique id associated with this user is already in use [email protected]

  console.log ../node_modules/graphql-tools/dist/schemaGenerator.js:514
    { Error: No user found for [email protected]
        at getUserPromise (/Users/ojongerius/repos/fcc-open-api/src/dataLayer/mongo/user.js:28:14)
        at <anonymous>
      originalMessage: 'No user found for [email protected]',
      message: 'Error in resolver Query.getUser\nNo user found for [email protected]' }

  console.error utils/asyncErrorHandler.js:6
    Error: No user found for [email protected]
        at getUserPromise (/Users/ojongerius/repos/fcc-open-api/src/dataLayer/mongo/user.js:28:14)
        at <anonymous>

  console.log ../node_modules/graphql-tools/dist/schemaGenerator.js:514
    { TypeError: Expected a valid email, got "Ooops!"
        at getUserPromise (/Users/ojongerius/repos/fcc-open-api/src/dataLayer/mongo/user.js:20:9)
        at new Promise (<anonymous>)
        at getUser (/Users/ojongerius/repos/fcc-open-api/src/dataLayer/mongo/user.js:17:10)
        at Object.<anonymous> (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/src/schemaGenerator.ts:683:22)
        at class_1.<anonymous> (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/src/schemaGenerator.ts:781:42)
        at step (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/dist/schemaGenerator.js:51:23)
        at Object.next (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/dist/schemaGenerator.js:32:53)
        at /Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/dist/schemaGenerator.js:26:71
        at new Promise (<anonymous>)
        at Object.<anonymous>.__awaiter (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/dist/schemaGenerator.js:22:12)
        at /Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/src/schemaGenerator.ts:781:13
        at isAuthenticatedOnQuery (/Users/ojongerius/repos/fcc-open-api/src/graphql/resolvers/directives.js:24:32)
        at field.resolve (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/src/schemaGenerator.ts:780:18)
        at resolveFieldValueOrError (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql/execution/execute.js:531:18)
        at resolveField (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql/execution/execute.js:495:16)
        at /Users/ojongerius/repos/fcc-open-api/node_modules/graphql/execution/execute.js:364:18
      originalMessage: 'Expected a valid email, got "Ooops!"',
      message: 'Error in resolver Query.getUser\nExpected a valid email, got "Ooops!"' }

  console.error utils/asyncErrorHandler.js:6
    TypeError: Expected a valid email, got "Ooops!"
        at getUserPromise (/Users/ojongerius/repos/fcc-open-api/src/dataLayer/mongo/user.js:20:9)
        at new Promise (<anonymous>)
        at getUser (/Users/ojongerius/repos/fcc-open-api/src/dataLayer/mongo/user.js:17:10)
        at Object.<anonymous> (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/src/schemaGenerator.ts:683:22)
        at class_1.<anonymous> (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/src/schemaGenerator.ts:781:42)
        at step (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/dist/schemaGenerator.js:51:23)
        at Object.next (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/dist/schemaGenerator.js:32:53)
        at /Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/dist/schemaGenerator.js:26:71
        at new Promise (<anonymous>)
        at Object.<anonymous>.__awaiter (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/dist/schemaGenerator.js:22:12)
        at /Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/src/schemaGenerator.ts:781:13
        at isAuthenticatedOnQuery (/Users/ojongerius/repos/fcc-open-api/src/graphql/resolvers/directives.js:24:32)
        at field.resolve (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql-tools/src/schemaGenerator.ts:780:18)
        at resolveFieldValueOrError (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql/execution/execute.js:531:18)
        at resolveField (/Users/ojongerius/repos/fcc-open-api/node_modules/graphql/execution/execute.js:495:16)
        at /Users/ojongerius/repos/fcc-open-api/node_modules/graphql/execution/execute.js:364:18

 PASS  graphql/resolvers/directive.test.js
  isAuthenticatedOnField
    ✓ should return null if authenication fails (3ms)
    ✓ should return the secretValue if authentication succeeds (1ms)
  isAuthenticatedOnQuery
    ✓ should throw an error is auth fails (1ms)
    ✓ should call next if auth succeeds

 PASS  dataLayer/mongo/user-dataLayer.test.js
  createUser
    ✓ should return a User object (24ms)
    ✓ should return user for accountLinkId if found in db (6ms)
  getUser
    ✓ should return a User object fo a valid request (3ms)
    ✓ should throw for a user not found (1ms)
    ✓ should throw if the supplied email is not valid (3ms)

Test Suites: 3 passed, 3 total
Tests:       18 passed, 18 total
Snapshots:   18 passed, 18 total
Time:        4.39s
Ran all test suites.
  console.error dataLayer/mongo/user.js:60
    Cannot create account, the unique id associated with this user is already in use [email protected]

  console.error dataLayer/mongo/user.js:60
    Cannot create account, the unique id associated with this user is already in use [email protected]

Bouncey · 2018-05-02T08:15:22Z

This is the reason for the --silent flag. 😁

ojongerius · 2018-05-02T08:59:28Z

😆

Would it not be neater to expect and handle those errors? Could we use tothrowerror and or tothrowerrormatchingsnapshot?

src/graphql/typeDefs/User/type.js

+  progressTimestamps: [ProgressTimestamp]!,
+  isBanned: Boolean
+  isCheater: Boolean
+  githubURL: String


ojongerius · 2018-05-03T00:07:37Z

I have no other concerns than that it looks messy to have uncaught errors in tests. Suppressing with a silent flag feels a little like a cover up.

ojongerius · 2018-05-03T00:10:54Z

Outstanding tasks AFAICS

remove or improve deleteGDPRUser
agree if tests should have uncaught errors

After that we should be good to merge, let's get this today or latest, tomorrow.

raisedadead · 2018-05-03T06:56:49Z

Yes, I agree.. I am going ahead and giving this a merge, to keep things moving! Thanks a lot @Bouncey this is some incredible work!

/cc @freeCodeCamp/open-api

Bouncey commented Apr 20, 2018

View reviewed changes

Bouncey changed the title ~~Feat/expand user types~~ Feat: Expand User type Apr 20, 2018

Bouncey force-pushed the feat/expandUserTypes branch from 90befaa to 8dc081d Compare April 21, 2018 13:29

ojongerius reviewed Apr 23, 2018

View reviewed changes

Bouncey added the WiP Work in Progress, not ready for QA/merge label Apr 23, 2018

raisedadead mentioned this pull request Apr 30, 2018

feat: bot should create issues from todo comments freeCodeCamp/camper-gh-bot#13

Open

Bouncey force-pushed the feat/expandUserTypes branch from 8dc081d to fff41fb Compare May 1, 2018 14:04

Bouncey commented May 1, 2018

View reviewed changes

src/dataLayer/mongo/user.js Outdated

newUser.accountLinkId = accountLinkId;

} else {

newUser.accountLinkId = uuid();

// TODO: appy accountLinkId to app_metadata (auth0)

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

ojongerius reviewed May 2, 2018

View reviewed changes

raisedadead reviewed May 2, 2018

View reviewed changes

src/graphql/typeDefs/User/type.js Outdated

progressTimestamps: [ProgressTimestamp]!,

isBanned: Boolean

isCheater: Boolean

githubURL: String

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

Bouncey added 3 commits May 3, 2018 00:29

chore(tools): use native promises

3d7d7e3

feat(user-types): create user and gdpr-user types

7961344

fix(resolvers): add checks to user resolvers

4c87b98

Bouncey added 4 commits May 3, 2018 00:30

chore(tests): add integration tests for createuser

92a9412

feat(accounts): link sso and freecodecamps together via accountlinktoken

26e246c

chore(model): slim the user model down

451d808

chore(tidy): a quick tidy up

13fa46f

Bouncey force-pushed the feat/expandUserTypes branch from fff41fb to 13fa46f Compare May 2, 2018 23:31

fix(createuser): remove possible search with a null accountlinkid

bd8ebc7

Bouncey removed the WiP Work in Progress, not ready for QA/merge label May 2, 2018

raisedadead mentioned this pull request May 3, 2018

Feat: User type Outstanding tasks #104

Closed

2 tasks

raisedadead merged commit a339616 into freeCodeCamp:staging May 3, 2018

Bouncey deleted the feat/expandUserTypes branch May 3, 2018 09:22

Bouncey mentioned this pull request May 3, 2018

As a new user, I want to create a new account #73

Closed

Feat: Expand User type #55

Feat: Expand User type #55

Conversation

Bouncey commented Apr 20, 2018

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

ojongerius commented Apr 20, 2018

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

Bouncey commented Apr 23, 2018

Bouncey commented Apr 24, 2018

ojongerius commented Apr 29, 2018

Bouncey commented Apr 30, 2018

ojongerius commented May 1, 2018

Bouncey commented May 1, 2018

Bouncey commented May 1, 2018

This comment was marked as off-topic.

This comment was marked as off-topic.

Bouncey commented May 1, 2018 • edited Loading

Yay for tests! 🎉

ojongerius commented May 1, 2018

Bouncey commented May 1, 2018

ojongerius commented May 1, 2018

ojongerius left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

ojongerius commented May 2, 2018 • edited Loading

Bouncey commented May 2, 2018

ojongerius commented May 2, 2018 • edited Loading

This comment was marked as off-topic.

This comment was marked as off-topic.

ojongerius commented May 3, 2018 • edited Loading

ojongerius commented May 3, 2018

raisedadead commented May 3, 2018

Bouncey commented May 1, 2018 •

edited

Loading

ojongerius commented May 2, 2018 •

edited

Loading

ojongerius commented May 2, 2018 •

edited

Loading

ojongerius commented May 3, 2018 •

edited

Loading