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
Show file tree
Hide file tree
Changes from 25 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
53 changes: 45 additions & 8 deletions static_src/actions/user_actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,35 @@ 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));
},

deleteUserIfNoSpaceAssociation(userGuid, orgGuid) {
let usersSpaces;
userActions.fetchUserAssociationsToOrgSpaces(userGuid, orgGuid)
.then((spaceUsers) => {
usersSpaces = spaceUsers.filter(spaceUser => spaceUser.guid === userGuid);
if (usersSpaces.length > 0) {
const description = 'This user can\'t be removed because they still have a space ' +
'role within the organization. Please remove all space ' +
Copy link
Contributor

Choose a reason for hiding this comment

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

The natural response to "Please remove all space associations" is "How/where do I do that?" Let's add a link or a few words to point them in the right direction.

'associations before removing this user from the organization. ' +
'To review how, click the "Managing Teammates" link below.';
userActions.createUserSpaceAssociationNotification(description);
} else {
userActions.deleteUser(userGuid, orgGuid);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

we need a catch here

},

deleteUser(userGuid, orgGuid) {
AppDispatcher.handleViewAction({
type: userActionTypes.USER_DELETE,
Expand Down Expand Up @@ -198,12 +227,24 @@ const userActions = {
inviting ${email}`));
},

clearInviteNotifications() {
clearUserListNotifications() {
AppDispatcher.handleViewAction({
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_INVITE_STATUS_DISMISSED
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 +264,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
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
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
35 changes: 14 additions & 21 deletions static_src/stores/user_store.js
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,14 +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);
});
cfApi.deleteUser(action.userGuid, action.orgGuid);

break;
}
Expand All @@ -179,7 +172,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 +190,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 +278,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 +360,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
18 changes: 9 additions & 9 deletions static_src/test/unit/actions/user_actions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,12 @@ describe('userActions', function() {
orgGuid: expectedOrgGuid
};

let spy = setupViewSpy(sandbox)
let spy = setupViewSpy(sandbox);

userActions.deleteUser(expectedUserGuid, expectedOrgGuid);

assertAction(spy, userActionTypes.USER_DELETE, expectedParams);
})
});
});

describe('deletedUser()', function() {
Expand Down Expand Up @@ -207,13 +207,13 @@ describe('userActions', function() {
});
});

describe('clearInviteNotifications()', function() {
describe('clearUserListNotifications()', function() {
it('should dispatch a view event of type clear invite notification', function() {
let spy = setupViewSpy(sandbox);

userActions.clearInviteNotifications();
userActions.clearUserListNotifications();

assertAction(spy, userActionTypes.USER_INVITE_STATUS_DISMISSED);
assertAction(spy, userActionTypes.USER_LIST_NOTICE_DISMISSED);
});
});

Expand All @@ -240,7 +240,7 @@ describe('userActions', function() {
let spy = setupViewSpy(sandbox);
sandbox.spy(userActions, 'receivedInviteStatus');
userActions.createInviteNotification(false, email);
assertAction(spy, userActionTypes.USER_INVITE_STATUS_DISPLAYED, expected);
assertAction(spy, userActionTypes.USER_LIST_NOTICE_CREATED, expected);
done();
});

Expand All @@ -253,12 +253,12 @@ describe('userActions', function() {
let spy = setupViewSpy(sandbox);
sandbox.spy(userActions, 'receivedInviteStatus');
userActions.createInviteNotification(true, email);
assertAction(spy, userActionTypes.USER_INVITE_STATUS_DISPLAYED, expected);
assertAction(spy, userActionTypes.USER_LIST_NOTICE_CREATED, expected);
done();
});
});

describe('userInviteError()', function() {
describe('userListNoticeError()', function() {
let err;
let message;

Expand All @@ -267,7 +267,7 @@ describe('userActions', function() {
message = 'something happened when invititing';
sandbox.stub(AppDispatcher, 'handleServerAction');

userActions.userInviteError(err, message).then(done, done.fail);
userActions.userListNoticeError(err, message).then(done, done.fail);
});

it('should dispatch server action of type user error with error and optional message',
Expand Down
Loading