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

UI: ACL Roles #5635

Merged
merged 20 commits into from
May 1, 2019
Merged

UI: ACL Roles #5635

merged 20 commits into from
May 1, 2019

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Apr 10, 2019

This PR adds basic CRUD for ACL Roles (which is very similar to Policies)

This involves:

  • A new ember triplet (Route/Controller/Template) for each new Route (index/edit/create), very similar to the previous policies triplet
  • A new ember-data triplet (Adapter/Serializer/Model), very similar to the previous policies triplet
  • A new Roles Repository for separation between Model layer and Controller/Route layer.
  • A new Form, Validator and Search filter implementation specific to Roles.
  • New tests for the Adapter url generation, serialization, Repository request/responses and higher level acceptance testing with the same coverage as Policies.

We initially looked at implementing this using the same approach as used for ACL Tokens > Policies (see #5521).

As the relationship here is a 3 level relationship (ACL Tokens > Roles > Policies), we found that whilst adding Tokens > Roles/Policies and Roles > Policies was relatively straightforwards, when it came to implementing a Token > Role > Policy (A modal form within another modal form within a form), it became more complicated. So we decided it was time to start moving things to components to make things easier to think about. This is a little earlier than we'd wanted to do this, and we have work still to do here so there is one area which is not quite how we'd like 🤷‍♂️

Main new things:

  1. Proxied EventSources are actually quite handy. We use these to almost not have to worry about asynchronous flow inside components. So we've added another 'type' of repository that returns these proxies without blocking (for routes we want them to block the UI). Eventually everything (routes and components) will be using blocking queries and therefore EventSources anyway, so this extra 'type' will probably go away once we are always using blocking queries and all areas support them.

  2. We added a couple of extra properties to these proxy objects, we chose close to closely follow observables and error. Both of these can now be used directly in templates or in computed properties.

  3. Luckily all EventSources used here right now are 'onces'. So they cleanup on their own (on emitting the first event). Down the line we'd probably look to reuse something similar to our 'listen' computed macro to cleanup if required, but also reuse the error functionality there. In the not too distant future we hope to add finer grained events there also so we can use blocking queries on the form pages to warn people about conflicts and possibly slightly different functionality for add and delete from lists instead of just 'change'

  4. We are re-using our searchables for the power select typeahead menus, which means all the search functionality from our general listing pages also works in the powerselect typeahead. This should mean in the future we could add our phrase-editor work (ui: Search improvements #5540) in here as well as upcoming filtering work.

  5. Additional selected property for easily making a specific tab in the tab-nav component as selected.

There are copious amounts of TODO comments in here, mainly because the work we are moving to now is all related to this and we hoped to PR this and keep working on the new features and possible tick off a few TODOs while we are there.

Oh lastly, we found that no matter what z-index you use, you'll always need a higher one 😂 , so we tried to bring a bit of sanity there. Rough rule for the moment is, if its less then 500 you are underneath the modal, greater than 500 means you are above the modal, and it follows a similar 100, 200, 300 approach to our colors etc in styles/base

P.S.

Lastly, all the mocks for the API aren't published yet, so the tests here won't pass (but they 'do on my machine(tm)' 😄 ). Once I'm more sure there are no more changes to be made to the mocks/API around the new ACL structure, I'll publish, which will be before the end of this series of PRs. If this is problematic, please let me know and I can publish a release of the mocks.

We've added all the mocks for this and for most of the upcoming things also, so tests should pass now

@johncowen johncowen added the theme/ui Anything related to the UI label Apr 10, 2019
@johncowen johncowen requested a review from a team April 10, 2019 13:29
@johncowen johncowen added this to the 1.5.0 milestone Apr 16, 2019
John Cowen added 18 commits April 29, 2019 11:31
1. ember-data adapter/serializer/model triplet for Roles
2. repository, form/validations and searching filter for Roles
3. Moves potentially, repeated, or soon to to repeated functionality
into a mixin (mainly for 'many policy' relationships)
4. A few styling tweaks for little edge cases around roles
5. Router additions, Route, Controller and templates for Roles
various tweaks and integrate Roles properly
1. Adds a 'close' event to CallableEventSources, which is upcoming
standard-ish
2. Adds closed and error props to EventSource Proxy objects so you can
access them in templates/computed
...and add special case for Proxies

Also adds slightly better testing
1. Addition of 'form' and 'selector' components (base components plus
specialized instances per type/noun)
2. New type of repositories that return eventsources without blocking
specifically for components
Potentially hoist this out better when we have to do this for
ServiceIdentities also
1. Move 'remove' actions to `actions` so we have `this`
2. Re-look at the dropdown listing filtering (so Roles you already have
don't show up in the dropdown list)
Temporarily replicate the 'data minimization' from policies awaiting on
HTTPAdapter to do something a little cleaner
1. Split things up more so they aren't as tightly coupled, forms are now
more independent, as are repos, still not completely decoupled but
better
2. Make certain fields on `forms` public that make sense i.e. name and
data

Theres a little fork for extra functionality here to only do it for our
new nouns as we don't want to alter too much functionality at once
Adding all these here is far from ideal, but I hear this is
going away in an ember upgrade
Initially we would have liked form-components to basically build themselves
if a form wasn't passed in. Right now we don't need this, and we found
that the approach we used produced an 'infinite rendering' bug but
_only_ during actual use (we've tried to write a test to replicate it
but it doesn't manifest during acceptance testing).

We've rethought how this works, but as we don't need to do it yet we've
reverted to retrieving forms on the base component
Adds support for ACL Roles and Service Identities CRUD, along with necessary changes to Tokens, and the CSS improvements required.

Also includes refinements/improvements for easier testing of deeply nested components.
@johncowen johncowen merged commit 0ff88f1 into ui-staging May 1, 2019
@johncowen johncowen deleted the feature/ui-roles branch May 1, 2019 18:09
johncowen added a commit that referenced this pull request May 1, 2019
Adds support for ACL Roles and Service Identities CRUD, along with necessary changes to Tokens, and the CSS improvements required.

Also includes refinements/improvements for easier testing of deeply nested components.

1. ember-data adapter/serializer/model triplet for Roles
2. repository, form/validations and searching filter for Roles
3. Moves potentially, repeated, or soon to to repeated functionality
into a mixin (mainly for 'many policy' relationships)
4. A few styling tweaks for little edge cases around roles
5. Router additions, Route, Controller and templates for Roles

Also see: 

* UI: ACL Roles cont. plus Service Identities (#5661 and #5720)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants