Adds a timestamp for each state we have time (and that isn't a state
downstream from the current state) in the CR timeline.
<img width="437" height="318" alt="image"
src="https://github.com/user-attachments/assets/a499e73f-c506-46a0-8d1a-7e4eb5ec4f7d"
/>
The timestamp respects the user's preferred locale and uses the `time`
element.
I've used the current name of the API payload's timestamps as it stands
on the enterprise main branch for now. This name is not set in any
schemas yet, so it is likely to change. Because it's not currently
exposed to any users, that will not cause any issues. Name suggestions
are welcome if you have them.
We only show timestamps for states up to and including the current
state. Timestamps for downstream states (such as "approved" if you're in
the "in review" state), will not be shown, even if they exist in the
payload. (There are some decisions to make on whether to include these
in the payload at all or not.)
There's no flags in this PR. They're not necessary If the API payload
doesn't contain the timestamp, we just don't render the timestamp, and
the timeline looks the way it always did:
<img width="447" height="399" alt="image"
src="https://github.com/user-attachments/assets/0062393a-190c-4099-bc16-29f9da82e7ea"
/>
## Bonus work
In the `ChangeRequestTimeline.tsx` file, I've made a few extra changes:
- `createTimelineItem` and `createScheduledTimelineItem` have become
normal React components (`TimelineItem` and `ScheduledTimelineItem`) and
are being called as such (in accordance with [React
recommendations](https://react.dev/reference/rules/react-calls-components-and-hooks#never-call-component-functions-directly)).
- I've updated the subtitles for schedules to also use the time element,
to improve HTML structure.
## Outstanding work
There's a few things that still need to be sorted out (primarily with
UX). Mainly about how we handle scheduled items, which already have time
information. For now, it looks like this:
<img width="426" height="394" alt="image"
src="https://github.com/user-attachments/assets/4bfc4ca2-c738-4954-9251-8d063143371e"
/>
<img width="700" height="246" alt="image"
src="https://github.com/user-attachments/assets/fe688b08-c5c8-40f8-a9d0-fe455e44665f"
/>
## About the changes
We've been using vitest for coverage and we use a github action to push
results to coveralls, so this should work just fine
Unlocks form-data to be able to fix
https://github.com/Unleash/unleash/security/dependabot/267
Also, removing coverage from PRs to avoid running tests twice
The console was complaining. I suspect it was because of the wrapping
fragment. So instead of doing everything within react, I switched to
using a standard case statement.
Also: because name is optional and not guaranteed to be unique, let's
use id for the key instead.
When deleting stale sessions, we sort them by createdAt. If both
sessions are created with the same createdAt, there's a chance we get a
different sort order and we end up with the wrong order:
https://github.com/Unleash/unleash/actions/runs/16438565746/job/46453700977
I think adding 10ms between inserts should be enough (1ms should do,
but this gives me more confidence and doesn't hurt that much)
---------
Co-authored-by: Thomas Heartman <thomas@getunleash.io>
https://linear.app/unleash/issue/2-3696/report-unknown-flags-when-sent-to-the-bulk-metrics-endpoint
Unifies metrics sifting logic across both metrics endpoints:
- `/metrics`
- `/metrics/bulk`
This PR improves consistency between the `/metrics` and `/metrics/bulk`
endpoints by introducing a shared `siftMetrics` method, now used within
`registerBulkMetrics`. Both endpoints already call this method at the
end of their respective logic flows, ensuring that metrics are sifted in
the same way regardless of the path taken.
While the primary goal was to enable reporting of unknown flags via the
`/metrics/bulk` endpoint, this change also improves bulk processing by
consistently dropping invalid or unknown flags before insertion, just
like in the regular `/metrics` endpoint.
## About the changes
Currently, if a PR is open and a push happens, the **e2e:frontend**
workflow will start running. If, shortly after a subsequent push on the
*same* PR happens, the workflow will start running again without
cancelling the previous (now obsolete) run. With these changes, the
first run would be cancelled, thus **saving compute resources** (see
below for quantity) that can be used to **speed up your overall CI/CD**,
without sacrificing functionality, since the second run will contain the
changes from the first push as well. 🌱
### Example
Here is an example of the behaviour described above: the commit
`b1b2e61` triggered
[this](https://github.com/Unleash/unleash/actions/runs/14493085673/)
workflow run, and shortly after the commit `9997fe1`, that happened on
top of the first commit, triggered
[this](https://github.com/Unleash/unleash/actions/runs/14493089012/)
workflow. Both workflows ran till the end, spending approximately 8 CPU
minutes each. With the proposed changes, the first run would be
cancelled, hence saving ~8 CPU minutes and clearing the queue for other
workflows. Note that this is an example of a single concurrent run; the
accumulated gain for all PRs would be higher, with a lower estimate at
**2 CPU hours** over the last few months.
The same holds for these workflow(s) as well: Dependency review, PR ->
Build Docs.
### Context
Hi,
We are a team of [researchers](https://www.ifi.uzh.ch/en/zest.html) from
University of Zurich and we are currently working on energy
optimizations in GitHub Actions workflows.
Kindly let us know (here or in the email below) if you would like more
details, if you want to reject the proposed changes for other reasons,
or if you have any question whatsoever.
Best regards,
[Konstantinos
Kitsios](https://www.ifi.uzh.ch/en/zest/team/konstantinos_kitsios.html)
konstantinos.kitsios@uzh.ch
Fixes a bug in the instance store where insert and bulkUpsert would
overwrite existing properties if there was a row there already. Now
it'll ignore any properties that are undefined.
The implementation is lifted directly from
`src/lib/db/client-applications-store.ts` (line 107 atm).
Additionally, I've renamed the `insert` method to `upsert` to make it
clearer what it does (and because we already have `bulkUpsert`). The
method seems to only be used in tests, anyway. I do not anticipate any
changes to be required in enterprise (I've checked).
## Discussion points:
This implementation uses `delete` to remove properties from the object.
Why didn't I do it some other way? Two main reasons:
1. We've had this implementation for 4 years in the client applications
store. If there were serious issues with it, we'd probably know by know.
(Probably.)
2. The only way I can think of without deleting, would be to use
`Object.fromEntries` and `Object.toEntries` and either map or reduce.
That'll double the amount of property iterations we'll need to do.
So naively, this strikes me as being more efficient. If you know better
solutions, I will of course be happy to take them. If not, I'd like to
leave this as is and then change it if we see that it's causing issues.
Fixes a bug where `registerInstance` and
`register{Frontend|Backend}Client` would overwrite each other's data in
the instance service, leading to the bulk update being made with partial
data, often missing SDK version. There's a different issue in the actual
store that causes sdk version and type to be overwritten when it's
updated (because we don't use `setLastSeen` anymore), but I'll handle
that in a different PR.
This PR adds tests for the changes I've made. Additionally, I've made
these semi-related bonus changes:
- In registerInstance, don't expect a partial `IClientApp`. We used to
validate that it was actual a metrics object instead. Instead, update
the signature to expect the actual properties we need from the cilent
metrics schema and set a default for instanceId the way Joi did.
- In `metrics.ts`, use the `ClientMetricsSchema` type in the function
signature, so that the request body is correctly typed in the function
(instead of being `any`).
- Delete two unused properties from the`createApplicationSchema`. They
would get ignored and were never used as far as I can tell. (`appName`
is taken from the URL, and applications don't store `sdkVersion`
information).
- Add `sdkVersion` to `IClientApp` because it's used in instance
service.
I've been very confused about all the weird type shenanigans we do in
the instance service (expecting `IClientApp`, then validating with a
different Joi schema etc). I think this makes it a little bit better and
updates the bits I'm touching, but I'm happy to take input if you
disagree.
