From 9f92c243082e8a4f599cc8991dd7772a9ce3ccbb Mon Sep 17 00:00:00 2001
From: Easton Crupper <65553218+ecrupper@users.noreply.github.com>
Date: Mon, 26 Jun 2023 14:17:45 -0600
Subject: [PATCH] fix(schedules): updated_at automatically updates to any
 change to row (#894)

* fix(schedules): updated_at automatically updates to any change to row

* revert docker compose

* change config to fields and improve comment

* what is your why
---
 api/schedule/create.go           |  2 +-
 api/schedule/update.go           |  2 +-
 cmd/vela-server/schedule.go      |  2 +-
 database/schedule/interface.go   |  2 +-
 database/schedule/update.go      | 19 +++++---
 database/schedule/update_test.go | 76 +++++++++++++++++++++++++++++++-
 6 files changed, 90 insertions(+), 13 deletions(-)

diff --git a/api/schedule/create.go b/api/schedule/create.go
index c8eb92741..365df170e 100644
--- a/api/schedule/create.go
+++ b/api/schedule/create.go
@@ -170,7 +170,7 @@ func CreateSchedule(c *gin.Context) {
 		dbSchedule.SetActive(true)
 
 		// send API call to update the schedule
-		err = database.FromContext(c).UpdateSchedule(dbSchedule)
+		err = database.FromContext(c).UpdateSchedule(dbSchedule, true)
 		if err != nil {
 			retErr := fmt.Errorf("unable to set schedule %s to active: %w", dbSchedule.GetName(), err)
 
diff --git a/api/schedule/update.go b/api/schedule/update.go
index 646e55fc2..b0862132a 100644
--- a/api/schedule/update.go
+++ b/api/schedule/update.go
@@ -123,7 +123,7 @@ func UpdateSchedule(c *gin.Context) {
 	}
 
 	// update the schedule within the database
-	err = database.FromContext(c).UpdateSchedule(s)
+	err = database.FromContext(c).UpdateSchedule(s, true)
 	if err != nil {
 		retErr := fmt.Errorf("unable to update scheduled %s: %w", scheduleName, err)
 
diff --git a/cmd/vela-server/schedule.go b/cmd/vela-server/schedule.go
index ab0a56b90..a9fd38234 100644
--- a/cmd/vela-server/schedule.go
+++ b/cmd/vela-server/schedule.go
@@ -350,7 +350,7 @@ func processSchedule(s *library.Schedule, compiler compiler.Engine, database dat
 	}
 
 	// send API call to update schedule for ensuring scheduled_at field is set
-	err = database.UpdateSchedule(s)
+	err = database.UpdateSchedule(s, false)
 	if err != nil {
 		return fmt.Errorf("unable to update schedule %s/%s: %w", r.GetFullName(), s.GetName(), err)
 	}
diff --git a/database/schedule/interface.go b/database/schedule/interface.go
index 8aa14efb5..afa163063 100644
--- a/database/schedule/interface.go
+++ b/database/schedule/interface.go
@@ -45,5 +45,5 @@ type ScheduleInterface interface {
 	// ListSchedulesForRepo defines a function that gets a list of schedules by repo ID.
 	ListSchedulesForRepo(*library.Repo, int, int) ([]*library.Schedule, int64, error)
 	// UpdateSchedule defines a function that updates an existing schedule.
-	UpdateSchedule(*library.Schedule) error
+	UpdateSchedule(*library.Schedule, bool) error
 }
diff --git a/database/schedule/update.go b/database/schedule/update.go
index b69f8eff0..dd03f62c5 100644
--- a/database/schedule/update.go
+++ b/database/schedule/update.go
@@ -2,7 +2,6 @@
 //
 // Use of this source code is governed by the LICENSE file in this repository.
 
-//nolint:dupl // ignore similar code with create.go
 package schedule
 
 import (
@@ -13,7 +12,7 @@ import (
 )
 
 // UpdateSchedule updates an existing schedule in the database.
-func (e *engine) UpdateSchedule(s *library.Schedule) error {
+func (e *engine) UpdateSchedule(s *library.Schedule, fields bool) error {
 	e.logger.WithFields(logrus.Fields{
 		"schedule": s.GetName(),
 	}).Tracef("updating schedule %s in the database", s.GetName())
@@ -27,9 +26,15 @@ func (e *engine) UpdateSchedule(s *library.Schedule) error {
 		return err
 	}
 
-	// send query to the database
-	return e.client.
-		Table(constants.TableSchedule).
-		Save(schedule).
-		Error
+	// If "fields" is true, update entire record; otherwise, just update scheduled_at (part of processSchedule)
+	//
+	// we do this because Gorm will automatically set `updated_at` with the Save function
+	// and the `updated_at` field should reflect the last time a user updated the record, rather than the scheduler
+	if fields {
+		err = e.client.Table(constants.TableSchedule).Save(schedule).Error
+	} else {
+		err = e.client.Table(constants.TableSchedule).Model(schedule).UpdateColumn("scheduled_at", s.GetScheduledAt()).Error
+	}
+
+	return err
 }
diff --git a/database/schedule/update_test.go b/database/schedule/update_test.go
index 05fff4286..2554cefdc 100644
--- a/database/schedule/update_test.go
+++ b/database/schedule/update_test.go
@@ -11,7 +11,7 @@ import (
 	"github.com/DATA-DOG/go-sqlmock"
 )
 
-func TestSchedule_Engine_UpdateSchedule(t *testing.T) {
+func TestSchedule_Engine_UpdateSchedule_Config(t *testing.T) {
 	_repo := testRepo()
 	_repo.SetID(1)
 	_repo.SetOrg("foo")
@@ -67,7 +67,79 @@ WHERE "id" = $10`).
 	// run tests
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err = test.database.UpdateSchedule(_schedule)
+			err = test.database.UpdateSchedule(_schedule, true)
+
+			if test.failure {
+				if err == nil {
+					t.Errorf("UpdateSchedule for %s should have returned err", test.name)
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Errorf("UpdateSchedule for %s returned err: %v", test.name, err)
+			}
+		})
+	}
+}
+
+func TestSchedule_Engine_UpdateSchedule_NotConfig(t *testing.T) {
+	_repo := testRepo()
+	_repo.SetID(1)
+	_repo.SetOrg("foo")
+	_repo.SetName("bar")
+	_repo.SetFullName("foo/bar")
+
+	_schedule := testSchedule()
+	_schedule.SetID(1)
+	_schedule.SetRepoID(1)
+	_schedule.SetName("nightly")
+	_schedule.SetEntry("0 0 * * *")
+	_schedule.SetCreatedAt(1)
+	_schedule.SetCreatedBy("user1")
+	_schedule.SetUpdatedAt(1)
+	_schedule.SetUpdatedBy("user2")
+	_schedule.SetScheduledAt(1)
+
+	_postgres, _mock := testPostgres(t)
+	defer func() { _sql, _ := _postgres.client.DB(); _sql.Close() }()
+
+	// ensure the mock expects the query
+	_mock.ExpectExec(`UPDATE "schedules" SET "scheduled_at"=$1 WHERE "id" = $2`).
+		WithArgs(1, 1).
+		WillReturnResult(sqlmock.NewResult(1, 1))
+
+	_sqlite := testSqlite(t)
+	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
+
+	err := _sqlite.CreateSchedule(_schedule)
+	if err != nil {
+		t.Errorf("unable to create test schedule for sqlite: %v", err)
+	}
+
+	// setup tests
+	tests := []struct {
+		failure  bool
+		name     string
+		database *engine
+	}{
+		{
+			failure:  false,
+			name:     "postgres",
+			database: _postgres,
+		},
+		{
+			failure:  false,
+			name:     "sqlite3",
+			database: _sqlite,
+		},
+	}
+
+	// run tests
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err = test.database.UpdateSchedule(_schedule, false)
 
 			if test.failure {
 				if err == nil {