-
Notifications
You must be signed in to change notification settings - Fork 18
Create space user's dropdown form for adding other org users #1172
Changes from 12 commits
e56bc8c
4ac8ee1
503e7f4
0bba2f7
931eef7
9fa5cbc
2461e0c
651e522
9c2815e
fa73119
04a8972
1d3f87f
55defcd
33a9778
4947863
42cb434
a94ab91
06ad595
02106fb
b98bf11
a7813cf
80ca7d6
fe47005
38ed848
ff12ede
5d8b81d
57ac1d0
339846c
f655f6e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,6 +11,7 @@ import OrgStore from '../stores/org_store.js'; | |
import SpaceStore from '../stores/space_store.js'; | ||
import UserList from './user_list.jsx'; | ||
import UsersInvite from './users_invite.jsx'; | ||
import UsersParentEntityUserSelector from './users_parent_entity_user_selector.jsx'; | ||
import Notification from './notification.jsx'; | ||
import UserStore from '../stores/user_store.js'; | ||
import ErrorMessage from './error_message.jsx'; | ||
|
@@ -33,12 +34,15 @@ function stateSetter() { | |
const isSaving = UserStore.isSaving; | ||
|
||
let users = []; | ||
let parentEntityUsers; | ||
let currentUserAccess = false; | ||
const inviteDisabled = UserStore.inviteDisabled(); | ||
const userParentEntityUserSelectDisabled = UserStore.userParentEntityUserSelectDisabled(); | ||
let entityGuid; | ||
|
||
if (currentType === SPACE_NAME) { | ||
users = UserStore.getAllInSpace(currentSpaceGuid); | ||
parentEntityUsers = UserStore.getAllInOrgAndNotSpace(currentSpaceGuid); | ||
entityGuid = currentSpaceGuid; | ||
currentUserAccess = UserStore.hasRole(currentUser.guid, currentSpaceGuid, | ||
SPACE_MANAGER); | ||
|
@@ -53,6 +57,7 @@ function stateSetter() { | |
currentUser, | ||
error: UserStore.getError(), | ||
inviteDisabled, | ||
userParentEntityUserSelectDisabled, | ||
currentUserAccess, | ||
currentOrgGuid, | ||
currentSpaceGuid, | ||
|
@@ -62,6 +67,7 @@ function stateSetter() { | |
loading: UserStore.loading, | ||
empty: !UserStore.loading && !users.length, | ||
users, | ||
parentEntityUsers, | ||
userListNotices: UserStore._userListNotification, | ||
userListNoticeError: UserStore.getUserListNotificationError() | ||
}; | ||
|
@@ -140,6 +146,13 @@ export default class Users extends React.Component { | |
return entityGuid; | ||
} | ||
|
||
get currentUserIsSpaceManager() { | ||
const { currentUser } = this.state; | ||
const { currentSpaceGuid } = SpaceStore; | ||
|
||
return UserStore.hasRole(currentUser.guid, currentSpaceGuid, SPACE_MANAGER); | ||
} | ||
|
||
get currentUserIsOrgManager() { | ||
const { currentUser } = this.state; | ||
const { currentOrgGuid } = OrgStore; | ||
|
@@ -186,6 +199,34 @@ export default class Users extends React.Component { | |
); | ||
} | ||
|
||
get userParentEntityUserSelector() { | ||
if (!this.isSpace) { | ||
return null; | ||
} | ||
|
||
if (!this.currentUserIsSpaceManager) { | ||
return ( | ||
<PanelDocumentation> | ||
ORG AND SPACE MANAGERS CAN DO THIS. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure you meant to have this text here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hahaha. No it should be :) |
||
</PanelDocumentation> | ||
); | ||
} | ||
|
||
return ( | ||
<UsersParentEntityUserSelector | ||
userParentEntityUserSelectDisabled={ this.state.userParentEntityUserSelectDisabled } | ||
parentEntityGuid={ this.state.currentOrgGuid } | ||
parentEntity={ ORG_ENTITY } | ||
currentEntityGuid={ this.entityGuid } | ||
currentEntity={ this.entityType } | ||
parentEntityUsers={ this.state.parentEntityUsers } | ||
inviteEntityType={ this.entityType } | ||
currentUserAccess={ this.state.currentUserAccess } | ||
error={ this.state.userListNoticeError } | ||
/> | ||
); | ||
} | ||
|
||
_onChange() { | ||
this.setState(stateSetter()); | ||
} | ||
|
@@ -203,6 +244,7 @@ export default class Users extends React.Component { | |
<div className="test-users"> | ||
<ErrorMessage error={this.state.error} /> | ||
{ this.userInvite } | ||
{ this.userParentEntityUserSelector } | ||
{ this.notification } | ||
<UserList | ||
users={ this.state.users } | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
/** | ||
* Renders a form that allows org users to invite new users | ||
* to cloud.gov | ||
*/ | ||
|
||
import PropTypes from 'prop-types'; | ||
import React from 'react'; | ||
import Action from './action.jsx'; | ||
import FormStore from '../stores/form_store'; | ||
import { Form, FormSelect } from './form'; | ||
import PanelDocumentation from './panel_documentation.jsx'; | ||
import userActions from '../actions/user_actions'; | ||
import { validateString } from '../util/validators'; | ||
|
||
const USERS_PARENT_ENTITY_USER_FORM_GUID = 'users-parent-entity-users-form'; | ||
|
||
const propTypes = { | ||
userParentEntityUserSelectDisabled: PropTypes.bool, | ||
currentUserAccess: PropTypes.bool, | ||
parentEntityUsers: PropTypes.array, | ||
error: PropTypes.object, | ||
parentEntityGuid: PropTypes.string, | ||
parentEntity: PropTypes.string, | ||
currentEntityGuid: PropTypes.string, | ||
currentEntity: PropTypes.string | ||
}; | ||
const defaultProps = { | ||
userParentEntityUserSelectDisabled: false, | ||
currentUserAccess: false, | ||
error: {} | ||
}; | ||
|
||
function stateSetter(props) { | ||
return { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is state needed in this component if everything is passed down from props? https://github.com/18F/cg-dashboard/blob/master/CONTRIBUTING.md#components There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its not. This is a great document. Thanks. |
||
userParentEntityUserSelectDisabled: props.userParentEntityUserSelectDisabled, | ||
currentUserAccess: props.currentUserAccess, | ||
parentEntityUsers: props.parentEntityUsers, | ||
parentEntityGuid: props.parentEntityGuid, | ||
parentEntity: props.parentEntity, | ||
currentEntityGuid: props.currentEntityGuid, | ||
currentEntity: props.currentEntity, | ||
error: props.error | ||
}; | ||
} | ||
|
||
export default class UsersParentEntityUserSelector extends React.Component { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we might want to change the name of this. at first glance it's not that clear. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I totally agree. Originally it was supposed to be a module. Not it only does one thing, check org users, if user is in a space. |
||
constructor(props) { | ||
super(props); | ||
FormStore.create(USERS_PARENT_ENTITY_USER_FORM_GUID); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Creating a form here could produce unwanted side effects e.g. if this component subscribed to the FormStore's change event. It should be called in |
||
|
||
this.state = stateSetter(props); | ||
|
||
this.validateString = validateString().bind(this); | ||
this._onSubmitForm = this._onSubmitForm.bind(this); | ||
} | ||
|
||
_onSubmitForm(errs, values) { | ||
const entityType = this.state.currentEntity; | ||
const entityGuid = this.state.currentEntityGuid; | ||
const apiKey = 'auditors'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets make these constants |
||
const roles = 'space_auditor'; | ||
if (values.userGuid) { | ||
const userGuid = values.userGuid.value; | ||
userActions.addUserRoles(roles, apiKey, userGuid, entityGuid, entityType); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think in order to prevent confusion, we should only show org users currently without space roles. then here, upon successful submission, we should remove the user from the list so we don't see them here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was working on last night, but didn't know quite how the EventEmitter on the UserStore worked. I need to dig in a bit. Basically, the logic to do this works. |
||
} | ||
} | ||
|
||
get errorMessage() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copy! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this is copied from the UserInvite component, seems like a good time to extract a out module or higher-order component. |
||
const { error } = this.props; | ||
|
||
if (!error) return ''; | ||
|
||
const message = error.contextualMessage; | ||
|
||
if (error.message) { | ||
return `${message}: ${error.message}.`; | ||
} | ||
|
||
return message; | ||
} | ||
|
||
get invitationMessage() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above ^^ |
||
const parentEntity = this.props.parentEntity; | ||
const currentEntity = this.props.currentEntity; | ||
|
||
return `Invite an existing user in this ${parentEntity}` + | ||
` to this ${currentEntity}.`; | ||
} | ||
|
||
get userSelector() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. test for this |
||
const orgUsers = this.state.parentEntityUsers.map((user) => | ||
({ value: user.guid, label: user.username }) | ||
); | ||
|
||
if (!orgUsers) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<FormSelect | ||
formGuid={ USERS_PARENT_ENTITY_USER_FORM_GUID } | ||
classes={ ['test-users_parent_entity_user_name'] } | ||
label="Username" | ||
name="userGuid" | ||
options={ orgUsers } | ||
validator={ this.validateString } | ||
/> | ||
); | ||
} | ||
|
||
render() { | ||
const { userParentEntityUserSelectDisabled } = this.props; | ||
|
||
if (!this.props.currentUserAccess) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<div className="test-users-invite"> | ||
<PanelDocumentation description> | ||
<p>{ this.invitationMessage }</p> | ||
</PanelDocumentation> | ||
<Form | ||
guid={ USERS_PARENT_ENTITY_USER_FORM_GUID } | ||
classes={ ['users_parent_entity_user_form'] } | ||
ref="form" | ||
onSubmit={ this._onSubmitForm } | ||
errorOverride={ this.errorMessage } | ||
> | ||
{ this.userSelector } | ||
<Action | ||
label="submit" | ||
type="submit" | ||
disabled={ userParentEntityUserSelectDisabled } | ||
> | ||
Add user to this { this.state.currentEntity } | ||
</Action> | ||
</Form> | ||
</div> | ||
); | ||
} | ||
|
||
} | ||
|
||
UsersParentEntityUserSelector.propTypes = propTypes; | ||
|
||
UsersParentEntityUserSelector.defaultProps = defaultProps; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import '../../global_setup.js'; | ||
|
||
import React from 'react'; | ||
import { shallow } from 'enzyme'; | ||
import { Form } from '../../../components/form'; | ||
import ParentUserSelector from '../../../components/users_parent_entity_user_selector.jsx'; | ||
|
||
describe('<ParentUserSelector />', function () { | ||
const entityType = 'space'; | ||
const props = { | ||
inviteEntityType: entityType, | ||
currentUserAccess: true | ||
}; | ||
let wrapper; | ||
|
||
describe('when user does not have ability to invite other users', () => { | ||
it('does not render <Form /> component', () => { | ||
const noAccessProps = Object.assign({}, props, { currentUserAccess: false }); | ||
wrapper = shallow(<ParentUserSelector { ...noAccessProps } />); | ||
|
||
expect(wrapper.find(Form).length).toEqual(0); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -682,6 +682,19 @@ describe('UserStore', function () { | |
}); | ||
}); | ||
|
||
describe('getAllInOrgAndNotSpace()', function() { | ||
it('should find all users that are in an org, without the space users', function() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am probably misreading this spec, but from the description, shouldn't the Should the corresponding method in |
||
var spaceGuid = 'asdfa'; | ||
var testUser = { guid: 'adfzxcv', roles: { [spaceGuid]: [ 'space_user'] } }; | ||
|
||
UserStore.push(testUser); | ||
|
||
let actual = UserStore.getAllInOrg(spaceGuid); | ||
|
||
expect(actual[0]).toEqual(testUser); | ||
}); | ||
}); | ||
|
||
describe('USER_FETCH', function () { | ||
let user; | ||
beforeEach(function () { | ||
|
@@ -909,6 +922,7 @@ describe('UserStore', function () { | |
}); | ||
}); | ||
}); | ||
|
||
describe('isAdmin()', function () { | ||
describe('user with _currentUserIsAdmin', function () { | ||
let user, space, org, actual; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It has a linking error because it's not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets delete it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍