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
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.
