Feature: Support workflow event dispatch via API #32059

bencurio · 2024-09-17T13:14:39Z

ref: #31765

routers/api/v1/api.go

modules/structs/repo_actions.go

services/actions/workflow.go

routers/api/v1/api.go

KodingDev · 2024-11-22T11:08:21Z

Any updates on this?

bencurio · 2024-12-04T08:13:20Z

I'm sorry, but due to my current workload, I don't have the capacity to handle the PR at the moment. However, if the code review is completed in full, I can address all the corrections at once. Thank you.

lunny · 2024-12-04T08:35:32Z

I'm sorry, but due to my current workload, I don't have the capacity to handle the PR at the moment. However, if the code review is completed in full, I can address all the corrections at once. Thank you.

would you mind maintainers send commits to your branch？

bencurio · 2024-12-04T09:28:31Z

I'm sorry, but due to my current workload, I don't have the capacity to handle the PR at the moment. However, if the code review is completed in full, I can address all the corrections at once. Thank you.

would you mind maintainers send commits to your branch？

I don't mind, feel free to send commits to my branch. Editing is enabled. Thank you!

services/actions/workflow.go

* uses all endpoints added here * checks if disabled workflows cannot dispatch * remove node_id * fix path field * test that the URL field of the workflow result is usable * additional test for non default branch workflow * return 200 when no workflows found, with an empty list

ChristopherHX · 2025-02-08T15:40:48Z

I understand this is still WIP, but wanted to note that at the moment when using the API only the first job from the workflow is executed, but when started from the web all jobs are executed.

Could you please reevaluate this now/later?

This now uses the exact same logic for UI and api to trigger workflow_dispatch

Also non default branch workflows are now read instead of using always the workflow of the default branch (changed in 1.24.0/nightly)

Summary

node_id removed from api
workflow path now includes the workflow path to repo root (instead of only the filename)
integration tests for all added endpoints
- no tests for the webui dispatch trigger path
workflow list now returns a different structure and uses the previously unused api struct
workflow dispatch logic is no longer a copy of the UI path
- used a custom error type to preserve the UI error messages (not all scenarious tested)
- the workflow dispatch http status for disabled workflows is not documented on GitHub Docs so currently http 500 can be changed on demand

Please let me know If you have some suggestions or ideas to improve this, until then I have currently no open points on my TODO list for this change

eldiaboloz · 2025-02-08T16:06:34Z

I understand this is still WIP, but wanted to note that at the moment when using the API only the first job from the workflow is executed, but when started from the web all jobs are executed.

Could you please reevaluate this now/later?

Just tested and it is fixed now - both manual and API execute all the jobs from the workflow.

ChristopherHX

I guess I'm now able to approve towards the lgtm bot, but didn't I changed way too much here

ChristopherHX · 2025-02-09T23:15:43Z

@bencurio I revoked my access to your fork, since this is done now. Thanks for your workflow api work.

* giteaofficial/main: Feature: Support workflow event dispatch via API (go-gitea#32059) Remove "class-name" from svg icon (go-gitea#33540) Add "No data available" display when list is empty (go-gitea#33517) Add a option "--user-type bot" to admin user create, improve role display (go-gitea#27885)

wxiaoguang · 2025-02-10T03:25:02Z

routers/api/v1/repo/action.go

+	workflowID := ctx.PathParam("workflow_id")
+	if len(workflowID) == 0 {
+		ctx.Error(http.StatusUnprocessableEntity, "MissingWorkflowParameter", util.NewInvalidArgumentErrorf("workflow_id is required parameter"))
+		return
+	}


That's impossible to happen, dead code ........

wxiaoguang · 2025-02-10T03:27:08Z

routers/api/v1/repo/action.go

+		if terr, ok := err.(*actions_service.TranslateableError); ok {
+			msg := ctx.Locale.TrString(terr.Translation, terr.Args...)
+			ctx.Error(terr.GetCode(), msg, fmt.Errorf("%s", msg))
+			return
+		}


We can't use locale in API.

IIRC the locale context is not properly prepared, and we never used that in code

wxiaoguang · 2025-02-10T03:28:50Z

routers/web/repo/actions/view.go

+					// Checkboxes (and radio buttons) are on/off switches that may be toggled by the user.
+					// A switch is "on" when the control element's checked attribute is set.
+					// When a form is submitted, only "on" checkbox controls can become successful.
+					(*inputs)[name] = strconv.FormatBool(value == "on")


There is ctx.FormBool to handle this case, no need to comment the "on" behavior everywhere .......

wxiaoguang · 2025-02-10T03:29:32Z

services/actions/workflow.go

+type TranslateableError struct {
+	Translation string
+	Args        []any
+	Code        int
+}
+
+func (t TranslateableError) Error() string {
+	return t.Translation
+}
+
+func (t TranslateableError) GetCode() int {
+	if t.Code == 0 {
+		return http.StatusInternalServerError
+	}
+	return t.Code
+}


We should not invent a new error system at the moment .........

wxiaoguang · 2025-02-10T03:29:58Z

Could we revert this change and propose a new and clear PR?

wxiaoguang · 2025-02-10T03:34:45Z

services/actions/workflow.go

+
+	URL := fmt.Sprintf("%s/actions/workflows/%s", ctx.Repo.Repository.APIURL(), entry.Name())
+	HTMLURL := fmt.Sprintf("%s/src/branch/%s/%s/%s", ctx.Repo.Repository.HTMLURL(ctx), defaultBranch, folder, entry.Name())
+	badgeURL := fmt.Sprintf("%s/actions/workflows/%s/badge.svg?branch=%s", ctx.Repo.Repository.HTMLURL(ctx), entry.Name(), ctx.Repo.Repository.DefaultBranch)


It forgets to escape ......

It would cause problems if there are special chars in names

)" This reverts commit 523751d.

wxiaoguang · 2025-02-10T03:36:46Z

Could we revert this change and propose a new and clear PR?

-> Revert "Feature: Support workflow event dispatch via API (#32059)" #33541

bencurio · 2025-02-10T07:27:51Z

@bencurio I revoked my access to your fork, since this is done now. Thanks for your workflow api work.

Thank you for continuing; unfortunately, I didn't have enough time to finish it at the end.

bencurio · 2025-02-10T07:34:54Z

@wxiaoguang A more collaborative tone can be achieved by explaining the reasoning behind concerns. Focusing on the issues and suggesting alternatives helps maintain a respectful and constructive dialogue, which is essential for effective collaboration.

wxiaoguang · 2025-02-10T07:58:10Z

@wxiaoguang A more collaborative tone can be achieved by explaining the reasoning behind concerns. Focusing on the issues and suggesting alternatives helps maintain a respectful and constructive dialogue, which is essential for effective collaboration.

I think I have explained some details for each review comment.

If there is still anything unclear, please point out and I will try to explain more.

Since I am not a native speaker, so I am not sure which "tone" could be more "collaborative": https://github.com/go-gitea/gitea/blob/main/CODE_OF_CONDUCT.md: Remember that people have varying communication styles and that not everyone is using their native language. (Meaning and tone can be lost in translation.)

And TBH I reviewed quite a lot of PRs (https://github.com/go-gitea/gitea/pulls?q=is%3Apr+is%3Aclosed+reviewed-by%3Awxiaoguang), usually I would assume that the readers could have some background knowledges about the problem, otherwise I need to type a lot of words again and again. And yes, if there is something unclear, I'd be happy to explain.

bencurio · 2025-02-10T08:01:35Z

I'm not a native speaker either; in fact, I'm asking ChatGPT to translate this. Nevertheless, I'm taking the trouble to avoid offending anyone by using an appropriate style. This was also translated by ChatGPT. As far as I'm concerned, I've finished this PR because I still don't have any more time to continue, but I'm happy to hand over the repo to someone else if needed.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 17, 2024

pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 17, 2024

github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Sep 17, 2024

bencurio force-pushed the main branch 2 times, most recently from 727cd1f to 8814e9f Compare September 17, 2024 16:00

pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 2, 2024

bencurio marked this pull request as ready for review October 2, 2024 10:18

bencurio changed the title ~~[WIP] Feature: Support workflow event dispatch via API~~ Feature: Support workflow event dispatch via API Oct 2, 2024

lunny reviewed Oct 3, 2024

View reviewed changes

routers/api/v1/api.go Outdated Show resolved Hide resolved

lunny reviewed Oct 6, 2024

View reviewed changes

modules/structs/repo_actions.go Show resolved Hide resolved

lunny reviewed Oct 11, 2024

View reviewed changes

services/actions/workflow.go Outdated Show resolved Hide resolved

bencurio force-pushed the main branch from 3d77a3f to 1969e6b Compare October 12, 2024 08:20

pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 12, 2024

lunny added this to the 1.23.0 milestone Oct 21, 2024

lunny reviewed Oct 21, 2024

View reviewed changes

routers/api/v1/api.go Outdated Show resolved Hide resolved

lunny reviewed Oct 21, 2024

View reviewed changes

routers/api/v1/api.go Outdated Show resolved Hide resolved

lunny modified the milestones: 1.23.0, 1.24.0 Nov 6, 2024

appleboy reviewed Jan 11, 2025

View reviewed changes

services/actions/workflow.go Outdated Show resolved Hide resolved

ChristopherHX added 3 commits February 8, 2025 12:39

update swagger

0ce8265

use assert empty

fb747b1

lunny approved these changes Feb 8, 2025

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 8, 2025

ChristopherHX approved these changes Feb 8, 2025

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 8, 2025

lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 9, 2025

GiteaBot added 2 commits February 9, 2025 11:25

Merge branch 'main' into main

db799e0

Merge branch 'main' into main

2f1c48c

lunny merged commit 523751d into go-gitea:main Feb 9, 2025
26 checks passed

GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 9, 2025

wxiaoguang reviewed Feb 10, 2025

View reviewed changes

wxiaoguang added a commit to wxiaoguang/gitea that referenced this pull request Feb 10, 2025

Revert "Feature: Support workflow event dispatch via API (go-gitea#32059

5c10670

)" This reverts commit 523751d.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Support workflow event dispatch via API #32059

Feature: Support workflow event dispatch via API #32059

bencurio commented Sep 17, 2024 •

edited

Loading

KodingDev commented Nov 22, 2024

bencurio commented Dec 4, 2024

lunny commented Dec 4, 2024

bencurio commented Dec 4, 2024

ChristopherHX commented Feb 8, 2025

eldiaboloz commented Feb 8, 2025

ChristopherHX left a comment

ChristopherHX commented Feb 9, 2025

wxiaoguang Feb 10, 2025

wxiaoguang Feb 10, 2025

wxiaoguang Feb 10, 2025

wxiaoguang Feb 10, 2025

wxiaoguang commented Feb 10, 2025

wxiaoguang Feb 10, 2025

wxiaoguang commented Feb 10, 2025 •

edited

Loading

bencurio commented Feb 10, 2025

bencurio commented Feb 10, 2025 •

edited

Loading

wxiaoguang commented Feb 10, 2025

bencurio commented Feb 10, 2025

Feature: Support workflow event dispatch via API #32059

Feature: Support workflow event dispatch via API #32059

Conversation

bencurio commented Sep 17, 2024 • edited Loading

KodingDev commented Nov 22, 2024

bencurio commented Dec 4, 2024

lunny commented Dec 4, 2024

bencurio commented Dec 4, 2024

ChristopherHX commented Feb 8, 2025

Summary

eldiaboloz commented Feb 8, 2025

ChristopherHX left a comment

Choose a reason for hiding this comment

ChristopherHX commented Feb 9, 2025

wxiaoguang Feb 10, 2025

Choose a reason for hiding this comment

wxiaoguang Feb 10, 2025

Choose a reason for hiding this comment

wxiaoguang Feb 10, 2025

Choose a reason for hiding this comment

wxiaoguang Feb 10, 2025

Choose a reason for hiding this comment

wxiaoguang commented Feb 10, 2025

wxiaoguang Feb 10, 2025

Choose a reason for hiding this comment

wxiaoguang commented Feb 10, 2025 • edited Loading

bencurio commented Feb 10, 2025

bencurio commented Feb 10, 2025 • edited Loading

wxiaoguang commented Feb 10, 2025

bencurio commented Feb 10, 2025

bencurio commented Sep 17, 2024 •

edited

Loading

wxiaoguang commented Feb 10, 2025 •

edited

Loading

bencurio commented Feb 10, 2025 •

edited

Loading