Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update permissions when when entities are updated #2221

Merged
merged 20 commits into from
Jun 5, 2018

Conversation

KlapTrap
Copy link
Contributor

CF Endpoint:

  • Connect - fetch permissions for newly connected endpoint
  • Disconnect - clear store of permissions
  • Unregister - clear store of permissions

Org:

  • Deletion - clear store of permissions

Space:

  • Deletion - clear store of permissions

We don't fetch permissions on creation as the current user won't have any permissions at that point.

KlapTrap and others added 9 commits May 22, 2018 14:52
* v2-master: (55 commits)
  Add code coverage config file
  Tweak to cover radio button in single select columns (map existing route table)
  Remove todo from firehose permission - tested for admin/non admin - admin read-only needs to be applied later on - doppler.firehose scope wasn't required to show the firehose
  Add user info to the response when connecting an endpoint
  Fix tests
  Fix typing issue, highlighted by unit tests
  Ensure we handle cases where the permissions haven't been fetched yet
  Revert "Add `has space role in any space in any cf org`"
  Real fix
  Add `has space role in any space in any cf org`
  Three changes - Ensure there's no dups in org spaceGuids list - Fetch list of spaceGuids from org and not space object - Wire in new all spaces
  Fix selection following merge - Fix issue in master where select column/multi actions not showing - Fix manage users selection columns
  Fix return to select users step (missing user rows) - The ngIf on selected users which determines if the step is visible flipped - This caused the table to disconnect, which was never connected again - Not sure how the step was even still visible
  Fix redirect for states without query params
  Re-add selected users list to modify roles step, fixed title when start with multi users - This was brought up in demo - Needed in some cases for warm fuzzies (user has preselected users list, either single or multiple) - Only case where we might not show it is when the select users step is shown, however for consistency felt it should be the same for all
  Fix stepper blocked indicator (failed to populate cf/users when nav direct and single user)
  Update app-service-binding-card.component.spec.ts
  Ensure query params are retained through auth guard/login redirect
  Increase jasmine timeout to 10 seconds
  Do proper check when checking all spaces
  ...
@cfdreddbot
Copy link

Hey KlapTrap!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@codecov
Copy link

codecov bot commented May 24, 2018

Codecov Report

Merging #2221 into v2-master will increase coverage by 6.2%.
The diff coverage is 47.21%.

@@             Coverage Diff              @@
##           v2-master    #2221     +/-   ##
============================================
+ Coverage      65.51%   71.71%   +6.2%     
============================================
  Files            571      572      +1     
  Lines          16782    23588   +6806     
  Branches        2151     5312   +3161     
============================================
+ Hits           10994    16916   +5922     
- Misses          5788     6672    +884

Copy link
Contributor

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

Might jump the gun with the review. Couple of comments and one issue

  • register endpoint and connect as admin and the 'admin' permission isn't updated

if (!state.cf[endpointGuid].organizations[guid]) {
return state;
}
// const spaceIds = state.cf[endpointGuid].organizations[guid].spaceIds;
Copy link
Contributor

Choose a reason for hiding this comment

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

comment

...state.cf[endpointGuid].organizations,
[orgId]: {
...state.cf[endpointGuid].organizations[orgId],
spaceIds: state.cf[endpointGuid].organizations[orgId].spaceGuids.filter(id => id !== spaceGuid)
Copy link
Contributor

Choose a reason for hiding this comment

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

Missed comment from review... spaceIds --> spaceGuids

@richard-cox richard-cox added needs attention This PR needs attention and removed in review labels Jun 1, 2018
@richard-cox richard-cox added in review needs attention This PR needs attention and removed needs attention This PR needs attention in review labels Jun 4, 2018
@KlapTrap KlapTrap added comments-addressed and removed needs attention This PR needs attention labels Jun 4, 2018
@richard-cox richard-cox added the conflicts Merge conflicts on PR label Jun 5, 2018
* v2-master: (136 commits)
  Fix merge, height of recent updated service instances + gate-check target
  Fixes - app breadcrumb for market place --> app (could be fixed in master) - quotes in cli pages (show) and `f` -> `cf` in one place - margin between cancel and next in stepper - app chip list margins and alignment - remove a debug line
  Removed uneeded files
  Resolve merge conflict
  Fixs
  Updates for week ending 1st June
  Fix for permissions issue with node
  Three fixes - Bind app to existing service with binding params - Updated default sort order to always show latest first - Update name of sort type of app service instance table (creation date --> bind date)
  Fix App SSH
  Addressed feedback
  Fix compression issue
  Fixed unit tests
  Remove AIO base image because of similarity to BK Build
  Changing node version the LTE
  Update to latest LTS for angular 6 migration
  Fixed merge issues
  Bumped node version again
  Updated node version on travis.yml file
  Updated node version
  Fix e2e target
  ...
@richard-cox richard-cox removed the conflicts Merge conflicts on PR label Jun 5, 2018
@KlapTrap KlapTrap merged commit cf16300 into v2-master Jun 5, 2018
@KlapTrap KlapTrap deleted the permissions-new-endpoint branch June 5, 2018 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants