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

Do not allow role checkboxes to update in error cases #1118

Merged
merged 2 commits into from
Jun 13, 2017

Conversation

jcscottiii
Copy link
Contributor

@jcscottiii jcscottiii commented Jun 13, 2017

Solves: https://favro.com/card/1e11108a2da81e3bd7153a7a/18F-5842

The problem was that even though there was an error from trying to
associate/dissociate spaces roles or just dissociate org roles, the
checkboxes would still reflect because addedUserRoles / deletedUserRoles
actions would be called.

This PR moves addedUserRoles and deletedUserRoles to in another place in
the promise chain.
Now, after succeeding with deleteSpaceUserPermissions and
deleteOrgUserPermissions, the deletedUserRoles action is called in error
either, it will call errorRemoveUser (which will be used for raising
errors to UI)

The problem with the association of space roles was that it was not
passing the error back up like the org space association. This PR
mirrors that same behavior. (There is no errorAddUser action right now
so no need to call that.)

There are tests in this PR to test that in success cases addedUserRoles
and deletedUserRoles actions are called. Also the tests test to make
sure those actions are NOT called in the case of an error.

Also added moxios so that we can easily test the axios library. (it's by the same author)

Also, this fixes typos.

Solves: https://favro.com/card/1e11108a2da81e3bd7153a7a/18F-5842

The problem was that even though there was an error from trying to
associate/dissociate spaces roles or just dissociate org roles, the
checkboxes would still reflect because addedUserRoles / deletedUserRoles
actions would be called.

This PR moves addedUserRoles and deletedUserRoles to in another place in
the promise chain.
Now, after succeeding with deleteSpaceUserPermissions and
deleteOrgUserPermissions, the deletedUserRoles action is called in error
either, it will call errorRemoveUser (which will be used for raising
errors to UI)

The problem with the association of space roles was that it was not
passing the error back up like the org space association. This PR
mirrors that same behavior. (There is no errorAddUser action right now
so no need to call that.)

There are tests in this PR to test that in success cases addedUserRoles
and deletedUserRoles actions are called. Also the tests test to make
sure those actions are NOT called in the case of an error.
Copy link
Contributor

@rememberlenny rememberlenny left a comment

Choose a reason for hiding this comment

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

This looks really good. Also the moxios tests look really good.

@rememberlenny rememberlenny merged commit fff18c9 into master Jun 13, 2017
@rememberlenny rememberlenny deleted the js-fix-checkbox branch June 13, 2017 23:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants