From 02d2aea7f2063e452f3fb4af4b79f5535c61c56b Mon Sep 17 00:00:00 2001 From: Abhineet Jain Date: Fri, 10 Jun 2022 15:24:21 -0400 Subject: [PATCH] feat: store and display template creator (#2228) * design commit * add owner_id to templates table * add owner information in apis and ui * update minWidth for statItem * rename owner to created_by * missing refactor to created_by * handle errors in fetching created_by names --- coderd/audit/diff_test.go | 2 + coderd/audit/table.go | 1 + coderd/database/databasefake/databasefake.go | 1 + coderd/database/dump.sql | 6 +- .../000022_template_created_by.down.sql | 1 + .../000022_template_created_by.up.sql | 1 + coderd/database/models.go | 1 + coderd/database/queries.sql.go | 22 +++-- coderd/database/queries/templates.sql | 5 +- coderd/templates.go | 83 +++++++++++++++++-- codersdk/templates.go | 2 + site/src/api/typesGenerated.ts | 8 +- .../TemplateStats/TemplateStats.stories.tsx | 9 ++ .../TemplateStats/TemplateStats.tsx | 9 +- .../WorkspaceBuildStats.tsx | 2 +- .../pages/TemplatesPage/TemplatesPageView.tsx | 4 + site/src/testHelpers/entities.ts | 2 + 17 files changed, 135 insertions(+), 24 deletions(-) create mode 100644 coderd/database/migrations/000022_template_created_by.down.sql create mode 100644 coderd/database/migrations/000022_template_created_by.up.sql diff --git a/coderd/audit/diff_test.go b/coderd/audit/diff_test.go index 53f2110f07..ba91692a07 100644 --- a/coderd/audit/diff_test.go +++ b/coderd/audit/diff_test.go @@ -88,6 +88,7 @@ func TestDiff(t *testing.T) { ActiveVersionID: uuid.UUID{3}, MaxTtl: int64(time.Hour), MinAutostartInterval: int64(time.Minute), + CreatedBy: uuid.NullUUID{UUID: uuid.UUID{4}, Valid: true}, }, exp: audit.Map{ "id": uuid.UUID{1}.String(), @@ -97,6 +98,7 @@ func TestDiff(t *testing.T) { "active_version_id": uuid.UUID{3}.String(), "max_ttl": int64(3600000000000), "min_autostart_interval": int64(60000000000), + "created_by": uuid.UUID{4}.String(), }, }, }) diff --git a/coderd/audit/table.go b/coderd/audit/table.go index efdf0de1d6..7562472f3c 100644 --- a/coderd/audit/table.go +++ b/coderd/audit/table.go @@ -72,6 +72,7 @@ var AuditableResources = auditMap(map[any]map[string]Action{ "description": ActionTrack, "max_ttl": ActionTrack, "min_autostart_interval": ActionTrack, + "created_by": ActionTrack, }, &database.TemplateVersion{}: { "id": ActionTrack, diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 2fb1c1fa98..fcc214a574 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1341,6 +1341,7 @@ func (q *fakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl Description: arg.Description, MaxTtl: arg.MaxTtl, MinAutostartInterval: arg.MinAutostartInterval, + CreatedBy: arg.CreatedBy, } q.templates = append(q.templates, template) return template, nil diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index eff1e9ea63..95ef9ff0df 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -248,7 +248,8 @@ CREATE TABLE templates ( active_version_id uuid NOT NULL, description character varying(128) DEFAULT ''::character varying NOT NULL, max_ttl bigint DEFAULT '604800000000000'::bigint NOT NULL, - min_autostart_interval bigint DEFAULT '3600000000000'::bigint NOT NULL + min_autostart_interval bigint DEFAULT '3600000000000'::bigint NOT NULL, + created_by uuid ); CREATE TABLE users ( @@ -476,6 +477,9 @@ ALTER TABLE ONLY template_versions ALTER TABLE ONLY template_versions ADD CONSTRAINT template_versions_template_id_fkey FOREIGN KEY (template_id) REFERENCES templates(id) ON DELETE CASCADE; +ALTER TABLE ONLY templates + ADD CONSTRAINT templates_created_by_fkey FOREIGN KEY (created_by) REFERENCES users(id) ON DELETE RESTRICT; + ALTER TABLE ONLY templates ADD CONSTRAINT templates_organization_id_fkey FOREIGN KEY (organization_id) REFERENCES organizations(id) ON DELETE CASCADE; diff --git a/coderd/database/migrations/000022_template_created_by.down.sql b/coderd/database/migrations/000022_template_created_by.down.sql new file mode 100644 index 0000000000..435291e5cc --- /dev/null +++ b/coderd/database/migrations/000022_template_created_by.down.sql @@ -0,0 +1 @@ +ALTER TABLE ONLY templates DROP COLUMN IF EXISTS created_by; diff --git a/coderd/database/migrations/000022_template_created_by.up.sql b/coderd/database/migrations/000022_template_created_by.up.sql new file mode 100644 index 0000000000..7ca6f23559 --- /dev/null +++ b/coderd/database/migrations/000022_template_created_by.up.sql @@ -0,0 +1 @@ +ALTER TABLE ONLY templates ADD COLUMN IF NOT EXISTS created_by uuid REFERENCES users (id) ON DELETE RESTRICT; diff --git a/coderd/database/models.go b/coderd/database/models.go index 22f47053c3..2454049da0 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -438,6 +438,7 @@ type Template struct { Description string `db:"description" json:"description"` MaxTtl int64 `db:"max_ttl" json:"max_ttl"` MinAutostartInterval int64 `db:"min_autostart_interval" json:"min_autostart_interval"` + CreatedBy uuid.NullUUID `db:"created_by" json:"created_by"` } type TemplateVersion struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a44b1458e6..edb2fc7745 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1603,7 +1603,7 @@ func (q *sqlQuerier) UpdateProvisionerJobWithCompleteByID(ctx context.Context, a const getTemplateByID = `-- name: GetTemplateByID :one SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by FROM templates WHERE @@ -1627,13 +1627,14 @@ func (q *sqlQuerier) GetTemplateByID(ctx context.Context, id uuid.UUID) (Templat &i.Description, &i.MaxTtl, &i.MinAutostartInterval, + &i.CreatedBy, ) return i, err } const getTemplateByOrganizationAndName = `-- name: GetTemplateByOrganizationAndName :one SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by FROM templates WHERE @@ -1665,13 +1666,14 @@ func (q *sqlQuerier) GetTemplateByOrganizationAndName(ctx context.Context, arg G &i.Description, &i.MaxTtl, &i.MinAutostartInterval, + &i.CreatedBy, ) return i, err } const getTemplatesByIDs = `-- name: GetTemplatesByIDs :many SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by FROM templates WHERE @@ -1699,6 +1701,7 @@ func (q *sqlQuerier) GetTemplatesByIDs(ctx context.Context, ids []uuid.UUID) ([] &i.Description, &i.MaxTtl, &i.MinAutostartInterval, + &i.CreatedBy, ); err != nil { return nil, err } @@ -1715,7 +1718,7 @@ func (q *sqlQuerier) GetTemplatesByIDs(ctx context.Context, ids []uuid.UUID) ([] const getTemplatesByOrganization = `-- name: GetTemplatesByOrganization :many SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by FROM templates WHERE @@ -1749,6 +1752,7 @@ func (q *sqlQuerier) GetTemplatesByOrganization(ctx context.Context, arg GetTemp &i.Description, &i.MaxTtl, &i.MinAutostartInterval, + &i.CreatedBy, ); err != nil { return nil, err } @@ -1775,10 +1779,11 @@ INSERT INTO active_version_id, description, max_ttl, - min_autostart_interval + min_autostart_interval, + created_by ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by ` type InsertTemplateParams struct { @@ -1792,6 +1797,7 @@ type InsertTemplateParams struct { Description string `db:"description" json:"description"` MaxTtl int64 `db:"max_ttl" json:"max_ttl"` MinAutostartInterval int64 `db:"min_autostart_interval" json:"min_autostart_interval"` + CreatedBy uuid.NullUUID `db:"created_by" json:"created_by"` } func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParams) (Template, error) { @@ -1806,6 +1812,7 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam arg.Description, arg.MaxTtl, arg.MinAutostartInterval, + arg.CreatedBy, ) var i Template err := row.Scan( @@ -1820,6 +1827,7 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam &i.Description, &i.MaxTtl, &i.MinAutostartInterval, + &i.CreatedBy, ) return i, err } @@ -1873,7 +1881,7 @@ SET WHERE id = $1 RETURNING - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by ` type UpdateTemplateMetaByIDParams struct { diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index ffc0b9dabf..c3b3753083 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -49,10 +49,11 @@ INSERT INTO active_version_id, description, max_ttl, - min_autostart_interval + min_autostart_interval, + created_by ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING *; + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) RETURNING *; -- name: UpdateTemplateActiveVersionByID :exec UPDATE diff --git a/coderd/templates.go b/coderd/templates.go index 386a360111..d79bd19f70 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -1,6 +1,7 @@ package coderd import ( + "context" "database/sql" "errors" "fmt" @@ -49,7 +50,16 @@ func (api *API) template(rw http.ResponseWriter, r *http.Request) { count = uint32(workspaceCounts[0].Count) } - httpapi.Write(rw, http.StatusOK, convertTemplate(template, count)) + createdByNameMap, err := getCreatedByNamesByTemplateIDs(r.Context(), api.Database, []database.Template{template}) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "Internal error fetching creator name.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(rw, http.StatusOK, convertTemplate(template, count, createdByNameMap[template.ID.String()])) } func (api *API) deleteTemplate(rw http.ResponseWriter, r *http.Request) { @@ -97,6 +107,7 @@ func (api *API) deleteTemplate(rw http.ResponseWriter, r *http.Request) { func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Request) { var createTemplate codersdk.CreateTemplateRequest organization := httpmw.OrganizationParam(r) + apiKey := httpmw.APIKey(r) if !api.Authorize(rw, r, rbac.ActionCreate, rbac.ResourceTemplate.InOrg(organization.ID)) { return } @@ -175,6 +186,10 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque Description: createTemplate.Description, MaxTtl: int64(maxTTL), MinAutostartInterval: int64(minAutostartInterval), + CreatedBy: uuid.NullUUID{ + UUID: apiKey.UserID, + Valid: true, + }, }) if err != nil { return xerrors.Errorf("insert template: %s", err) @@ -208,7 +223,12 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque } } - template = convertTemplate(dbTemplate, 0) + createdByNameMap, err := getCreatedByNamesByTemplateIDs(r.Context(), db, []database.Template{dbTemplate}) + if err != nil { + return xerrors.Errorf("get creator name: %w", err) + } + + template = convertTemplate(dbTemplate, 0, createdByNameMap[dbTemplate.ID.String()]) return nil }) if err != nil { @@ -258,7 +278,16 @@ func (api *API) templatesByOrganization(rw http.ResponseWriter, r *http.Request) return } - httpapi.Write(rw, http.StatusOK, convertTemplates(templates, workspaceCounts)) + createdByNameMap, err := getCreatedByNamesByTemplateIDs(r.Context(), api.Database, templates) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "Internal error fetching creator names.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(rw, http.StatusOK, convertTemplates(templates, workspaceCounts, createdByNameMap)) } func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Request) { @@ -304,7 +333,16 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re count = uint32(workspaceCounts[0].Count) } - httpapi.Write(rw, http.StatusOK, convertTemplate(template, count)) + createdByNameMap, err := getCreatedByNamesByTemplateIDs(r.Context(), api.Database, []database.Template{template}) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "Internal error fetching creator name.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(rw, http.StatusOK, convertTemplate(template, count, createdByNameMap[template.ID.String()])) } func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { @@ -400,10 +438,35 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(rw, http.StatusOK, convertTemplate(updated, count)) + createdByNameMap, err := getCreatedByNamesByTemplateIDs(r.Context(), api.Database, []database.Template{updated}) + if err != nil { + httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ + Message: "Internal error fetching creator name.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(rw, http.StatusOK, convertTemplate(updated, count, createdByNameMap[updated.ID.String()])) } -func convertTemplates(templates []database.Template, workspaceCounts []database.GetWorkspaceOwnerCountsByTemplateIDsRow) []codersdk.Template { +func getCreatedByNamesByTemplateIDs(ctx context.Context, db database.Store, templates []database.Template) (map[string]string, error) { + creators := make(map[string]string, len(templates)) + for _, template := range templates { + if template.CreatedBy.Valid { + creator, err := db.GetUserByID(ctx, template.CreatedBy.UUID) + if err != nil { + return map[string]string{}, err + } + creators[template.ID.String()] = creator.Username + } else { + creators[template.ID.String()] = "" + } + } + return creators, nil +} + +func convertTemplates(templates []database.Template, workspaceCounts []database.GetWorkspaceOwnerCountsByTemplateIDsRow, createdByNameMap map[string]string) []codersdk.Template { apiTemplates := make([]codersdk.Template, 0, len(templates)) for _, template := range templates { found := false @@ -411,18 +474,18 @@ func convertTemplates(templates []database.Template, workspaceCounts []database. if workspaceCount.TemplateID.String() != template.ID.String() { continue } - apiTemplates = append(apiTemplates, convertTemplate(template, uint32(workspaceCount.Count))) + apiTemplates = append(apiTemplates, convertTemplate(template, uint32(workspaceCount.Count), createdByNameMap[template.ID.String()])) found = true break } if !found { - apiTemplates = append(apiTemplates, convertTemplate(template, uint32(0))) + apiTemplates = append(apiTemplates, convertTemplate(template, uint32(0), createdByNameMap[template.ID.String()])) } } return apiTemplates } -func convertTemplate(template database.Template, workspaceOwnerCount uint32) codersdk.Template { +func convertTemplate(template database.Template, workspaceOwnerCount uint32, createdByName string) codersdk.Template { return codersdk.Template{ ID: template.ID, CreatedAt: template.CreatedAt, @@ -435,5 +498,7 @@ func convertTemplate(template database.Template, workspaceOwnerCount uint32) cod Description: template.Description, MaxTTLMillis: time.Duration(template.MaxTtl).Milliseconds(), MinAutostartIntervalMillis: time.Duration(template.MinAutostartInterval).Milliseconds(), + CreatedByID: template.CreatedBy, + CreatedByName: createdByName, } } diff --git a/codersdk/templates.go b/codersdk/templates.go index e137c3fca0..31f1ea97d9 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -25,6 +25,8 @@ type Template struct { Description string `json:"description"` MaxTTLMillis int64 `json:"max_ttl_ms"` MinAutostartIntervalMillis int64 `json:"min_autostart_interval_ms"` + CreatedByID uuid.NullUUID `json:"created_by_id"` + CreatedByName string `json:"created_by_name"` } type UpdateActiveTemplateVersion struct { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index ce0fae95ce..79423268ac 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -247,6 +247,8 @@ export interface Template { readonly description: string readonly max_ttl_ms: number readonly min_autostart_interval_ms: number + readonly created_by_id?: string + readonly created_by_name: string } // From codersdk/templateversions.go:14:6 @@ -276,12 +278,12 @@ export interface TemplateVersionParameter { readonly default_source_value: boolean } -// From codersdk/templates.go:98:6 +// From codersdk/templates.go:100:6 export interface TemplateVersionsByTemplateRequest extends Pagination { readonly template_id: string } -// From codersdk/templates.go:30:6 +// From codersdk/templates.go:32:6 export interface UpdateActiveTemplateVersion { readonly id: string } @@ -291,7 +293,7 @@ export interface UpdateRoles { readonly roles: string[] } -// From codersdk/templates.go:34:6 +// From codersdk/templates.go:36:6 export interface UpdateTemplateMeta { readonly description?: string readonly max_ttl_ms?: number diff --git a/site/src/components/TemplateStats/TemplateStats.stories.tsx b/site/src/components/TemplateStats/TemplateStats.stories.tsx index 3056187d11..fb7f36d894 100644 --- a/site/src/components/TemplateStats/TemplateStats.stories.tsx +++ b/site/src/components/TemplateStats/TemplateStats.stories.tsx @@ -23,3 +23,12 @@ UsedByMany.args = { }, activeVersion: Mocks.MockTemplateVersion, } + +export const UnknownCreator = Template.bind({}) +UnknownCreator.args = { + template: { + ...Mocks.MockTemplate, + created_by_name: "", + }, + activeVersion: Mocks.MockTemplateVersion, +} diff --git a/site/src/components/TemplateStats/TemplateStats.tsx b/site/src/components/TemplateStats/TemplateStats.tsx index 24da983939..fed612f45f 100644 --- a/site/src/components/TemplateStats/TemplateStats.tsx +++ b/site/src/components/TemplateStats/TemplateStats.tsx @@ -13,6 +13,8 @@ const Language = { lastUpdateLabel: "Last updated", userPlural: "users", userSingular: "user", + createdByLabel: "Created by", + defaultTemplateCreator: "", } export interface TemplateStatsProps { @@ -45,6 +47,11 @@ export const TemplateStats: FC = ({ template, activeVersion {dayjs().to(dayjs(template.updated_at))} +
+
+ {Language.createdByLabel} + {template.created_by_name || Language.defaultTemplateCreator} +
) } @@ -63,7 +70,7 @@ const useStyles = makeStyles((theme) => ({ }, statItem: { - minWidth: theme.spacing(20), + minWidth: "20%", padding: theme.spacing(2), paddingTop: theme.spacing(1.75), }, diff --git a/site/src/components/WorkspaceBuildStats/WorkspaceBuildStats.tsx b/site/src/components/WorkspaceBuildStats/WorkspaceBuildStats.tsx index 0db8dfc143..4b00dd0584 100644 --- a/site/src/components/WorkspaceBuildStats/WorkspaceBuildStats.tsx +++ b/site/src/components/WorkspaceBuildStats/WorkspaceBuildStats.tsx @@ -69,7 +69,7 @@ const useStyles = makeStyles((theme) => ({ }, statItem: { - minWidth: theme.spacing(20), + minWidth: "16%", padding: theme.spacing(2), paddingTop: theme.spacing(1.75), }, diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index a27cf2cacf..deda1cad6c 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -49,6 +49,8 @@ export const Language = { templateTooltipTitle: "What is template?", templateTooltipText: "With templates you can create a common configuration for your workspaces using Terraform.", templateTooltipLink: "Manage templates", + createdByLabel: "Created by", + defaultTemplateCreator: "", } const TemplateHelpTooltip: React.FC = () => { @@ -95,6 +97,7 @@ export const TemplatesPageView: FC = (props) => { {Language.nameLabel} {Language.usedByLabel} {Language.lastUpdatedLabel} + {Language.createdByLabel} @@ -137,6 +140,7 @@ export const TemplatesPageView: FC = (props) => { {Language.developerCount(template.workspace_owner_count)} {dayjs().to(dayjs(template.updated_at))} + {template.created_by_name || Language.defaultTemplateCreator}
diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 53414283e4..b37e038529 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -114,6 +114,8 @@ export const MockTemplate: TypesGen.Template = { description: "This is a test description.", max_ttl_ms: 604800000, min_autostart_interval_ms: 3600000, + created_by_id: "test-creator-id", + created_by_name: "test_creator", } export const MockWorkspaceAutostartDisabled: TypesGen.UpdateWorkspaceAutostartRequest = {