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

action-test-results-to-slack doesn't support workflow_dispatch #39412

Open
testlabauto opened this issue Sep 16, 2024 · 7 comments
Open

action-test-results-to-slack doesn't support workflow_dispatch #39412

testlabauto opened this issue Sep 16, 2024 · 7 comments
Labels
[Action] Test Results to Slack Good For Community [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Comments

@testlabauto
Copy link

testlabauto commented Sep 16, 2024

Impacted plugin

None / Other

Quick summary

I just started using this plugin and noticed that all workflow_dispatch events will be grouped because the eventName is not handled.

I think it would be possibly sufficient to add:

if ( eventName === 'workflow_dispatch' ) {
    msgId = `workflow_dispatch-${ Date.now() }`;
}

at: https://github.com/Automattic/action-test-results-to-slack/blob/trunk/src/message.js#L98

But, of course, another implementation would be fine! Just trying to avoid all workflow_dispatch being grouped as it is easy to lose track of those messages in the history.###

Steps to reproduce

  1. Run a workflow_dispatch workflow using the plugin
  2. Make code changes
  3. Run it a second time

A clear and concise description of what you expected to happen.

Separate messages are sent to slack.

What actually happened

All workflow_dispatch messages are grouped.

Impact

One

Available workarounds?

No and the platform is unusable

If the above answer is "Yes...", outline the workaround.

No response

Platform (Simple and/or Atomic)

No response

Logs or notes

No response

@testlabauto testlabauto added [Type] Bug When a feature is broken and / or not performing as intended Needs triage Ticket needs to be triaged labels Sep 16, 2024
@github-actions github-actions bot added [Status] Priority Review Triggered The guild in charge of triage has been notified of this issue in Slack [Pri] High labels Sep 16, 2024
@anomiex anomiex added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it Good For Community [Action] Test Results to Slack and removed [Type] Bug When a feature is broken and / or not performing as intended [Pri] High Needs triage Ticket needs to be triaged [Status] Priority Review Triggered The guild in charge of triage has been notified of this issue in Slack labels Sep 17, 2024
@anomiex
Copy link
Contributor

anomiex commented Sep 17, 2024

I'd recommend someone taking on this request to allow for using appropriately named inputs to optionally specify a sha and source repo, in the same manner that the handling for 'repository_dispatch' uses client_payload.

@testlabauto
Copy link
Author

Thank you! in the meantime, if I use a fork, is this what you are suggesting @anomiex ?

	if ( eventName === 'workflow_dispatch' ) {
		target = `for event _*${ payload.action }*_`;

		if ( payload.client_payload?.sha ) {
			const upstreamSha = payload.client_payload.sha;
			msgId = `workflow_dispatch-${ upstreamSha }`;
			contextElements.push(
				getTextContextElement( `Last commit: ${ upstreamSha.substring( 0, 8 ) }` )
			);

			if ( payload.client_payload?.repository ) {
				const commitUrl = `${ serverUrl }/${ payload.client_payload.repository }/commit/${ upstreamSha }`;
				buttons.push( getButton( `Commit ${ upstreamSha.substring( 0, 8 ) }`, commitUrl ) );
			}
		} else {
			msgId = `workflow_dispatch-${ Date.now() }`;
		}
	}

@anomiex
Copy link
Contributor

anomiex commented Sep 19, 2024

If you're going to make a fork, you could also submit a PR for us to review. 🙂

As for the code, it looks ok to me except that you'd want to do payload.inputs? rather than payload.client_payload? everywhere in there. That should correspond to a workflow yaml something like

on:
  workflow_dispatch:
    inputs:
      sha:
        description: 'Commit tested'
        type: string
      repository:
        description: 'Repository tested'
        type: string

@testlabauto
Copy link
Author

Thank you! Will do! I will test on my fork first and then submit a PR.

@testlabauto
Copy link
Author

Went ahead and created a PR. Happy to "edit down" to the minimal changes if that makes sense:
Automattic/action-test-results-to-slack#1

@testlabauto
Copy link
Author

Looks like it may have been automatically closed, but as I mentioned, I really think these are the changes that matter:

	if ( eventName === 'workflow_dispatch' ) {
		target = `for manual run on ${ refType } _*${ refName }*_`;

		if ( payload.inputs?.sha ) {
			const upstreamSha = payload.inputs.sha;
			msgId = `workflow_dispatch-${ upstreamSha }`;
			contextElements.push(
				getTextContextElement( `Last commit: ${ upstreamSha.substring( 0, 8 ) }` )
			);

			if ( payload.inputs?.repository ) {
				const commitUrl = `${ serverUrl }/${ payload.inputs.repository }/commit/${ upstreamSha }`;
				buttons.push( getButton( `Commit ${ upstreamSha.substring( 0, 8 ) }`, commitUrl ) );
			}
		} else {
			msgId = `workflow_dispatch-${ Date.now() }`;
		}
	}

@anomiex
Copy link
Contributor

anomiex commented Sep 27, 2024

You'll need to make the PR against the Automattic/jetpack repository. The corresponding file is at projects/github-actions/test-results-to-slack/src/message.js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Action] Test Results to Slack Good For Community [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
Development

No branches or pull requests

2 participants