Fixes an issue where SSO group sync would delete a syncable group that a
user was manually added to
## Discussion points
Is this the longterm fix for this? Or would we want another column in
the mapping table for future-proofing this?
https://linear.app/unleash/issue/2-1253/add-support-for-more-events-in-the-slack-app-integration
Adds support for a lot more events in our integrations. Here is how the
full list looks like:
- ADDON_CONFIG_CREATED
- ADDON_CONFIG_DELETED
- ADDON_CONFIG_UPDATED
- API_TOKEN_CREATED
- API_TOKEN_DELETED
- CHANGE_ADDED
- CHANGE_DISCARDED
- CHANGE_EDITED
- CHANGE_REQUEST_APPLIED
- CHANGE_REQUEST_APPROVAL_ADDED
- CHANGE_REQUEST_APPROVED
- CHANGE_REQUEST_CANCELLED
- CHANGE_REQUEST_CREATED
- CHANGE_REQUEST_DISCARDED
- CHANGE_REQUEST_REJECTED
- CHANGE_REQUEST_SENT_TO_REVIEW
- CONTEXT_FIELD_CREATED
- CONTEXT_FIELD_DELETED
- CONTEXT_FIELD_UPDATED
- FEATURE_ARCHIVED
- FEATURE_CREATED
- FEATURE_DELETED
- FEATURE_ENVIRONMENT_DISABLED
- FEATURE_ENVIRONMENT_ENABLED
- FEATURE_ENVIRONMENT_VARIANTS_UPDATED
- FEATURE_METADATA_UPDATED
- FEATURE_POTENTIALLY_STALE_ON
- FEATURE_PROJECT_CHANGE
- FEATURE_REVIVED
- FEATURE_STALE_OFF
- FEATURE_STALE_ON
- FEATURE_STRATEGY_ADD
- FEATURE_STRATEGY_REMOVE
- FEATURE_STRATEGY_UPDATE
- FEATURE_TAGGED
- FEATURE_UNTAGGED
- GROUP_CREATED
- GROUP_DELETED
- GROUP_UPDATED
- PROJECT_CREATED
- PROJECT_DELETED
- SEGMENT_CREATED
- SEGMENT_DELETED
- SEGMENT_UPDATED
- SERVICE_ACCOUNT_CREATED
- SERVICE_ACCOUNT_DELETED
- SERVICE_ACCOUNT_UPDATED
- USER_CREATED
- USER_DELETED
- USER_UPDATED
I added the events that I thought were relevant based on my own
discretion. Know of any event we should add? Let me know and I'll add it
🙂
For now I only added these events to the new Slack App integration, but
we can add them to the other integrations as well since they are now
supported.
The event formatter was refactored and changed quite a bit in order to
make it easier to maintain and add new events in the future. As a
result, events are now posted with different text. Do we consider this a
breaking change? If so, I can keep the old event formatter around,
create a new one and only use it for the new Slack App integration.
I noticed we don't have good 404 behaviors in the UI for things that are
deleted in the meantime, that's why I avoided some links to specific
resources (like feature strategies, integration configurations, etc),
but we could add them later if we improve this.
This PR also tries to add some consistency to the the way we log events.
This commit changes our linter/formatter to biome (https://biomejs.dev/)
Causing our prehook to run almost instantly, and our "yarn lint" task to
run in sub 100ms.
Some trade-offs:
* Biome isn't quite as well established as ESLint
* Are we ready to install a different vscode plugin (the biome plugin)
instead of the prettier plugin
The configuration set for biome also has a set of recommended rules,
this is turned on by default, in order to get to something that was
mergeable I have turned off a couple the rules we seemed to violate the
most, that we also explicitly told eslint to ignore.
## About the changes
This fixes a bunch of openHandles from our tests
I've used this script to find out the ones that leave them:
`find src -name "*.test.ts" -printf "%f\n" | xargs -i sh -c "echo =====
{} && yarn test {}"`
If there's an issue, the script will halt and the last filename will be
the one that has to be fixed.
Each commit fixes one problem so it's easy to review
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>
We love all open-source Unleash users. in 2022 we built the [segment
capability](https://docs.getunleash.io/reference/segments) (v4.13) as an
enterprise feature, simplify life for our customers.
Now it is time to contribute it to the world 🌏
---------
Co-authored-by: Thomas Heartman <thomas@getunleash.io>
## About the changes
This enables us to use names instead of permission ids across all our
APIs at the computational cost of searching for the ids in the DB but
improving the API user experience
## Open topics
We're using methods that are test-only and circumvent our business
logic. This makes our test to rely on assumptions that are not always
true because these assumptions are not validated frequently.
i.e. We are expecting that after removing a permission it's no longer
there, but to test this, the permission has to be there before:
78273e4ff3/src/test/e2e/services/access-service.e2e.test.ts (L367-L375)
But it seems that's not the case.
We'll look into improving this later.
This test enforces that descriptions and examples are cleared if they
are not present in the feature naming data.
This behavior is already present, but it hasn't been encoded in any
tests before.
This PR updates the back-end handling of feature naming patterns to add
implicit leading `^`s and trailing `$`s to the regexes when comparing
them.
It also adds tests for the new behavior, both for new flag names and for
examples.
## Discussion points
Regarding stripping incoming ^ and $: We don't actually need to strip
incoming `^`s and `$`s: it appears that `^^^^^x$$$$$` is just as valid
as `^x$`. As such, we can leave that in. However, if we think it's
better to strip, we can do that too.
Second, I'm considering moving the flag naming validation into a
dedicated module to encapsulate everything a little better. Not sure if
this is the time or where it would live, but open to hearing
suggestions.
This PR adds feature name pattern validation to the import validation
step. When errors occur, they are rendered with all the offending
features, the pattern to match, plus the pattern's description and
example if available.
To achieve this I've added an extra method to the feature toggle service
that checks feature names without throwing errors (because catching `n`
async errors in a loop became tricky and hard to grasp). This method is
also reused in the existing feature name validation method and handles
the feature enabled chcek.
In doing so, I've also added tests to check that the pattern is applied.
Does what it says on the tin, should help with cleaning up
https://github.com/Unleash/unleash/pull/4512 and respective schema
changes.
---------
Co-authored-by: Gastón Fournier <gaston@getunleash.io>
Adds a first iteration of feature flag naming patterns. Currently behind a flag.
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Co-authored-by: Thomas Heartman <thomas@getunleash.io>
Co-authored-by: andreas-unleash <andreas@getunleash.ai>
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
## About the changes
Returns Not Found on create and get project api tokens when given a
project id that doesn't exist
## Discussion points
- This is an extra lookup per execution of the endpoint
## About the changes
Returns either 400 or 404 when token isn't found or doesn't match single
project must be provided projectId criteria
<!-- Does it close an issue? Multiple? -->
Closes #
Linear 2-1003
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
Is projects.length > 1 a 400?
https://linear.app/unleash/issue/2-1128/change-the-api-to-support-adding-multiple-roles-to-a-usergroup-on-ahttps://linear.app/unleash/issue/2-1125/be-able-to-fetch-all-roles-for-a-user-in-a-projecthttps://linear.app/unleash/issue/2-1127/adapt-the-ui-to-be-able-to-do-a-multi-select-on-role-permissions-for
- Allows assigning project roles to groups with root roles
- Implements new methods that support assigning, editing, removing and
retrieving multiple project roles in project access, along with other
auxiliary methods
- Adds new events for updating and removing assigned roles
- Adapts `useProjectApi` to new methods that use new endpoints that
support multiple roles
- Adds the `multipleRoles` feature flag that controls the possibility of
selecting multiple roles on the UI
- Adapts `ProjectAccessAssign` to support multiple role, using the new
methods
- Adds a new `MultipleRoleSelect` component that allows you to select
multiple roles based on the `RoleSelect` component
- Adapts the `RoleCell` component to support either a single role or
multiple roles
- Updates the `access.spec.ts` Cypress e2e test to reflect our new logic
- Updates `access-service.e2e.test.ts` with tests covering the multiple
roles logic and covering some corner cases
- Updates `project-service.e2e.test.ts` to adapt to the new logic,
adding a test that covers adding access with `[roles], [groups],
[users]`
- Misc refactors and boy scouting
---------
Co-authored-by: David Leek <david@getunleash.io>
Co-authored-by: Mateusz Kwasniewski <kwasniewski.mateusz@gmail.com>
Co-authored-by: Nuno Góis <github@nunogois.com>
<!-- 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
This PR adds an e2e test to the OpenAPI tests that checks that all
openapi operations have both summaries and descriptions. It also fixes
the few schemas that were missing one or the other.
Enable strict schema validation by default. It can still be overridden
by explicitly setting it to false.
I've also fixed the validation errors that appeared when turning it on.
I've opted for the simplest route and changed the schemas to comply with
the tests.
This PR does **one** thing:
it changes the events for potentially stale to:
- Only being emitted when potentially stale gets turned on
- In doing so, it also simplifies the event that's getting emitted,
removing the `data` property.
- The event is also renamed to better match the existing
`feature-stale-on` and `...-off` events.
The addon listening was broken out into a separate PR (#4279)
## Old description
This change lets all addons listen for events when features get marked
or unmarked as potentially stale.
### Discussion
#### All addons?
Should this be available to all addons? I can't see a reason why it
shouldn't be available to all addons, but I might be missing
something.
**Update**: spoke to a couple people. Can see no reason why this isn't
okay.
#### Should it be behind a flag?
The feature is still behind a flag, but the event type is not. Should
we gate the event being available until we actually emit the event?
That would require some more code, but could yield less potential
confusion.
Open to hearing your thoughts.
This PR updates the feature type service by adding a new
`updateLifetime` method. This method handles the connection between the
API (#4256) and the store (#4252).
I've also added some new e2e tests to ensure that the API behaves as
expected.
<!-- 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! ❤️ -->
When reordering strategies for a feature environment:
- Adds stop when CR are enabled
- Emits an event
## 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. -->
<!-- Does it close an issue? Multiple? -->
Closes #
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
This PR adds updates the potentially stale status change events whenever
the potentially stale update function is run.
No events are emitted yet. While the emission is only a few lines of
code, I'd like to do that in a separate PR so that we can give it the
attention it deserves in the form of tests, etc.
This PR also moves the potentially stale update functionality from the
`update` method to only being done in the
`updatePotentiallyStaleFeatures` method. This keeps all functionality
related to marking `potentiallyStale` in one place.
The emission implementation was removed in
4fb7cbde03
## The update queries
While it would be possible to do the state updates in a single query
instead of three separate ones, wrangling this into knex proved to be
troublesome (and would also probably be harder to understand and reason
about). The current solution uses three smaller queries (one select, two
updates), as Jaanus suggested in a private slack thread.
This reverts commit 16e3799b9a.
<!-- 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. -->
<!-- Does it close an issue? Multiple? -->
Closes #
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
https://linear.app/unleash/issue/2-1232/implement-first-iteration-of-the-new-slack-app-addon
This PR implements the first iteration of the new Slack App addon.
Unlike the old Slack addon, this one uses a Slack App (bot) that is
installed to Slack workspaces in order to post messages. This uses
`@slack/web-api`, which internally uses the latest Slack API endpoints
like `postMessage`.
This is currently behind a flag: `slackAppAddon`.
The current flow is that the Unleash Slack App is installed from
whatever source:
- Unleash addons page;
- Direct link;
- https://unleash-slack-app.vercel.app/ (temporary URL);
- Slack App Directory (in the future);
- Etc;
After installed, we resolve the authorization to an `access_token` that
the user can paste into the Unleash Slack App addon configuration form.
https://github.com/Unleash/unleash/assets/14320932/6a6621b9-5b8a-4921-a279-30668be6d46c
Co-authored by: @daveleek
---------
Co-authored-by: David Leek <david@getunleash.io>
This PR lays most of the groundwork required for emitting events when
features are marked as potentially stale by Unleash. It does **not**
emit any events just yet. The summary is:
- periodically look for features that are potentially stale and mark
them (set to run every 10 seconds for now; can be changed)
- when features are updated, if the update data contains changes to the
feature's type or createdAt date, also update the potentially stale
status.
It is currently about 220 lines of tests and about 100 lines of
application code (primarily db migration and two new methods on the
IFeatureToggleStore interface).
The reason I wanted to put this into a single PR (instead of just the db
migration, then just the potentially stale marking, then the update
logic) is:
If users get the db migration first, but not the rest of the update
logic until the events are fired, then they could get a bunch of new
events for features that should have been marked as potentially stale
several days/weeks/months ago. That seemed undesirable to me, so I
decided to bunch those changes together. Of course, I'd be happy to
break it into smaller parts.
## Rules
A toggle will be marked as potentially stale iff:
- it is not already stale
- its createdAt date is older than its feature type's expected lifetime
would dictate
## Migration
The migration adds a new `potentially_stale` column to the features
table and sets this to true for any toggles that have exceeded their
expected lifetime and that have not already been marked as `stale`.
## Discussion
### The `currentTime` parameter of `markPotentiallyStaleFeatures`
The `markPotentiallyStaleFetaures` method takes an optional
`currentTime` parameter. This was added to make it easier to test (so
you can test "into the future"), but it's not used in the application.
We can rewrite the tests to instead update feature toggles manually, but
that wouldn't test the actual marking method. Happy to discuss.
## About the changes
Fix un-awaited promise on batch variant update - reduce function allowed
TS to skip Promise type.
---------
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! ❤️ -->
Adds description and summary to `favorite` endpoints
## 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. -->
<!-- Does it close an issue? Multiple? -->
Closes #
[1-1098](https://linear.app/unleash/issue/1-1098/openapi-features-favorites)
<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->
### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->
## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
In some of the places we used `NoAccessError` for permissions, other
places we used it for a more generic 403 error with a different
message. This refactoring splits the error type into two distinct
types instead to make the error messages more consistent.
This PR fixes an issue where events generated during a db transaction
would get published before the transaction was complete. This caused
errors in some of our services that expected the data to be stored
before the transaction had been commited. Refer to [linear issue
1-1049](https://linear.app/unleash/issue/1-1049/event-emitter-should-emit-events-after-db-transaction-is-commited-not)
for more info.
Fixes 1-1049.
## Changes
The most important change here is that the `eventStore` no longer emits
events when they happen (because that can be in the middle of a
transaction). Instead, events are stored with a new `announced` column.
The new event announcer service runs on a schedule (every second) and
publishes any new events that have not been published.
Parts of the code have largely been lifted from the
`client-application-store`, which uses a similar logic.
I have kept the emitting of the event within the event store because a
lot of other services listen to events from this store, so removing that
would require a large rewrite. It's something we could look into down
the line, but it seems like too much of a change to do right now.
## Discussion
### Terminology:
Published vs announced? We should settle on one or the other. Announced
is consistent with the client-application store, but published sounds
more fitting for events.
### Publishing and marking events as published
The current implementation fetches all events that haven't been marked
as announced, sets them as announced, and then emits them. It's possible
that Unleash would crash in the interim or something else might happen,
causing the events not to get published. Maybe it would make sense to
just fetch the events and only mark them as published after the
announcement? On the other hand, that might get us into other problems.
Any thoughts on this would be much appreciated.
### What
This PR adds documentation for our endpoints that are covered by our
"Events" tag. It also adds a type for all valid events, and then uses
this as valid values for type argument.
This PR updates endpoints and schemas for the API tokens tag.
As part of that, they also handle oneOf openapi validation errors and improve the console output for the enforcer tests.
Background (general): I don’t think it creates any value. I don’t think
people check the snapshot explicitly (especially now that it’s so large
that it gets hidden by default), and it only creates an extra chore that
you forget to do all the time (at least I do).
Background (why now?): I’ve been fighting a weird issue with the
snapshot test for the [API tokens OpenAPI
PR](https://github.com/Unleash/unleash/pull/4137). The tests run fine
locally, but fail in CI. The tests seem to be using an old version of
the snapshot for the test. However, when I check the snapshot in the
files tab, it’s got the new and correct values. I’ve spent probably more
than an hour trying to fix and debug this today but to no avail.
## What
This adds openapi documentation for the Auth tagged operations and
connected schemas.
## Discussion points
Our user schema seems to be exposing quite a bit of internal fields, I
flagged the isApi field as deprecated, I can imagine quite a few of
these fields also being deprecated to prepare for removal in next major
version, but I was unsure which ones were safe to do so with.
## Observation
We have some technical debt around the shape of the schema we're
claiming we're returning and what we actually are returning. I believe
@gastonfournier also observed this when we turned on validation for our
endpoints.
---------
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
<!-- 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. -->
Adds a UI that shows current status of version and feature usage
collection configuration, and a presence in the configuration menu +
menu bar.
Configuring these features is done by setting environment variables. The
version info collection is an existing feature that we're making more
visible, the feature usage collection feature is a new feature that has
it's own environment configuration but also depends on version info
collection being active to work.
When version collection is turned off and the experimental feature flag
for feature usage collection is turned off:
<img width="1269" alt="image"
src="https://github.com/Unleash/unleash/assets/707867/435a07da-d238-4b5b-a150-07e3bd6b816f">
When version collection is turned on and the experimental feature flag
is off:
<img width="1249" alt="image"
src="https://github.com/Unleash/unleash/assets/707867/8d1a76c5-99c9-4551-9a4f-86d477bbbf6f">
When the experimental feature flag is enabled, and version+telemetry is
turned off:
<img width="1239" alt="image"
src="https://github.com/Unleash/unleash/assets/707867/e0bc532b-be94-4076-bee1-faef9bc48a5b">
When version collection is turned on, the experimental feature flag is
enabled, and telemetry collection is turned off:
<img width="1234" alt="image"
src="https://github.com/Unleash/unleash/assets/707867/1bd190c1-08fe-4402-bde3-56f863a33289">
When version collection is turned on, the experimental feature flag is
enabled, and telemetry collection is turned on:
<img width="1229" alt="image"
src="https://github.com/Unleash/unleash/assets/707867/848912cd-30bd-43cf-9b81-c58a4cbad1e4">
When version collection is turned off, the experimental feature flag is
enabled, and telemetry collection is turned on:
<img width="1241" alt="image"
src="https://github.com/Unleash/unleash/assets/707867/d2b981f2-033f-4fae-a115-f93e0653729b">
---------
Co-authored-by: sighphyre <liquidwicked64@gmail.com>
Co-authored-by: Nuno Góis <github@nunogois.com>
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
### What
Added documentation for context fields and endpoints tagged with the
"Context" tag.
---------
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
This PR adds strategy titles as an optional bit of data added to client
features. It's only added when prompted.
## Discussion points:
### getPlaygroundFeatures
The optional `includeStrategyId` parameter has been replaced by a
`getPlaygroundFeatures` in the service (and in the underlying store).
The playground was the only place that used this specific include, so
instead of adding more and making the interface for that method more
complex, I created a new method that deals specifically with the
playground.
The underlying store still uses an `optionalIncludes` parameter,
however. I have a plan to make that interface more fluid, but I'd like
to propose that in a follow-up PR.
