Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI: Show more information in Promotion Table view #1822

Closed
rbreeze opened this issue Apr 11, 2024 · 4 comments
Closed

UI: Show more information in Promotion Table view #1822

rbreeze opened this issue Apr 11, 2024 · 4 comments

Comments

@rbreeze
Copy link
Contributor

rbreeze commented Apr 11, 2024

Currently, we don't show much detail in the Promotions Table of the Stage panel.

We should display:

  • URL (to PR or commit)
  • More information about Freight, or at least link to more detail (name isn't helpful)
  • Information about the Argo CD Application status (if applicable)

CleanShot 2024-04-11 at 14 24 55@2x

@rbreeze
Copy link
Contributor Author

rbreeze commented May 9, 2024

TLDR; #1979 is the best we will get for this without some questionable API decisions.

In #1979, I added links to Freight Details in this table. I also added a Tooltip to the Freight ID column, such that when you hover over it, the Freight alias appears.

Implementation-wise, this is accomplished by doing an ad-hoc API request every time a user hovers. This is necessary because the ListPromotions API does not contain freight information besides just the freight name (ID), nor should it IMO (would require additional K8S API calls in the endpoint to accomplish). Likewise, the UI shouldn't call the GetFreight API repeatedly on this page.

@krancour @hiddeco do you have thoughts here?

@hiddeco
Copy link
Contributor

hiddeco commented May 15, 2024

The only optimization I can think of is temporarily caching the response in the frontend as the Freight is almost always static (the single exception being: if someone manually changes the alias).

Providing e.g. a special endpoint for a more lightweight response would not yield a better result for the backend, which would still have to retrieve the object to provide a compressed view.

@krancour
Copy link
Member

krancour commented May 15, 2024

ListPromotions API does not contain freight information besides just the freight name (ID), nor should it IMO

I agree with this. Aliases are mutable and if we included information about aliases in a FreightReference, we'd have to track down and fix those any time a piece of Freight's alias changes. It's simply not worth it.

Edit: I also agree with @hiddeco about not augmenting list promo results with that information.

We could, theoretically, add a more bespoke API endpoint just for use with this view, where you supply a list of Freight IDs and get back the corresponding list of aliases.

I wouldn't object if you wanted to do that.

@rbreeze
Copy link
Contributor Author

rbreeze commented May 15, 2024

@hiddeco @krancour I agree with you both.

We could, theoretically, add a more bespoke API endpoint just for use with this view, where you supply a list of Freight IDs and get back the corresponding list of aliases.

TBH, I don't think it's worth it. The tooltip solution isn't as nice as if we had the alias inline, but I think it gets 90% of the way there. I'll go ahead and close this issue but feel free to reopen if you feel we should do more here

@rbreeze rbreeze closed this as completed May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants