Merge pull request #2667 from weseek/imprv/use-properly-USER_PUBLIC_FIELDS

Imprv/use properly user public fields

src/client/js/components/Me/PasswordSettings.jsx

@@ -3,7 +3,6 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { withTranslation } from 'react-i18next';
 
-
 import { toastSuccess, toastError } from '../../util/apiNotification';
 import { withUnstatedContainers } from '../UnstatedUtils';
 
@@ -23,6 +22,7 @@ class PasswordSettings extends React.Component {
       oldPassword: '',
       newPassword: '',
       newPasswordConfirm: '',
+      isPasswordSet: false,
     };
 
     this.onClickSubmit = this.onClickSubmit.bind(this);
@@ -30,6 +30,21 @@ class PasswordSettings extends React.Component {
 
   }
 
+  async componentDidMount() {
+    const { appContainer } = this.props;
+
+    try {
+      const res = await appContainer.apiv3Get('/personal-setting/is-password-set');
+      const { isPasswordSet } = res.data;
+      this.setState({ isPasswordSet });
+    }
+    catch (err) {
+      toastError(err);
+      this.setState({ retrieveError: err });
+    }
+
+  }
+
   async onClickSubmit() {
     const { t, appContainer, personalContainer } = this.props;
     const { oldPassword, newPassword, newPasswordConfirm } = this.state;
@@ -61,22 +76,26 @@ class PasswordSettings extends React.Component {
   }
 
   render() {
-    const { t, personalContainer } = this.props;
+    const { t } = this.props;
     const { newPassword, newPasswordConfirm } = this.state;
     const isIncorrectConfirmPassword = (newPassword !== newPasswordConfirm);
 
+    if (this.state.retrieveError != null) {
+      throw new Error(this.state.retrieveError.message);
+    }
+
     return (
       <React.Fragment>
-        { (!personalContainer.state.isPasswordSet) && (
+        { (!this.state.isPasswordSet) && (
           <div className="alert alert-warning">{ t('personal_settings.password_is_not_set') }</div>
         ) }
 
         <div className="container-fluid my-4">
-          {(personalContainer.state.isPasswordSet)
+          {(this.state.isPasswordSet)
             ? <h2 className="border-bottom">{t('personal_settings.update_password')}</h2>
           : <h2 className="border-bottom">{t('personal_settings.set_new_password')}</h2>}
         </div>
-        {(personalContainer.state.isPasswordSet)
+        {(this.state.isPasswordSet)
         && (
           <div className="row mb-3">
             <label htmlFor="oldPassword" className="col-md-3 text-md-right">{ t('personal_settings.current_password') }</label>
@@ -124,7 +143,7 @@ class PasswordSettings extends React.Component {
               type="button"
               className="btn btn-primary"
               onClick={this.onClickSubmit}
-              disabled={this.state.retrieveError != null || isIncorrectConfirmPassword}
+              disabled={isIncorrectConfirmPassword}
             >
               {t('Update')}
             </button>

src/client/js/services/PersonalContainer.js

@@ -29,7 +29,6 @@ export default class PersonalContainer extends Container {
       isUploadedPicture: false,
       uploadedPictureSrc: this.getUploadedPictureSrc(this.appContainer.currentUser),
       externalAccounts: [],
-      isPasswordSet: false,
       apiToken: '',
     };
 
@@ -55,7 +54,6 @@ export default class PersonalContainer extends Container {
         isEmailPublished: currentUser.isEmailPublished,
         lang: currentUser.lang,
         isGravatarEnabled: currentUser.isGravatarEnabled,
-        isPasswordSet: (currentUser.password != null),
         apiToken: currentUser.apiToken,
       });
     }

src/server/models/user.js

@@ -384,7 +384,7 @@ module.exports = function(crowi) {
     option = option || {};
 
     const sort = option.sort || { createdAt: -1 };
-    const fields = option.fields || USER_PUBLIC_FIELDS;
+    const fields = option.fields || {};
 
     let status = option.status || [STATUS_ACTIVE, STATUS_SUSPENDED];
     if (!Array.isArray(status)) {
@@ -403,7 +403,7 @@ module.exports = function(crowi) {
 
     const sort = option.sort || { createdAt: -1 };
     const status = option.status || STATUS_ACTIVE;
-    const fields = option.fields || USER_PUBLIC_FIELDS;
+    const fields = option.fields || {};
 
     return this.find({ _id: { $in: ids }, status })
       .select(fields)
@@ -414,31 +414,6 @@ module.exports = function(crowi) {
     return this.find({ admin: true });
   };
 
-  userSchema.statics.findUsersByPartOfEmail = function(emailPart, options) {
-    const status = options.status || null;
-    const emailPartRegExp = new RegExp(emailPart.replace(/[-/\\^$*+?.()|[\]{}]/g, '\\$&'));
-    const User = this;
-
-    return new Promise((resolve, reject) => {
-      const query = User.find({ email: emailPartRegExp }, USER_PUBLIC_FIELDS);
-
-      if (status) {
-        query.and({ status });
-      }
-
-      query
-        .limit(PAGE_ITEMS + 1)
-        .exec((err, userData) => {
-          if (err) {
-            return reject(err);
-          }
-
-          return resolve(userData);
-        });
-    });
-  };
-
-
   userSchema.statics.findUserByUsername = function(username) {
     if (username == null) {
       return Promise.resolve(null);

src/server/routes/admin.js

@@ -337,23 +337,6 @@ module.exports = function(crowi, app) {
 
   actions.api = {};
 
-  // app.get('/_api/admin/users.search' , admin.api.userSearch);
-  actions.api.usersSearch = function(req, res) {
-    const User = crowi.model('User');
-    const email = req.query.email;
-
-    User.findUsersByPartOfEmail(email, {})
-      .then((users) => {
-        const result = {
-          data: users,
-        };
-        return res.json(ApiResponse.success(result));
-      })
-      .catch((err) => {
-        return res.json(ApiResponse.error());
-      });
-  };
-
   /**
    * save esa settings, update config cache, and response json
    *

src/server/routes/apiv3/personal-setting.js

@@ -123,8 +123,52 @@ module.exports = (crowi) => {
    *                      description: personal params
    */
   router.get('/', accessTokenParser, loginRequiredStrictly, async(req, res) => {
-    const currentUser = await User.findUserByUsername(req.user.username);
-    return res.apiv3({ currentUser });
+    const { username } = req.user;
+
+    try {
+      const user = await User.findUserByUsername(username);
+      const currentUser = user.toObject();
+      return res.apiv3({ currentUser });
+
+    }
+    catch (err) {
+      logger.error(err);
+      return res.apiv3Err('update-personal-settings-failed');
+    }
+  });
+
+  /**
+   * @swagger
+   *
+   *    /personal-setting/is-password-set:
+   *      get:
+   *        tags: [PersonalSetting]
+   *        operationId: getIsPasswordSet
+   *        summary: /personal-setting
+   *        description: Get whether a password has been set
+   *        responses:
+   *          200:
+   *            description: Whether a password has been set
+   *            content:
+   *              application/json:
+   *                schema:
+   *                  properties:
+   *                    isPasswordSet:
+   *                      type: boolean
+   */
+  router.get('/is-password-set', accessTokenParser, loginRequiredStrictly, async(req, res) => {
+    const { username } = req.user;
+
+    try {
+      const user = await User.findUserByUsername(username);
+      const isPasswordSet = user.isPasswordSet();
+      return res.apiv3({ isPasswordSet });
+    }
+    catch (err) {
+      logger.error(err);
+      return res.apiv3Err('fail-to-get-whether-password-is-set');
+    }
+
   });
 
   /**

src/server/routes/apiv3/users.js

@@ -119,7 +119,7 @@ module.exports = (crowi) => {
    *            description: page number
    *            schema:
    *              type: number
-   *          - name:  selectedStatusList
+   *          - name: selectedStatusList
    *            in: query
    *            description: status list
    *            schema:
@@ -184,6 +184,7 @@ module.exports = (crowi) => {
           sort: sortOutput,
           page,
           limit: PAGE_ITEMS,
+          select: User.USER_PUBLIC_FIELDS,
         },
       );
       return res.apiv3({ paginateResult });

src/server/routes/index.js

@@ -89,7 +89,6 @@ module.exports = function(crowi, app) {
   app.get('/admin/notification'              , loginRequiredStrictly , adminRequired , admin.notification.index);
   app.get('/admin/notification/slackAuth'    , loginRequiredStrictly , adminRequired , admin.notification.slackAuth);
   app.get('/admin/notification/slackSetting/disconnect', loginRequiredStrictly , adminRequired , admin.notification.disconnectFromSlack);
-  app.get('/_api/admin/users.search'         , loginRequiredStrictly , adminRequired , admin.api.usersSearch);
   app.get('/admin/global-notification/new'   , loginRequiredStrictly , adminRequired , admin.globalNotification.detail);
   app.get('/admin/global-notification/:id'   , loginRequiredStrictly , adminRequired , admin.globalNotification.detail);
 

src/test/models/user.test.js

@@ -42,13 +42,6 @@ describe('User', () => {
         expect(user.name).toBe('Example for User Test');
       });
 
-      test('should be found by findUsersByPartOfEmail', async() => {
-        const users = await User.findUsersByPartOfEmail('usert', {});
-        expect(users).toBeInstanceOf(Array);
-        expect(users.length).toBe(2);
-        expect(users[0]).toBeInstanceOf(User);
-        expect(users[1]).toBeInstanceOf(User);
-      });
     });
   });