feat: Add template display name (backend) (#4966)

* Rename to nameValidator

* Refactor: NameValid

* Fix: comment

* Define new migration

* Include display_name

* Update typesGenerated.ts

* Update meta

* Adjust tests

* CLI tests

* Fix: audit

* Fix: omitempty

* site: display_name is optional

* unit: TestUsernameValid

* entities.ts: add display_name

* site: TemplateSettingsPage.test.tsx

* Fix: TemplateSettingsForm.tsx

* Adjust tests

* Add comment to display_name column

* Fix: rename

* Fix: make

* Loosen regexp

* Fix: err check

* Fix: template name length

* Allow for whitespaces

* Update migration number
This commit is contained in:
Marcin Tojek
2022-11-10 21:51:09 +01:00
committed by GitHub
parent f3eb662208
commit 2042b575dc
23 changed files with 219 additions and 57 deletions

View File

@ -1303,6 +1303,7 @@ func (q *fakeQuerier) UpdateTemplateMetaByID(_ context.Context, arg database.Upd
}
tpl.UpdatedAt = database.Now()
tpl.Name = arg.Name
tpl.DisplayName = arg.DisplayName
tpl.Description = arg.Description
tpl.Icon = arg.Icon
tpl.DefaultTtl = arg.DefaultTtl

View File

@ -353,11 +353,14 @@ CREATE TABLE templates (
created_by uuid NOT NULL,
icon character varying(256) DEFAULT ''::character varying NOT NULL,
user_acl jsonb DEFAULT '{}'::jsonb NOT NULL,
group_acl jsonb DEFAULT '{}'::jsonb NOT NULL
group_acl jsonb DEFAULT '{}'::jsonb NOT NULL,
display_name character varying(64) DEFAULT ''::character varying NOT NULL
);
COMMENT ON COLUMN templates.default_ttl IS 'The default duration for auto-stop for workspaces created from this template.';
COMMENT ON COLUMN templates.display_name IS 'Display name is a custom, human-friendly template name that user can set.';
CREATE TABLE user_links (
user_id uuid NOT NULL,
login_type login_type NOT NULL,

View File

@ -0,0 +1 @@
ALTER TABLE templates DROP COLUMN display_name;

View File

@ -0,0 +1,4 @@
ALTER TABLE templates ADD COLUMN display_name VARCHAR(64) NOT NULL DEFAULT '';
COMMENT ON COLUMN templates.display_name
IS 'Display name is a custom, human-friendly template name that user can set.';

View File

@ -589,6 +589,8 @@ type Template struct {
Icon string `db:"icon" json:"icon"`
UserACL TemplateACL `db:"user_acl" json:"user_acl"`
GroupACL TemplateACL `db:"group_acl" json:"group_acl"`
// Display name is a custom, human-friendly template name that user can set.
DisplayName string `db:"display_name" json:"display_name"`
}
type TemplateVersion struct {

View File

@ -3019,7 +3019,7 @@ func (q *sqlQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg GetTem
const getTemplateByID = `-- name: GetTemplateByID :one
SELECT
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name
FROM
templates
WHERE
@ -3046,13 +3046,14 @@ func (q *sqlQuerier) GetTemplateByID(ctx context.Context, id uuid.UUID) (Templat
&i.Icon,
&i.UserACL,
&i.GroupACL,
&i.DisplayName,
)
return i, err
}
const getTemplateByOrganizationAndName = `-- name: GetTemplateByOrganizationAndName :one
SELECT
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name
FROM
templates
WHERE
@ -3087,12 +3088,13 @@ func (q *sqlQuerier) GetTemplateByOrganizationAndName(ctx context.Context, arg G
&i.Icon,
&i.UserACL,
&i.GroupACL,
&i.DisplayName,
)
return i, err
}
const getTemplates = `-- name: GetTemplates :many
SELECT id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl FROM templates
SELECT id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name FROM templates
ORDER BY (name, id) ASC
`
@ -3120,6 +3122,7 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) {
&i.Icon,
&i.UserACL,
&i.GroupACL,
&i.DisplayName,
); err != nil {
return nil, err
}
@ -3136,7 +3139,7 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) {
const getTemplatesWithFilter = `-- name: GetTemplatesWithFilter :many
SELECT
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name
FROM
templates
WHERE
@ -3199,6 +3202,7 @@ func (q *sqlQuerier) GetTemplatesWithFilter(ctx context.Context, arg GetTemplate
&i.Icon,
&i.UserACL,
&i.GroupACL,
&i.DisplayName,
); err != nil {
return nil, err
}
@ -3228,10 +3232,11 @@ INSERT INTO
created_by,
icon,
user_acl,
group_acl
group_acl,
display_name
)
VALUES
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name
`
type InsertTemplateParams struct {
@ -3248,6 +3253,7 @@ type InsertTemplateParams struct {
Icon string `db:"icon" json:"icon"`
UserACL TemplateACL `db:"user_acl" json:"user_acl"`
GroupACL TemplateACL `db:"group_acl" json:"group_acl"`
DisplayName string `db:"display_name" json:"display_name"`
}
func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParams) (Template, error) {
@ -3265,6 +3271,7 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam
arg.Icon,
arg.UserACL,
arg.GroupACL,
arg.DisplayName,
)
var i Template
err := row.Scan(
@ -3282,6 +3289,7 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam
&i.Icon,
&i.UserACL,
&i.GroupACL,
&i.DisplayName,
)
return i, err
}
@ -3295,7 +3303,7 @@ SET
WHERE
id = $3
RETURNING
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name
`
type UpdateTemplateACLByIDParams struct {
@ -3322,6 +3330,7 @@ func (q *sqlQuerier) UpdateTemplateACLByID(ctx context.Context, arg UpdateTempla
&i.Icon,
&i.UserACL,
&i.GroupACL,
&i.DisplayName,
)
return i, err
}
@ -3376,11 +3385,12 @@ SET
description = $3,
default_ttl = $4,
name = $5,
icon = $6
icon = $6,
display_name = $7
WHERE
id = $1
RETURNING
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl
id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name
`
type UpdateTemplateMetaByIDParams struct {
@ -3390,6 +3400,7 @@ type UpdateTemplateMetaByIDParams struct {
DefaultTtl int64 `db:"default_ttl" json:"default_ttl"`
Name string `db:"name" json:"name"`
Icon string `db:"icon" json:"icon"`
DisplayName string `db:"display_name" json:"display_name"`
}
func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTemplateMetaByIDParams) (Template, error) {
@ -3400,6 +3411,7 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl
arg.DefaultTtl,
arg.Name,
arg.Icon,
arg.DisplayName,
)
var i Template
err := row.Scan(
@ -3417,6 +3429,7 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl
&i.Icon,
&i.UserACL,
&i.GroupACL,
&i.DisplayName,
)
return i, err
}

View File

@ -69,10 +69,11 @@ INSERT INTO
created_by,
icon,
user_acl,
group_acl
group_acl,
display_name
)
VALUES
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13) RETURNING *;
($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING *;
-- name: UpdateTemplateActiveVersionByID :exec
UPDATE
@ -100,7 +101,8 @@ SET
description = $3,
default_ttl = $4,
name = $5,
icon = $6
icon = $6,
display_name = $7
WHERE
id = $1
RETURNING

View File

@ -47,7 +47,7 @@ func ConvertConfig(entries []codersdk.GitAuthConfig, accessURL *url.URL) ([]*Con
// Default to the type.
entry.ID = string(typ)
}
if valid := httpapi.UsernameValid(entry.ID); valid != nil {
if valid := httpapi.NameValid(entry.ID); valid != nil {
return nil, xerrors.Errorf("git auth provider %q doesn't have a valid id: %w", entry.ID, valid)
}

View File

@ -33,13 +33,14 @@ func init() {
}
return name
})
nameValidator := func(fl validator.FieldLevel) bool {
f := fl.Field().Interface()
str, ok := f.(string)
if !ok {
return false
}
valid := UsernameValid(str)
valid := NameValid(str)
return valid == nil
}
for _, tag := range []string{"username", "template_name", "workspace_name"} {
@ -48,6 +49,20 @@ func init() {
panic(err)
}
}
templateDisplayNameValidator := func(fl validator.FieldLevel) bool {
f := fl.Field().Interface()
str, ok := f.(string)
if !ok {
return false
}
valid := TemplateDisplayNameValid(str)
return valid == nil
}
err := validate.RegisterValidation("template_display_name", templateDisplayNameValidator)
if err != nil {
panic(err)
}
}
// Convenience error functions don't take contexts since their responses are

View File

@ -11,10 +11,34 @@ import (
var (
UsernameValidRegex = regexp.MustCompile("^[a-zA-Z0-9]+(?:-[a-zA-Z0-9]+)*$")
usernameReplace = regexp.MustCompile("[^a-zA-Z0-9-]*")
templateDisplayName = regexp.MustCompile(`^[^\s](.*[^\s])?$`)
)
// UsernameValid returns whether the input string is a valid username.
func UsernameValid(str string) error {
// UsernameFrom returns a best-effort username from the provided string.
//
// It first attempts to validate the incoming string, which will
// be returned if it is valid. It then will attempt to extract
// the username from an email address. If no success happens during
// these steps, a random username will be returned.
func UsernameFrom(str string) string {
if valid := NameValid(str); valid == nil {
return str
}
emailAt := strings.LastIndex(str, "@")
if emailAt >= 0 {
str = str[:emailAt]
}
str = usernameReplace.ReplaceAllString(str, "")
if valid := NameValid(str); valid == nil {
return str
}
return strings.ReplaceAll(namesgenerator.GetRandomName(1), "_", "-")
}
// NameValid returns whether the input string is a valid name.
// It is a generic validator for any name (user, workspace, template, etc.).
func NameValid(str string) error {
if len(str) > 32 {
return xerrors.New("must be <= 32 characters")
}
@ -28,23 +52,17 @@ func UsernameValid(str string) error {
return nil
}
// UsernameFrom returns a best-effort username from the provided string.
//
// It first attempts to validate the incoming string, which will
// be returned if it is valid. It then will attempt to extract
// the username from an email address. If no success happens during
// these steps, a random username will be returned.
func UsernameFrom(str string) string {
if valid := UsernameValid(str); valid == nil {
return str
// TemplateDisplayNameValid returns whether the input string is a valid template display name.
func TemplateDisplayNameValid(str string) error {
if len(str) == 0 {
return nil // empty display_name is correct
}
emailAt := strings.LastIndex(str, "@")
if emailAt >= 0 {
str = str[:emailAt]
if len(str) > 64 {
return xerrors.New("must be <= 64 characters")
}
str = usernameReplace.ReplaceAllString(str, "")
if valid := UsernameValid(str); valid == nil {
return str
matched := templateDisplayName.MatchString(str)
if !matched {
return xerrors.New("must be alphanumeric with spaces")
}
return strings.ReplaceAll(namesgenerator.GetRandomName(1), "_", "-")
return nil
}

View File

@ -8,7 +8,7 @@ import (
"github.com/coder/coder/coderd/httpapi"
)
func TestValid(t *testing.T) {
func TestUsernameValid(t *testing.T) {
t.Parallel()
// Tests whether usernames are valid or not.
testCases := []struct {
@ -59,7 +59,62 @@ func TestValid(t *testing.T) {
testCase := testCase
t.Run(testCase.Username, func(t *testing.T) {
t.Parallel()
valid := httpapi.UsernameValid(testCase.Username)
valid := httpapi.NameValid(testCase.Username)
require.Equal(t, testCase.Valid, valid == nil)
})
}
}
func TestTemplateDisplayNameValid(t *testing.T) {
t.Parallel()
// Tests whether display names are valid.
testCases := []struct {
Name string
Valid bool
}{
{"", true},
{"1", true},
{"12", true},
{"1 2", true},
{"123 456", true},
{"1234 678901234567890", true},
{"<b> </b>", true},
{"S", true},
{"a1", true},
{"a1K2", true},
{"!!!!1 ?????", true},
{"k\r\rm", true},
{"abcdefghijklmnopqrst", true},
{"Wow Test", true},
{"abcdefghijklmnopqrstu-", true},
{"a1b2c3d4e5f6g7h8i9j0k-", true},
{"BANANAS_wow", true},
{"test--now", true},
{"123456789012345678901234567890123", true},
{"1234567890123456789012345678901234567890123456789012345678901234", true},
{"-a1b2c3d4e5f6g7h8i9j0k", true},
{" ", false},
{"\t", false},
{"\r\r", false},
{"\t1 ", false},
{" a", false},
{"\ra ", false},
{" 1", false},
{"1 ", false},
{" aa", false},
{"aa\r", false},
{" 12", false},
{"12 ", false},
{"\fa1", false},
{"a1\t", false},
{"12345678901234567890123456789012345678901234567890123456789012345", false},
}
for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.Name, func(t *testing.T) {
t.Parallel()
valid := httpapi.TemplateDisplayNameValid(testCase.Name)
require.Equal(t, testCase.Valid, valid == nil)
})
}
@ -92,7 +147,7 @@ func TestFrom(t *testing.T) {
t.Parallel()
converted := httpapi.UsernameFrom(testCase.From)
t.Log(converted)
valid := httpapi.UsernameValid(converted)
valid := httpapi.NameValid(converted)
require.True(t, valid == nil)
if testCase.Match == "" {
require.NotEqual(t, testCase.From, converted)

View File

@ -472,6 +472,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
if req.Name == template.Name &&
req.Description == template.Description &&
req.DisplayName == template.DisplayName &&
req.Icon == template.Icon &&
req.DefaultTTLMillis == time.Duration(template.DefaultTtl).Milliseconds() {
return nil
@ -479,6 +480,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
// Update template metadata -- empty fields are not overwritten.
name := req.Name
displayName := req.DisplayName
desc := req.Description
icon := req.Icon
maxTTL := time.Duration(req.DefaultTTLMillis) * time.Millisecond
@ -486,6 +488,9 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
if name == "" {
name = template.Name
}
if displayName == "" {
displayName = template.DisplayName
}
if desc == "" {
desc = template.Description
}
@ -494,6 +499,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
ID: template.ID,
UpdatedAt: database.Now(),
Name: name,
DisplayName: displayName,
Description: desc,
Icon: icon,
DefaultTtl: int64(maxTTL),
@ -738,6 +744,7 @@ func (api *API) convertTemplate(
UpdatedAt: template.UpdatedAt,
OrganizationID: template.OrganizationID,
Name: template.Name,
DisplayName: template.DisplayName,
Provisioner: codersdk.ProvisionerType(template.Provisioner),
ActiveVersionID: template.ActiveVersionID,
WorkspaceOwnerCount: workspaceOwnerCount,

View File

@ -283,13 +283,10 @@ func TestPatchTemplateMeta(t *testing.T) {
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, Auditor: auditor})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
ctr.Description = "original description"
ctr.Icon = "/icons/original-icon.png"
ctr.DefaultTTLMillis = ptr.Ref(24 * time.Hour.Milliseconds())
})
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
req := codersdk.UpdateTemplateMeta{
Name: "new-template-name",
DisplayName: "Displayed Name 456",
Description: "lorem ipsum dolor sit amet et cetera",
Icon: "/icons/new-icon.png",
DefaultTTLMillis: 12 * time.Hour.Milliseconds(),
@ -305,6 +302,7 @@ func TestPatchTemplateMeta(t *testing.T) {
require.NoError(t, err)
assert.Greater(t, updated.UpdatedAt, template.UpdatedAt)
assert.Equal(t, req.Name, updated.Name)
assert.Equal(t, req.DisplayName, updated.DisplayName)
assert.Equal(t, req.Description, updated.Description)
assert.Equal(t, req.Icon, updated.Icon)
assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis)
@ -314,6 +312,7 @@ func TestPatchTemplateMeta(t *testing.T) {
require.NoError(t, err)
assert.Greater(t, updated.UpdatedAt, template.UpdatedAt)
assert.Equal(t, req.Name, updated.Name)
assert.Equal(t, req.DisplayName, updated.DisplayName)
assert.Equal(t, req.Description, updated.Description)
assert.Equal(t, req.Icon, updated.Icon)
assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis)

View File

@ -261,7 +261,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
// The username is a required property in Coder. We make a best-effort
// attempt at using what the claims provide, but if that fails we will
// generate a random username.
usernameValid := httpapi.UsernameValid(username)
usernameValid := httpapi.NameValid(username)
if usernameValid != nil {
// If no username is provided, we can default to use the email address.
// This will be converted in the from function below, so it's safe