This PR updates the personal dashboard project endpoint to return owners
and roles. It also adds the impl for getting roles (via the access
store).
I'm filtering the roles for a project to only include project roles for
now, but we might wanna change this later.
Tests and UI update will follow.
This PR is part 1 of returning project owners and your project roles for
the personal dashboard single-project endpoint.
It moves the responsibility of adding owners and roles to the project to
the service from the controller and adds a new method to the project
owners read model to take care of it.
I'll add roles and tests in follow-up PRs.
This PR adds tests for the new admins property of the personal dashboard
API payload.
It checks that only user admins are added and that their image URL is
not an empty string. In doing this, also fixes an issue where the image
URL wouldn't be generated correctly.
## Discussion points
Some of the test feels like it might be better testing on a deeper level
(i.e. the account store). However, from an initial glance, I think that
would require more setup and work, so I'm leaving it in the dashboard
test for now as that's where it's ultimately useful. But we can discuss
if we should move it.
Adds Unleash admins to the personal dashboard payload.
Uses the access store (and a new method) to fetch admins and maps it to
a new `MinimalUser` type. We already have a `User` class, but it
contains a lot of information we don't care about here, such as `isAPI`,
SCIM data etc.
In the UI, admins will be shown to users who are not part of any
projects. This is the default state for new viewer users, and can also
happen for editors if you archive the default project, for instance.
Tests in a follow-up PR
This PR adds all user-type owners of projects that you have access to to
the personal dashboard payload. It adds the new `projectOwners` property
regardless of whether you have access to any projects or not because it
required less code and fewer conditionals, but we can do the filtering
if we want to.
To add the owners, it uses the private project checker to get accessible
projects before passing those to the project owner read model, which has
a new method to fetch user owners for projects.
This change removes the flag used to anonymize project owners on the
way out. It was an issue in demo when we'd forgotten to configure the
email encryption. However, this issue has been resolved and we can
remove this check now.
This PR adds project owner information to the personal dashboard's
project payload.
To do so, it uses the existing project owners read model.
I've had to make a few changes to the project owners read model to
accomodate this:
- make the input type to `addOwners` more lenient. We only need the
project ids, so we can make that the only required property
- fall back to using email as the name if the user has no name or
username (such as if you sign up with the demo auth)
This PR adds some of the necessary project data to the personal
dashboard API: project names and ids, and the roles that the user has in
each of these projects.
I have not added project owners yet, as that would increase the
complexity a bit and I'd rather focus on that in a separate PR.
I have also not added projects you are part of through a group, though I
have added a placeholder test for that. I will address this in a
follow-up.
https://linear.app/unleash/issue/2-2664/implement-event-tooltips
Implements event tooltips in the new event timeline.
This leverages our current `feature-event-formatter-md` to provide both
a label and a summary of the event. Whenever our new `eventTimeline`
flag is enabled, we enrich our events in our event search endpoint with
this information. We've discussed different options here and reached the
conclusion that this is the best path forward for now. This way we are
being consistent, DRY, relatively performant and it also gives us a
happy path forward if we decide to scope in the event log revamp, since
this data will already be present there.
We also added a new `label` property to each of our event types
currently in our event formatter. This way we can have a concise,
human-readable name for each event type, instead of exposing the
internal event type string.
~~We also fixed the way the event formatter handled bold text (as in,
**bold**). Before, it was wrapping them in *single asterisks*, but now
we're using **double asterisks**. We also abstracted this away into a
helper method aptly named `bold`. Of course, this change meant that a
bunch of snapshots and tests needed to be updated.~~
~~This new `bold` method also makes it super easy to revert this
decision if we choose to, for any reason. However I believe we should
stick with markdown formatting, since it is the most commonly supported
formatting syntax, so I see this as an important fix. It's also in the
name of the formatter (`md`). I also believe bold was the original
intent. If we want italic formatting we should implement it separately
at a later point.~~
Edit: It was _bold_ of me to assume this would work out of the box on
Slack. It does when you manually try it on the app, but not when using
the Slack client. See: https://github.com/Unleash/unleash/pull/8222
We now have customers that exceed INT capacity, so we need to change
this to BIGINT in client_metrics_env_variants_daily as well.
Even heavy users only have about 10000 rows here, so should be a quick
enough operation.
## Background
In #6380 we fixed a privilege escalation bug that allowed members of a
project that had permission to add users to the project with roles that
had a higher permission set than themselves. The PR linked essentially
constricts you only be able to assign users to roles that you possess
yourself if you are not an Admin or Project owner.
This fix broke expectations for another customer who needed to have a
project owner without the DELETE_PROJECT permission. The fix above made
it so that their custom project owner role only was able to assign users
to the project with the role that they posessed.
## Fix
Instead of looking directly at which role the role granter has, this PR
addresses the issue by making the assessment based on the permission
sets of the user and the roles to be granted. If the granter has all the
permissions of the role being granted, the granter is permitted to
assign the role.
## Other considerations
The endpoint to get roles was changed in this PR. It previously only
retrieved the roles that the user had in the project. This no-longer
makes sense because the user should be able to see other project roles
than the one they themselves hold when assigning users to the project.
The drawback of returning all project roles is that there may be a
project role in the list that the user does not have access to assign,
because they do not hold all the permissions required of the role. This
was discussed internally and we decided that it's an acceptable
trade-off for now because the complexities of returning a role list
based on comparing permissions set is not trivial. We would have to
retrieve each project role with permissions from the database, and run
the same in-memory check against the users permission to determine which
roles to return from this endpoint. Instead we opted for returning all
project roles and display an error if you try to assign a role that you
do not have access to.
## Follow up
When this is merged, there's no longer need for the frontend logic that
filters out roles in the role assignment form. I deliberately left this
out of the scope for this PR because I couldn't wrap my head around
everything that was going on there and I thought it was better to pair
on this with @chriswk or @nunogois in order to make sure we get this
right as the logic for this filtering seemed quite complex and was
touching multiple different components.
---------
Co-authored-by: Fredrik Strand Oseberg <fredrikstrandoseberg@Fredrik-sin-MacBook-Pro.local>
This appears to have been an oversight in the original implementation
of this endpoint. This seems to be the primary point of this
permission. Additionally, the docs mention that this permission should
allow you to do just that.
Note: I've not added any tests for this, because we don't typically add
tests for it. If we have an example to follow, I'd be very happy to add
it, though
https://linear.app/unleash/issue/2-2592/updateimprove-a-segment-via-api-call
Related to https://github.com/Unleash/unleash/issues/7987
This does not make the endpoint necessarily better - It's still a PUT
that acts as a PUT in some ways (expects specific required fields to be
present, resets the project to `null` if it's not included in the body)
and a PATCH in others (ignores most fields if they're not included in
the body). We need to have a more in-depth discussion about developing
long-term strategies for our API and respective OpenAPI spec.
However this at least includes the proper schema for the request body,
which is slightly better than before.
