Fix(settings): normalize responses (#16603)

client/epics/index.js

@@ -2,7 +2,6 @@ import analyticsEpic from './analytics-epic.js';
 import errEpic from './err-epic.js';
 import hardGoToEpic from './hard-go-to-epic.js';
 import mouseTrapEpic from './mouse-trap-epic.js';
-import nightModeEpic from './night-mode-epic.js';
 import titleEpic from './title-epic.js';
 
 export default [
@@ -10,6 +9,5 @@ export default [
   errEpic,
   hardGoToEpic,
   mouseTrapEpic,
-  nightModeEpic,
   titleEpic
 ];

client/epics/night-mode-epic.js

@@ -1,59 +0,0 @@
-import _ from 'lodash';
-import { Observable } from 'rx';
-import { ofType } from 'redux-epic';
-import store from 'store';
-
-import { themes } from '../../common/utils/themes.js';
-import { postJSON$ } from '../../common/utils/ajax-stream.js';
-import {
-  types,
-
-  postThemeComplete,
-  createErrorObservable,
-
-  themeSelector,
-  usernameSelector,
-  csrfSelector
-} from '../../common/app/redux';
-
-function persistTheme(theme) {
-  store.set('fcc-theme', theme);
-}
-
-export default function nightModeSaga(
-  actions,
-  { getState },
-  { document: { body } }
-) {
-  const toggleBodyClass = actions
-    ::ofType(
-      types.fetchUser.complete,
-      types.toggleNightMode,
-      types.postThemeComplete
-    )
-    .map(_.flow(getState, themeSelector))
-    // catch existing night mode users
-    .do(persistTheme)
-    .do(theme => {
-      if (theme === themes.night) {
-        body.classList.add(themes.night);
-      } else {
-        body.classList.remove(themes.night);
-      }
-    })
-    .ignoreElements();
-
-  const postThemeEpic = actions::ofType(types.toggleNightMode)
-    .debounce(250)
-    .flatMapLatest(() => {
-      const _csrf = csrfSelector(getState());
-      const theme = themeSelector(getState());
-      const username = usernameSelector(getState());
-      return postJSON$('/update-my-theme', { _csrf, theme })
-        .pluck('updatedTo')
-        .map(theme => postThemeComplete(username, theme))
-        .catch(createErrorObservable);
-    });
-
-  return Observable.merge(toggleBodyClass, postThemeEpic);
-}

common/app/Flash/Flash.jsx

@@ -4,26 +4,26 @@ import { CloseButton } from 'react-bootstrap';
 import { connect } from 'react-redux';
 
 import ns from './ns.json';
-import { alertTypes } from './redux/utils.js';
+import { alertTypes } from '../../utils/flash.js';
 import {
   latestMessageSelector,
   clickOnClose
 } from './redux';
 
 const propTypes = {
-  alertType: PropTypes.oneOf(Object.keys(alertTypes)),
   clickOnClose: PropTypes.func.isRequired,
-  message: PropTypes.string
+  message: PropTypes.string,
+  type: PropTypes.oneOf(Object.keys(alertTypes))
 };
 const mapStateToProps = latestMessageSelector;
 const mapDispatchToProps = { clickOnClose };
 
-export function Flash({ alertType, clickOnClose, message }) {
+export function Flash({ type, clickOnClose, message }) {
   if (!message) {
     return null;
   }
   return (
-    <div className={`${ns}-container bg-${alertType}`}>
+    <div className={`${ns}-container bg-${type}`}>
       <div className={`${ns}-content`}>
         <p className={ `${ns}-message` }>
           { message }

common/app/Flash/redux/index.js

@@ -11,6 +11,8 @@ import * as utils from './utils.js';
 import getMessagesEpic from './get-messages-epic.js';
 import ns from '../ns.json';
 
+// export all the utils
+export { utils };
 export const epics = [getMessagesEpic];
 export const types = createTypes([
   'clickOnClose',
@@ -45,13 +47,10 @@ export default composeReducers(
   ),
   function metaReducer(state = defaultState, action) {
     if (utils.isFlashAction(action)) {
-      const { payload: { alertType, message } } = utils.getFlashAction(action);
+      const { payload } = utils.getFlashAction(action);
       return [
         ...state,
-        {
+        ...payload
-          alertType: utils.normalizeAlertType(alertType),
-          message: _.escape(message)
-        }
       ];
     }
     return state;

common/app/Flash/redux/utils.js

@@ -1,29 +1,43 @@
 import _ from 'lodash/fp';
+import { alertTypes, normalizeAlertType } from '../../../utils/flash.js';
 
-export const alertTypes = _.keyBy(_.identity)([
+// interface ExpressFlash {
-  'success',
+//   [alertType]: [String...]
-  'info',
+// }
-  'warning',
+// interface StackFlash {
-  'danger'
+//   type: AlertType,
-]);
+//   message: String
+// }
+export const expressToStack = _.flow(
+  _.toPairs,
+  _.flatMap(([ type, messages ]) => messages.map(msg => ({
+    message: msg,
+    type: normalizeAlertType(type)
+  })))
+);
 
-export const normalizeAlertType = alertType => alertTypes[alertType] || 'info';
+export const isExpressFlash = _.flow(
+  _.keys,
+  _.every(type => alertTypes[type])
+);
 
 export const getFlashAction = _.flow(
   _.property('meta'),
   _.property('flash')
 );
 
+// FlashMessage
+// createFlashMetaAction(payload: ExpressFlash|StackFlash
+export const createFlashMetaAction = payload => {
+  if (isExpressFlash(payload)) {
+    payload = expressToStack(payload);
+  } else {
+    payload = [payload];
+  }
+  return { flash: { payload } };
+};
+
 export const isFlashAction = _.flow(
   getFlashAction,
   Boolean
 );
-
-export const expressToStack = _.flow(
-  _.toPairs,
-  _.flatMap(([ type, messages ]) => messages.map(msg => ({
-    message: msg,
-    alertType: normalizeAlertType(type)
-  })))
-);
-

common/app/redux/index.js

@@ -12,9 +12,11 @@ import { createSelector } from 'reselect';
 import fetchUserEpic from './fetch-user-epic.js';
 import updateMyCurrentChallengeEpic from './update-my-challenge-epic.js';
 import fetchChallengesEpic from './fetch-challenges-epic.js';
+import nightModeEpic from './night-mode-epic.js';
 
 import { createFilesMetaCreator } from '../files';
 import { updateThemeMetacreator, entitiesSelector } from '../entities';
+import { utils } from '../Flash/redux';
 import { types as challenges } from '../routes/Challenges/redux';
 import { challengeToFiles } from '../routes/Challenges/utils';
 
@@ -23,8 +25,9 @@ import ns from '../ns.json';
 import { themes, invertTheme } from '../../utils/themes.js';
 
 export const epics = [
-  fetchUserEpic,
   fetchChallengesEpic,
+  fetchUserEpic,
+  nightModeEpic,
   updateMyCurrentChallengeEpic
 ];
 
@@ -48,7 +51,7 @@ export const types = createTypes([
 
   // night mode
   'toggleNightMode',
-  'postThemeComplete'
+  createAsyncTypes('postTheme')
 ], ns);
 
 const throwIfUndefined = () => {
@@ -130,6 +133,7 @@ export const createErrorObservable = error => Observable.just({
   type: types.handleError,
   error
 });
+// use sparingly
 // doActionOnError(
 //   actionCreator: (() => Action|Null)
 // ) => (error: Error) => Observable[Action]
@@ -147,9 +151,18 @@ export const toggleNightMode = createAction(
   (username, theme) => updateThemeMetacreator(username, invertTheme(theme))
 );
 export const postThemeComplete = createAction(
-  types.postThemeComplete,
+  types.postTheme.complete,
   null,
-  updateThemeMetacreator
+  utils.createFlashMetaAction
+);
+
+export const postThemeError = createAction(
+  types.postTheme.error,
+  null,
+  (username, theme, err) => ({
+    ...updateThemeMetacreator(username, invertTheme(theme)),
+    ...utils.createFlashMetaAction(err)
+  })
 );
 
 const defaultState = {

common/app/redux/night-mode-epic.js

@@ -0,0 +1,64 @@
+import _ from 'lodash';
+import { Observable } from 'rx';
+import { ofType } from 'redux-epic';
+import store from 'store';
+
+import { themes } from '../../utils/themes.js';
+import { postJSON$ } from '../../utils/ajax-stream.js';
+import {
+  csrfSelector,
+  postThemeComplete,
+  postThemeError,
+  themeSelector,
+  types,
+  usernameSelector
+} from './index.js';
+
+function persistTheme(theme) {
+  store.set('fcc-theme', theme);
+}
+
+export default function nightModeEpic(
+  actions,
+  { getState },
+  { document }
+) {
+  return Observable.of(document)
+    // if document is undefined we do nothing (ssr trap)
+    .filter(Boolean)
+    .flatMap(({ body }) => {
+      const toggleBodyClass = actions
+        ::ofType(
+          types.fetchUser.complete,
+          types.toggleNightMode,
+          types.postTheme.complete,
+          types.postTheme.error
+        )
+        .map(_.flow(getState, themeSelector))
+        // catch existing night mode users
+        .do(persistTheme)
+        .do(theme => {
+          if (theme === themes.night) {
+            body.classList.add(themes.night);
+          } else {
+            body.classList.remove(themes.night);
+          }
+        })
+        .ignoreElements();
+
+      const postThemeEpic = actions::ofType(types.toggleNightMode)
+        .debounce(250)
+        .flatMapLatest(() => {
+          const _csrf = csrfSelector(getState());
+          const theme = themeSelector(getState());
+          const username = usernameSelector(getState());
+          return postJSON$('/update-my-theme', { _csrf, theme })
+            .map(postThemeComplete)
+            .catch(err => {
+              return Observable.of(postThemeError(username, theme, err));
+            });
+        });
+
+      return Observable.merge(toggleBodyClass, postThemeEpic);
+    });
+}

common/app/routes/Settings/redux/index.js

@@ -2,11 +2,13 @@ import { isLocationAction } from 'redux-first-router';
 import {
   addNS,
   createAction,
+  createAsyncTypes,
   createTypes
 } from 'berkeleys-redux-utils';
 
 import userUpdateEpic from './update-user-epic.js';
 import ns from '../ns.json';
+import { utils } from '../../../Flash/redux';
 
 export const epics = [
   userUpdateEpic
@@ -14,7 +16,7 @@ export const epics = [
 
 export const types = createTypes([
   'toggleUserFlag',
-  'updateMyEmail',
+  createAsyncTypes('updateMyEmail'),
   'updateMyLang',
   'onRouteSettings',
   'onRouteUpdateEmail'
@@ -24,7 +26,18 @@ export const types = createTypes([
 export const onRouteSettings = createAction(types.onRouteSettings);
 export const onRouteUpdateEmail = createAction(types.onRouteUpdateEmail);
 export const toggleUserFlag = createAction(types.toggleUserFlag);
-export const updateMyEmail = createAction(types.updateMyEmail);
+export const updateMyEmail = createAction(types.updateMyEmail.start);
+export const updateMyEmailComplete = createAction(
+  types.updateMyEmail.complete,
+  null,
+  utils.createFlashMetaAction
+);
+
+export const updateMyEmailError = createAction(
+  types.updateMyEmail.error,
+  null,
+  utils.createFlashMetaAction
+);
 
 export const updateMyLang = createAction(
   types.updateMyLang,

common/models/user.js

@@ -703,9 +703,7 @@ module.exports = function(User) {
       );
       return Promise.reject(err);
     }
-    return this.update$({ theme })
+    return this.update$({ theme }).toPromise();
-      .map({ updatedTo: theme })
-      .toPromise();
   };
 
   // deprecated. remove once live

common/utils/ajax-stream.js

@@ -64,11 +64,27 @@ function getCORSRequest() {
   }
 }
 
+function parseXhrResponse(responseType, xhr) {
+  switch (responseType) {
+    case 'json':
+      if ('response' in xhr) {
+        return xhr.responseType ?
+          xhr.response :
+          JSON.parse(xhr.response || xhr.responseText || 'null');
+      } else {
+        return JSON.parse(xhr.responseText || 'null');
+      }
+    case 'xml':
+      return xhr.responseXML;
+    case 'text':
+    default:
+      return ('response' in xhr) ? xhr.response : xhr.responseText;
+  }
+}
+
 function normalizeAjaxSuccessEvent(e, xhr, settings) {
-  var response = ('response' in xhr) ? xhr.response : xhr.responseText;
-  response = settings.responseType === 'json' ? JSON.parse(response) : response;
   return {
-    response: response,
+    response: parseXhrResponse(settings.responseType || xhr.responseType, xhr),
     status: xhr.status,
     responseType: xhr.responseType,
     xhr: xhr,
@@ -266,7 +282,8 @@ export function postJSON$(url, body) {
     headers: {
       'Content-Type': 'application/json',
       Accept: 'application/json'
-    }
+    },
+    normalizeError: (e, xhr) => parseXhrResponse('json', xhr)
   })
     .map(({ response }) => response);
 }
@@ -291,6 +308,7 @@ export function getJSON$(url) {
     headers: {
       'Content-Type': 'application/json',
       Accept: 'application/json'
-    }
+    },
+    normalizeError: (e, xhr) => parseXhrResponse('json', xhr)
   }).map(({ response }) => response);
 }

common/utils/flash.js

@@ -0,0 +1,10 @@
+import _ from 'lodash';
+
+export const alertTypes = _.keyBy([
+  'success',
+  'info',
+  'warning',
+  'danger'
+], _.identity);
+
+export const normalizeAlertType = alertType => alertTypes[alertType] || 'info';

server/boot/authentication.js

@@ -3,13 +3,13 @@ import { Observable } from 'rx';
 import dedent from 'dedent';
 // import debugFactory from 'debug';
 import { isEmail } from 'validator';
-import { check, validationResult } from 'express-validator/check';
+import { check } from 'express-validator/check';
 
-import { ifUserRedirectTo } from '../utils/middleware';
 import {
-  wrapHandledError,
+  ifUserRedirectTo,
-  createValidatorErrorFormatter
+  createValidatorErrorHandler
-} from '../utils/create-handled-error.js';
+} from '../utils/middleware';
+import { wrapHandledError } from '../utils/create-handled-error.js';
 
 const isSignUpDisabled = !!process.env.DISABLE_SIGNUP;
 // const debug = debugFactory('fcc:boot:auth');
@@ -82,13 +82,6 @@ module.exports = function enableAuthentication(app) {
         token: authTokenId
       } = {}
     } = req;
-    const validation = validationResult(req)
-      .formatWith(createValidatorErrorFormatter('errors', '/email-signup'));
-
-    if (!validation.isEmpty()) {
-      const errors = validation.array();
-      return next(errors.pop());
-    }
 
     const email = User.decodeEmail(encodedEmail);
     if (!isEmail(email)) {
@@ -188,6 +181,7 @@ module.exports = function enableAuthentication(app) {
     '/passwordless-auth',
     ifUserRedirect,
     passwordlessGetValidators,
+    createValidatorErrorHandler('errors', '/email-signup'),
     getPasswordlessAuth
   );
 
@@ -198,12 +192,6 @@ module.exports = function enableAuthentication(app) {
   ];
   function postPasswordlessAuth(req, res, next) {
     const { body: { email } = {} } = req;
-    const validation = validationResult(req)
-      .formatWith(createValidatorErrorFormatter('errors', '/email-signup'));
-    if (!validation.isEmpty()) {
-      const errors = validation.array();
-      return next(errors.pop());
-    }
 
     return User.findOne$({ where: { email } })
       .flatMap(_user => Observable.if(
@@ -222,6 +210,7 @@ module.exports = function enableAuthentication(app) {
     '/passwordless-auth',
     ifUserRedirect,
     passwordlessPostValidators,
+    createValidatorErrorHandler('errors', '/email-signup'),
     postPasswordlessAuth
   );
 

server/boot/settings.js

@@ -1,7 +1,12 @@
-import { isMongoId } from 'validator';
+import { check } from 'express-validator/check';
 
-import { ifNoUser401 } from '../utils/middleware';
+import {
+  ifNoUser401,
+  createValidatorErrorHandler
+} from '../utils/middleware';
 import supportedLanguages from '../../common/utils/supported-languages.js';
+import { themes } from '../../common/utils/themes.js';
+import { alertTypes } from '../../common/utils/flash.js';
 
 export default function settingsController(app) {
   const api = app.loopback.Router();
@@ -19,15 +24,29 @@ export default function settingsController(app) {
       );
   };
 
+  const updateMyEmailValidators = [
+    check('email')
+      .isEmail()
+      .withMessage('Email format is invalid.')
+  ];
+
   function updateMyEmail(req, res, next) {
     const { user, body: { email } } = req;
     return user.requestUpdateEmail(email)
       .subscribe(
-        (message) => res.json({ message }),
+        message => res.sendFlash(alertTypes.info, message),
         next
       );
   }
 
+  api.post(
+    '/update-my-email',
+    ifNoUser401,
+    updateMyEmailValidators,
+    createValidatorErrorHandler(alertTypes.danger),
+    updateMyEmail
+  );
+
   function updateMyLang(req, res, next) {
     const { user, body: { lang } = {} } = req;
     const langName = supportedLanguages[lang];
@@ -51,11 +70,14 @@ export default function settingsController(app) {
       );
   }
 
+  const updateMyCurrentChallengeValidators = [
+    check('currentChallengeId')
+      .isMongoId()
+      .withMessage('currentChallengeId is not a valid challenge ID')
+  ];
+
   function updateMyCurrentChallenge(req, res, next) {
     const { user, body: { currentChallengeId } } = req;
-    if (!isMongoId('' + currentChallengeId)) {
-      return next(new Error(`${currentChallengeId} is not a valid ObjectId`));
-    }
     return user.update$({ currentChallengeId }).subscribe(
       () => res.json({
         message:
@@ -65,22 +87,37 @@ export default function settingsController(app) {
     );
   }
 
+  api.post(
+    '/update-my-current-challenge',
+    ifNoUser401,
+    updateMyCurrentChallengeValidators,
+    createValidatorErrorHandler(alertTypes.danger),
+    updateMyCurrentChallenge
+  );
+
+  const updateMyThemeValidators = [
+    check('theme')
+      .isIn(Object.keys(themes))
+      .withMessage('Theme is invalid.')
+  ];
   function updateMyTheme(req, res, next) {
-    req.checkBody('theme', 'Theme is invalid.').isLength({ min: 4 });
     const { body: { theme } } = req;
-    const errors = req.validationErrors(true);
-    if (errors) {
-      return res.status(403).json({ errors });
-    }
     if (req.user.theme === theme) {
-      return res.json({ msg: 'Theme already set' });
+      return res.sendFlash(alertTypes.info, 'Theme already set');
     }
-    return req.user.updateTheme('' + theme)
+    return req.user.updateTheme(theme)
       .then(
-        data => res.json(data),
+        () => res.sendFlash(alertTypes.info, 'Your theme has been updated'),
         next
       );
   }
+  api.post(
+    '/update-my-theme',
+    ifNoUser401,
+    updateMyThemeValidators,
+    createValidatorErrorHandler(alertTypes.danger),
+    updateMyTheme
+  );
 
   api.post(
     '/toggle-available-for-hire',
@@ -107,28 +144,11 @@ export default function settingsController(app) {
     ifNoUser401,
     toggleUserFlag('sendQuincyEmail')
   );
-  api.post(
-    '/update-my-email',
-    ifNoUser401,
-    updateMyEmail
-  );
   api.post(
     '/update-my-lang',
     ifNoUser401,
     updateMyLang
   );
 
-  api.post(
-    '/update-my-current-challenge',
-    ifNoUser401,
-    updateMyCurrentChallenge
-  );
-
-  api.post(
-    '/update-my-theme',
-    ifNoUser401,
-    updateMyTheme
-  );
-
   app.use(api);
 }

server/middlewares/express-extensions.js

@@ -9,6 +9,12 @@ export default function() {
     res.renderWithoutFlash = res.render;
     // render to observable stream using build in render
     res.render$ = Observable.fromNodeCallback(res.render, res);
+    res.sendFlash = (type, message) => {
+      if (type && message) {
+        req.flash(type, message);
+      }
+      return res.json(req.flash());
+    };
     next();
   };
 }

server/utils/create-handled-error.js

@@ -18,13 +18,15 @@ export function wrapHandledError(err, {
   return err;
 }
 
-export const createValidatorErrorFormatter = (type, redirectTo, status) =>
+// for use with express-validator error formatter
+export const createValidatorErrorFormatter = (type, redirectTo) =>
   ({ msg }) => wrapHandledError(
     new Error(msg),
     {
       type,
       message: msg,
       redirectTo,
-      status
+      // we default to 400 as these are malformed requests
+      status: 400
     }
   );

server/utils/middleware.js

@@ -1,4 +1,7 @@
 import dedent from 'dedent';
+import { validationResult } from 'express-validator/check';
+
+import { createValidatorErrorFormatter } from './create-handled-error.js';
 
 export function ifNoUserRedirectTo(url, message, type = 'errors') {
   return function(req, res, next) {
@@ -56,3 +59,16 @@ export function ifUserRedirectTo(path = '/', status) {
     return next();
   };
 }
+
+// for use with express-validator error formatter
+export const createValidatorErrorHandler = (...args) => (req, res, next) => {
+  const validation = validationResult(req)
+    .formatWith(createValidatorErrorFormatter(...args));
+
+  if (!validation.isEmpty()) {
+    const errors = validation.array();
+    return next(errors.pop());
+  }
+
+  return next();
+};