From c138d0968613ecc27a92bd58507f848914f435db Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 26 Apr 2023 10:07:26 -0800 Subject: [PATCH 1/3] Extract randWorkspace to shared class --- src/test/groovy/CoderCLIManagerTest.groovy | 26 +--------------------- src/test/groovy/DataGen.groovy | 26 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 25 deletions(-) create mode 100644 src/test/groovy/DataGen.groovy diff --git a/src/test/groovy/CoderCLIManagerTest.groovy b/src/test/groovy/CoderCLIManagerTest.groovy index 8f09d4c7..9c135c88 100644 --- a/src/test/groovy/CoderCLIManagerTest.groovy +++ b/src/test/groovy/CoderCLIManagerTest.groovy @@ -1,10 +1,5 @@ package com.coder.gateway.sdk -import com.coder.gateway.models.WorkspaceAgentModel -import com.coder.gateway.models.WorkspaceAndAgentStatus -import com.coder.gateway.models.WorkspaceVersionStatus -import com.coder.gateway.sdk.v2.models.WorkspaceStatus -import com.coder.gateway.sdk.v2.models.WorkspaceTransition import com.sun.net.httpserver.HttpExchange import com.sun.net.httpserver.HttpHandler import com.sun.net.httpserver.HttpServer @@ -364,25 +359,6 @@ class CoderCLIManagerTest extends Specification { Path.of("/tmp/coder-gateway-test/localappdata/coder-gateway") == dataDir() } - private WorkspaceAgentModel randWorkspace(String name) { - return new WorkspaceAgentModel( - UUID.randomUUID(), - name, - name, - UUID.randomUUID(), - "template-name", - "template-icon-path", - null, - WorkspaceVersionStatus.UPDATED, - WorkspaceStatus.RUNNING, - WorkspaceAndAgentStatus.READY, - WorkspaceTransition.START, - null, - null, - null - ) - } - def "configures an SSH file"() { given: def sshConfigPath = tmpdir.resolve(input + "_to_" + output + ".conf") @@ -401,7 +377,7 @@ class CoderCLIManagerTest extends Specification { .replace("/tmp/coder-gateway/test.coder.invalid/coder-linux-amd64", ccm.localBinaryPath.toString()) when: - ccm.configSsh(workspaces.collect { randWorkspace(it) }) + ccm.configSsh(workspaces.collect { DataGen.workspace(it) }) then: sshConfigPath.toFile().text == expectedConf diff --git a/src/test/groovy/DataGen.groovy b/src/test/groovy/DataGen.groovy new file mode 100644 index 00000000..af609f99 --- /dev/null +++ b/src/test/groovy/DataGen.groovy @@ -0,0 +1,26 @@ +import com.coder.gateway.models.WorkspaceAgentModel +import com.coder.gateway.models.WorkspaceAndAgentStatus +import com.coder.gateway.models.WorkspaceVersionStatus +import com.coder.gateway.sdk.v2.models.WorkspaceStatus +import com.coder.gateway.sdk.v2.models.WorkspaceTransition + +class DataGen { + static WorkspaceAgentModel workspace(String name, String workspaceName = name) { + return new WorkspaceAgentModel( + UUID.randomUUID(), + workspaceName, + name, + UUID.randomUUID(), + "template-name", + "template-icon-path", + null, + WorkspaceVersionStatus.UPDATED, + WorkspaceStatus.RUNNING, + WorkspaceAndAgentStatus.READY, + WorkspaceTransition.START, + null, + null, + null + ) + } +} From f9c8573979de390b9a8a058f6e23418d41b6ebe6 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 26 Apr 2023 10:59:10 -0800 Subject: [PATCH 2/3] Break out workspaces table I want to write a unit test for the selection logic. --- .../views/steps/CoderWorkspacesStepView.kt | 71 +++++++++---------- 1 file changed, 33 insertions(+), 38 deletions(-) diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt index a0fa03df..11ab21b7 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -108,16 +108,9 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod private var tfUrl: JTextField? = null private var cbExistingToken: JCheckBox? = null - private var listTableModelOfWorkspaces = ListTableModel( - WorkspaceIconColumnInfo(""), - WorkspaceNameColumnInfo("Name"), - WorkspaceTemplateNameColumnInfo("Template"), - WorkspaceVersionColumnInfo("Version"), - WorkspaceStatusColumnInfo("Status") - ) private val notificationBanner = NotificationBanner() - private var tableOfWorkspaces = TableView(listTableModelOfWorkspaces).apply { + private var tableOfWorkspaces = WorkspacesTable().apply { setEnableAntialiasing(true) rowSelectionAllowed = true columnSelectionAllowed = false @@ -348,7 +341,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } override fun onInit(wizardModel: CoderWorkspacesWizardModel) { - listTableModelOfWorkspaces.items = emptyList() + tableOfWorkspaces.listTableModel.items = emptyList() if (localWizardModel.coderURL.isNotBlank() && localWizardModel.token != null) { triggerWorkspacePolling(true) } else { @@ -454,7 +447,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod // Clear out old deployment details. poller?.cancel() tableOfWorkspaces.setEmptyState("Connecting to $deploymentURL...") - listTableModelOfWorkspaces.items = emptyList() + tableOfWorkspaces.listTableModel.items = emptyList() // Authenticate and load in a background process with progress. // TODO: Make this cancelable. @@ -677,7 +670,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } withContext(Dispatchers.Main) { val selectedWorkspace = tableOfWorkspaces.selectedObject?.name - listTableModelOfWorkspaces.items = ws.toList() + tableOfWorkspaces.listTableModel.items = ws.toList() if (selectedWorkspace != null) { tableOfWorkspaces.selectItem(selectedWorkspace) } @@ -764,7 +757,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod else CoderCLIManager.getDataDir(), settings.binarySource, ) - cliManager.configSsh(listTableModelOfWorkspaces.items) + cliManager.configSsh(tableOfWorkspaces.items) logger.info("Opening IDE and Project Location window for ${workspace.name}") return true @@ -776,6 +769,18 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod cs.cancel() } + companion object { + val logger = Logger.getInstance(CoderWorkspacesStepView::class.java.simpleName) + } +} + +class WorkspacesTableModel : ListTableModel( + WorkspaceIconColumnInfo(""), + WorkspaceNameColumnInfo("Name"), + WorkspaceTemplateNameColumnInfo("Template"), + WorkspaceVersionColumnInfo("Version"), + WorkspaceStatusColumnInfo("Status") +) { private class WorkspaceIconColumnInfo(columnName: String) : ColumnInfo(columnName) { override fun valueOf(workspace: WorkspaceAgentModel?): String? { return workspace?.templateName @@ -803,7 +808,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } } - private inner class WorkspaceNameColumnInfo(columnName: String) : ColumnInfo(columnName) { + private class WorkspaceNameColumnInfo(columnName: String) : ColumnInfo(columnName) { override fun valueOf(workspace: WorkspaceAgentModel?): String? { return workspace?.name } @@ -822,7 +827,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod text = value } - font = RelativeFont.BOLD.derive(this@CoderWorkspacesStepView.tableOfWorkspaces.tableHeader.font) + font = RelativeFont.BOLD.derive(table.tableHeader.font) border = JBUI.Borders.empty(0, 8) return this } @@ -830,7 +835,8 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } } - private inner class WorkspaceTemplateNameColumnInfo(columnName: String) : ColumnInfo(columnName) { + private class WorkspaceTemplateNameColumnInfo(columnName: String) : + ColumnInfo(columnName) { override fun valueOf(workspace: WorkspaceAgentModel?): String? { return workspace?.templateName } @@ -842,11 +848,6 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } override fun getRenderer(item: WorkspaceAgentModel?): TableCellRenderer { - val simpleH3 = this@CoderWorkspacesStepView.tableOfWorkspaces.tableHeader.font - - val h3AttributesWithUnderlining = simpleH3.attributes as MutableMap - h3AttributesWithUnderlining[TextAttribute.UNDERLINE] = UNDERLINE_ON - val underlinedH3 = JBFont.h3().deriveFont(h3AttributesWithUnderlining) return object : DefaultTableCellRenderer() { override fun getTableCellRendererComponent(table: JTable, value: Any, isSelected: Boolean, hasFocus: Boolean, row: Int, column: Int): Component { super.getTableCellRendererComponent(table, value, isSelected, hasFocus, row, column) @@ -855,10 +856,13 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } border = JBUI.Borders.empty(0, 8) + val simpleH3 = table.tableHeader.font if (table.getClientProperty(MOUSE_OVER_TEMPLATE_NAME_COLUMN_ON_ROW) != null) { val mouseOverRow = table.getClientProperty(MOUSE_OVER_TEMPLATE_NAME_COLUMN_ON_ROW) as Int if (mouseOverRow >= 0 && mouseOverRow == row) { - font = underlinedH3 + val h3AttributesWithUnderlining = simpleH3.attributes as MutableMap + h3AttributesWithUnderlining[TextAttribute.UNDERLINE] = UNDERLINE_ON + font = JBFont.h3().deriveFont(h3AttributesWithUnderlining) return this } } @@ -869,7 +873,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } } - private inner class WorkspaceVersionColumnInfo(columnName: String) : ColumnInfo(columnName) { + private class WorkspaceVersionColumnInfo(columnName: String) : ColumnInfo(columnName) { override fun valueOf(workspace: WorkspaceAgentModel?): String? { return workspace?.status?.label } @@ -881,7 +885,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod if (value is String) { text = value } - font = this@CoderWorkspacesStepView.tableOfWorkspaces.tableHeader.font + font = table.tableHeader.font border = JBUI.Borders.empty(0, 8) return this } @@ -889,7 +893,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } } - private inner class WorkspaceStatusColumnInfo(columnName: String) : ColumnInfo(columnName) { + private class WorkspaceStatusColumnInfo(columnName: String) : ColumnInfo(columnName) { override fun valueOf(workspace: WorkspaceAgentModel?): String? { return workspace?.agentStatus?.label } @@ -902,29 +906,24 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod override fun getRenderer(item: WorkspaceAgentModel?): TableCellRenderer { return object : DefaultTableCellRenderer() { - override fun getTableCellRendererComponent( - table: JTable, - value: Any, - isSelected: Boolean, - hasFocus: Boolean, - row: Int, - column: Int, - ): Component { + override fun getTableCellRendererComponent(table: JTable, value: Any, isSelected: Boolean, hasFocus: Boolean, row: Int, column: Int): Component { super.getTableCellRendererComponent(table, value, isSelected, hasFocus, row, column) if (value is String) { text = value foreground = WorkspaceAndAgentStatus.from(value).statusColor() toolTipText = WorkspaceAndAgentStatus.from(value).description } - font = this@CoderWorkspacesStepView.tableOfWorkspaces.tableHeader.font + font = table.tableHeader.font border = JBUI.Borders.empty(0, 8) return this } } } } +} - private fun TableView.selectItem(workspaceName: String?) { +class WorkspacesTable : TableView(WorkspacesTableModel()) { + fun selectItem(workspaceName: String?) { if (workspaceName != null) { this.items.forEachIndexed { index, workspaceAgentModel -> if (workspaceAgentModel.name == workspaceName) { @@ -935,8 +934,4 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } } } - - companion object { - val logger = Logger.getInstance(CoderWorkspacesStepView::class.java.simpleName) - } } From a83b0bc0329000f94ee96e5f2bf73ac66fa59fe2 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 26 Apr 2023 11:24:19 -0800 Subject: [PATCH 3/3] Preserve selection when workspaces starts or stops When a workspace starts the workspace row is replaced with a row for the agent(s) and you have to select it again which is annoying (similarly for when the workspace stops). This preserves the selection when the workspace transitions. --- .../gateway/models/WorkspaceAgentModel.kt | 8 ++- .../views/steps/CoderWorkspacesStepView.kt | 40 +++++++++----- .../groovy/CoderWorkspacesStepViewTest.groovy | 54 +++++++++++++++++++ 3 files changed, 88 insertions(+), 14 deletions(-) create mode 100644 src/test/groovy/CoderWorkspacesStepViewTest.groovy diff --git a/src/main/kotlin/com/coder/gateway/models/WorkspaceAgentModel.kt b/src/main/kotlin/com/coder/gateway/models/WorkspaceAgentModel.kt index 6cfc103b..c4893b9c 100644 --- a/src/main/kotlin/com/coder/gateway/models/WorkspaceAgentModel.kt +++ b/src/main/kotlin/com/coder/gateway/models/WorkspaceAgentModel.kt @@ -7,10 +7,16 @@ import com.coder.gateway.sdk.v2.models.WorkspaceTransition import java.util.UUID import javax.swing.Icon +// TODO: Refactor to have a list of workspaces that each have agents. We +// present in the UI as a single flat list in the table (when there are no +// agents we display a row for the workspace) but still, a list of workspaces +// each with a list of agents might reflect reality more closely. When we +// iterate over the list we can add the workspace row if it has no agents +// otherwise iterate over the agents and then flatten the result. data class WorkspaceAgentModel( val workspaceID: UUID, val workspaceName: String, - val name: String, + val name: String, // Name of the workspace OR the agent if this is for an agent. val templateID: UUID, val templateName: String, val templateIconPath: String, diff --git a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt index 11ab21b7..62d4c1d3 100644 --- a/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt +++ b/src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt @@ -669,11 +669,9 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod } } withContext(Dispatchers.Main) { - val selectedWorkspace = tableOfWorkspaces.selectedObject?.name + val selectedWorkspace = tableOfWorkspaces.selectedObject tableOfWorkspaces.listTableModel.items = ws.toList() - if (selectedWorkspace != null) { - tableOfWorkspaces.selectItem(selectedWorkspace) - } + tableOfWorkspaces.selectItem(selectedWorkspace) } } @@ -923,15 +921,31 @@ class WorkspacesTableModel : ListTableModel( } class WorkspacesTable : TableView(WorkspacesTableModel()) { - fun selectItem(workspaceName: String?) { - if (workspaceName != null) { - this.items.forEachIndexed { index, workspaceAgentModel -> - if (workspaceAgentModel.name == workspaceName) { - selectionModel.addSelectionInterval(convertRowIndexToView(index), convertRowIndexToView(index)) - // fix cell selection case - columnModel.selectionModel.addSelectionInterval(0, columnCount - 1) - } - } + /** + * Given either a workspace or an agent select in order of preference: + * 1. That same agent or workspace. + * 2. The first match for the workspace (workspace itself or first agent). + */ + fun selectItem(workspace: WorkspaceAgentModel?) { + val index = getNewSelection(workspace) + if (index > -1) { + selectionModel.addSelectionInterval(convertRowIndexToView(index), convertRowIndexToView(index)) + // Fix cell selection case. + columnModel.selectionModel.addSelectionInterval(0, columnCount - 1) } } + + private fun getNewSelection(oldSelection: WorkspaceAgentModel?): Int { + if (oldSelection == null) { + return -1 + } + val index = listTableModel.items.indexOfFirst { + it.name == oldSelection.name && it.workspaceName == oldSelection.workspaceName + } + if (index > -1) { + return index + } + return listTableModel.items.indexOfFirst { it.workspaceName == oldSelection.workspaceName } + } + } diff --git a/src/test/groovy/CoderWorkspacesStepViewTest.groovy b/src/test/groovy/CoderWorkspacesStepViewTest.groovy new file mode 100644 index 00000000..c684e963 --- /dev/null +++ b/src/test/groovy/CoderWorkspacesStepViewTest.groovy @@ -0,0 +1,54 @@ +import com.coder.gateway.views.steps.WorkspacesTable +import spock.lang.Specification +import spock.lang.Unroll + +@Unroll +class CoderWorkspacesStepViewTest extends Specification { + def "gets new selection"() { + given: + def table = new WorkspacesTable() + table.listTableModel.items = List.of( + // An off workspace. + DataGen.workspace("ws1", "ws1"), + + // On workspaces. + DataGen.workspace("agent1", "ws2"), + DataGen.workspace("agent2", "ws2"), + DataGen.workspace("agent3", "ws3"), + + // Another off workspace. + DataGen.workspace("ws4", "ws4"), + + // In practice we do not list both agents and workspaces + // together but here test that anyway with an agent first and + // then with a workspace first. + DataGen.workspace("agent2", "ws5"), + DataGen.workspace("ws5", "ws5"), + DataGen.workspace("ws6", "ws6"), + DataGen.workspace("agent3", "ws6"), + ) + + expect: + table.getNewSelection(selected) == expected + + where: + selected | expected + null | -1 // No selection. + DataGen.workspace("gone", "gone") | -1 // No workspace that matches. + DataGen.workspace("ws1", "ws1") | 0 // Workspace exact match. + DataGen.workspace("gone", "ws1") | 0 // Agent gone, select workspace. + DataGen.workspace("ws2", "ws2") | 1 // Workspace gone, select first agent. + DataGen.workspace("agent1", "ws2") | 1 // Agent exact match. + DataGen.workspace("agent2", "ws2") | 2 // Agent exact match. + DataGen.workspace("ws3", "ws3") | 3 // Workspace gone, select first agent. + DataGen.workspace("agent3", "ws3") | 3 // Agent exact match. + DataGen.workspace("gone", "ws4") | 4 // Agent gone, select workspace. + DataGen.workspace("ws4", "ws4") | 4 // Workspace exact match. + DataGen.workspace("agent2", "ws5") | 5 // Agent exact match. + DataGen.workspace("gone", "ws5") | 5 // Agent gone, another agent comes first. + DataGen.workspace("ws5", "ws5") | 6 // Workspace exact match. + DataGen.workspace("ws6", "ws6") | 7 // Workspace exact match. + DataGen.workspace("gone", "ws6") | 7 // Agent gone, workspace comes first. + DataGen.workspace("agent3", "ws6") | 8 // Agent exact match. + } +}