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

Fix org and space role association #1119

Merged
merged 2 commits into from
Jun 15, 2017
Merged

Fix org and space role association #1119

merged 2 commits into from
Jun 15, 2017

Conversation

jcscottiii
Copy link
Contributor

@jcscottiii jcscottiii commented Jun 14, 2017

First commit: Fix functional test server to check for correct roles

This modification makes sure that all requests to do
association/dissociation of roles use the right api key for the role.

This will break tests because currently the code doesn't use the right
api key for role modification but will be fixed in following commits.

Other commits: fixing the api calls and tests

James C. Scott added 2 commits June 14, 2017 11:44
This modification makes sure that all requests to do
association/dissociation of roles use the right api key for the role.

This will break tests because currently the code doesn't use the right
api key for role modification
The data to how to map a role to various things was spread out.
This PR moves them all into one object.

Also, with moving into one object, we have the apiKey field.
This field is now used
@@ -164,6 +150,7 @@ export class UserStore extends BaseStore {
const orgPermissionsReq = cfApi.deleteOrgUserPermissions(
action.userGuid,
action.orgGuid,
'users',
Copy link
Contributor

Choose a reason for hiding this comment

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

@jcscottiii This might be a duplicated line

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's meant to be duplicated but let me re-work it so it's more clear as to why it's like that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good overall.

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.

Looks good with exception to the one comment

@rememberlenny rememberlenny merged commit 0b29739 into master Jun 15, 2017
@rememberlenny rememberlenny deleted the js-fix-role-api branch June 15, 2017 14:25
@rememberlenny
Copy link
Contributor

@jcscottiii Merged because it does what it is supposed to do.

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