✅ Deploy Preview for docs-kargo-akuity-io ready!

Name	Link
🔨 Latest commit	503ed79
🔍 Latest deploy log	https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/6635617436cd2c00084201a1
😎 Deploy Preview	https://deploy-preview-1949.kargo.akuity.io
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 46.25%. Comparing base (ba3601b) to head (503ed79).
Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main    #1949      +/-   ##
==========================================
+ Coverage   46.20%   46.25%   +0.04%     
==========================================
  Files         235      235              
  Lines       15990    16004      +14     
==========================================
+ Hits         7388     7402      +14     
+ Misses       8244     8242       -2     
- Partials      358      360       +2     

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

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.

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

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.

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
];

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.

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

I think we would at least want it in the API, and for the frontend we have to await further instructions from @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.

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.

@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 ?

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

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

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

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.