feat: add user-level parameter autofill (#11731)

This PR solves #10478 by auto-filling previously used template values in create and update workspace flows.

I decided against explicit user values in settings for these reasons:

* Autofill is far easier to implement
* Users benefit from autofill _by default_ — we don't need to teach them new concepts
* If we decide that autofill creates more harm than good, we can remove it without breaking compatibility
This commit is contained in:
Ammar Bandukwala
2024-01-30 16:02:21 -06:00
committed by GitHub
parent aeb4112513
commit adbb025e74
44 changed files with 862 additions and 137 deletions
+57
View File
@@ -4142,6 +4142,50 @@ const docTemplate = `{
}
}
},
"/users/{user}/autofill-parameters": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": [
"application/json"
],
"tags": [
"Users"
],
"summary": "Get autofill build parameters for user",
"operationId": "get-autofill-build-parameters-for-user",
"parameters": [
{
"type": "string",
"description": "User ID, username, or me",
"name": "user",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template ID",
"name": "template_id",
"in": "query",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/codersdk.UserParameter"
}
}
}
}
}
},
"/users/{user}/convert-login": {
"post": {
"security": [
@@ -10493,6 +10537,7 @@ const docTemplate = `{
"api_key",
"user",
"user_data",
"user_workspace_build_parameters",
"organization_member",
"license",
"deployment_config",
@@ -10518,6 +10563,7 @@ const docTemplate = `{
"ResourceAPIKey",
"ResourceUser",
"ResourceUserData",
"ResourceUserWorkspaceBuildParameters",
"ResourceOrganizationMember",
"ResourceLicense",
"ResourceDeploymentValues",
@@ -11923,6 +11969,17 @@ const docTemplate = `{
}
}
},
"codersdk.UserParameter": {
"type": "object",
"properties": {
"name": {
"type": "string"
},
"value": {
"type": "string"
}
}
},
"codersdk.UserQuietHoursScheduleConfig": {
"type": "object",
"properties": {
+53
View File
@@ -3638,6 +3638,46 @@
}
}
},
"/users/{user}/autofill-parameters": {
"get": {
"security": [
{
"CoderSessionToken": []
}
],
"produces": ["application/json"],
"tags": ["Users"],
"summary": "Get autofill build parameters for user",
"operationId": "get-autofill-build-parameters-for-user",
"parameters": [
{
"type": "string",
"description": "User ID, username, or me",
"name": "user",
"in": "path",
"required": true
},
{
"type": "string",
"description": "Template ID",
"name": "template_id",
"in": "query",
"required": true
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"type": "array",
"items": {
"$ref": "#/definitions/codersdk.UserParameter"
}
}
}
}
}
},
"/users/{user}/convert-login": {
"post": {
"security": [
@@ -9439,6 +9479,7 @@
"api_key",
"user",
"user_data",
"user_workspace_build_parameters",
"organization_member",
"license",
"deployment_config",
@@ -9464,6 +9505,7 @@
"ResourceAPIKey",
"ResourceUser",
"ResourceUserData",
"ResourceUserWorkspaceBuildParameters",
"ResourceOrganizationMember",
"ResourceLicense",
"ResourceDeploymentValues",
@@ -10810,6 +10852,17 @@
}
}
},
"codersdk.UserParameter": {
"type": "object",
"properties": {
"name": {
"type": "string"
},
"value": {
"type": "string"
}
}
},
"codersdk.UserQuietHoursScheduleConfig": {
"type": "object",
"properties": {
+1
View File
@@ -821,6 +821,7 @@ func New(options *Options) *API {
r.Post("/convert-login", api.postConvertLoginType)
r.Delete("/", api.deleteUser)
r.Get("/", api.userByName)
r.Get("/autofill-parameters", api.userAutofillParameters)
r.Get("/login-type", api.userLoginType)
r.Put("/profile", api.putUserProfile)
r.Route("/status", func(r chi.Router) {
+11
View File
@@ -1768,6 +1768,17 @@ func (q *querier) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([
return q.db.GetUserLinksByUserID(ctx, userID)
}
func (q *querier) GetUserWorkspaceBuildParameters(ctx context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) {
u, err := q.db.GetUserByID(ctx, params.OwnerID)
if err != nil {
return nil, err
}
if err := q.authorizeContext(ctx, rbac.ActionRead, u.UserWorkspaceBuildParametersObject()); err != nil {
return nil, err
}
return q.db.GetUserWorkspaceBuildParameters(ctx, params)
}
func (q *querier) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]database.GetUsersRow, error) {
// This does the filtering in SQL.
prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceUser.Type)
+11
View File
@@ -1052,6 +1052,17 @@ func (s *MethodTestSuite) TestUser() {
UpdatedAt: u.UpdatedAt,
}).Asserts(u.UserDataRBACObject(), rbac.ActionUpdate).Returns(u)
}))
s.Run("GetUserWorkspaceBuildParameters", s.Subtest(func(db database.Store, check *expects) {
u := dbgen.User(s.T(), db, database.User{})
check.Args(
database.GetUserWorkspaceBuildParametersParams{
OwnerID: u.ID,
TemplateID: uuid.UUID{},
},
).Asserts(u.UserWorkspaceBuildParametersObject(), rbac.ActionRead).Returns(
[]database.GetUserWorkspaceBuildParametersRow{},
)
}))
s.Run("UpdateUserAppearanceSettings", s.Subtest(func(db database.Store, check *expects) {
u := dbgen.User(s.T(), db, database.User{})
check.Args(database.UpdateUserAppearanceSettingsParams{
+59
View File
@@ -3797,6 +3797,65 @@ func (q *FakeQuerier) GetUserLinksByUserID(_ context.Context, userID uuid.UUID)
return uls, nil
}
func (q *FakeQuerier) GetUserWorkspaceBuildParameters(_ context.Context, params database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()
userWorkspaceIDs := make(map[uuid.UUID]struct{})
for _, ws := range q.workspaces {
if ws.OwnerID != params.OwnerID {
continue
}
if ws.TemplateID != params.TemplateID {
continue
}
userWorkspaceIDs[ws.ID] = struct{}{}
}
userWorkspaceBuilds := make(map[uuid.UUID]struct{})
for _, wb := range q.workspaceBuilds {
if _, ok := userWorkspaceIDs[wb.WorkspaceID]; !ok {
continue
}
userWorkspaceBuilds[wb.ID] = struct{}{}
}
templateVersions := make(map[uuid.UUID]struct{})
for _, tv := range q.templateVersions {
if tv.TemplateID.UUID != params.TemplateID {
continue
}
templateVersions[tv.ID] = struct{}{}
}
tvps := make(map[string]struct{})
for _, tvp := range q.templateVersionParameters {
if _, ok := templateVersions[tvp.TemplateVersionID]; !ok {
continue
}
if _, ok := tvps[tvp.Name]; !ok && !tvp.Ephemeral {
tvps[tvp.Name] = struct{}{}
}
}
userWorkspaceBuildParameters := make(map[string]database.GetUserWorkspaceBuildParametersRow)
for _, wbp := range q.workspaceBuildParameters {
if _, ok := userWorkspaceBuilds[wbp.WorkspaceBuildID]; !ok {
continue
}
if _, ok := tvps[wbp.Name]; !ok {
continue
}
userWorkspaceBuildParameters[wbp.Name] = database.GetUserWorkspaceBuildParametersRow{
Name: wbp.Name,
Value: wbp.Value,
}
}
return maps.Values(userWorkspaceBuildParameters), nil
}
func (q *FakeQuerier) GetUsers(_ context.Context, params database.GetUsersParams) ([]database.GetUsersRow, error) {
if err := validateDatabaseType(params); err != nil {
return nil, err
+7
View File
@@ -1012,6 +1012,13 @@ func (m metricsStore) GetUserLinksByUserID(ctx context.Context, userID uuid.UUID
return r0, r1
}
func (m metricsStore) GetUserWorkspaceBuildParameters(ctx context.Context, ownerID database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) {
start := time.Now()
r0, r1 := m.s.GetUserWorkspaceBuildParameters(ctx, ownerID)
m.queryLatencies.WithLabelValues("GetUserWorkspaceBuildParameters").Observe(time.Since(start).Seconds())
return r0, r1
}
func (m metricsStore) GetUsers(ctx context.Context, arg database.GetUsersParams) ([]database.GetUsersRow, error) {
start := time.Now()
users, err := m.s.GetUsers(ctx, arg)
+15
View File
@@ -2104,6 +2104,21 @@ func (mr *MockStoreMockRecorder) GetUserLinksByUserID(arg0, arg1 any) *gomock.Ca
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserLinksByUserID", reflect.TypeOf((*MockStore)(nil).GetUserLinksByUserID), arg0, arg1)
}
// GetUserWorkspaceBuildParameters mocks base method.
func (m *MockStore) GetUserWorkspaceBuildParameters(arg0 context.Context, arg1 database.GetUserWorkspaceBuildParametersParams) ([]database.GetUserWorkspaceBuildParametersRow, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetUserWorkspaceBuildParameters", arg0, arg1)
ret0, _ := ret[0].([]database.GetUserWorkspaceBuildParametersRow)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetUserWorkspaceBuildParameters indicates an expected call of GetUserWorkspaceBuildParameters.
func (mr *MockStoreMockRecorder) GetUserWorkspaceBuildParameters(arg0, arg1 any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetUserWorkspaceBuildParameters", reflect.TypeOf((*MockStore)(nil).GetUserWorkspaceBuildParameters), arg0, arg1)
}
// GetUsers mocks base method.
func (m *MockStore) GetUsers(arg0 context.Context, arg1 database.GetUsersParams) ([]database.GetUsersRow, error) {
m.ctrl.T.Helper()
+4
View File
@@ -256,6 +256,10 @@ func (u User) UserDataRBACObject() rbac.Object {
return rbac.ResourceUserData.WithID(u.ID).WithOwner(u.ID.String())
}
func (u User) UserWorkspaceBuildParametersObject() rbac.Object {
return rbac.ResourceUserWorkspaceBuildParameters.WithID(u.ID).WithOwner(u.ID.String())
}
func (u GetUsersRow) RBACObject() rbac.Object {
return rbac.ResourceUserObject(u.ID)
}
+1
View File
@@ -211,6 +211,7 @@ type sqlcQuerier interface {
GetUserLinkByLinkedID(ctx context.Context, linkedID string) (UserLink, error)
GetUserLinkByUserIDLoginType(ctx context.Context, arg GetUserLinkByUserIDLoginTypeParams) (UserLink, error)
GetUserLinksByUserID(ctx context.Context, userID uuid.UUID) ([]UserLink, error)
GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error)
// This will never return deleted users.
GetUsers(ctx context.Context, arg GetUsersParams) ([]GetUsersRow, error)
// This shouldn't check for deleted, because it's frequently used
+56
View File
@@ -9939,6 +9939,62 @@ func (q *sqlQuerier) InsertWorkspaceAppStats(ctx context.Context, arg InsertWork
return err
}
const getUserWorkspaceBuildParameters = `-- name: GetUserWorkspaceBuildParameters :many
SELECT DISTINCT ON (tvp.name)
tvp.name,
wbp.value
FROM
workspace_build_parameters wbp
JOIN
workspace_builds wb ON wb.id = wbp.workspace_build_id
JOIN
workspaces w ON w.id = wb.workspace_id
JOIN
template_version_parameters tvp ON tvp.template_version_id = wb.template_version_id
WHERE
w.owner_id = $1
AND wb.transition = 'start'
AND w.template_id = $2
AND tvp.ephemeral = false
AND tvp.name = wbp.name
ORDER BY
tvp.name, wb.created_at DESC
LIMIT 100
`
type GetUserWorkspaceBuildParametersParams struct {
OwnerID uuid.UUID `db:"owner_id" json:"owner_id"`
TemplateID uuid.UUID `db:"template_id" json:"template_id"`
}
type GetUserWorkspaceBuildParametersRow struct {
Name string `db:"name" json:"name"`
Value string `db:"value" json:"value"`
}
func (q *sqlQuerier) GetUserWorkspaceBuildParameters(ctx context.Context, arg GetUserWorkspaceBuildParametersParams) ([]GetUserWorkspaceBuildParametersRow, error) {
rows, err := q.db.QueryContext(ctx, getUserWorkspaceBuildParameters, arg.OwnerID, arg.TemplateID)
if err != nil {
return nil, err
}
defer rows.Close()
var items []GetUserWorkspaceBuildParametersRow
for rows.Next() {
var i GetUserWorkspaceBuildParametersRow
if err := rows.Scan(&i.Name, &i.Value); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Close(); err != nil {
return nil, err
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const getWorkspaceBuildParameters = `-- name: GetWorkspaceBuildParameters :many
SELECT
workspace_build_id, name, value
@@ -14,3 +14,27 @@ FROM
workspace_build_parameters
WHERE
workspace_build_id = $1;
-- name: GetUserWorkspaceBuildParameters :many
-- name: GetUserWorkspaceBuildParameters :many
SELECT DISTINCT ON (tvp.name)
tvp.name,
wbp.value
FROM
workspace_build_parameters wbp
JOIN
workspace_builds wb ON wb.id = wbp.workspace_build_id
JOIN
workspaces w ON w.id = wb.workspace_id
JOIN
template_version_parameters tvp ON tvp.template_version_id = wb.template_version_id
WHERE
w.owner_id = $1
AND wb.transition = 'start'
AND w.template_id = $2
AND tvp.ephemeral = false
AND tvp.name = wbp.name
ORDER BY
tvp.name, wb.created_at DESC
LIMIT 100;
+6
View File
@@ -148,6 +148,12 @@ var (
Type: "user_data",
}
// ResourceUserWorkspaceBuildParameters is the user's workspace build
// parameter history.
ResourceUserWorkspaceBuildParameters = Object{
Type: "user_workspace_build_parameters",
}
// ResourceOrganizationMember is a user's membership in an organization.
// Has ONLY an organization owner.
// create/delete = Create/delete member from org.
+1
View File
@@ -25,6 +25,7 @@ func AllResources() []Object {
ResourceTemplateInsights,
ResourceUser,
ResourceUserData,
ResourceUserWorkspaceBuildParameters,
ResourceWildcard,
ResourceWorkspace,
ResourceWorkspaceApplicationConnect,
+6 -4
View File
@@ -154,7 +154,8 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
Permissions(map[string][]Action{
// Users cannot do create/update/delete on themselves, but they
// can read their own details.
ResourceUser.Type: {ActionRead},
ResourceUser.Type: {ActionRead},
ResourceUserWorkspaceBuildParameters.Type: {ActionRead},
// Users can create provisioner daemons scoped to themselves.
ResourceProvisionerDaemon.Type: {ActionCreate, ActionRead, ActionUpdate},
})...,
@@ -209,9 +210,10 @@ func ReloadBuiltinRoles(opts *RoleOptions) {
Name: userAdmin,
DisplayName: "User Admin",
Site: Permissions(map[string][]Action{
ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceUserData.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceRoleAssignment.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceUser.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceUserData.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceUserWorkspaceBuildParameters.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
// Full perms to manage org members
ResourceOrganizationMember.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
ResourceGroup.Type: {ActionCreate, ActionRead, ActionUpdate, ActionDelete},
+51
View File
@@ -569,6 +569,57 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(ctx, rw, http.StatusOK, db2sdk.User(user, organizationIDs))
}
// Returns recent build parameters for the signed-in user.
//
// @Summary Get autofill build parameters for user
// @ID get-autofill-build-parameters-for-user
// @Security CoderSessionToken
// @Produce json
// @Tags Users
// @Param user path string true "User ID, username, or me"
// @Param template_id query string true "Template ID"
// @Success 200 {array} codersdk.UserParameter
// @Router /users/{user}/autofill-parameters [get]
func (api *API) userAutofillParameters(rw http.ResponseWriter, r *http.Request) {
user := httpmw.UserParam(r)
p := httpapi.NewQueryParamParser().Required("template_id")
templateID := p.UUID(r.URL.Query(), uuid.UUID{}, "template_id")
p.ErrorExcessParams(r.URL.Query())
if len(p.Errors) > 0 {
httpapi.Write(r.Context(), rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid query parameters.",
Validations: p.Errors,
})
return
}
params, err := api.Database.GetUserWorkspaceBuildParameters(
r.Context(),
database.GetUserWorkspaceBuildParametersParams{
OwnerID: user.ID,
TemplateID: templateID,
},
)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching user's parameters.",
Detail: err.Error(),
})
return
}
sdkParams := []codersdk.UserParameter{}
for _, param := range params {
sdkParams = append(sdkParams, codersdk.UserParameter{
Name: param.Name,
Value: param.Value,
})
}
httpapi.Write(r.Context(), rw, http.StatusOK, sdkParams)
}
// Returns the user's login type. This only works if the api key for authorization
// and the requested user match. Eg: 'me'
//
+124
View File
@@ -23,6 +23,7 @@ import (
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
"github.com/coder/coder/v2/coderd/database/dbfake"
"github.com/coder/coder/v2/coderd/database/dbgen"
"github.com/coder/coder/v2/coderd/database/dbtime"
"github.com/coder/coder/v2/coderd/rbac"
@@ -1721,6 +1722,129 @@ func TestSuspendedPagination(t *testing.T) {
require.Equal(t, expected, page.Users, "expected page")
}
func TestUserAutofillParameters(t *testing.T) {
t.Parallel()
t.Run("NotSelf", func(t *testing.T) {
t.Parallel()
client1, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{})
u1 := coderdtest.CreateFirstUser(t, client1)
client2, u2 := coderdtest.CreateAnotherUser(t, client1, u1.OrganizationID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
db := api.Database
version := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{
CreatedBy: u1.UserID,
OrganizationID: u1.OrganizationID,
}).Params(database.TemplateVersionParameter{
Name: "param",
Required: true,
}).Do()
_, err := client2.UserAutofillParameters(
ctx,
u1.UserID.String(),
version.Template.ID,
)
var apiErr *codersdk.Error
require.ErrorAs(t, err, &apiErr)
require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
// u1 should be able to read u2's parameters as u1 is site admin.
_, err = client1.UserAutofillParameters(
ctx,
u2.ID.String(),
version.Template.ID,
)
require.NoError(t, err)
})
t.Run("FindsParameters", func(t *testing.T) {
t.Parallel()
client1, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{})
u1 := coderdtest.CreateFirstUser(t, client1)
client2, u2 := coderdtest.CreateAnotherUser(t, client1, u1.OrganizationID)
db := api.Database
version := dbfake.TemplateVersion(t, db).Seed(database.TemplateVersion{
CreatedBy: u1.UserID,
OrganizationID: u1.OrganizationID,
}).Params(database.TemplateVersionParameter{
Name: "param",
Required: true,
},
database.TemplateVersionParameter{
Name: "param2",
Ephemeral: true,
},
).Do()
dbfake.WorkspaceBuild(t, db, database.Workspace{
OwnerID: u2.ID,
TemplateID: version.Template.ID,
OrganizationID: u1.OrganizationID,
}).Params(
database.WorkspaceBuildParameter{
Name: "param",
Value: "foo",
},
database.WorkspaceBuildParameter{
Name: "param2",
Value: "bar",
},
).Do()
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
// Use client2 since client1 is site admin, so
// we don't get good coverage on RBAC working.
params, err := client2.UserAutofillParameters(
ctx,
u2.ID.String(),
version.Template.ID,
)
require.NoError(t, err)
require.Equal(t, 1, len(params))
require.Equal(t, "param", params[0].Name)
require.Equal(t, "foo", params[0].Value)
// Verify that latest parameter value is returned.
dbfake.WorkspaceBuild(t, db, database.Workspace{
OrganizationID: u1.OrganizationID,
OwnerID: u2.ID,
TemplateID: version.Template.ID,
}).Params(
database.WorkspaceBuildParameter{
Name: "param",
Value: "foo_new",
},
).Do()
params, err = client2.UserAutofillParameters(
ctx,
u2.ID.String(),
version.Template.ID,
)
require.NoError(t, err)
require.Equal(t, 1, len(params))
require.Equal(t, "param", params[0].Name)
require.Equal(t, "foo_new", params[0].Value)
})
}
// TestPaginatedUsers creates a list of users, then tries to paginate through
// them using different page sizes.
func TestPaginatedUsers(t *testing.T) {