Skip to content

Update job status on rerun - avoid duplicate email notification #35575

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.

Sign up for GitHub

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

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
+0 −1
Draft

Update job status on rerun - avoid duplicate email notification #35575

Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8d799c2
use the correct context data for PR link template in issue card
badhezi Apr 15, 2025
f2a2acf
Merge branch 'main' into main
badhezi Apr 15, 2025
5cc1bda
Merge branch 'main' into main
GiteaBot Apr 16, 2025
54d37d1
Merge branch 'main' into main
GiteaBot Apr 16, 2025
ac25150
Merge branch 'go-gitea:main' into main
badhezi Apr 16, 2025
e11a339
Merge branch 'go-gitea:main' into main
badhezi Apr 20, 2025
104eecc
Merge branch 'go-gitea:main' into main
badhezi Apr 21, 2025
bcc4ade
Merge branch 'go-gitea:main' into main
badhezi Apr 22, 2025
a7aaa79
Merge branch 'go-gitea:main' into main
badhezi Apr 27, 2025
b46d314
Merge branch 'go-gitea:main' into main
badhezi Apr 28, 2025
4e2434b
Merge branch 'go-gitea:main' into main
badhezi Apr 29, 2025
7f72fe9
Merge branch 'go-gitea:main' into main
badhezi May 2, 2025
c6acfc1
Merge branch 'go-gitea:main' into main
badhezi May 11, 2025
016c2f3
Merge branch 'go-gitea:main' into main
badhezi May 12, 2025
a3c2953
Merge branch 'go-gitea:main' into main
badhezi May 13, 2025
4d7ea0d
Merge branch 'go-gitea:main' into main
badhezi May 20, 2025
bfa2d10
Merge branch 'go-gitea:main' into main
badhezi May 28, 2025
0d50b75
Merge branch 'go-gitea:main' into main
badhezi Jun 1, 2025
5ad8708
Merge branch 'go-gitea:main' into main
badhezi Jul 15, 2025
7fb93fe
Merge branch 'go-gitea:main' into main
badhezi Jul 29, 2025
80e3c30
Merge branch 'go-gitea:main' into main
badhezi Oct 3, 2025
d0386a6
update job.Status on re-run trigger to avoid duplicate email notifica…
badhezi Oct 3, 2025
540b853
fix comment
badhezi Oct 3, 2025
854cd39
skip redundant workflow run status update
badhezi Oct 3, 2025
686a4f6
Merge branch 'main' into hezi/update-run-status-on-rerun
badhezi Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion routers/web/repo/actions/view.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,14 @@ func Rerun(ctx *context_module.Context) {
return
}

// reset run's start and stop time when it is done
// reset run's start stop time and status if it is done
if run.Status.IsDone() {
run.PreviousDuration = run.Duration()
run.Started = 0
run.Stopped = 0
// Set run to waiting status so next job.Status evaluations reflect the status after the re-run trigger
// Avoid re-triggering email notification before the new job run ends
run.Status = 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the new code does nothing. It isn't updated into datebase (see UpdateRun(ctx, run, "started", "stopped", "previous_duration"))

And can you add a test to make sure that the behavior is expected?

Copy link
Contributor Author

@badhezi badhezi Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, you are of course correct. I mixed up ActionRun and ActionRunJob also.
I'll add test cases for this as well.

@NorthRealm is there a particular reason why we need this call at all?

gitea/routers/web/repo/actions/view.go

Line 437 in c453210

notify_service.WorkflowRunStatusUpdate(ctx, run.Repo, run.TriggerUser, run)

It's only being called on a rerun and it's sole purpose is sending email updates about the status of the ActoinJobRun which is yet to be changed. (we only change the ActionJob at this stage)
I don't see why we need call it before the run started (I might be missing something?)

IMO we can just remove this line and the notification will be sent at the end of the run without sending a redundant one when re-running.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6bcdd7e

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never produced the issue 😅 sorry, I kinda lose track
@ChristopherHX Could you please look at it again 🤦

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see now the problem is that the status is not set back to in progress.

Seems fine for me to temporary remove this event, until it's rewritten to send an expected event.

It's only being called on a rerun and it's sole purpose is sending email updates about the status of the ActoinJobRun which is yet to be changed. (we only change the ActionJob at this stage) I don't see why we need call it before the run started (I might be missing something?)

IMO we can just remove this line and the notification will be sent at the end of the run without sending a redundant one when re-running.

IMO We need to sent a workflow_run event that is requested not completed.

It is not expected to have a email delivered here, but I expect an event.

A test that checks a rerun does not create a workflow_run completion event is preferred.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actially I have written tests that verify the existence of this event..., need to look at this.

if err := actions_model.UpdateRun(ctx, run, "started", "stopped", "previous_duration"); err != nil {
ctx.ServerError("UpdateRun", err)
return
Expand Down