Skip to content

Commit

Permalink
Merge branch 'fix/schedules' of github.com:go-vela/server into fix/sc…
Browse files Browse the repository at this point in the history
…hedules
  • Loading branch information
jbrockopp committed Jun 27, 2023
2 parents cc3b18c + 589a3ce commit 15d8699
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 13 deletions.
2 changes: 1 addition & 1 deletion api/schedule/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion api/schedule/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion cmd/vela-server/schedule.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,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)
}
Expand Down
2 changes: 1 addition & 1 deletion database/schedule/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
19 changes: 12 additions & 7 deletions database/schedule/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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())
Expand All @@ -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
}
76 changes: 74 additions & 2 deletions database/schedule/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 15d8699

Please sign in to comment.