From 386e7f82086454a2fc18bf4ff667560a5dd3c981 Mon Sep 17 00:00:00 2001 From: christopher-besch Date: Tue, 29 Apr 2025 06:58:05 +0000 Subject: [PATCH] send mail on failed or recovered workflow run (#7509) Send a Mail when an action run fails or a workflow recovers. This PR depends on https://codeberg.org/forgejo/forgejo/pulls/7491 closes #3719 ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests - I added test coverage for Go changes... - [x] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I added test coverage for JavaScript changes... - [ ] in `web_src/js/*.test.js` if it can be unit tested. - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)). ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] I do not want this change to show in the release notes. - [x] I want the title to show in the release notes with a link to this pull request. - [ ] I want the content of the `release-notes/.md` to be be used for the release notes instead of the title. ## Release notes - Features - [PR](https://codeberg.org/forgejo/forgejo/pulls/7509): send mail on failed or recovered workflow run Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7509 Reviewed-by: Earl Warren Co-authored-by: christopher-besch Co-committed-by: christopher-besch --- options/locale_next/locale_en-US.json | 8 + services/mailer/mail_actions.go | 84 ++++++++++ services/mailer/mail_actions_now_done_test.go | 146 ++++++++++++++++++ services/mailer/mail_admin_new_user_test.go | 13 +- services/mailer/main_test.go | 8 + services/mailer/notify.go | 11 ++ templates/mail/actions/now_done.tmpl | 38 +++++ 7 files changed, 298 insertions(+), 10 deletions(-) create mode 100644 services/mailer/mail_actions.go create mode 100644 services/mailer/mail_actions_now_done_test.go create mode 100644 templates/mail/actions/now_done.tmpl diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index 94b1100aeb..9e620f4b1c 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -21,5 +21,13 @@ "alert.asset_load_failed": "Failed to load asset files from {path}. Please make sure the asset files can be accessed.", "alert.range_error": " must be a number between %[1]s and %[2]s.", "install.invalid_lfs_path": "Unable to create the LFS root at the specified path: %[1]s", + "mail.actions.successful_run_after_failure_subject": "Workflow %[1]s recovered in repository %[2]s", + "mail.actions.not_successful_run_subject": "Workflow %[1]s failed in repository %[2]s", + "mail.actions.successful_run_after_failure": "Workflow %[1]s recovered in repository %[2]s", + "mail.actions.not_successful_run": "Workflow %[1]s failed in repository %[2]s", + "mail.actions.run_info_cur_status": "This Run's Status: %[1]s (just updated from %[2]s)", + "mail.actions.run_info_previous_status": "Previous Run's Status: %[1]s", + "mail.actions.run_info_ref": "Branch: %[1]s (%[2]s)", + "mail.actions.run_info_trigger": "Triggered because: %[1]s by: %[2]s", "meta.last_line": "Thank you for translating Forgejo! This line isn't seen by the users but it serves other purposes in the translation management. You can place a fun fact in the translation instead of translating it." } diff --git a/services/mailer/mail_actions.go b/services/mailer/mail_actions.go new file mode 100644 index 0000000000..7c63603a98 --- /dev/null +++ b/services/mailer/mail_actions.go @@ -0,0 +1,84 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later +package mailer + +import ( + "bytes" + + actions_model "forgejo.org/models/actions" + user_model "forgejo.org/models/user" + "forgejo.org/modules/base" + "forgejo.org/modules/setting" + "forgejo.org/modules/translation" +) + +const ( + tplActionNowDone base.TplName = "actions/now_done" +) + +// requires !run.Status.IsSuccess() or !lastRun.Status.IsSuccess() +func MailActionRun(run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) error { + if setting.MailService == nil { + // No mail service configured + return nil + } + + if run.TriggerUser.Email != "" && run.TriggerUser.EmailNotificationsPreference != user_model.EmailNotificationsDisabled { + if err := sendMailActionRun(run.TriggerUser, run, priorStatus, lastRun); err != nil { + return err + } + } + + if run.Repo.Owner.Email != "" && run.Repo.Owner.Email != run.TriggerUser.Email && run.Repo.Owner.EmailNotificationsPreference != user_model.EmailNotificationsDisabled { + if err := sendMailActionRun(run.Repo.Owner, run, priorStatus, lastRun); err != nil { + return err + } + } + + return nil +} + +func sendMailActionRun(to *user_model.User, run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) error { + var ( + locale = translation.NewLocale(to.Language) + content bytes.Buffer + ) + + var subject string + if run.Status.IsSuccess() { + subject = locale.TrString("mail.actions.successful_run_after_failure_subject", run.Title, run.Repo.FullName()) + } else { + subject = locale.TrString("mail.actions.not_successful_run", run.Title, run.Repo.FullName()) + } + + commitSHA := run.CommitSHA + if len(commitSHA) > 7 { + commitSHA = commitSHA[:7] + } + branch := run.PrettyRef() + + data := map[string]any{ + "locale": locale, + "Link": run.HTMLURL(), + "Subject": subject, + "Language": locale.Language(), + "RepoFullName": run.Repo.FullName(), + "Run": run, + "TriggerUserLink": run.TriggerUser.HTMLURL(), + "LastRun": lastRun, + "PriorStatus": priorStatus, + "CommitSHA": commitSHA, + "Branch": branch, + "IsSuccess": run.Status.IsSuccess(), + } + + if err := bodyTemplates.ExecuteTemplate(&content, string(tplActionNowDone), data); err != nil { + return err + } + + msg := NewMessage(to.EmailTo(), subject, content.String()) + msg.Info = subject + SendAsync(msg) + + return nil +} diff --git a/services/mailer/mail_actions_now_done_test.go b/services/mailer/mail_actions_now_done_test.go new file mode 100644 index 0000000000..0d832f2b36 --- /dev/null +++ b/services/mailer/mail_actions_now_done_test.go @@ -0,0 +1,146 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package mailer + +import ( + "testing" + + actions_model "forgejo.org/models/actions" + "forgejo.org/models/db" + repo_model "forgejo.org/models/repo" + user_model "forgejo.org/models/user" + "forgejo.org/modules/setting" + notify_service "forgejo.org/services/notify" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func getActionsNowDoneTestUsers(t *testing.T) []*user_model.User { + t.Helper() + newTriggerUser := new(user_model.User) + newTriggerUser.Name = "new_trigger_user" + newTriggerUser.Language = "en_US" + newTriggerUser.IsAdmin = false + newTriggerUser.Email = "new_trigger_user@example.com" + newTriggerUser.LastLoginUnix = 1693648327 + newTriggerUser.CreatedUnix = 1693648027 + newTriggerUser.EmailNotificationsPreference = user_model.EmailNotificationsEnabled + require.NoError(t, user_model.CreateUser(db.DefaultContext, newTriggerUser)) + + newOwner := new(user_model.User) + newOwner.Name = "new_owner" + newOwner.Language = "en_US" + newOwner.IsAdmin = false + newOwner.Email = "new_owner@example.com" + newOwner.LastLoginUnix = 1693648329 + newOwner.CreatedUnix = 1693648029 + newOwner.EmailNotificationsPreference = user_model.EmailNotificationsEnabled + require.NoError(t, user_model.CreateUser(db.DefaultContext, newOwner)) + + return []*user_model.User{newTriggerUser, newOwner} +} + +func assertTranslatedLocaleMailActionsNowDone(t *testing.T, msgBody string) { + AssertTranslatedLocale(t, msgBody, "mail.actions.successful_run_after_failure", "mail.actions.not_successful_run", "mail.actions.run_info_cur_status", "mail.actions.run_info_ref", "mail.actions.run_info_previous_status", "mail.actions.run_info_trigger", "mail.view_it_on") +} + +func TestActionRunNowDoneNotificationMail(t *testing.T) { + ctx := t.Context() + + users := getActionsNowDoneTestUsers(t) + defer CleanUpUsers(ctx, users) + triggerUser := users[0] + ownerUser := users[1] + + repo := repo_model.Repository{ + Name: "some repo", + Description: "rockets are cool", + Owner: ownerUser, + OwnerID: ownerUser.ID, + } + + // Do some funky stuff with the action run's ids: + // The run with the larger ID finished first. + // This is odd but something that must work. + run1 := &actions_model.ActionRun{ID: 2, Repo: &repo, RepoID: repo.ID, Title: "some workflow", TriggerUser: triggerUser, TriggerUserID: triggerUser.ID, Status: actions_model.StatusFailure, Stopped: 1745821796, TriggerEvent: "workflow_dispatch"} + run2 := &actions_model.ActionRun{ID: 1, Repo: &repo, RepoID: repo.ID, Title: "some workflow", TriggerUser: triggerUser, TriggerUserID: triggerUser.ID, Status: actions_model.StatusSuccess, Stopped: 1745822796, TriggerEvent: "push"} + + notify_service.RegisterNotifier(NewNotifier()) + + t.Run("DontSendNotificationEmailOnFirstActionSuccess", func(t *testing.T) { + defer MockMailSettings(func(msgs ...*Message) { + assert.Fail(t, "no mail should be sent") + })() + notify_service.ActionRunNowDone(ctx, run2, actions_model.StatusRunning, nil) + }) + + t.Run("SendNotificationEmailOnActionRunFailed", func(t *testing.T) { + mailSentToOwner := false + mailSentToTriggerUser := false + defer MockMailSettings(func(msgs ...*Message) { + assert.LessOrEqual(t, len(msgs), 2) + for _, msg := range msgs { + switch msg.To { + case triggerUser.EmailTo(): + assert.False(t, mailSentToTriggerUser, "sent mail twice") + mailSentToTriggerUser = true + case ownerUser.EmailTo(): + assert.False(t, mailSentToOwner, "sent mail twice") + mailSentToOwner = true + default: + assert.Fail(t, "sent mail to unknown sender", msg.To) + } + assert.Contains(t, msg.Body, triggerUser.HTMLURL()) + assert.Contains(t, msg.Body, triggerUser.Name) + // what happened + assert.Contains(t, msg.Body, "failed") + // new status of run + assert.Contains(t, msg.Body, "failure") + // prior status of this run + assert.Contains(t, msg.Body, "waiting") + assertTranslatedLocaleMailActionsNowDone(t, msg.Body) + } + })() + notify_service.ActionRunNowDone(ctx, run1, actions_model.StatusWaiting, nil) + assert.True(t, mailSentToOwner) + assert.True(t, mailSentToTriggerUser) + }) + + t.Run("SendNotificationEmailOnActionRunRecovered", func(t *testing.T) { + mailSentToOwner := false + mailSentToTriggerUser := false + defer MockMailSettings(func(msgs ...*Message) { + assert.LessOrEqual(t, len(msgs), 2) + for _, msg := range msgs { + switch msg.To { + case triggerUser.EmailTo(): + assert.False(t, mailSentToTriggerUser, "sent mail twice") + mailSentToTriggerUser = true + case ownerUser.EmailTo(): + assert.False(t, mailSentToOwner, "sent mail twice") + mailSentToOwner = true + default: + assert.Fail(t, "sent mail to unknown sender", msg.To) + } + assert.Contains(t, msg.Body, triggerUser.HTMLURL()) + assert.Contains(t, msg.Body, triggerUser.Name) + // what happened + assert.Contains(t, msg.Body, "recovered") + // old status of run + assert.Contains(t, msg.Body, "failure") + // new status of run + assert.Contains(t, msg.Body, "success") + // prior status of this run + assert.Contains(t, msg.Body, "running") + assertTranslatedLocaleMailActionsNowDone(t, msg.Body) + } + })() + assert.NotNil(t, setting.MailService) + + notify_service.ActionRunNowDone(ctx, run2, actions_model.StatusRunning, run1) + assert.True(t, mailSentToOwner) + assert.True(t, mailSentToTriggerUser) + }) +} diff --git a/services/mailer/mail_admin_new_user_test.go b/services/mailer/mail_admin_new_user_test.go index 9273691792..58afcfcda6 100644 --- a/services/mailer/mail_admin_new_user_test.go +++ b/services/mailer/mail_admin_new_user_test.go @@ -4,7 +4,6 @@ package mailer import ( - "context" "strconv" "testing" @@ -17,7 +16,7 @@ import ( "github.com/stretchr/testify/require" ) -func getTestUsers(t *testing.T) []*user_model.User { +func getAdminNewUserTestUsers(t *testing.T) []*user_model.User { t.Helper() admin := new(user_model.User) admin.Name = "testadmin" @@ -38,16 +37,10 @@ func getTestUsers(t *testing.T) []*user_model.User { return []*user_model.User{admin, newUser} } -func cleanUpUsers(ctx context.Context, users []*user_model.User) { - for _, u := range users { - db.DeleteByID[user_model.User](ctx, u.ID) - } -} - func TestAdminNotificationMail_test(t *testing.T) { ctx := t.Context() - users := getTestUsers(t) + users := getAdminNewUserTestUsers(t) t.Run("SendNotificationEmailOnNewUser_true", func(t *testing.T) { defer test.MockVariableValue(&setting.Admin.SendNotificationEmailOnNewUser, true)() @@ -75,5 +68,5 @@ func TestAdminNotificationMail_test(t *testing.T) { MailNewUser(ctx, users[1]) }) - cleanUpUsers(ctx, users) + CleanUpUsers(ctx, users) } diff --git a/services/mailer/main_test.go b/services/mailer/main_test.go index 9ef71dbdb3..47e5d5d175 100644 --- a/services/mailer/main_test.go +++ b/services/mailer/main_test.go @@ -7,7 +7,9 @@ import ( "context" "testing" + "forgejo.org/models/db" "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" "forgejo.org/modules/setting" "forgejo.org/modules/templates" "forgejo.org/modules/test" @@ -46,3 +48,9 @@ func MockMailSettings(send func(msgs ...*Message)) func() { } } } + +func CleanUpUsers(ctx context.Context, users []*user_model.User) { + for _, u := range users { + db.DeleteByID[user_model.User](ctx, u.ID) + } +} diff --git a/services/mailer/notify.go b/services/mailer/notify.go index 8acfa86fb6..7461a67181 100644 --- a/services/mailer/notify.go +++ b/services/mailer/notify.go @@ -7,6 +7,7 @@ import ( "context" "fmt" + actions_model "forgejo.org/models/actions" activities_model "forgejo.org/models/activities" issues_model "forgejo.org/models/issues" repo_model "forgejo.org/models/repo" @@ -208,3 +209,13 @@ func (m *mailNotifier) RepoPendingTransfer(ctx context.Context, doer, newOwner * func (m *mailNotifier) NewUserSignUp(ctx context.Context, newUser *user_model.User) { MailNewUser(ctx, newUser) } + +func (m *mailNotifier) ActionRunNowDone(ctx context.Context, run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) { + // Only send a mail on a successful run when the workflow recovered (i.e., the run before failed). + if run.Status.IsSuccess() && (lastRun == nil || lastRun.Status.IsSuccess()) { + return + } + if err := MailActionRun(run, priorStatus, lastRun); err != nil { + log.Error("MailActionRunNowDone: %v", err) + } +} diff --git a/templates/mail/actions/now_done.tmpl b/templates/mail/actions/now_done.tmpl new file mode 100644 index 0000000000..a890411055 --- /dev/null +++ b/templates/mail/actions/now_done.tmpl @@ -0,0 +1,38 @@ + + + + + + + +{{$repo_link := HTMLFormat "%s" .Run.Repo.HTMLURL .RepoFullName}} +{{$action_run_link := HTMLFormat "%s" .Link .Run.Title}} +{{$trigger_user_link := HTMLFormat "@%s" .Run.TriggerUser.HTMLURL .Run.TriggerUser.Name}} + +

+ {{if .IsSuccess}} + {{.locale.Tr "mail.actions.successful_run_after_failure" $action_run_link $repo_link}} + {{else}} + {{.locale.Tr "mail.actions.not_successful_run" $action_run_link $repo_link}} + {{end}} + +
+ + {{.locale.Tr "mail.actions.run_info_cur_status" .Run.Status .PriorStatus}}
+ {{.locale.Tr "mail.actions.run_info_ref" .Branch .CommitSHA}}
+ {{if .LastRun}} + {{.locale.Tr "mail.actions.run_info_previous_status" .LastRun.Status}}
+ {{end}} + {{.locale.Tr "mail.actions.run_info_trigger" .Run.TriggerEvent $trigger_user_link}} +

+
+

+ --- +
+ {{.locale.Tr "mail.view_it_on" AppName}}. +

+
+ +