Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

Add error message when Space user exists on Org user removal. #1163

Merged
merged 33 commits into from
Jul 24, 2017
Merged

Add error message when Space user exists on Org user removal. #1163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
88c99a3
Create a new catch to resole issues with existing space association
rememberlenny Jul 19, 2017
c48c3eb
Rename the constants associated to user invite notification and make …
rememberlenny Jul 19, 2017
4487850
Update tests and naming of actions associated to user store
rememberlenny Jul 19, 2017
28b56da
Add the new errors in the user store and change the invite naming
rememberlenny Jul 19, 2017
b123cd9
Update the naming of invite notification tests
rememberlenny Jul 19, 2017
f4b550f
Update the user actions to remove direct invite affiliation during na…
rememberlenny Jul 19, 2017
e8c967e
Update naming convention around the user notifications on the user list
rememberlenny Jul 19, 2017
1e30a66
Lint
rememberlenny Jul 19, 2017
d0a8766
Fix up errors
rememberlenny Jul 19, 2017
faf2819
Updates for syntax and simplicity
rememberlenny Jul 19, 2017
f782d65
Remove the USER_INVITE_STATUS_CREATED
rememberlenny Jul 20, 2017
24b0ba7
add test for emitting description of being unable to remove user
Jul 20, 2017
01eef76
Create a preliminary check before the users are verified for deletion
rememberlenny Jul 20, 2017
ba99e8d
Add an action to check a current user's space associations
rememberlenny Jul 20, 2017
67d0596
Create the additional cf commands for the space/org
rememberlenny Jul 20, 2017
0ed162f
Create a wrapper around the promises
rememberlenny Jul 20, 2017
3c883d5
move error handling logic to deleteUser
Jul 20, 2017
792214e
remove pre-emptive calls
Jul 20, 2017
9e8ea37
remove extra calls
Jul 20, 2017
fd13220
Merge
rememberlenny Jul 20, 2017
46bbd74
Fix the recall of the user delete
rememberlenny Jul 20, 2017
684ab14
Update the linting around promises
rememberlenny Jul 21, 2017
45bc78b
Update comments on the constants for user list notices
rememberlenny Jul 21, 2017
b7815d0
Update the changes to the notification
rememberlenny Jul 21, 2017
3c72fe2
Add tests for receivedOrgSpacesToExtractSpaceUsers
rememberlenny Jul 21, 2017
d9984cc
Add missing spaces
fureigh Jul 21, 2017
49465de
Add missing cf api request for fetchAllOrgSpaces
rememberlenny Jul 21, 2017
22ecabf
Create a promise resolve on the deleteUserIfNoSpaceAssociation
rememberlenny Jul 21, 2017
e00d4a6
Add tests on all functions associated to the deleteUserIfNoSpaceAssoc…
rememberlenny Jul 21, 2017
2afb68e
Merge with space update
rememberlenny Jul 21, 2017
07e0ddb
bring back code
Jul 24, 2017
237dc7e
fix tests for post deleteUser action
Jul 24, 2017
607daf9
fix test comment
Jul 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 63 additions & 8 deletions static_src/actions/user_actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import OrgStore from '../stores/org_store';
import SpaceStore from '../stores/space_store';

const ORG_NAME = OrgStore.cfName;
const MSG_USER_HAS_SPACE_ROLES = 'This user can\'t be removed because they still have a space ' +
'role within the organization. Please remove all space ' +
'associations before removing this user from the organization. ' +
'To review how, click the "Managing Teammates" link below.';

const userActions = {
fetchOrgUsers(orgGuid) {
Expand Down Expand Up @@ -60,12 +64,55 @@ const userActions = {
});
},

receivedOrgSpacesToExtractSpaceUsers(orgSpaces) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these methods need some tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added!

const orgSpaceUsers = orgSpaces.map((orgSpace) => Promise.resolve(
cfApi.fetchSpaceUserRoles(orgSpace.guid)
));
return Promise.all(orgSpaceUsers);
},

fetchUserAssociationsToOrgSpaces(userGuid, orgGuid) {
return Promise.resolve(cfApi.fetchAllOrgSpaces(orgGuid))
.then((orgSpaces) => userActions.receivedOrgSpacesToExtractSpaceUsers(orgSpaces));
},

deleteUserOrDisplayNotice(spaceUsers, userGuid, orgGuid) {
const usersSpaces = spaceUsers.filter(spaceUser => spaceUser.guid === userGuid);
if (usersSpaces.length > 0) {
userActions.createUserSpaceAssociationNotification(MSG_USER_HAS_SPACE_ROLES);
} else {
userActions.deleteUser(userGuid, orgGuid);
}
},

deleteUserIfNoSpaceAssociation(userGuid, orgGuid) {
return Promise.resolve(userActions.fetchUserAssociationsToOrgSpaces(userGuid, orgGuid))
.then((spaceUsers) => userActions.deleteUserOrDisplayNotice(spaceUsers, userGuid, orgGuid));
},

deleteUser(userGuid, orgGuid) {
AppDispatcher.handleViewAction({
type: userActionTypes.USER_DELETE,
userGuid,
orgGuid
});

return cfApi.deleteUser(userGuid, orgGuid)
.then(() => userActions.deletedUser(userGuid, orgGuid))
.catch(error => {
// Check whether we got caught on user roles in spaces
const userHasSpaceRoles = (error &&
error.response &&
error.response.status === 400 &&
error.response.data.error_code === 'CF-AssociationNotEmpty'
);
if (userHasSpaceRoles) {
this.createUserSpaceAssociationNotification(MSG_USER_HAS_SPACE_ROLES);
} else {
// else use generic error
this.errorRemoveUser(userGuid, error.response.data);
}
});
},

deletedUser(userGuid, orgGuid) {
Expand Down Expand Up @@ -198,12 +245,24 @@ const userActions = {
inviting ${email}`));
},

clearInviteNotifications() {
clearUserListNotifications() {
AppDispatcher.handleViewAction({
type: userActionTypes.USER_INVITE_STATUS_DISMISSED
type: userActionTypes.USER_LIST_NOTICE_DISMISSED
});
},

createUserListNotification(noticeType, description) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should start writing new actions in this format:

type: RAD_TYPE,
data: {
  datum1: false,
  datum2: true
}

Then every action presents a consistent interface regardless of the information being passed in. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think thats a good idea. It currently uses the ES6 syntax for exampling the variable name as the key. Instead of data, it does what you are describing with action. I think it would be a lot of refactoring to change them all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like the idea as well. but let's save that for a broader discussion.

AppDispatcher.handleViewAction({
type: userActionTypes.USER_LIST_NOTICE_CREATED,
noticeType,
description
});
},

createUserSpaceAssociationNotification(notification) {
userActions.createUserListNotification('error', notification);
},

createInviteNotification(verified, email) {
let description;
const noticeType = 'finish';
Expand All @@ -223,14 +282,10 @@ const userActions = {
'be controlled below.';
}

AppDispatcher.handleViewAction({
type: userActionTypes.USER_INVITE_STATUS_DISPLAYED,
noticeType,
description
});
userActions.createUserListNotification(noticeType, description);
},

userInviteError(err, contextualMessage) {
userListNoticeError(err, contextualMessage) {
AppDispatcher.handleServerAction({
type: userActionTypes.USER_INVITE_ERROR,
err,
Expand Down
14 changes: 7 additions & 7 deletions static_src/components/users.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ function stateSetter() {
loading: UserStore.loading,
empty: !UserStore.loading && !users.length,
users,
inviteNotices: UserStore._inviteNotification,
userInviteError: UserStore.getInviteError()
userListNotices: UserStore._userListNotification,
userListNoticeError: UserStore.getUserListNotificationError()
};
}

Expand All @@ -82,7 +82,7 @@ export default class Users extends React.Component {

onNotificationDismiss(ev) {
ev.preventDefault();
userActions.clearInviteNotifications();
userActions.clearUserListNotifications();
}

handleAddPermissions(roleKey, apiKey, userGuid) {
Expand Down Expand Up @@ -143,14 +143,14 @@ export default class Users extends React.Component {
let notification;
let userInvite;

if (this.state.inviteNotices.description) {
const notice = this.state.inviteNotices;
if (this.state.userListNotices.description) {
const notice = this.state.userListNotices;
notification = (
<Notification
message={ notice.description }
actions={ [] }
onDismiss={ this.onNotificationDismiss }
status="finish"
status={ notice.noticeType }
/>
);
} else {
Expand All @@ -164,7 +164,7 @@ export default class Users extends React.Component {
<UsersInvite
inviteDisabled={ this.state.inviteDisabled }
currentUserAccess={ this.state.currentUserAccess }
error={ this.state.userInviteError }
error={ this.state.userListNoticeError }
/>
</div>
);
Expand Down
8 changes: 4 additions & 4 deletions static_src/constants.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ const userActionTypes = keymirror({
USER_INVITE_TRIGGER: null,
// Action to trigger when invite status is triggered for front end.
USER_INVITE_STATUS_UPDATED: null,
// Action to trigger when user list notice is created.
USER_LIST_NOTICE_CREATED: null,
// Action to trigger email sent to user with cloud.gov invite url.
USER_ORG_ASSOCIATE: null,
// Action to associate user to organization on the server.
Expand All @@ -266,10 +268,8 @@ const userActionTypes = keymirror({
USER_ASSOCIATED_ORG_DISPLAYED: null,
// Action when something goes wrong in user invite and email process.
USER_INVITE_ERROR: null,
// Action to display an invite notification
USER_INVITE_STATUS_DISPLAYED: null,
// Action to dismiss an invite notification
USER_INVITE_STATUS_DISMISSED: null,
// Action to dismiss an user list notification.
USER_LIST_NOTICE_DISMISSED: null,
// Action to delete a user from an org.
USER_DELETE: null,
// Action when a user was deleted from an org on the server.
Expand Down
36 changes: 14 additions & 22 deletions static_src/stores/user_store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class UserStore extends BaseStore {
this._error = null;
this._saving = false;
this._inviteDisabled = false;
this._inviteNotification = {};
this._userListNotification = {};
this._loading = {};
}

Expand Down Expand Up @@ -64,7 +64,7 @@ export class UserStore extends BaseStore {

case userActionTypes.USER_INVITE_TRIGGER: {
this._inviteDisabled = true;
this._inviteError = null;
this._userListNotificationError = null;
this.emitChange();
break;
}
Expand Down Expand Up @@ -153,15 +153,7 @@ export class UserStore extends BaseStore {
}

case userActionTypes.USER_DELETE: {
const orgPermissionsReq = cfApi.deleteOrgUserPermissions(
action.userGuid,
action.orgGuid,
'users');

orgPermissionsReq.then(() => {
cfApi.deleteUser(action.userGuid, action.orgGuid);
});

// Nothing should happen.
break;
}

Expand All @@ -179,7 +171,7 @@ export class UserStore extends BaseStore {
}

case userActionTypes.USER_INVITE_ERROR: {
this._inviteError = Object.assign({}, action.err, {
this._userListNotificationError = Object.assign({}, action.err, {
contextualMessage: action.contextualMessage
});
this._inviteDisabled = false;
Expand All @@ -197,18 +189,18 @@ export class UserStore extends BaseStore {
break;
}

case userActionTypes.USER_INVITE_STATUS_DISPLAYED: {
case userActionTypes.USER_LIST_NOTICE_CREATED: {
this._inviteDisabled = false;
const noticeType = action.noticeType;
const description = action.description;
const notice = Object.assign({}, { noticeType }, { description });
this._inviteNotification = notice;
this._userListNotification = notice;
this.emitChange();
break;
}

case userActionTypes.USER_INVITE_STATUS_DISMISSED: {
this._inviteNotification = {};
case userActionTypes.USER_LIST_NOTICE_DISMISSED: {
this._userListNotification = {};
this.emitChange();
break;
}
Expand Down Expand Up @@ -285,8 +277,8 @@ export class UserStore extends BaseStore {
case errorActionTypes.CLEAR: {
this._error = null;
this._saving = false;
this._inviteNotification = {};
this._inviteError = null;
this._userListNotification = {};
this._userListNotificationError = null;
this._loading = {};
this.emitChange();
break;
Expand Down Expand Up @@ -367,12 +359,12 @@ export class UserStore extends BaseStore {
return this._currentUserIsAdmin;
}

getInviteNotification() {
return this._inviteNotification;
getUserListNotification() {
return this._userListNotification;
}

getInviteError() {
return this._inviteError;
getUserListNotificationError() {
return this._userListNotificationError;
}

get isSaving() {
Expand Down
Loading