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

PR link no longer displayed in the Promotion UI #3024

Closed
4 tasks done
simonartige opened this issue Nov 30, 2024 · 4 comments · Fixed by #3129
Closed
4 tasks done

PR link no longer displayed in the Promotion UI #3024

simonartige opened this issue Nov 30, 2024 · 4 comments · Fixed by #3129

Comments

@simonartige
Copy link

simonartige commented Nov 30, 2024

Checklist

  • I've searched the issue queue to verify this is not a duplicate bug report.
  • I've included steps to reproduce the bug.
  • I've pasted the output of kargo version.
  • I've pasted logs, if applicable.

Description

With the new promotion steps, which are indeed more flexible now, it seems that we have lost the link to the opened PR in the promotion view on the Kargo UI.
This feature on the UI was very handy as it allowed to quickly see the pull request opened for the promotion.

Looking at the code, we can see the functionality got removed with this change:
https://github.com/akuity/kargo/pull/2694/files#diff-831008a406138fc2b4e0288bf2dfc2fe4c9ed871de1268ee4c67edaa0f7977f2
Which looks like the intended behavior.

We can still find the PR number by browsing in the new promotion details panel
image
But this requires more navigation and there is no link available - only the PR number

Are there any future plans to show the list of PRs opened in the promotion steps on the UI?

I logged a bug, but this is a functional regression. But I would understand if this is seen as a feature request as this is a feature from the 0.x Kargo version, that was before the new promotion steps mechanism. With the new system, there could be multiple PRs opened in a single promotion, therefore there may be UX challenges to display them. But I would say it is expected to find these PR links somehow when we see this icon
image

Screenshots

image

Steps to Reproduce

Create a promotion with the following configuration:

        - uses: git-commit
          config:
            path: ./app
            messageFromSteps:
              - helm-update-image
        - uses: git-push
          as: push
          config:
            path: ./app
            generateTargetBranch: true
        - uses: git-open-pr
          as: open-pr
          config:
            repoURL: https://github.com/x/y
            provider: github
            sourceBranchFromStep: push
            targetBranch: main
        - uses: git-wait-for-pr
          config:
            repoURL: https://github.com/x/y
            provider: github
            prNumberFromStep: open-pr

Promote a freight using that configuration
A PR is opened during the promotion
Looking at the promotion history in the stage details, we don't have the link to the PR anymore (see screenshot)

I don't have a screenshot to the previous UI sorry, but the associated code was https://github.com/akuity/kargo/pull/2694/files#diff-831008a406138fc2b4e0288bf2dfc2fe4c9ed871de1268ee4c67edaa0f7977f2

Version

v1.0.3

@simonartige simonartige changed the title PR link no longer displayed in the Promotion PR link no longer displayed in the Promotion UI Nov 30, 2024
@simonartige
Copy link
Author

Alternatively, maybe we could include the link to the PR in this area of the new promotion details panel?
image

@Marvin9
Copy link
Contributor

Marvin9 commented Dec 1, 2024

There is easy fix to this but the thing is any metadata that is related to promotion steps have potential to show up at meaningful place in the UI. The easy fix is tightly coupled UI code. This might not scale well with third-party promotions. Although this hasn't been discussed yet which way to go but it might become clearer once we progress on third party promotion.

@krancour
Copy link
Member

krancour commented Dec 2, 2024

Although I hate to say it, this is one of those spots where we're going to have to accept the less-than-ideal, tightly-coupled solution, at least temporarily.

@krancour krancour added this to the v1.1.1 milestone Dec 2, 2024
@jessesuen
Copy link
Member

This is another use case for #2752

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants