Sort array items before running compare. Feature flag certain properties
of strategy that were previously not present in the /api/admin/features
endpoint.
This PR is the first step in separating the client and admin stores.
Currently our feature toggle services uses the client store to serve
multiple purposes.
Admin API uses the feature toggle service to serve both the feature
toggle list and playground features, while the client API uses the
feature toggle service to serve client features. The admin API can
change often and have very different requirements than the client API,
which changes infrequently and generally keeps the same stable structure
for long periods of time. This architecture is error prone, because when
you need to make changes to the admin API, you can very easily affect
the client API.
I aim to put up a stone wall between the two APIs. Complete separation
between the two APIs, at the cost of some duplication.
In this PR I have created a feature oriented architecture for client
features and disconnected the client API from the feature toggle
service. It now goes through it's own service to it's own store. For
feature toggle service I have duplicated and replaced the functionality
that serves /api/admin/features, I have kept a lot of the ugliness in
the code and haven't removed anything in order to avoid breaking
changes.
Next steps:
* Move playground to admin API
* Remove client-feature-toggle-store from feature-toggle-service
https://linear.app/unleash/issue/2-1403/consider-refactoring-the-way-tags-are-fetched-for-the-events
This adds 2 methods to `EventService`:
- `storeEvent`;
- `storeEvents`;
This allows us to run event-specific logic inside these methods. In the
case of this PR, this means fetching the feature tags in case the event
contains a `featureName` and there are no tags specified in the event.
This prevents us from having to remember to fetch the tags in order to
store feature-related events except for very specific cases, like the
deletion of a feature - You can't fetch tags for a feature that no
longer exists, so in that case we need to pre-fetch the tags before
deleting the feature.
This also allows us to do any event-specific post-processing to the
event before reaching the DB layer.
In general I think it's also nicer that we reference the event service
instead of the event store directly.
There's a lot of changes and a lot of files touched, but most of it is
boilerplate to inject the `eventService` where needed instead of using
the `eventStore` directly.
Hopefully this will be a better approach than
https://github.com/Unleash/unleash/pull/4729
---------
Co-authored-by: Gastón Fournier <gaston@getunleash.io>
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->
Fixes an issue where project role deletion validation didn't validate
against project roles being connected to groups
## About the changes
Implements custom root roles, encompassing a lot of different areas of
the project, and slightly refactoring the current roles logic. It
includes quite a clean up.
This feature itself is behind a flag: `customRootRoles`
This feature covers root roles in:
- Users;
- Service Accounts;
- Groups;
Apologies in advance. I may have gotten a bit carried away 🙈
### Roles
We now have a new admin tab called "Roles" where we can see all root
roles and manage custom ones. We are not allowed to edit or remove
*predefined* roles.
![image](https://github.com/Unleash/unleash/assets/14320932/1ad8695c-8c3f-440d-ac32-39746720d588)
This meant slightly pushing away the existing roles to `project-roles`
instead. One idea we want to explore in the future is to unify both
types of roles in the UI instead of having 2 separate tabs. This
includes modernizing project roles to fit more into our current design
and decisions.
Hovering the permissions cell expands detailed information about the
role:
![image](https://github.com/Unleash/unleash/assets/14320932/81c4aae7-8b4d-4cb4-92d1-8f1bc3ef1f2a)
### Create and edit role
Here's how the role form looks like (create / edit):
![image](https://github.com/Unleash/unleash/assets/14320932/85baec29-bb10-48c5-a207-b3e9a8de838a)
Here I categorized permissions so it's easier to visualize and manage
from a UX perspective.
I'm using the same endpoint as before. I tried to unify the logic and
get rid of the `projectRole` specific hooks. What distinguishes custom
root roles from custom project roles is the extra `root-custom` type we
see on the payload. By default we assume `custom` (custom project role)
instead, which should help in terms of backwards compatibility.
### Delete role
When we delete a custom role we try to help the end user make an
informed decision by listing all the entities which currently use this
custom root role:
![image](https://github.com/Unleash/unleash/assets/14320932/352ed529-76be-47a8-88da-5e924fb191d4)
~~As mentioned in the screenshot, when deleting a custom role, we demote
all entities associated with it to the predefined `Viewer` role.~~
**EDIT**: Apparently we currently block this from the API
(access-service deleteRole) with a message:
![image](https://github.com/Unleash/unleash/assets/14320932/82a8e50f-8dc5-4c18-a2ba-54e2ae91b91c)
What should the correct behavior be?
### Role selector
I added a new easy-to-use role selector component that is present in:
- Users
![image](https://github.com/Unleash/unleash/assets/14320932/76953139-7fb6-437e-b3fa-ace1d9187674)
- Service Accounts
![image](https://github.com/Unleash/unleash/assets/14320932/2b80bd55-9abb-4883-b715-15650ae752ea)
- Groups
![image](https://github.com/Unleash/unleash/assets/14320932/ab438f7c-2245-4779-b157-2da1689fe402)
### Role description
I also added a new role description component that you can see below the
dropdown in the selector component, but it's also used to better
describe each role in the respective tables:
![image](https://github.com/Unleash/unleash/assets/14320932/a3eecac1-2a34-4500-a68c-e3f62ebfa782)
I'm not listing all the permissions of predefined roles. Those simply
show the description in the tooltip:
![image](https://github.com/Unleash/unleash/assets/14320932/7e5b2948-45f0-4472-8311-bf533409ba6c)
### Role badge
Groups is a bit different, since it uses a list of cards, so I added yet
another component - Role badge:
![image](https://github.com/Unleash/unleash/assets/14320932/1d62c3db-072a-4c97-b86f-1d8ebdd3523e)
I'm using this same component on the profile tab:
![image](https://github.com/Unleash/unleash/assets/14320932/214272db-a828-444e-8846-4f39b9456bc6)
## Discussion points
- Are we being defensive enough with the use of the flag? Should we
cover more?
- Are we breaking backwards compatibility in any way?
- What should we do when removing a role? Block or demote?
- Maybe some existing permission-related issues will surface with this
change: Are we being specific enough with our permissions? A lot of
places are simply checking for `ADMIN`;
- We may want to get rid of the API roles coupling we have with the
users and SAs and instead use the new hooks (e.g. `useRoles`)
explicitly;
- We should update the docs;
- Maybe we could allow the user to add a custom role directly from the
role selector component;
---------
Co-authored-by: Gastón Fournier <gaston@getunleash.io>