Fix selection when workspace name collides

code-asher · code-asher · commit 30839c8613f8 · 2024-10-03T18:07:50.000-08:00
This can happen if multiple users are using the same name for their
workspaces.

Turns out we can just match the workspace ID.

It hardly seems worth a test to see that a == is working, so I did not
bother to update it.
diff --git a/src/main/kotlin/com/coder/gateway/models/WorkspaceAgentListModel.kt b/src/main/kotlin/com/coder/gateway/models/WorkspaceAgentListModel.kt
@@ -16,6 +16,7 @@ data class WorkspaceAgentListModel(
     var icon: Icon? = null,
     // The combined status of the workspace and agent to display on the row.
     val status: WorkspaceAndAgentStatus = WorkspaceAndAgentStatus.from(workspace, agent),
-    // The combined `workspace.agent` name to display on the row.
+    // The combined `workspace.agent` name to display on the row.  Users can have workspaces with the same name, so it
+    // must not be used as a unique identifier.
     val name: String = if (agent != null) "${workspace.name}.${agent.name}" else workspace.name,
 )
diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt
@@ -989,17 +989,19 @@ class WorkspacesTable : TableView<WorkspaceAgentListModel>(WorkspacesTableModel(
         }
     }
 
-    fun getNewSelection(oldSelection: WorkspaceAgentListModel?): Int {
+    /**
+     * If a row becomes unselected because the workspace turned on, find the
+     * first agent row and select that.
+     *
+     * If a row becomes unselected because the workspace turned off, find the
+     * workspace row and select that.
+     */
+    private fun getNewSelection(oldSelection: WorkspaceAgentListModel?): Int {
         if (oldSelection == null) {
             return -1
         }
-        val index = listTableModel.items.indexOfFirst { it.name == oldSelection.name }
-        if (index > -1) {
-            return index
-        }
-        // If there is no matching agent, try matching on just the workspace.
-        // It is possible it turned off so it no longer has agents displaying;
-        // in this case we want to keep it highlighted.
-        return listTableModel.items.indexOfFirst { it.workspace.name == oldSelection.workspace.name }
+        // Both cases are handled by just looking for the ID, since we only ever
+        // show agents or a workspace but never both.
+        return listTableModel.items.indexOfFirst { it.workspace.id == oldSelection.workspace.id }
     }
 }
diff --git a/src/test/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepViewTest.kt b/src/test/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepViewTest.kt

Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@ data class WorkspaceAgentListModel(`
`16`	`16`	`var icon: Icon? = null,`
`17`	`17`	`// The combined status of the workspace and agent to display on the row.`
`18`	`18`	`val status: WorkspaceAndAgentStatus = WorkspaceAndAgentStatus.from(workspace, agent),`
`19`		- // The combined `workspace.agent` name to display on the row.
	`19`	+ // The combined `workspace.agent` name to display on the row. Users can have workspaces with the same name, so it
	`20`	`+ // must not be used as a unique identifier.`
`20`	`21`	`val name: String = if (agent != null) "${workspace.name}.${agent.name}" else workspace.name,`
`21`	`22`	`)`
Original file line number	Diff line number	Diff line change
`@@ -989,17 +989,19 @@ class WorkspacesTable : TableView<WorkspaceAgentListModel>(WorkspacesTableModel(`
`989`	`989`	`}`
`990`	`990`	`}`
`991`	`991`
`992`		`- fun getNewSelection(oldSelection: WorkspaceAgentListModel?): Int {`
	`992`	`+ /**`
	`993`	`+ * If a row becomes unselected because the workspace turned on, find the`
	`994`	`+ * first agent row and select that.`
	`995`	`+ *`
	`996`	`+ * If a row becomes unselected because the workspace turned off, find the`
	`997`	`+ * workspace row and select that.`
	`998`	`+ */`
	`999`	`+ private fun getNewSelection(oldSelection: WorkspaceAgentListModel?): Int {`
`993`	`1000`	`if (oldSelection == null) {`
`994`	`1001`	`return -1`
`995`	`1002`	`}`
`996`		`- val index = listTableModel.items.indexOfFirst { it.name == oldSelection.name }`
`997`		`- if (index > -1) {`
`998`		`- return index`
`999`		`- }`
`1000`		`- // If there is no matching agent, try matching on just the workspace.`
`1001`		`- // It is possible it turned off so it no longer has agents displaying;`
`1002`		`- // in this case we want to keep it highlighted.`
`1003`		`- return listTableModel.items.indexOfFirst { it.workspace.name == oldSelection.workspace.name }`
	`1003`	`+ // Both cases are handled by just looking for the ID, since we only ever`
	`1004`	`+ // show agents or a workspace but never both.`
	`1005`	`+ return listTableModel.items.indexOfFirst { it.workspace.id == oldSelection.workspace.id }`
`1004`	`1006`	`}`
`1005`	`1007`	`}`