diff --git a/models/actions/run.go b/models/actions/run.go index 55def805ed..69592120e9 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -284,16 +284,10 @@ func GetLatestRun(ctx context.Context, repoID int64) (*ActionRun, error) { return &run, nil } -// GetRunBefore returns the last run that completed a given timestamp (not inclusive). -func GetRunBefore(ctx context.Context, repoID int64, timestamp timeutil.TimeStamp) (*ActionRun, error) { - var run ActionRun - has, err := db.GetEngine(ctx).Where("repo_id=? AND stopped IS NOT NULL AND stopped v34 NewMigration("Add `notify-email` column to `action_run` table", AddNotifyEmailToActionRun), // v34 -> v35 - NewMigration("Add index to `stopped` column in `action_run` table", AddIndexToActionRunStopped), + NewMigration("Noop because of https://codeberg.org/forgejo/forgejo/issues/8373", NoopAddIndexToActionRunStopped), } // GetCurrentDBVersion returns the current Forgejo database version. diff --git a/models/forgejo_migrations/v35.go b/models/forgejo_migrations/v35.go index 0fb3b43e2c..ca412d7951 100644 --- a/models/forgejo_migrations/v35.go +++ b/models/forgejo_migrations/v35.go @@ -4,16 +4,10 @@ package forgejo_migrations //nolint:revive import ( - "forgejo.org/modules/timeutil" - "xorm.io/xorm" ) -func AddIndexToActionRunStopped(x *xorm.Engine) error { - type ActionRun struct { - ID int64 - Stopped timeutil.TimeStamp `xorm:"index"` - } - - return x.Sync(&ActionRun{}) +// see https://codeberg.org/forgejo/forgejo/issues/8373 +func NoopAddIndexToActionRunStopped(x *xorm.Engine) error { + return nil } diff --git a/services/actions/notifier.go b/services/actions/notifier.go index f1e9a6d7e9..a5bac730be 100644 --- a/services/actions/notifier.go +++ b/services/actions/notifier.go @@ -788,7 +788,7 @@ func (n *actionsNotifier) MigrateRepository(ctx context.Context, doer, u *user_m // the ActionRun of the same workflow that finished before priorRun/updatedRun. func sendActionRunNowDoneNotificationIfNeeded(ctx context.Context, priorRun, updatedRun *actions_model.ActionRun) error { if !priorRun.Status.IsDone() && updatedRun.Status.IsDone() { - lastRun, err := actions_model.GetRunBefore(ctx, updatedRun.RepoID, updatedRun.Stopped) + lastRun, err := actions_model.GetRunBefore(ctx, updatedRun) if err != nil && !errors.Is(err, util.ErrNotExist) { return err } diff --git a/tests/integration/actions_run_now_done_notification_test.go b/tests/integration/actions_run_now_done_notification_test.go index d5142096c5..480d67a73d 100644 --- a/tests/integration/actions_run_now_done_notification_test.go +++ b/tests/integration/actions_run_now_done_notification_test.go @@ -49,41 +49,22 @@ func (m *mockNotifier) ActionRunNowDone(ctx context.Context, run *actions_model. assert.Equal(m.t, m.runID, run.ID) assert.Equal(m.t, actions_model.StatusFailure, run.Status) assert.Equal(m.t, actions_model.StatusRunning, priorStatus) - assert.Equal(m.t, m.lastRunID, lastRun.ID) - assert.Equal(m.t, actions_model.StatusSuccess, lastRun.Status) assert.True(m.t, run.NotifyEmail) case 2: assert.Equal(m.t, m.runID, run.ID) assert.Equal(m.t, actions_model.StatusCancelled, run.Status) assert.Equal(m.t, actions_model.StatusRunning, priorStatus) - assert.Equal(m.t, m.lastRunID, lastRun.ID) - assert.Equal(m.t, actions_model.StatusFailure, lastRun.Status) - assert.True(m.t, run.NotifyEmail) - case 3: - assert.Equal(m.t, m.runID, run.ID) - assert.Equal(m.t, actions_model.StatusSuccess, run.Status) - assert.Equal(m.t, actions_model.StatusRunning, priorStatus) - assert.Equal(m.t, m.lastRunID, lastRun.ID) - assert.Equal(m.t, actions_model.StatusCancelled, lastRun.Status) - assert.True(m.t, run.NotifyEmail) - case 4: - assert.Equal(m.t, m.runID, run.ID) - assert.Equal(m.t, actions_model.StatusSuccess, run.Status) - assert.Equal(m.t, actions_model.StatusRunning, priorStatus) - assert.Equal(m.t, m.lastRunID, lastRun.ID) - assert.Equal(m.t, actions_model.StatusSuccess, lastRun.Status) assert.True(m.t, run.NotifyEmail) default: assert.Fail(m.t, "too many notifications") } - m.lastRunID = m.runID m.runID++ m.testIdx++ } // ensure all tests have been run func (m *mockNotifier) complete() { - assert.Equal(m.t, 5, m.testIdx) + assert.Equal(m.t, 3, m.testIdx) } func TestActionNowDoneNotification(t *testing.T) { @@ -159,24 +140,6 @@ func TestActionNowDoneNotification(t *testing.T) { task = runner.fetchTask(t) require.NoError(t, actions_service.StopTask(db.DefaultContext, task.Id, actions_model.StatusCancelled)) - // we can't differentiate different runs without a delay - time.Sleep(time.Millisecond * 2000) - - // 3: successful run after failure - _, _, err = workflow.Dispatch(db.DefaultContext, inputGetter, repo, user2) - require.NoError(t, err) - task = runner.fetchTask(t) - runner.succeedAtTask(t, task) - - // we can't differentiate different runs without a delay - time.Sleep(time.Millisecond * 2000) - - // 4: successful run after success - _, _, err = workflow.Dispatch(db.DefaultContext, inputGetter, repo, user2) - require.NoError(t, err) - task = runner.fetchTask(t) - runner.succeedAtTask(t, task) - notifier.complete() }) }