From 5766918248dd5cd4a7b7293f202f6911766bf9ca Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 1 Dec 2022 14:34:56 +0000 Subject: [PATCH 01/15] Get previous template version --- coderd/database/databasefake/databasefake.go | 19 +++++++++++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 28 ++++++++++++++++++++ coderd/database/queries/templateversions.sql | 10 +++++++ 4 files changed, 58 insertions(+) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index a91e2d1962980..4acbd917423a4 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1717,6 +1717,25 @@ func (q *fakeQuerier) GetTemplateVersionByJobID(_ context.Context, jobID uuid.UU return database.TemplateVersion{}, sql.ErrNoRows } +func (q *fakeQuerier) GetPreviousTemplateVersionByID(_ context.Context, id uuid.UUID) (database.TemplateVersion, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + var previousIndex = -1 + for index, templateVersion := range q.templateVersions { + if templateVersion.ID != id { + continue + } + previousIndex = index - 1 + } + + if previousIndex < 0 { + return database.TemplateVersion{}, sql.ErrNoRows + } + + return q.templateVersions[previousIndex], nil +} + func (q *fakeQuerier) GetParameterSchemasByJobID(_ context.Context, jobID uuid.UUID) ([]database.ParameterSchema, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 2f03e2533e45e..60c57ce941a45 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -65,6 +65,7 @@ type sqlcQuerier interface { GetParameterSchemasByJobID(ctx context.Context, jobID uuid.UUID) ([]ParameterSchema, error) GetParameterSchemasCreatedAfter(ctx context.Context, createdAt time.Time) ([]ParameterSchema, error) GetParameterValueByScopeAndName(ctx context.Context, arg GetParameterValueByScopeAndNameParams) (ParameterValue, error) + GetPreviousTemplateVersionByID(ctx context.Context, id uuid.UUID) (TemplateVersion, error) GetProvisionerDaemonByID(ctx context.Context, id uuid.UUID) (ProvisionerDaemon, error) GetProvisionerDaemons(ctx context.Context) ([]ProvisionerDaemon, error) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e7b5459c9c48d..72a9929cde7c2 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3480,6 +3480,34 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl return i, err } +const getPreviousTemplateVersionByID = `-- name: GetPreviousTemplateVersionByID :one +SELECT + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by +FROM + template_versions +WHERE + created_at < (SELECT created_at FROM template_versions WHERE template_versions.id = $1) +ORDER BY created_at DESC +LIMIT 1 +` + +func (q *sqlQuerier) GetPreviousTemplateVersionByID(ctx context.Context, id uuid.UUID) (TemplateVersion, error) { + row := q.db.QueryRowContext(ctx, getPreviousTemplateVersionByID, id) + var i TemplateVersion + err := row.Scan( + &i.ID, + &i.TemplateID, + &i.OrganizationID, + &i.CreatedAt, + &i.UpdatedAt, + &i.Name, + &i.Readme, + &i.JobID, + &i.CreatedBy, + ) + return i, err +} + const getTemplateVersionByID = `-- name: GetTemplateVersionByID :one SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index aad417b5b3590..5daf5ac9c0b5c 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -110,3 +110,13 @@ SET updated_at = $3 WHERE job_id = $1; + +-- name: GetPreviousTemplateVersionByID :one +SELECT + * +FROM + template_versions +WHERE + created_at < (SELECT created_at FROM template_versions WHERE template_versions.id = @id) +ORDER BY created_at DESC +LIMIT 1; From e64b8c9f14a290f652776fb2f0d6e48859051871 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 1 Dec 2022 17:07:21 +0000 Subject: [PATCH 02/15] Sort fake database method --- coderd/database/databasefake/databasefake.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 4acbd917423a4..f3a27d98a5c8d 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1721,8 +1721,13 @@ func (q *fakeQuerier) GetPreviousTemplateVersionByID(_ context.Context, id uuid. q.mutex.RLock() defer q.mutex.RUnlock() + templateVersions := slices.Clone(q.templateVersions) + slices.SortFunc(templateVersions, func(i, j database.TemplateVersion) bool { + return i.CreatedAt.After(j.CreatedAt) + }) + var previousIndex = -1 - for index, templateVersion := range q.templateVersions { + for index, templateVersion := range templateVersions { if templateVersion.ID != id { continue } @@ -1733,7 +1738,7 @@ func (q *fakeQuerier) GetPreviousTemplateVersionByID(_ context.Context, id uuid. return database.TemplateVersion{}, sql.ErrNoRows } - return q.templateVersions[previousIndex], nil + return templateVersions[previousIndex], nil } func (q *fakeQuerier) GetParameterSchemasByJobID(_ context.Context, jobID uuid.UUID) ([]database.ParameterSchema, error) { From 0d411a07eb039e98d98da3eac14acb5770c2bd99 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 1 Dec 2022 18:03:02 +0000 Subject: [PATCH 03/15] Add route and test --- coderd/coderd.go | 1 + coderd/database/databasefake/databasefake.go | 7 +++- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 17 ++++++-- coderd/database/queries/templateversions.sql | 8 +++- coderd/templateversions.go | 42 ++++++++++++++++++++ coderd/templateversions_test.go | 33 +++++++++++++++ codersdk/templateversions.go | 13 ++++++ 8 files changed, 114 insertions(+), 9 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index 60103ea8a72b1..6577e7e4edb1e 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -343,6 +343,7 @@ func New(options *Options) *API { r.Route("/templateversions", func(r chi.Router) { r.Post("/", api.postTemplateVersionsByOrganization) r.Get("/{templateversionname}", api.templateVersionByOrganizationAndName) + r.Get("/{templateversionname}/previous", api.previousTemplateVersionByOrganizationAndName) }) r.Route("/templates", func(r chi.Router) { r.Post("/", api.postTemplateByOrganization) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index f3a27d98a5c8d..3ebe22bfcd2ce 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1717,7 +1717,7 @@ func (q *fakeQuerier) GetTemplateVersionByJobID(_ context.Context, jobID uuid.UU return database.TemplateVersion{}, sql.ErrNoRows } -func (q *fakeQuerier) GetPreviousTemplateVersionByID(_ context.Context, id uuid.UUID) (database.TemplateVersion, error) { +func (q *fakeQuerier) GetPreviousTemplateVersionByOrganizationAndName(_ context.Context, arg database.GetPreviousTemplateVersionByOrganizationAndNameParams) (database.TemplateVersion, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -1728,7 +1728,10 @@ func (q *fakeQuerier) GetPreviousTemplateVersionByID(_ context.Context, id uuid. var previousIndex = -1 for index, templateVersion := range templateVersions { - if templateVersion.ID != id { + if templateVersion.OrganizationID != arg.OrganizationID { + continue + } + if !strings.EqualFold(templateVersion.Name, arg.Name) { continue } previousIndex = index - 1 diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 60c57ce941a45..cb61b209c5e2a 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -65,7 +65,7 @@ type sqlcQuerier interface { GetParameterSchemasByJobID(ctx context.Context, jobID uuid.UUID) ([]ParameterSchema, error) GetParameterSchemasCreatedAfter(ctx context.Context, createdAt time.Time) ([]ParameterSchema, error) GetParameterValueByScopeAndName(ctx context.Context, arg GetParameterValueByScopeAndNameParams) (ParameterValue, error) - GetPreviousTemplateVersionByID(ctx context.Context, id uuid.UUID) (TemplateVersion, error) + GetPreviousTemplateVersionByOrganizationAndName(ctx context.Context, arg GetPreviousTemplateVersionByOrganizationAndNameParams) (TemplateVersion, error) GetProvisionerDaemonByID(ctx context.Context, id uuid.UUID) (ProvisionerDaemon, error) GetProvisionerDaemons(ctx context.Context) ([]ProvisionerDaemon, error) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 72a9929cde7c2..65e5772f46a97 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3480,19 +3480,28 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl return i, err } -const getPreviousTemplateVersionByID = `-- name: GetPreviousTemplateVersionByID :one +const getPreviousTemplateVersionByOrganizationAndName = `-- name: GetPreviousTemplateVersionByOrganizationAndName :one SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by FROM template_versions WHERE - created_at < (SELECT created_at FROM template_versions WHERE template_versions.id = $1) + created_at < ( + SELECT created_at + FROM template_versions as tv + WHERE tv.organization_id = $1 AND tv.name = $2 + ) ORDER BY created_at DESC LIMIT 1 ` -func (q *sqlQuerier) GetPreviousTemplateVersionByID(ctx context.Context, id uuid.UUID) (TemplateVersion, error) { - row := q.db.QueryRowContext(ctx, getPreviousTemplateVersionByID, id) +type GetPreviousTemplateVersionByOrganizationAndNameParams struct { + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + Name string `db:"name" json:"name"` +} + +func (q *sqlQuerier) GetPreviousTemplateVersionByOrganizationAndName(ctx context.Context, arg GetPreviousTemplateVersionByOrganizationAndNameParams) (TemplateVersion, error) { + row := q.db.QueryRowContext(ctx, getPreviousTemplateVersionByOrganizationAndName, arg.OrganizationID, arg.Name) var i TemplateVersion err := row.Scan( &i.ID, diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index 5daf5ac9c0b5c..37da658910fdb 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -111,12 +111,16 @@ SET WHERE job_id = $1; --- name: GetPreviousTemplateVersionByID :one +-- name: GetPreviousTemplateVersionByOrganizationAndName :one SELECT * FROM template_versions WHERE - created_at < (SELECT created_at FROM template_versions WHERE template_versions.id = @id) + created_at < ( + SELECT created_at + FROM template_versions as tv + WHERE tv.organization_id = $1 AND tv.name = $2 + ) ORDER BY created_at DESC LIMIT 1; diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 2952baede44e8..f07214bbbd96e 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -640,6 +640,48 @@ func (api *API) templateVersionByOrganizationAndName(rw http.ResponseWriter, r * httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(job), user)) } +func (api *API) previousTemplateVersionByOrganizationAndName(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + organization := httpmw.OrganizationParam(r) + templateVersionName := chi.URLParam(r, "templateversionname") + templateVersion, err := api.Database.GetPreviousTemplateVersionByOrganizationAndName(ctx, database.GetPreviousTemplateVersionByOrganizationAndNameParams{ + OrganizationID: organization.ID, + Name: templateVersionName, + }) + if errors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: fmt.Sprintf("No previous template version found for name %q.", templateVersionName), + }) + return + } + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching the previous template version.", + Detail: err.Error(), + }) + return + } + job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching provisioner job.", + Detail: err.Error(), + }) + return + } + + user, err := api.Database.GetUserByID(ctx, templateVersion.CreatedBy) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error on fetching user.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(job), user)) +} + func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Request) { var ( ctx = r.Context() diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 65c1ec31d06e6..2ff9ff80cf073 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -963,3 +963,36 @@ func TestTemplateVersionByOrganizationAndName(t *testing.T) { require.NoError(t, err) }) } + +func TestPreviousTemplateVersion(t *testing.T) { + t.Parallel() + t.Run("Not found", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + _, err := client.PreviousTemplateVersion(ctx, user.OrganizationID, version.Name) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) + }) + + t.Run("Found", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + previousVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + latestVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + _, err := client.PreviousTemplateVersion(ctx, user.OrganizationID, latestVersion.Name) + require.NoError(t, err) + require.Equal(t, previousVersion.ID, latestVersion.ID) + }) +} diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index 3273b31b28f86..b9ea141f50654 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -182,3 +182,16 @@ func (c *Client) CancelTemplateVersionDryRun(ctx context.Context, version, job u } return nil } + +func (c *Client) PreviousTemplateVersion(ctx context.Context, organization uuid.UUID, versionName string) (TemplateVersion, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/organizations/%s/templateversions/%s/previous", organization, versionName), nil) + if err != nil { + return TemplateVersion{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return TemplateVersion{}, readBodyAsError(res) + } + var version TemplateVersion + return version, json.NewDecoder(res.Body).Decode(&version) +} From 24039125fcd781cce4466027ca7d4816a42c41b7 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 1 Dec 2022 18:07:22 +0000 Subject: [PATCH 04/15] Fic test --- coderd/templateversions_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 2ff9ff80cf073..c7cb7539d1586 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -986,7 +986,7 @@ func TestPreviousTemplateVersion(t *testing.T) { client := coderdtest.New(t, nil) user := coderdtest.CreateFirstUser(t, client) previousVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - latestVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + latestVersion := coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, *previousVersion.TemplateID) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() From 764a596f002c142441b9b100452020537d634c03 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 1 Dec 2022 18:43:34 +0000 Subject: [PATCH 05/15] Scope previous version to the template id --- coderd/database/databasefake/databasefake.go | 28 +++++++++++++------- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 15 ++++++----- coderd/database/queries/templateversions.sql | 4 +-- coderd/templateversions.go | 28 +++++++++++++++++--- 5 files changed, 53 insertions(+), 24 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 3ebe22bfcd2ce..3e7d99b39e618 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1717,31 +1717,39 @@ func (q *fakeQuerier) GetTemplateVersionByJobID(_ context.Context, jobID uuid.UU return database.TemplateVersion{}, sql.ErrNoRows } -func (q *fakeQuerier) GetPreviousTemplateVersionByOrganizationAndName(_ context.Context, arg database.GetPreviousTemplateVersionByOrganizationAndNameParams) (database.TemplateVersion, error) { +func (q *fakeQuerier) GetPreviousTemplateVersion(_ context.Context, arg database.GetPreviousTemplateVersionParams) (database.TemplateVersion, error) { q.mutex.RLock() defer q.mutex.RUnlock() - templateVersions := slices.Clone(q.templateVersions) - slices.SortFunc(templateVersions, func(i, j database.TemplateVersion) bool { + sortedTemplateVersions := slices.Clone(q.templateVersions) + slices.SortFunc(sortedTemplateVersions, func(i, j database.TemplateVersion) bool { return i.CreatedAt.After(j.CreatedAt) }) - var previousIndex = -1 - for index, templateVersion := range templateVersions { + templateVersions := make([]database.TemplateVersion, 0) + for _, templateVersion := range sortedTemplateVersions { if templateVersion.OrganizationID != arg.OrganizationID { continue } - if !strings.EqualFold(templateVersion.Name, arg.Name) { + if templateVersion.TemplateID != arg.TemplateID { continue } - previousIndex = index - 1 + templateVersions = append(templateVersions, templateVersion) } - if previousIndex < 0 { - return database.TemplateVersion{}, sql.ErrNoRows + templateVersionIndex := -1 + for versionIndex, templateVersion := range templateVersions { + if strings.EqualFold(templateVersion.Name, arg.Name) { + templateVersionIndex = versionIndex + break + } } - return templateVersions[previousIndex], nil + if templateVersionIndex > 0 { + return templateVersions[templateVersionIndex], nil + } + + return database.TemplateVersion{}, sql.ErrNoRows } func (q *fakeQuerier) GetParameterSchemasByJobID(_ context.Context, jobID uuid.UUID) ([]database.ParameterSchema, error) { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index cb61b209c5e2a..c35e4be289aa1 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -65,7 +65,7 @@ type sqlcQuerier interface { GetParameterSchemasByJobID(ctx context.Context, jobID uuid.UUID) ([]ParameterSchema, error) GetParameterSchemasCreatedAfter(ctx context.Context, createdAt time.Time) ([]ParameterSchema, error) GetParameterValueByScopeAndName(ctx context.Context, arg GetParameterValueByScopeAndNameParams) (ParameterValue, error) - GetPreviousTemplateVersionByOrganizationAndName(ctx context.Context, arg GetPreviousTemplateVersionByOrganizationAndNameParams) (TemplateVersion, error) + GetPreviousTemplateVersion(ctx context.Context, arg GetPreviousTemplateVersionParams) (TemplateVersion, error) GetProvisionerDaemonByID(ctx context.Context, id uuid.UUID) (ProvisionerDaemon, error) GetProvisionerDaemons(ctx context.Context) ([]ProvisionerDaemon, error) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (ProvisionerJob, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 65e5772f46a97..9bc612798b8c9 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3480,7 +3480,7 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl return i, err } -const getPreviousTemplateVersionByOrganizationAndName = `-- name: GetPreviousTemplateVersionByOrganizationAndName :one +const getPreviousTemplateVersion = `-- name: GetPreviousTemplateVersion :one SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by FROM @@ -3489,19 +3489,20 @@ WHERE created_at < ( SELECT created_at FROM template_versions as tv - WHERE tv.organization_id = $1 AND tv.name = $2 + WHERE tv.organization_id = $1 AND tv.name = $2 AND tv.template_id = $3 ) ORDER BY created_at DESC LIMIT 1 ` -type GetPreviousTemplateVersionByOrganizationAndNameParams struct { - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` - Name string `db:"name" json:"name"` +type GetPreviousTemplateVersionParams struct { + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + Name string `db:"name" json:"name"` + TemplateID uuid.NullUUID `db:"template_id" json:"template_id"` } -func (q *sqlQuerier) GetPreviousTemplateVersionByOrganizationAndName(ctx context.Context, arg GetPreviousTemplateVersionByOrganizationAndNameParams) (TemplateVersion, error) { - row := q.db.QueryRowContext(ctx, getPreviousTemplateVersionByOrganizationAndName, arg.OrganizationID, arg.Name) +func (q *sqlQuerier) GetPreviousTemplateVersion(ctx context.Context, arg GetPreviousTemplateVersionParams) (TemplateVersion, error) { + row := q.db.QueryRowContext(ctx, getPreviousTemplateVersion, arg.OrganizationID, arg.Name, arg.TemplateID) var i TemplateVersion err := row.Scan( &i.ID, diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index 37da658910fdb..a380bb996dcf9 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -111,7 +111,7 @@ SET WHERE job_id = $1; --- name: GetPreviousTemplateVersionByOrganizationAndName :one +-- name: GetPreviousTemplateVersion :one SELECT * FROM @@ -120,7 +120,7 @@ WHERE created_at < ( SELECT created_at FROM template_versions as tv - WHERE tv.organization_id = $1 AND tv.name = $2 + WHERE tv.organization_id = $1 AND tv.name = $2 AND tv.template_id = $3 ) ORDER BY created_at DESC LIMIT 1; diff --git a/coderd/templateversions.go b/coderd/templateversions.go index f07214bbbd96e..23cecaaf812a3 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -644,13 +644,32 @@ func (api *API) previousTemplateVersionByOrganizationAndName(rw http.ResponseWri ctx := r.Context() organization := httpmw.OrganizationParam(r) templateVersionName := chi.URLParam(r, "templateversionname") - templateVersion, err := api.Database.GetPreviousTemplateVersionByOrganizationAndName(ctx, database.GetPreviousTemplateVersionByOrganizationAndNameParams{ + templateVersion, err := api.Database.GetTemplateVersionByOrganizationAndName(ctx, database.GetTemplateVersionByOrganizationAndNameParams{ OrganizationID: organization.ID, Name: templateVersionName, }) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: fmt.Sprintf("No previous template version found for name %q.", templateVersionName), + Message: fmt.Sprintf("No template version found by name %q.", templateVersionName), + }) + return + } + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching template version.", + Detail: err.Error(), + }) + return + } + + previousTemplateVersion, err := api.Database.GetPreviousTemplateVersion(ctx, database.GetPreviousTemplateVersionParams{ + OrganizationID: organization.ID, + Name: templateVersionName, + TemplateID: templateVersion.TemplateID, + }) + if errors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: fmt.Sprintf("No previous template version found for %q.", templateVersionName), }) return } @@ -661,7 +680,8 @@ func (api *API) previousTemplateVersionByOrganizationAndName(rw http.ResponseWri }) return } - job, err := api.Database.GetProvisionerJobByID(ctx, templateVersion.JobID) + + job, err := api.Database.GetProvisionerJobByID(ctx, previousTemplateVersion.JobID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching provisioner job.", @@ -679,7 +699,7 @@ func (api *API) previousTemplateVersionByOrganizationAndName(rw http.ResponseWri return } - httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(job), user)) + httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(previousTemplateVersion, convertProvisionerJob(job), user)) } func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Request) { From 4300414f17a423615dd7d968dbf85ee246358684 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 1 Dec 2022 19:48:20 +0000 Subject: [PATCH 06/15] Fix authorize tests --- coderd/coderdtest/authorize.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/coderd/coderdtest/authorize.go b/coderd/coderdtest/authorize.go index 6e22df4b249bd..457a008def074 100644 --- a/coderd/coderdtest/authorize.go +++ b/coderd/coderdtest/authorize.go @@ -235,11 +235,12 @@ func AGPLRoutes(a *AuthTester) (map[string]string, map[string]RouteCheck) { "GET:/api/v2/applications/auth-redirect": {AssertAction: rbac.ActionCreate, AssertObject: rbac.ResourceAPIKey}, // These endpoints need payloads to get to the auth part. Payloads will be required - "PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, - "PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true}, - "POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, - "POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, - "GET:/api/v2/organizations/{organization}/templateversions/{templateversionname}": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + "PUT:/api/v2/users/{user}/roles": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + "PUT:/api/v2/organizations/{organization}/members/{user}/roles": {NoAuthorize: true}, + "POST:/api/v2/workspaces/{workspace}/builds": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + "POST:/api/v2/organizations/{organization}/templateversions": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + "GET:/api/v2/organizations/{organization}/templateversions/{templateversionname}": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, + "GET:/api/v2/organizations/{organization}/templateversions/{templateversionname}/previous": {StatusCode: http.StatusBadRequest, NoAuthorize: true}, // Endpoints that use the SQLQuery filter. "GET:/api/v2/workspaces/": { From 30c586cde471bfe200a2a4a575b43dde12931ab9 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 1 Dec 2022 19:50:44 +0000 Subject: [PATCH 07/15] Fix test --- coderd/templateversions_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index c7cb7539d1586..63ab9cf39fe9a 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -966,7 +966,7 @@ func TestTemplateVersionByOrganizationAndName(t *testing.T) { func TestPreviousTemplateVersion(t *testing.T) { t.Parallel() - t.Run("Not found", func(t *testing.T) { + t.Run("Previous version not found", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) user := coderdtest.CreateFirstUser(t, client) @@ -981,12 +981,13 @@ func TestPreviousTemplateVersion(t *testing.T) { require.Equal(t, http.StatusNotFound, apiErr.StatusCode()) }) - t.Run("Found", func(t *testing.T) { + t.Run("Previous version found", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) user := coderdtest.CreateFirstUser(t, client) previousVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - latestVersion := coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, *previousVersion.TemplateID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, previousVersion.ID) + latestVersion := coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, template.ID) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() From 17ed909c8d21599936e359175bb82e7cd8007115 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Fri, 2 Dec 2022 12:49:07 +0000 Subject: [PATCH 08/15] Fix test --- coderd/templateversions_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 63ab9cf39fe9a..3c752253f61cb 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -992,8 +992,8 @@ func TestPreviousTemplateVersion(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() - _, err := client.PreviousTemplateVersion(ctx, user.OrganizationID, latestVersion.Name) + result, err := client.PreviousTemplateVersion(ctx, user.OrganizationID, latestVersion.Name) require.NoError(t, err) - require.Equal(t, previousVersion.ID, latestVersion.ID) + require.Equal(t, previousVersion.ID, result.ID) }) } From 6c7c8b940af3de60a0a324505f3aa038788faac6 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Fri, 2 Dec 2022 13:30:40 +0000 Subject: [PATCH 09/15] Fix tests --- coderd/database/databasefake/databasefake.go | 41 +++++++++++--------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 3e7d99b39e618..cf389ec45ab82 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1721,35 +1721,40 @@ func (q *fakeQuerier) GetPreviousTemplateVersion(_ context.Context, arg database q.mutex.RLock() defer q.mutex.RUnlock() - sortedTemplateVersions := slices.Clone(q.templateVersions) - slices.SortFunc(sortedTemplateVersions, func(i, j database.TemplateVersion) bool { - return i.CreatedAt.After(j.CreatedAt) - }) - - templateVersions := make([]database.TemplateVersion, 0) - for _, templateVersion := range sortedTemplateVersions { - if templateVersion.OrganizationID != arg.OrganizationID { + var currentTemplateVersion database.TemplateVersion + for _, templateVersion := range q.templateVersions { + if templateVersion.TemplateID != arg.TemplateID { continue } - if templateVersion.TemplateID != arg.TemplateID { + if templateVersion.Name != arg.Name { + continue + } + if templateVersion.OrganizationID != arg.OrganizationID { continue } - templateVersions = append(templateVersions, templateVersion) + currentTemplateVersion = templateVersion + break } - templateVersionIndex := -1 - for versionIndex, templateVersion := range templateVersions { - if strings.EqualFold(templateVersion.Name, arg.Name) { - templateVersionIndex = versionIndex - break + previousTemplateVersions := make([]database.TemplateVersion, 0) + for _, templateVersion := range q.templateVersions { + if templateVersion.ID == currentTemplateVersion.ID { + continue + } + if templateVersion.CreatedAt.Before(currentTemplateVersion.CreatedAt) { + previousTemplateVersions = append(previousTemplateVersions, templateVersion) } } - if templateVersionIndex > 0 { - return templateVersions[templateVersionIndex], nil + if len(previousTemplateVersions) == 0 { + return database.TemplateVersion{}, sql.ErrNoRows } - return database.TemplateVersion{}, sql.ErrNoRows + sort.Slice(previousTemplateVersions, func(i, j int) bool { + return previousTemplateVersions[i].CreatedAt.After(previousTemplateVersions[j].CreatedAt) + }) + + return previousTemplateVersions[0], nil } func (q *fakeQuerier) GetParameterSchemasByJobID(_ context.Context, jobID uuid.UUID) ([]database.ParameterSchema, error) { From b16b9b281ee889ea9a54e357d139b31c1fa635b1 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Fri, 2 Dec 2022 11:32:11 -0300 Subject: [PATCH 10/15] Update coderd/database/queries/templateversions.sql Co-authored-by: Mathias Fredriksson --- coderd/database/queries/templateversions.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index a380bb996dcf9..721a77f93902a 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -119,7 +119,7 @@ FROM WHERE created_at < ( SELECT created_at - FROM template_versions as tv + FROM template_versions AS tv WHERE tv.organization_id = $1 AND tv.name = $2 AND tv.template_id = $3 ) ORDER BY created_at DESC From c80d66a827b0683d83eda1adecf39fe4059c76ed Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Fri, 2 Dec 2022 15:23:35 +0000 Subject: [PATCH 11/15] Fix fmt --- coderd/database/queries.sql.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 9bc612798b8c9..4a231f69da1ab 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3488,7 +3488,7 @@ FROM WHERE created_at < ( SELECT created_at - FROM template_versions as tv + FROM template_versions AS tv WHERE tv.organization_id = $1 AND tv.name = $2 AND tv.template_id = $3 ) ORDER BY created_at DESC From 0f3c2b4343ea7a4b5c9e430b7d32edaaf4209249 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 5 Dec 2022 13:05:39 -0300 Subject: [PATCH 12/15] Update coderd/database/databasefake/databasefake.go Co-authored-by: Mathias Fredriksson --- coderd/database/databasefake/databasefake.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index cf389ec45ab82..c9f2a31d93930 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1738,7 +1738,7 @@ func (q *fakeQuerier) GetPreviousTemplateVersion(_ context.Context, arg database previousTemplateVersions := make([]database.TemplateVersion, 0) for _, templateVersion := range q.templateVersions { - if templateVersion.ID == currentTemplateVersion.ID { + if templateVersion.ID == currentTemplateVersion.ID || templateVersion.OrganizationID != arg.OrganizationID || templateVersion.TemplateID != arg.TemplateID { continue } if templateVersion.CreatedAt.Before(currentTemplateVersion.CreatedAt) { From d218d3270e2111ca2c49fd6e7f58f5628597abbe Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 5 Dec 2022 13:05:56 -0300 Subject: [PATCH 13/15] Update coderd/templateversions.go Co-authored-by: Mathias Fredriksson --- coderd/templateversions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 23cecaaf812a3..a523297a80531 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -648,7 +648,7 @@ func (api *API) previousTemplateVersionByOrganizationAndName(rw http.ResponseWri OrganizationID: organization.ID, Name: templateVersionName, }) - if errors.Is(err, sql.ErrNoRows) { + if xerrors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ Message: fmt.Sprintf("No template version found by name %q.", templateVersionName), }) From 23fda224c2094dd664d3322b2585203505322ee2 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Mon, 5 Dec 2022 17:41:14 +0000 Subject: [PATCH 14/15] Wrap errors inside of err!= nil --- coderd/templateversions.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 23cecaaf812a3..858911971ce38 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -648,13 +648,14 @@ func (api *API) previousTemplateVersionByOrganizationAndName(rw http.ResponseWri OrganizationID: organization.ID, Name: templateVersionName, }) - if errors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: fmt.Sprintf("No template version found by name %q.", templateVersionName), - }) - return - } if err != nil { + if errors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: fmt.Sprintf("No template version found by name %q.", templateVersionName), + }) + return + } + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching template version.", Detail: err.Error(), @@ -667,13 +668,15 @@ func (api *API) previousTemplateVersionByOrganizationAndName(rw http.ResponseWri Name: templateVersionName, TemplateID: templateVersion.TemplateID, }) - if errors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: fmt.Sprintf("No previous template version found for %q.", templateVersionName), - }) - return - } + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: fmt.Sprintf("No previous template version found for %q.", templateVersionName), + }) + return + } + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error fetching the previous template version.", Detail: err.Error(), From 2ada7c81e93c96cb6853ce31ff81ebf74c475f56 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Tue, 6 Dec 2022 14:05:44 +0000 Subject: [PATCH 15/15] Fix fake database --- coderd/database/databasefake/databasefake.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index e24737d4bc9e6..f724f5448d03a 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1739,7 +1739,7 @@ func (q *fakeQuerier) GetPreviousTemplateVersion(_ context.Context, arg database previousTemplateVersions := make([]database.TemplateVersion, 0) for _, templateVersion := range q.templateVersions { - if templateVersion.ID == currentTemplateVersion.ID || templateVersion.OrganizationID != arg.OrganizationID || templateVersion.TemplateID != arg.TemplateID { + if templateVersion.ID == currentTemplateVersion.ID { continue } if templateVersion.CreatedAt.Before(currentTemplateVersion.CreatedAt) {