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

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.

wxiaoguang · 2025-02-10T08:05:38Z

To be clear, I am not saying this PR is wrong. Instead, it is awesome and thank you everyone for the work.

The problem is: some designs and approaches introduced by this PR are not suitable (well, how to find a proper word to not offend people?) and need to improve, so I think we need to "propose a new and clear PR (#32059 (comment))" .

bencurio · 2025-02-10T08:11:58Z

If it helps, this functionality has already been independently implemented in Forgejo: https://codeberg.org/forgejo/forgejo/commit/51735c415bb29e40b1b02c14bb4f9854e8e27b24

wxiaoguang · 2025-02-10T08:16:51Z

Yup, many features are independently implemented in Gitea and Forgejo.

You could compare them, I believe the Gitea's solutions are better in most cases. For example: Sync fork, Wiki public access, markup rendering, including the workflow dispatch.

…33541) This reverts commit 523751d.

ref: go-gitea#31765 --------- Signed-off-by: Bence Santha <[email protected]> Co-authored-by: Lunny Xiao <[email protected]> Co-authored-by: Christopher Homberger <[email protected]>

wxiaoguang · 2025-02-10T09:48:28Z

I opened a new PR: Feature: Support workflow event dispatch via API #33545

Will try to make some changes and please help to review then, thank you very much!

wxiaoguang · 2025-02-10T12:00:14Z

I opened a new PR: Feature: Support workflow event dispatch via API #33545

CI passes and it is ready for review

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

GiteaBot added the lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. label 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.

This comment has been minimized.

Sign in to view

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

Revert "Feature: Support workflow event dispatch via API (#32059)" (#…

704b65e

…33541) This reverts commit 523751d.

wxiaoguang mentioned this pull request Feb 10, 2025

Feature: Support workflow event dispatch via API #33545

Merged

go-gitea locked and limited conversation to collaborators Feb 10, 2025

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

wxiaoguang commented Feb 10, 2025

bencurio commented Feb 10, 2025

wxiaoguang commented Feb 10, 2025

This comment has been minimized.

wxiaoguang commented Feb 10, 2025

wxiaoguang 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 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

wxiaoguang commented Feb 10, 2025

bencurio commented Feb 10, 2025

wxiaoguang commented Feb 10, 2025

This comment has been minimized.

wxiaoguang commented Feb 10, 2025

wxiaoguang 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