### What
Adds `createdByUserId` to all events exposed by unleash. In addition
this PR updates all tests and usages of the methods in this codebase to
include the required number.
Sort array items before running compare. Feature flag certain properties
of strategy that were previously not present in the /api/admin/features
endpoint.
## About the changes
This small improvement aims to help developers when instantiating
services. They need to be constructed without injecting services or
stores created elsewhere so they can be bound to the same transactional
scope.
This suggests that you need to create the services and stores on your
own
## About the changes
This splits the interfaces for import and export, especially because the
import functionality has to be replaced in enterprise repo.
This is a breaking change because of the service renames, but I'll have
the PR for the other repository ready so we reduce the time to fix. I
intentionally avoided doing it backward compatible because of time.
## About the changes
This transactional implementation decorates a service with a
transactional method that removes the need to start transactions in the
method using the service.
This is a gradual rollout with a feature toggle, just because
transactions are not easy.
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.
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>
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.
This PR fixes an issue where trying to use an admin token to import
features via the API resulted in a 500 (due to missing properties).
The solution is to catch when the user is using and admin token in the
controller, throw a 400, and tell them to use personal access tokens or
service accounts.
The PR includes a new test file for this specific use case. We don't
really test these cases many other places, so it seemed the logical
choice.
This PR attempts to improve the error handling introduced in #3607.
## About the changes
## **tl;dr:**
- Make `UnleashError` constructor protected
- Make all custom errors inherit from `UnleashError`.
- Add tests to ensure that all special error cases include their
relevant data
- Remove `PasswordMismatchError` and `BadRequestError`. These don't
exist.
- Add a few new error types: `ContentTypeError`, `NotImplementedError`,
`UnauthorizedError`
- Remove the `...rest` parameter from error constructor
- Add an unexported `GenericUnleashError` class
- Move OpenAPI conversion function to `BadDataError` clas
- Remove explicit `Error.captureStackTrace`. This is done automatically.
- Extract `getPropFromString` function and add tests
### **In a more verbose fashion**
The main thing is that all our internal errors now inherit
from`UnleashError`. This allows us to simplify the `UnleashError`
constructor and error handling in general while still giving us the
extra benefits we added to that class. However, it _does_ also mean that
I've had to update **all** existing error classes.
The constructor for `UnleashError` is now protected and all places that
called that constructor directly have been updated. Because the base
error isn't available anymore, I've added three new errors to cover use
cases that we didn't already have covered: `NotImplementedError`,
`UnauthorizedError`, `ContentTypeError`. This is to stay consistent in
how we report errors to the user.
There is also an internal class, `GenericUnleashError` that inherits
from the base error. This class is only used in conversions for cases
where we don't know what the error is. It is not exported.
In making all the errors inherit, I've also removed the `...rest`
parameter from the `UnleashError` constructor. We don't need this
anymore.
Following on from the fixes with missing properties in #3638, I have
added tests for all errors that contain extra data.
Some of the error names that were originally used when creating the list
don't exist in the backend. `BadRequestError` and
`PasswordMismatchError` have been removed.
The `BadDataError` class now contains the conversion code for OpenAPI
validation errors. In doing so, I extracted and tested the
`getPropFromString` function.
### Main files
Due to the nature of the changes, there's a lot of files to look at. So
to make it easier to know where to turn your attention:
The changes in `api-error.ts` contain the main changes: protected
constructor, removal of OpenAPI conversion (moved into `BadDataError`.
`api-error.test.ts` contains tests to make sure that errors work as
expected.
Aside from `get-prop-from-string.ts` and the tests, everything else is
just the required updates to go through with the changes.
## Discussion points
I've gone for inheritance of the Error type over composition. This is in
large part because throwing actual Error instances instead of just
objects is preferable (because they collect stack traces, for instance).
However, it's quite possible that we could solve the same thing in a
more elegant fashion using composition.
## For later / suggestions for further improvements
The `api-error` files still contain a lot of code. I think it might be
beneficial to break each Error into a separate folder that includes the
error, its tests, and its schema (if required). It would help decouple
it a bit.
We don't currently expose the schema anywhere, so it's not available in
the openapi spec. We should look at exposing it too.
Finally, it would be good to go through each individual error message
and update each one to be as helpful as possible.
