diff --git a/services/mailer/mail_actions.go b/services/mailer/mail_actions.go index 09763e164e..fa0d2635f1 100644 --- a/services/mailer/mail_actions.go +++ b/services/mailer/mail_actions.go @@ -16,8 +16,8 @@ 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 { +var MailActionRun = mailActionRun // make it mockable +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 diff --git a/services/mailer/mail_actions_now_done_test.go b/services/mailer/mail_actions_now_done_test.go index 6a01ea7631..f4c597c99c 100644 --- a/services/mailer/mail_actions_now_done_test.go +++ b/services/mailer/mail_actions_now_done_test.go @@ -57,6 +57,91 @@ 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 TestActionRunNowDoneStatusMatrix(t *testing.T) { + successStatuses := []actions_model.Status{ + actions_model.StatusSuccess, + actions_model.StatusSkipped, + actions_model.StatusCancelled, + } + failureStatuses := []actions_model.Status{ + actions_model.StatusFailure, + } + + for _, testCase := range []struct { + name string + statuses []actions_model.Status + hasLastRun bool + lastStatuses []actions_model.Status + run bool + }{ + { + name: "FailureNoLastRun", + statuses: failureStatuses, + run: true, + }, + { + name: "SuccessNoLastRun", + statuses: successStatuses, + run: false, + }, + { + name: "FailureLastRunSuccess", + statuses: failureStatuses, + hasLastRun: true, + lastStatuses: successStatuses, + run: true, + }, + { + name: "FailureLastRunFailure", + statuses: failureStatuses, + hasLastRun: true, + lastStatuses: failureStatuses, + run: true, + }, + { + name: "SuccessLastRunFailure", + statuses: successStatuses, + hasLastRun: true, + lastStatuses: failureStatuses, + run: true, + }, + { + name: "SuccessLastRunSuccess", + statuses: successStatuses, + hasLastRun: true, + lastStatuses: successStatuses, + run: false, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + var called bool + defer test.MockVariableValue(&MailActionRun, func(run *actions_model.ActionRun, priorStatus actions_model.Status, lastRun *actions_model.ActionRun) error { + called = true + return nil + })() + for _, status := range testCase.statuses { + for _, lastStatus := range testCase.lastStatuses { + called = false + n := NewNotifier() + var lastRun *actions_model.ActionRun + if testCase.hasLastRun { + lastRun = &actions_model.ActionRun{ + Status: lastStatus, + } + } + n.ActionRunNowDone(t.Context(), + &actions_model.ActionRun{ + Status: status, + }, + actions_model.StatusUnknown, + lastRun) + assert.Equal(t, testCase.run, called, "status = %s, lastStatus = %s", status, lastStatus) + } + } + }) + } +} + func TestActionRunNowDoneNotificationMail(t *testing.T) { ctx := t.Context() diff --git a/services/mailer/notify.go b/services/mailer/notify.go index 7461a67181..640de31fcc 100644 --- a/services/mailer/notify.go +++ b/services/mailer/notify.go @@ -212,7 +212,7 @@ func (m *mailNotifier) NewUserSignUp(ctx context.Context, newUser *user_model.Us 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()) { + if !run.Status.IsFailure() && (lastRun == nil || !lastRun.Status.IsFailure()) { return } if err := MailActionRun(run, priorStatus, lastRun); err != nil {