Completed redux-logic transition for linkGithubIdentity code (#1672)

* CHECKPOINT jest

* Fix unnecessary addition of an eslint-disable directive

* Initial transition to react-logic. Moved over unlickGithubIdentity logic. Still need to get react-saga and react-logic to play nicely.

* Got redux-saga and redux-logic working at the same time.

* Attempt to get jest working on IntelliJ

* Make jest config more consistent; transform lodash-es

* Mocks for bugsnag and firebase libraries

These libraries are set up in code that runs at module load time, which
is probably not a great practice, but for now we just mock out the
libraries themselves so our modules will load without error.

* Revert unnecessary change to karma config

* Add some jest rules to eslint config

* Write test for unlinkGithubIdentityTest.js

* Small refactor

* Fix yarn.lock with deduplicate

* Fix yarn.lock again?

* Refactored test, installed jest-extended (but didn't use it)

* Fix package.json after rebase

* Fix yarn.lock again

* Fix incorrect merge with .eslintrc

* Add jest override to .eslintrc

* Lint fixing

* Start redux-logic transition for linkGithubIdentity code

* Completed redux-logic transition for linkGithubIdentity code

* Fix easy nit-pits. Will work on Rosie and factory approach later.

* Fix eslint warnings triggered by Travis

* Add support for factories

Cherry-picked from CodeMirror branch

* Attempt to use Rosie into jest testing

* Label all factory objects with a suffix of "Factory". Added mock user object.

* Fix eslint failing in Travis

* 2nd attempt at fixing eslint failing in Travis

* Added more realistic mock data for githubProfileFactory. Revert changes to .eslintrc

* Upgrade eslint-plugin-import, fix import order

As expected, `eslint-plugin-import` released a fix that corrects the
behavior with internal modules that use module lookup aliases. So now
the modules aliased under `@factory` are correctly recognized as
internal.

* Created new client factory github.js and moved relevant code there. Adjusted test description.

* outoftime requested changes

Author: jwang1919

.eslintrc

@@ -41,7 +41,7 @@
         "jest": true
       },
       "files": [
-        "**/__tests__/*.js"
+        "**/{__factories__,__mocks__,__tests__}/*.js{,x}"
       ],
       "rules": {
         "jest/no-alias-methods": "warn",
@@ -64,7 +64,17 @@
         "react",
         "promise",
         "private-props"
-      ]
+      ],
+      "settings": {
+        "import/resolver": {
+          "@popcodeorg/eslint-import-resolver-jest": {
+            "jestConfigFile": "./jest.config.js"
+          },
+          "node": {
+            "extensions": [".js", ".jsx"]
+          }
+        }
+      }
     },
     {
       "files": "src/**/*",

__factories__/clients/firebase.js

@@ -0,0 +1,51 @@
+import {Factory} from 'rosie';
+
+class FirebaseError extends Error {
+  constructor(args) {
+    super(args.name);
+    this.code = args.name;
+    this.credential = args.credential;
+    Error.captureStackTrace(this, FirebaseError);
+  }
+}
+
+export const credentialFactory = new Factory().attrs({
+  providerId: 'github.com',
+  accessToken: 'abc123',
+});
+
+export const firebaseErrorFactory = Factory.define(
+  'firebaseError',
+  FirebaseError,
+).attrs({
+  name: 'some other error',
+});
+
+export const credentialInUseErrorFactory = new Factory().extend(
+  firebaseErrorFactory,
+).attrs({
+  name: 'auth/credential-already-in-use',
+  credential: () => credentialFactory.build(),
+});
+
+export const userProviderDataFactory = new Factory().attrs({
+  displayName: 'popcoder',
+  email: null,
+  phoneNumber: null,
+  photoUrl: null,
+  providerId: 'github.com',
+  uid: '1234567',
+});
+
+export const userFactory = new Factory().extend(
+  userProviderDataFactory,
+).attrs({
+  emailVerified: false,
+  isAnonymous: false,
+  metadata: {
+    creationTime: Date.now(),
+    lastSignInTime: Date.now(),
+  },
+  providerData: () => [userProviderDataFactory.build()],
+  refreshToken: 'token123',
+});

__factories__/clients/github.js

@@ -0,0 +1,47 @@
+import {Factory} from 'rosie';
+
+export const githubProfileFactory = new Factory().attrs({
+  login: 'popcoder',
+  id: 123456,
+  node_id: 'ABC123',
+  avatar_url: null,
+  gravatar_id: null,
+  url: 'https://api.github.com/users/popcoder',
+  html_url: 'https://github.com/popcoder',
+  followers_url: 'https://api.github.com/users/popcoder/followers',
+  following_url: 'https://api.github.com/users/popcoder/following{/other_user}',
+  gists_url: 'https://api.github.com/users/popcoder/gists{/gist_id}',
+  starred_url: 'https://api.github.com/users/popcoder/starred{/owner}{/repo}',
+  subscriptions_url: 'https://api.github.com/users/popcoder/subscriptions',
+  organizations_url: 'https://api.github.com/users/popcoder/orgs',
+  repos_url: 'https://api.github.com/users/popcoder/repos',
+  events_url: 'https://api.github.com/users/popcoder/events{/privacy}',
+  received_events_url: 'https://api.github.com/users/popcoder/received_events',
+  type: 'User',
+  site_admin: false,
+  name: 'Popcoder',
+  company: null,
+  blog: '',
+  location: null,
+  email: null,
+  hireable: null,
+  bio: null,
+  public_repos: 0,
+  public_gists: 0,
+  followers: 0,
+  following: 0,
+  created_at: '2014-09-12T15:02:43Z',
+  updated_at: '2019-05-02T13:50:32Z',
+  private_gists: 0,
+  total_private_repos: 0,
+  owned_private_repos: 0,
+  disk_usage: 0,
+  collaborators: 0,
+  two_factor_authentication: false,
+  plan: {
+    name: 'free',
+    space: 1234567,
+    collaborators: 0,
+    private_repos: 10000,
+  },
+});

jest.config.js

@@ -5,6 +5,10 @@
 // https://jestjs.io/docs/en/configuration.html
 
 module.exports = {
+  clearMocks: true,
+  moduleNameMapper: {
+    '@factories/(.*)$': '<rootDir>/__factories__/$1',
+  },
   testPathIgnorePatterns: [
     '/node_modules/',
     '/bower_components/',

package.json

@@ -288,6 +288,7 @@
     "@babel/polyfill": "^7.2.5",
     "@babel/preset-env": "^7.3.4",
     "@babel/preset-react": "^7.0.0",
+    "@popcodeorg/eslint-import-resolver-jest": "^2.1.1-popcode.1",
     "@redux-saga/testing-utils": "^1.0.1",
     "almost-equal": "^1.1.0",
     "babel-eslint": "^10.0.1",
@@ -306,7 +307,7 @@
     "eslint": "^5.6.0",
     "eslint-import-resolver-webpack": "^0.10.1",
     "eslint-loader": "^2.1.1",
-    "eslint-plugin-import": "^2.14.0",
+    "eslint-plugin-import": "^2.17.2",
     "eslint-plugin-jest": "^22.3.0",
     "eslint-plugin-private-props": "^0.3.0",
     "eslint-plugin-promise": "^4.0.1",
@@ -342,6 +343,7 @@
     "postcss-preset-env": "^5.3.0",
     "raw-loader": "^0.5.1",
     "redux-saga-test-plan": "^4.0.0-beta.2",
+    "rosie": "^2.0.1",
     "script-ext-html-webpack-plugin": "^2.0.1",
     "sinon": "^5.0.7",
     "source-map-explorer": "^1.4.0",

src/logic/__tests__/linkGithubIdentityTest.js

@@ -0,0 +1,89 @@
+import linkGithubIdentity from '../linkGithubIdentity';
+import {
+  linkGithub,
+  saveCredentialForCurrentUser,
+} from '../../clients/firebase';
+import {getProfileForAuthenticatedUser} from '../../clients/github';
+import {bugsnagClient} from '../../util/bugsnag';
+
+import {
+  credentialFactory,
+  credentialInUseErrorFactory,
+  firebaseErrorFactory,
+  userFactory,
+} from '@factories/clients/firebase';
+
+import {githubProfileFactory} from '@factories/clients/github';
+
+jest.mock('../../clients/firebase.js');
+jest.mock('../../clients/github.js');
+jest.mock('../../util/bugsnag.js');
+
+describe('linkGithubIdentity', () => {
+  test('success', async() => {
+    const mockCredential = credentialFactory.build();
+    const mockUser = userFactory.build();
+
+    linkGithub.mockResolvedValue({
+      user: mockUser,
+      credential: mockCredential,
+    });
+
+    const {
+      type,
+      payload: {
+        credential: {providerId},
+        user,
+      },
+    } = await linkGithubIdentity.process();
+
+    expect(linkGithub).toHaveBeenCalledWith();
+    expect(saveCredentialForCurrentUser).toHaveBeenCalledWith(mockCredential);
+    expect(type).toBe('IDENTITY_LINKED');
+    expect(providerId).toBe(mockCredential.providerId);
+    expect(user).toEqual(mockUser);
+  });
+
+  test('credential already in use', async() => {
+    const error = credentialInUseErrorFactory.build();
+    const githubProfile = githubProfileFactory.build();
+
+    linkGithub.mockRejectedValue(error);
+    getProfileForAuthenticatedUser.mockResolvedValue(githubProfile);
+
+    const {
+      type,
+      payload: {
+        credential: {
+          providerId,
+          accessToken,
+        },
+      },
+    } = await linkGithubIdentity.process();
+    expect(linkGithub).toHaveBeenCalledWith();
+    expect(getProfileForAuthenticatedUser).toHaveBeenCalledWith(
+      error.credential.accessToken,
+    );
+    expect(type).toBe('ACCOUNT_MIGRATION_NEEDED');
+    expect(providerId).toBe(error.credential.providerId);
+    expect(accessToken).toBe(error.credential.accessToken);
+  });
+
+  test('other error', async() => {
+    const otherError = firebaseErrorFactory.build();
+
+    linkGithub.mockRejectedValue(otherError);
+    bugsnagClient.notify.mockResolvedValue();
+
+    const {
+      type,
+      error,
+      payload: {message},
+    } = await linkGithubIdentity.process();
+    expect(linkGithub).toHaveBeenCalledWith();
+    expect(bugsnagClient.notify).toHaveBeenCalledWith(otherError);
+    expect(type).toBe('LINK_IDENTITY_FAILED');
+    expect(error).toBe(true);
+    expect(message).toBe(otherError.code);
+  });
+});

src/logic/index.js

@@ -1,3 +1,7 @@
+import linkGithubIdentity from './linkGithubIdentity';
 import unlinkGithubIdentity from './unlinkGithubIdentity';
 
-export default [unlinkGithubIdentity];
+export default [
+  linkGithubIdentity,
+  unlinkGithubIdentity,
+];

src/logic/linkGithubIdentity.js

@@ -0,0 +1,33 @@
+import {createLogic} from 'redux-logic';
+
+import {
+  linkGithub,
+  saveCredentialForCurrentUser,
+} from '../clients/firebase';
+import {
+  accountMigrationNeeded,
+  identityLinked,
+  linkIdentityFailed,
+} from '../actions/user';
+import {getProfileForAuthenticatedUser} from '../clients/github';
+import {bugsnagClient} from '../util/bugsnag';
+
+export default createLogic({
+  type: 'LINK_GITHUB_IDENTITY',
+  async process() {
+    try {
+      const {user: userData, credential} = await linkGithub();
+      await saveCredentialForCurrentUser(credential);
+      return identityLinked(userData, credential);
+    } catch (e) {
+      if (e.code === 'auth/credential-already-in-use') {
+        const {data: githubProfile} = await getProfileForAuthenticatedUser(
+          e.credential.accessToken,
+        );
+        return accountMigrationNeeded(githubProfile, e.credential);
+      }
+      await bugsnagClient.notify(e);
+      return linkIdentityFailed(e);
+    }
+  },
+});

src/sagas/user.js

@@ -11,43 +11,14 @@ import {
 } from 'redux-saga/effects';
 import {
   accountMigrationComplete,
-  accountMigrationNeeded,
   accountMigrationUndoPeriodExpired,
-  identityLinked,
-  linkIdentityFailed,
   accountMigrationError,
 } from '../actions/user';
 import {getCurrentAccountMigration} from '../selectors';
 import {
-  linkGithub,
   migrateAccount,
   signOut,
-  saveCredentialForCurrentUser,
 } from '../clients/firebase';
-import {getProfileForAuthenticatedUser} from '../clients/github';
-
-export function* linkGithubIdentity() {
-  try {
-    const {user: userData, credential} = yield call(linkGithub);
-    yield call(saveCredentialForCurrentUser, credential);
-    yield put(identityLinked(userData, credential));
-  } catch (e) {
-    switch (e.code) {
-      case 'auth/credential-already-in-use': {
-        const {data: githubProfile} = yield call(
-          getProfileForAuthenticatedUser,
-          e.credential.accessToken,
-        );
-        yield put(accountMigrationNeeded(githubProfile, e.credential));
-        break;
-      }
-
-      default:
-        yield call([bugsnagClient, 'notify'], e);
-        yield put(linkIdentityFailed(e));
-    }
-  }
-}
 
 export function* startAccountMigration() {
   const {shouldContinue} = yield race({
@@ -82,7 +53,6 @@ export function* logOut() {
 
 export default function* user() {
   yield all([
-    takeEvery('LINK_GITHUB_IDENTITY', linkGithubIdentity),
     takeEvery('LOG_OUT', logOut),
     takeEvery('START_ACCOUNT_MIGRATION', startAccountMigration),
   ]);