This PR adds a strategy sorting algorithm to be used for the segment
deletion dialog. It assumes that you have a list of existing strategies
and a list of change request strategies. Based on the content of these
two lists, it will create one unified list sorted after a number of
criteria (as listed in the test).
# Discussion point:
This impl does the sorting on the front end, but could we do it on the
back end? Instead of adding a new property to the segment data, could we
simply fold the change request strategies in with the existing segment
strategies and return it using the old property? If the only place we do
that is in this view, then that might be a good suggestion.
Response:
I'll leave this in the front end for now. The reason is that we can't add change request strategies to the existing `strategies` property of the API payload without it being a breaking change. The OpenAPI schema says that `id` is a required field on a strategy, and that field doesn't exist on strategies that have only been added in change requests, but not yet applied.
This PR updates the returned value about segments to also include the CR
title and to be one list item per strategy per change request. This
means that if the same strategy is used multiple times in multiple
change requests, they each get their own line (as has been discussed
with Nicolae).
Because of this, this pr removes a collection step in the query and
fixes some test cases.
The previous check would return `false` if the value was 0, causing a
bug where the usage data wouldn't be included.
This also adds tests to ensure that usage data for CR segments is
propagated correctly because that's where I first encountered the issue.
Before this fix, if the values were 0, the data would display like the
bottom element in the screenshot:
See [Linear issue
2-1627](https://linear.app/unleash/issue/2-1627/db-permissions-update-docs-and-migration-guides).
Since we now use functions, we need more permissions than just GRANT ALL
PERMISSIONS ON DATBASE. In addition we need to be allowed to create
functions in the schema that's actually being used. This PR takes the
easy way out and say that we need OWNER privileges on the database. That
guarantees that we can do all we do in our migration scripts.
### Discussion points
We might encounter some pushback on this, if so, we'll need to say that
we need
`GRANT ALL PRIVILEGES ON ALL FUNCTIONS IN <schema>` in addition to GRANT
ALL PRIVILEGES ON DATABASE, where <schema> is the schema selected in
their configuration.
We've been caught out here, we added a migration that used PG>=11
syntax, but our docs still state that we support v10, when it reached
EoL Nov 2022. This updates our docs to state v14 and newer, v14 came out
in 2021 and as such won't be EoL until 2026 according to [Postgres 5
year versioning scheme
support](https://www.postgresql.org/support/versioning/)
- Create 2 new events to replace the SCHEDULED_CHANGE_REQUEST_EXECUTED
event
- Handle the 3 events in slack-app and webhook addon definitions
3 events handled:
- CHANGE_REQUEST_SCHEDULED
- CHANGE_REQUEST_SCHEDULED_APPLICATION_SUCCESS
- CHANGE_REQUEST_SCHEDULED_APPLICATION_FAILURE
Closes #
[1-1555](https://linear.app/unleash/issue/1-1555/update-change-request-scheduled-and-scheduled-change-request-executed)
Note: SCHEDULED_CHANGE_REQUEST_EXECUTED will be removed in follow up PR
not to break current enterprise build
---------
Signed-off-by: andreas-unleash <andreas@getunleash.ai>
This PR addresses some cleanup related to removing the
useLastSeenRefactor flag:
* Added fallback last seen to the feature table last_seen_at column
* Remove foreign key on environment since we can not guarantee that we
will get valid data in this field
* Add environments to cleanup function
* Add test for cleanup environments
This PR changes the behavior of the API a little bit. Instead of
removing any strategies from `changeRequestStrategies` that are also
in `strategies`, we keep them in instead.
The reason for this is that the overview of where a segment is used is
incomplete if it shows only strategies but not CRs. Imagine this:
You want to delete a segment, but you're told it's only used in strategy
S.
So you go and remove it from strategy S, but then you're told it's
suddenly used in CRs A, B, and C. This is now a two-step operation
with a bad surprise. Instead, we could show you immediately that this
segment is used in strategy S and CRs A, B, and C.
Otherwise, we might accidentally display CR data to open source users.
But more importantly, it might keep them from being able to delete a
segment that's in use by a CR in their database that they can't touch.
So by checking that they're on an enterprise instance, we avoid this
potential blocker.
I've added the `includeChangeRequestUsageData` parameter as a boolean
now, but I'm open to other suggestions.
This PR handles the case where a single strategy is used in multiple
change requests. Instead of listing the strategy several times in the
output, we consolidate the entries and add a new `changeRequestIds`
property. This is a non-empty list that points to all the change
requests it is used in.
This is required for us to be able to link back to the change requests
from the UI overview.
This change is just a refactor, removing code that's no longer used. Instead of
checking just whether a segment is in use, we now extract the list of
strategies that use this segment. This is slightly more costly,
perhaps, but it will be necessary for the upcoming implementation.
This PR changes the payload of the strategiesBySegment endpoint when the
flag is active. In addition to returning just the strategies, the object
will also contain a new property, called `changeRequestStrategies`
containing the strategies that are used in change requests.
This PR does not update the schema. That can be done later when the
changes go into beta. This also allows us some time to iterate on the
payload without changing the public API.
## Discussion points:
Should `strategies` and `changeRequestStrategies` ever contain
duplicates? Take this scenario:
- Strategy S uses segment T.
- There is an open change request that updates the list of segments for
S to T and a new segment U.
- In this case, strategy S would show up both in `strategies` _and_ in
`changeRequestStrategies`.
We have two options:
1. Filter the list of change request strategies, so that they don't
contain any duplicates (this is currently how it's implemented)
2. Ignore the duplicates and just send both lists as is.
We're doing option 2 for now.
Removing a user from a project was impossible if you only had 1 owner.
It worked fine when having more than an owner. This should fix it and
we'll add tests later
In short the issue is that after our last seen improvements, we did not
update where we are getting last_seen field. It was still using features
table, which is not the source of last seen anymore.
