feat: (api/ui) Sort promotions by phase and name

[webstradev]

Closes #1930

This PR handles sorting of promotions in two places.

In the api call we simply sort on the Name which contains a prefix that is always the same for promotions you want to sort followed by a ULID. I have also adjusted the tests to test for this sorting instead of on creation time. I do this by simply creating for ULID and using them in the tests.

In te UI we have slightly more complex sorting logic and I'm not certain we want that in the API, so I have essentially made the same change in the sortPromotions function. Instead of sorting on create time we sort simply on name. I didin't use ULIDs here in the tests because it doesn't make sense to me to die the frontend to that implementation, so I just used lexicographically sortable strings in the ui tests as well.

[netlify]

✅ Deploy Preview for docs-kargo-akuity-io canceled.

Name	Link
🔨 Latest commit	a5801c4
🔍 Latest deploy log	https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/665f1226b1635f000926c349

[codecov]

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 45.91%. Comparing base (7150274) to head (a5801c4).

Files	Patch %	Lines
api/v1alpha1/promotion_helpers.go	90.90%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1949      +/-   ##
==========================================
+ Coverage   45.88%   45.91%   +0.02%     
==========================================
  Files         238      238              
  Lines       16551    16549       -2     
==========================================
+ Hits         7595     7599       +4     
+ Misses       8584     8578       -6     
  Partials      372      372              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[krancour]

In te UI we have slightly more complex sorting logic

Ah... I think when commenting on #1930, I failed to realize that both the API server and UI were sorting these.

I would prefer the sorting to be done only on the back end, but I will ask @rbreeze to also weigh in on this opinion in case there are UI-specific reasons I am unaware of for sorting any differently than what the API server will return.

[webstradev]

In te UI we have slightly more complex sorting logic

Ah... I think when commenting on #1930, I failed to realize that both the API server and UI were sorting these.

I would prefer the sorting to be done only on the back end, but I will ask @rbreeze to also weigh in on this opinion in case there are UI-specific reasons I am unaware of for sorting any differently than what the API server will return.

All good, let me know if you want me to move it completely from UI to the API. I'm pretty sure it should be easy enough, but just to confirm they are currently different.

Backend only sorts on creationTime
Frontend Sorts on a combination of phase and creation time

[krancour]

Frontend Sorts on a combination of phase and creation time

@rbreeze was this intended? Sorting by time should be adequate, right?

In case it's not clear... promos are always processed in the order they were created.

[webstradev]

Just for your clarity @krancour
This test (from current main) shows how we sort in the frontend currently:

const testPromotions = [
  running1,
  pending2,
  running3,
  pending4,
  succeeded5,
  failed6,
  unknown7,
  running8,
  pending9,
  running10,
  pending11,
  succeeded12,
  errored13,
  unknown14
];

const orderedPromotions = [
  running10,
  running8,
  running3,
  running1,
  pending11,
  pending9,
  pending4,
  pending2,
  unknown14,
  errored13,
  succeeded12,
  unknown7,
  failed6,
  succeeded5
];

[hiddeco]

The way I feel you want them sorted in the UI (and arguably the API), is a bit awkward.

For things which are non-terminal (Running and Pending), you want them to be from oldest -> newest. Because that's how the queue is going through them.

For things which are terminal however (Succeeded, Errored, etc.), you want them from newest -> oldest. Because that gives you the latest Promotion which has been processed first.

Non-terminal should then go before terminal, to ensure the user has a clear view of what's being worked on.

I already wrote something which handles this type of ordering (in Go) in #1919.

[webstradev]

So are we moving this completely from frontend to backend then and keeping all the sorting logic in the backend?

[hiddeco]

I think we would at least want it in the API, and for the frontend we have to await further instructions from @rbreeze.

[rbreeze]

I have no opposition to sorting lexicographically on the backend, and agree that we should rely on the backend to sort... with one caveat. The background for why the UI does sorting as it does can be found in this PR and the linked issue: #1314

To summarize, currently running promotions were often buried on the last page where they were difficult to find. Hence the phase creeping into the sorting logic. I don't think this PR will affect that logic.

[webstradev]

I have no opposition to sorting lexicographically on the backend, and agree that we should rely on the backend to sort... with one caveat. The background for why the UI does sorting as it does can be found in this PR and the linked issue: #1314

To summarize, currently running promotions were often buried on the last page where they were difficult to find. Hence the phase creeping into the sorting logic. I don't think this PR will affect that logic.

Well the PR as it is currently I think is mergeable, but the suggestion was to move the sorting logic completely to the backend, and removing sorting all together from the frontend (whilst keeping the same sorting logic as describe in #1314)

Happy to change it to that if everyone is in agreement.

[rbreeze]

@webstradev Got it, makes sense. I am okay with either way. IMO the frontend sorting logic to surface currently running promotions is more of a UI feature than a "sort". So it comes down to whether we think this surfacing of running promos should happen at the API level (and therefore change the behavior of the CLI). WDYT @hiddeco @krancour ?

[hiddeco]

As the phase is also being shown for kargo get promotions, I feel the ordering I suggested in #1949 (comment) makes sense for both the frontend and the CLI (which arguably is a "TUI").

[krancour]

Also agree with @hiddeco's suggestions in #1949 (comment)

[krancour]

We're looking at a v0.7.0 release toward the end of this week. @webstradev should we defer this to v0.8.0?

[webstradev]

We're looking at a v0.7.0 release toward the end of this week. @webstradev should we defer this to v0.8.0?

yeah sorry I've been quite busy and I've got some holidays coming up. So either defer or happy to have someone else take it over and use what I wrote so far.

[akuitybot]

Successfully created backport PR for release-0.7:

• chore(backport release-0.7): feat: (api/ui) Sort promotions by phase and name #2106