Manage user groups in UserEdit

client/app/assets/less/ant.less

@@ -20,6 +20,7 @@
 @import '~antd/lib/tag/style/index';
 @import '~antd/lib/grid/style/index';
 @import '~antd/lib/switch/style/index';
+@import '~antd/lib/empty/style/index';
 @import 'inc/ant-variables';
 
 // Remove bold in labels for Ant checkboxes and radio buttons

client/app/components/dynamic-form/DynamicForm.jsx

@@ -7,6 +7,7 @@ import Checkbox from 'antd/lib/checkbox';
 import Button from 'antd/lib/button';
 import Upload from 'antd/lib/upload';
 import Icon from 'antd/lib/icon';
+import Select from 'antd/lib/select';
 import { includes } from 'lodash';
 import { react2angular } from 'react2angular';
 import { toastr } from '@/services/ng';
@@ -130,6 +131,25 @@ export const DynamicForm = Form.create()(class DynamicForm extends React.Compone
     return getFieldDecorator(name, fileOptions)(upload);
   }
 
+  renderSelect(field, props) {
+    const { getFieldDecorator } = this.props.form;
+    const { name, options, mode, initialValue, readOnly } = field;
+    const { Option } = Select;
+
+    const decoratorOptions = {
+      rules: fieldRules(field),
+      initialValue,
+    };
+
+    return getFieldDecorator(name, decoratorOptions)(
+      <Select {...props} mode={mode}>
+        {options && options.map(({ value, title }) => (
+          <Option key={`${value}`} value={value} disabled={readOnly}>{ title || value }</Option>
+        ))}
+      </Select>,
+    );
+  }
+
   renderField(field, props) {
     const { getFieldDecorator } = this.props.form;
     const { name, type, initialValue } = field;
@@ -145,6 +165,8 @@ export const DynamicForm = Form.create()(class DynamicForm extends React.Compone
       return getFieldDecorator(name, options)(<Checkbox {...props}>{fieldLabel}</Checkbox>);
     } else if (type === 'file') {
       return this.renderUpload(field, props);
+    } else if (type === 'select') {
+      return this.renderSelect(field, props);
     } else if (type === 'number') {
       return getFieldDecorator(name, options)(<InputNumber {...props} />);
     }

client/app/components/proptypes.js

@@ -34,8 +34,15 @@ export const RefreshScheduleDefault = {
 export const Field = PropTypes.shape({
   name: PropTypes.string.isRequired,
   title: PropTypes.string,
-  type: PropTypes.oneOf(['text', 'email', 'password', 'number', 'checkbox', 'file']).isRequired,
-  initialValue: PropTypes.oneOfType([PropTypes.string, PropTypes.number, PropTypes.bool]),
+  type: PropTypes.oneOf(['text', 'email', 'password', 'number', 'checkbox', 'file', 'select']).isRequired,
+  initialValue: PropTypes.oneOfType([
+    PropTypes.string,
+    PropTypes.number,
+    PropTypes.bool,
+    PropTypes.arrayOf(PropTypes.string),
+    PropTypes.arrayOf(PropTypes.number),
+  ]),
+  mode: PropTypes.string,
   required: PropTypes.bool,
   readOnly: PropTypes.bool,
   minLength: PropTypes.number,

client/app/components/users/UserEdit.jsx

@@ -4,7 +4,9 @@ import Button from 'antd/lib/button';
 import Form from 'antd/lib/form';
 import Modal from 'antd/lib/modal';
 import { react2angular } from 'react2angular';
+import { includes, reject, isNull } from 'lodash';
 import { User } from '@/services/user';
+import { Group } from '@/services/group';
 import { currentUser } from '@/services/auth';
 import { absoluteUrl } from '@/services/utils';
 import { UserProfile } from '../proptypes';
@@ -21,11 +23,16 @@ export class UserEdit extends React.Component {
     super(props);
     this.state = {
       user: this.props.user,
+      groups: [],
       regeneratingApiKey: false,
       sendingPasswordEmail: false,
       resendingInvitation: false,
       togglingUser: false,
     };
+
+    Group.query((groups) => {
+      this.setState({ groups: groups.map(({ id, name }) => ({ value: id, title: name })) });
+    });
   }
 
   changePassword = () => {
@@ -101,22 +108,34 @@ export class UserEdit extends React.Component {
     });
   };
 
-  renderBasicInfoForm() {
+  renderUserInfoForm() {
     const { user } = this.state;
-    const formFields = [
+
+    const formFields = reject([
       {
         name: 'name',
         title: 'Name',
         type: 'text',
         initialValue: user.name,
+        required: true,
       },
       {
         name: 'email',
         title: 'Email',
         type: 'email',
         initialValue: user.email,
+        required: true,
       },
-    ].map(field => ({ ...field, readOnly: user.isDisabled, required: true }));
+      (currentUser.isAdmin && currentUser.id !== user.id) ? {
+        name: 'group_ids',
+        title: 'Groups',
+        type: 'select',
+        mode: 'multiple',
+        options: this.state.groups,
+        initialValue: this.state.groups.filter(group => includes(user.groupIds, group.value))
+          .map(group => group.value),
+      } : null,
+    ], isNull).map(field => ({ ...field, readOnly: user.isDisabled }));
 
     return (
       <DynamicForm
@@ -227,7 +246,7 @@ export class UserEdit extends React.Component {
         />
         <h3 className="profile__h3">{user.name}</h3>
         <hr />
-        {this.renderBasicInfoForm()}
+        {this.renderUserInfoForm()}
         {!user.isDisabled && (
           <Fragment>
             {this.renderApiKey()}

client/app/services/user.js

@@ -73,6 +73,7 @@ function convertUserInfo(user) {
     email: user.email,
     profileImageUrl: user.profile_image_url,
     apiKey: user.api_key,
+    groupIds: user.groups,
     isDisabled: user.is_disabled,
     isInvitationPending: user.is_invitation_pending,
   };

cypress/integration/user/edit_profile_spec.js

@@ -37,6 +37,7 @@ describe('Edit Profile', () => {
   });
 
   it('takes a screenshot', () => {
+    cy.getByTestId('Groups').contains('admin');
     cy.getByTestId('ApiKey').then(($apiKey) => {
       $apiKey.val('secret');
     });

redash/handlers/users.py

@@ -204,7 +204,7 @@ def post(self, user_id):
 
         req = request.get_json(True)
 
-        params = project(req, ('email', 'name', 'password', 'old_password', 'groups'))
+        params = project(req, ('email', 'name', 'password', 'old_password', 'group_ids'))
 
         if 'password' in params and 'old_password' not in params:
             abort(403, message="Must provide current password to update password.")
@@ -216,7 +216,7 @@ def post(self, user_id):
             user.hash_password(params.pop('password'))
             params.pop('old_password')
 
-        if 'groups' in params and not self.current_user.has_permission('admin'):
+        if 'group_ids' in params and not self.current_user.has_permission('admin'):
             abort(403, message="Must be admin to change groups membership.")
 
         if 'email' in params:

tests/handlers/test_users.py

@@ -273,6 +273,15 @@ def test_changing_email_does_not_end_current_session(self):
         # make sure the session's `user_id` has changed to reflect the new identity, thus not logging the user out
         self.assertNotEquals(previous, current)
 
+    def test_admin_can_change_user_groups(self):
+        admin_user = self.factory.create_admin()
+        other_user = self.factory.create_user(group_ids=[1])
+
+        rv = self.make_request('post', "/api/users/{}".format(other_user.id), data={"group_ids": []}, user=admin_user)
+
+        self.assertEqual(rv.status_code, 200)
+        self.assertEqual(models.User.query.get(other_user.id).group_ids, [])
+
     def test_admin_can_delete_user(self):
         admin_user = self.factory.create_admin()
         other_user = self.factory.create_user(is_invitation_pending=True)