refactor(db): return schedule on create and update (#915)

* refactor(db): return schedule on create and update

* fix integration tests

---------

Co-authored-by: David May <49894298+wass3rw3rk@users.noreply.github.com>

api/schedule/create.go

@@ -171,30 +171,24 @@ func CreateSchedule(c *gin.Context) {
 		dbSchedule.SetActive(true)
 
 		// send API call to update the schedule
-		err = database.FromContext(c).UpdateSchedule(ctx, dbSchedule, true)
+		s, err = database.FromContext(c).UpdateSchedule(ctx, dbSchedule, true)
 		if err != nil {
 			retErr := fmt.Errorf("unable to set schedule %s to active: %w", dbSchedule.GetName(), err)
 
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
 			return
 		}
-
-		// send API call to capture the updated schedule
-		s, _ = database.FromContext(c).GetScheduleForRepo(ctx, r, dbSchedule.GetName())
 	} else {
 		// send API call to create the schedule
-		err = database.FromContext(c).CreateSchedule(ctx, s)
+		s, err = database.FromContext(c).CreateSchedule(ctx, s)
 		if err != nil {
 			retErr := fmt.Errorf("unable to create new schedule %s: %w", r.GetName(), err)
 
 			util.HandleError(c, http.StatusInternalServerError, retErr)
 
 			return
 		}
-
-		// send API call to capture the created schedule
-		s, _ = database.FromContext(c).GetScheduleForRepo(ctx, r, input.GetName())
 	}
 
 	c.JSON(http.StatusCreated, s)

api/schedule/update.go

@@ -129,7 +129,7 @@ func UpdateSchedule(c *gin.Context) {
 	s.SetUpdatedBy(u.GetName())
 
 	// update the schedule within the database
-	err = database.FromContext(c).UpdateSchedule(ctx, s, true)
+	s, err = database.FromContext(c).UpdateSchedule(ctx, s, true)
 	if err != nil {
 		retErr := fmt.Errorf("unable to update scheduled %s: %w", scheduleName, err)
 
@@ -138,8 +138,5 @@ func UpdateSchedule(c *gin.Context) {
 		return
 	}
 
-	// capture the updated scheduled
-	s, _ = database.FromContext(c).GetScheduleForRepo(ctx, r, scheduleName)
-
 	c.JSON(http.StatusOK, s)
 }

cmd/vela-server/schedule.go

@@ -116,7 +116,7 @@ func processSchedules(ctx context.Context, start time.Time, compiler compiler.En
 		schedule.SetScheduledAt(time.Now().UTC().Unix())
 
 		// send API call to update schedule for ensuring scheduled_at field is set
-		err = database.UpdateSchedule(ctx, schedule, false)
+		_, err = database.UpdateSchedule(ctx, schedule, false)
 		if err != nil {
 			logrus.WithError(err).Warnf("%s %s", scheduleErr, schedule.GetName())
 

database/integration_test.go

@@ -928,7 +928,7 @@ func testSchedules(t *testing.T, db Interface, resources *Resources) {
 
 	// create the schedules
 	for _, schedule := range resources.Schedules {
-		err := db.CreateSchedule(ctx, schedule)
+		_, err := db.CreateSchedule(ctx, schedule)
 		if err != nil {
 			t.Errorf("unable to create schedule %d: %v", schedule.GetID(), err)
 		}
@@ -1004,16 +1004,11 @@ func testSchedules(t *testing.T, db Interface, resources *Resources) {
 	// update the schedules
 	for _, schedule := range resources.Schedules {
 		schedule.SetUpdatedAt(time.Now().UTC().Unix())
-		err = db.UpdateSchedule(ctx, schedule, true)
+		got, err := db.UpdateSchedule(ctx, schedule, true)
 		if err != nil {
 			t.Errorf("unable to update schedule %d: %v", schedule.GetID(), err)
 		}
 
-		// lookup the schedule by ID
-		got, err := db.GetSchedule(ctx, schedule.GetID())
-		if err != nil {
-			t.Errorf("unable to get schedule %d by ID: %v", schedule.GetID(), err)
-		}
 		if !reflect.DeepEqual(got, schedule) {
 			t.Errorf("GetSchedule() is %v, want %v", got, schedule)
 		}

database/schedule/count_active_test.go

@@ -47,12 +47,12 @@ func TestSchedule_Engine_CountActiveSchedules(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
+	_, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/count_repo_test.go

@@ -51,12 +51,12 @@ func TestSchedule_Engine_CountSchedulesForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
+	_, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/count_test.go

@@ -45,12 +45,12 @@ func TestSchedule_Engine_CountSchedules(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
+	_, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/create.go

@@ -7,14 +7,15 @@ package schedule
 
 import (
 	"context"
+
 	"github.com/go-vela/types/constants"
 	"github.com/go-vela/types/database"
 	"github.com/go-vela/types/library"
 	"github.com/sirupsen/logrus"
 )
 
 // CreateSchedule creates a new schedule in the database.
-func (e *engine) CreateSchedule(ctx context.Context, s *library.Schedule) error {
+func (e *engine) CreateSchedule(ctx context.Context, s *library.Schedule) (*library.Schedule, error) {
 	e.logger.WithFields(logrus.Fields{
 		"schedule": s.GetName(),
 	}).Tracef("creating schedule %s in the database", s.GetName())
@@ -25,12 +26,11 @@ func (e *engine) CreateSchedule(ctx context.Context, s *library.Schedule) error
 	// validate the necessary fields are populated
 	err := schedule.Validate()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// send query to the database
-	return e.client.
-		Table(constants.TableSchedule).
-		Create(schedule).
-		Error
+	result := e.client.Table(constants.TableSchedule).Create(schedule)
+
+	return schedule.ToLibrary(), result.Error
 }

database/schedule/create_test.go

@@ -6,6 +6,7 @@ package schedule
 
 import (
 	"context"
+	"reflect"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -59,7 +60,7 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10) RETURNING "id"`).
 	// run tests
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err := test.database.CreateSchedule(context.TODO(), _schedule)
+			got, err := test.database.CreateSchedule(context.TODO(), _schedule)
 
 			if test.failure {
 				if err == nil {
@@ -72,6 +73,10 @@ VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10) RETURNING "id"`).
 			if err != nil {
 				t.Errorf("CreateSchedule for %s returned err: %v", test.name, err)
 			}
+
+			if !reflect.DeepEqual(got, _schedule) {
+				t.Errorf("CreateSchedule for %s returned %s, want %s", test.name, got, _schedule)
+			}
 		})
 	}
 }

database/schedule/delete_test.go

@@ -33,7 +33,7 @@ func TestSchedule_Engine_DeleteSchedule(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _schedule)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _schedule)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/get_repo_test.go

@@ -44,7 +44,7 @@ func TestSchedule_Engine_GetScheduleForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _schedule)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _schedule)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/get_test.go

@@ -38,7 +38,7 @@ func TestSchedule_Engine_GetSchedule(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _schedule)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _schedule)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/interface.go

@@ -6,6 +6,7 @@ package schedule
 
 import (
 	"context"
+
 	"github.com/go-vela/types/library"
 )
 
@@ -32,7 +33,7 @@ type ScheduleInterface interface {
 	// CountSchedulesForRepo defines a function that gets the count of schedules by repo ID.
 	CountSchedulesForRepo(context.Context, *library.Repo) (int64, error)
 	// CreateSchedule defines a function that creates a new schedule.
-	CreateSchedule(context.Context, *library.Schedule) error
+	CreateSchedule(context.Context, *library.Schedule) (*library.Schedule, error)
 	// DeleteSchedule defines a function that deletes an existing schedule.
 	DeleteSchedule(context.Context, *library.Schedule) error
 	// GetSchedule defines a function that gets a schedule by ID.
@@ -46,5 +47,5 @@ type ScheduleInterface interface {
 	// ListSchedulesForRepo defines a function that gets a list of schedules by repo ID.
 	ListSchedulesForRepo(context.Context, *library.Repo, int, int) ([]*library.Schedule, int64, error)
 	// UpdateSchedule defines a function that updates an existing schedule.
-	UpdateSchedule(context.Context, *library.Schedule, bool) error
+	UpdateSchedule(context.Context, *library.Schedule, bool) (*library.Schedule, error)
 }

database/schedule/list_active_test.go

@@ -56,12 +56,12 @@ func TestSchedule_Engine_ListActiveSchedules(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
+	_, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/list_repo_test.go

@@ -60,12 +60,12 @@ func TestSchedule_Engine_ListSchedulesForRepo(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
+	_, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/list_test.go

@@ -55,12 +55,12 @@ func TestSchedule_Engine_ListSchedules(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _scheduleOne)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}
 
-	err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
+	_, err = _sqlite.CreateSchedule(context.TODO(), _scheduleTwo)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}

database/schedule/update.go

@@ -6,14 +6,15 @@ package schedule
 
 import (
 	"context"
+
 	"github.com/go-vela/types/constants"
 	"github.com/go-vela/types/database"
 	"github.com/go-vela/types/library"
 	"github.com/sirupsen/logrus"
 )
 
 // UpdateSchedule updates an existing schedule in the database.
-func (e *engine) UpdateSchedule(ctx context.Context, s *library.Schedule, fields bool) error {
+func (e *engine) UpdateSchedule(ctx context.Context, s *library.Schedule, fields bool) (*library.Schedule, error) {
 	e.logger.WithFields(logrus.Fields{
 		"schedule": s.GetName(),
 	}).Tracef("updating schedule %s in the database", s.GetName())
@@ -24,7 +25,7 @@ func (e *engine) UpdateSchedule(ctx context.Context, s *library.Schedule, fields
 	// validate the necessary fields are populated
 	err := schedule.Validate()
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// If "fields" is true, update entire record; otherwise, just update scheduled_at (part of processSchedule)
@@ -37,5 +38,5 @@ func (e *engine) UpdateSchedule(ctx context.Context, s *library.Schedule, fields
 		err = e.client.Table(constants.TableSchedule).Model(schedule).UpdateColumn("scheduled_at", s.GetScheduledAt()).Error
 	}
 
-	return err
+	return schedule.ToLibrary(), err
 }

database/schedule/update_test.go

@@ -6,6 +6,7 @@ package schedule
 
 import (
 	"context"
+	"reflect"
 	"testing"
 
 	"github.com/DATA-DOG/go-sqlmock"
@@ -41,7 +42,7 @@ WHERE "id" = $10`).
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _schedule)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _schedule)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}
@@ -67,7 +68,8 @@ WHERE "id" = $10`).
 	// run tests
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err = test.database.UpdateSchedule(context.TODO(), _schedule, true)
+			got, err := test.database.UpdateSchedule(context.TODO(), _schedule, true)
+			_schedule.SetUpdatedAt(got.GetUpdatedAt())
 
 			if test.failure {
 				if err == nil {
@@ -80,6 +82,10 @@ WHERE "id" = $10`).
 			if err != nil {
 				t.Errorf("UpdateSchedule for %s returned err: %v", test.name, err)
 			}
+
+			if !reflect.DeepEqual(got, _schedule) {
+				t.Errorf("UpdateSchedule for %s returned %s, want %s", test.name, got, _schedule)
+			}
 		})
 	}
 }
@@ -113,7 +119,7 @@ func TestSchedule_Engine_UpdateSchedule_NotConfig(t *testing.T) {
 	_sqlite := testSqlite(t)
 	defer func() { _sql, _ := _sqlite.client.DB(); _sql.Close() }()
 
-	err := _sqlite.CreateSchedule(context.TODO(), _schedule)
+	_, err := _sqlite.CreateSchedule(context.TODO(), _schedule)
 	if err != nil {
 		t.Errorf("unable to create test schedule for sqlite: %v", err)
 	}
@@ -139,7 +145,7 @@ func TestSchedule_Engine_UpdateSchedule_NotConfig(t *testing.T) {
 	// run tests
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			err = test.database.UpdateSchedule(context.TODO(), _schedule, false)
+			got, err := test.database.UpdateSchedule(context.TODO(), _schedule, false)
 
 			if test.failure {
 				if err == nil {
@@ -152,6 +158,10 @@ func TestSchedule_Engine_UpdateSchedule_NotConfig(t *testing.T) {
 			if err != nil {
 				t.Errorf("UpdateSchedule for %s returned err: %v", test.name, err)
 			}
+
+			if !reflect.DeepEqual(got, _schedule) {
+				t.Errorf("CreateSchedule for %s returned %s, want %s", test.name, got, _schedule)
+			}
 		})
 	}
 }