Skip to content

Commit 67644cc

Browse files
EmyrkEmyrk
committed
remove need to constantly send owner id
1 parent f651edc commit 67644cc

File tree

3 files changed

+26
-18
lines changed

3 files changed

+26
-18
lines changed

‎coderd/dynamicparameters/render.go

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ func (r *Loader) Load(ctx context.Context, db database.Store) error {
105105
return nil
106106
}
107107

108-
func (r *Loader) loaded() bool {
108+
func (r *Loader) isReady() bool {
109109
return r.templateVersion != nil && r.job != nil && r.terraformValues != nil
110110
}
111111

@@ -117,7 +117,7 @@ func (r *Loader) loaded() bool {
117117
// do not have the database state to support dynamic parameters. A constant
118118
// warning will be displayed for these template versions.
119119
func (r *Loader) Renderer(ctx context.Context, db database.Store, cache *files.Cache) (Renderer, error) {
120-
if !r.loaded() {
120+
if !r.isReady() {
121121
return nil, xerrors.New("Load() must be called before Renderer()")
122122
}
123123

@@ -154,11 +154,11 @@ func (r *Loader) dynamicRenderer(ctx context.Context, db database.Store, cache *
154154
}
155155

156156
return &DynamicRenderer{
157-
data: r,
158-
templateFS: templateFS,
159-
db: db,
160-
plan: plan,
161-
failedOwners: make(map[uuid.UUID]error),
157+
data: r,
158+
templateFS: templateFS,
159+
db: db,
160+
plan: plan,
161+
ownerErrors: make(map[uuid.UUID]error),
162162
close: func() {
163163
cache.Release(r.job.FileID)
164164
if moduleFilesFS != nil {
@@ -174,22 +174,23 @@ type DynamicRenderer struct {
174174
templateFS fs.FS
175175
plan json.RawMessage
176176

177-
failedOwners map[uuid.UUID]error
178-
currentOwner *previewtypes.WorkspaceOwner
177+
ownerErrors map[uuid.UUID]error
178+
currentOwner *previewtypes.WorkspaceOwner
179+
currentOwnerID uuid.UUID
179180

180181
once sync.Once
181182
close func()
182183
}
183184

184185
func (r *DynamicRenderer) Render(ctx context.Context, ownerID uuid.UUID, values map[string]string) (*preview.Output, hcl.Diagnostics) {
185186
// Always start with the cached error, if we have one.
186-
ownerErr := r.failedOwners[ownerID]
187+
ownerErr := r.ownerErrors[ownerID]
187188
if ownerErr == nil {
188189
ownerErr = r.getWorkspaceOwnerData(ctx, ownerID)
189190
}
190191

191192
if ownerErr != nil || r.currentOwner == nil {
192-
r.failedOwners[ownerID] = ownerErr
193+
r.ownerErrors[ownerID] = ownerErr
193194
return nil, hcl.Diagnostics{
194195
{
195196
Severity: hcl.DiagError,

‎coderd/parameters.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request
168168
// As the user types into the form, reprocess the state using their input,
169169
// and respond with updates.
170170
updates := stream.Chan()
171+
ownerID := initial.OwnerID
171172
for {
172173
select {
173174
case <-ctx.Done():
@@ -179,6 +180,14 @@ func (api *API) handleParameterWebsocket(rw http.ResponseWriter, r *http.Request
179180
return
180181
}
181182

183+
// Take a nil uuid to mean the previous owner ID.
184+
// This just removes the need to constantly send who you are.
185+
if update.OwnerID == uuid.Nil {
186+
update.OwnerID = ownerID
187+
}
188+
189+
ownerID = update.OwnerID
190+
182191
result, diagnostics := render.Render(ctx, update.OwnerID, update.Inputs)
183192
response := codersdk.DynamicParametersResponse{
184193
ID: update.ID,

‎enterprise/coderd/parameters_test.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ func TestDynamicParametersOwnerGroups(t *testing.T) {
3131
Options: &coderdtest.Options{IncludeProvisionerDaemon: true},
3232
},
3333
)
34-
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.RoleTemplateAdmin())
34+
templateAdmin, templateAdminUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID, rbac.ScopedRoleOrgTemplateAdmin(owner.OrganizationID))
3535
_, noGroupUser := coderdtest.CreateAnotherUser(t, ownerClient, owner.OrganizationID)
3636

3737
// Create the group to be asserted
@@ -79,36 +79,34 @@ func TestDynamicParametersOwnerGroups(t *testing.T) {
7979
require.NoError(t, err)
8080
defer stream.Close(websocket.StatusGoingAway)
8181

82-
previews := stream.Chan()
82+
previews, pop := coderdtest.SynchronousStream(stream)
8383

8484
// Should automatically send a form state with all defaulted/empty values
85-
preview := testutil.RequireReceive(ctx, t, previews)
85+
preview := pop()
8686
require.Equal(t, -1, preview.ID)
8787
require.Empty(t, preview.Diagnostics)
8888
require.Equal(t, "group", preview.Parameters[0].Name)
8989
require.True(t, preview.Parameters[0].Value.Valid)
9090
require.Equal(t, database.EveryoneGroup, preview.Parameters[0].Value.Value)
9191

9292
// Send a new value, and see it reflected
93-
err = stream.Send(codersdk.DynamicParametersRequest{
93+
preview, err = previews(codersdk.DynamicParametersRequest{
9494
ID: 1,
9595
Inputs: map[string]string{"group": group.Name},
9696
})
9797
require.NoError(t, err)
98-
preview = testutil.RequireReceive(ctx, t, previews)
9998
require.Equal(t, 1, preview.ID)
10099
require.Empty(t, preview.Diagnostics)
101100
require.Equal(t, "group", preview.Parameters[0].Name)
102101
require.True(t, preview.Parameters[0].Value.Valid)
103102
require.Equal(t, group.Name, preview.Parameters[0].Value.Value)
104103

105104
// Back to default
106-
err = stream.Send(codersdk.DynamicParametersRequest{
105+
preview, err = previews(codersdk.DynamicParametersRequest{
107106
ID: 3,
108107
Inputs: map[string]string{},
109108
})
110109
require.NoError(t, err)
111-
preview = testutil.RequireReceive(ctx, t, previews)
112110
require.Equal(t, 3, preview.ID)
113111
require.Empty(t, preview.Diagnostics)
114112
require.Equal(t, "group", preview.Parameters[0].Name)

0 commit comments

Comments
 (0)