code review updates

Signed-off-by: Spike Curtis <spike@coder.com>
coder · spikecurtis · Oct 6, 2023 · May 10, 2023 · May 10, 2023 · May 10, 2023
commit 27e899412007dc79c4480b2e13e33eb675fd76fe
diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go
@@ -122,7 +122,6 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
 			workspace = coderdtest.MustTransitionWorkspace(
 				t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
 
-			// Given: the workspace template has been updated
 			orgs, err := client.OrganizationsByUser(ctx, workspace.OwnerID.String())
 			require.NoError(t, err)
 			require.Len(t, orgs, 1)
@@ -149,6 +148,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
 				}
 			}
 
+			// Given: the workspace template has been updated
 			newVersion := coderdtest.UpdateTemplateVersion(t, client, orgs[0].ID, res, workspace.TemplateID)
 			coderdtest.AwaitTemplateVersionJobCompleted(t, client, newVersion.ID)
 			require.NoError(t, client.UpdateActiveTemplateVersion(
@@ -166,24 +166,25 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
 
 			stats := <-statsCh
 			assert.NoError(t, stats.Error)
-			if tc.expectStart {
-				// Then: the workspace should be started
-				assert.Len(t, stats.Transitions, 1)
-				assert.Contains(t, stats.Transitions, workspace.ID)
-				assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[workspace.ID])
-				ws := coderdtest.MustWorkspace(t, client, workspace.ID)
-				if tc.expectUpdate {
-					// Then: uses the updated version
-					assert.Equal(t, newVersion.ID, ws.LatestBuild.TemplateVersionID,
-						"expected workspace build to be using the updated template version")
-				} else {
-					// Then: uses the previous template version
-					assert.Equal(t, workspace.LatestBuild.TemplateVersionID, ws.LatestBuild.TemplateVersionID,
-						"expected workspace build to be using the old template version")
-				}
-			} else {
+			if !tc.expectStart {
 				// Then: the workspace should not be started
 				assert.Len(t, stats.Transitions, 0)
+				return
+			}
+
+			// Then: the workspace should be started
+			assert.Len(t, stats.Transitions, 1)
+			assert.Contains(t, stats.Transitions, workspace.ID)
+			assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[workspace.ID])
+			ws := coderdtest.MustWorkspace(t, client, workspace.ID)
+			if tc.expectUpdate {
+				// Then: uses the updated version
+				assert.Equal(t, newVersion.ID, ws.LatestBuild.TemplateVersionID,
+					"expected workspace build to be using the updated template version")
+			} else {
+				// Then: uses the previous template version
+				assert.Equal(t, workspace.LatestBuild.TemplateVersionID, ws.LatestBuild.TemplateVersionID,
+					"expected workspace build to be using the old template version")
 			}
 		})
 	}

diff --git a/coderd/workspaces.go b/coderd/workspaces.go
@@ -1038,7 +1038,7 @@ func (api *API) putWorkspaceAutoupdates(rw http.ResponseWriter, r *http.Request)
 	}
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-			Message: "Internal error updating workspace autostart schedule.",
+			Message: "Internal error updating workspace automatic updates setting",
 			Detail:  err.Error(),
 		})
 		return

diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -2178,13 +2178,14 @@ func TestWorkspaceUpdateAutomaticUpdates_OK(t *testing.T) {
 	t.Parallel()
 
 	var (
-		auditor   = audit.NewMock()
-		client    = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, Auditor: auditor})
-		user      = coderdtest.CreateFirstUser(t, client)
-		version   = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
-		_         = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
-		project   = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
-		workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
+		auditor      = audit.NewMock()
+		adminClient  = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, Auditor: auditor})
+		admin        = coderdtest.CreateFirstUser(t, adminClient)
+		client, user = coderdtest.CreateAnotherUser(t, adminClient, admin.OrganizationID)
+		version      = coderdtest.CreateTemplateVersion(t, adminClient, admin.OrganizationID, nil)
+		_            = coderdtest.AwaitTemplateVersionJobCompleted(t, adminClient, version.ID)
+		project      = coderdtest.CreateTemplate(t, adminClient, admin.OrganizationID, version.ID)
+		workspace    = coderdtest.CreateWorkspace(t, client, admin.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
 			cwr.AutostartSchedule = nil
 			cwr.TTLMillis = nil
 			cwr.AutomaticUpdates = codersdk.AutomaticUpdatesNever
@@ -2210,11 +2211,12 @@ func TestWorkspaceUpdateAutomaticUpdates_OK(t *testing.T) {
 	require.Equal(t, codersdk.AutomaticUpdatesAlways, updated.AutomaticUpdates)
 
 	require.Eventually(t, func() bool {
-		if len(auditor.AuditLogs()) < 7 {
-			return false
-		}
-		return auditor.AuditLogs()[6].Action == database.AuditActionWrite
+		return len(auditor.AuditLogs()) >= 9
 	}, testutil.WaitShort, testutil.IntervalFast)
+	l := auditor.AuditLogs()[8]
+	require.Equal(t, database.AuditActionWrite, l.Action)
+	require.Equal(t, user.ID, l.UserID)
+	require.Equal(t, workspace.ID, l.ResourceID)
 }
 
 func TestUpdateWorkspaceAutomaticUpdates_NotFound(t *testing.T) {