Skip to content

Preserve selection when workspace starts/stops #233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 27, 2023
Merged

Preserve selection when workspace starts/stops #233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c138d09
Extract randWorkspace to shared class
code-asher Apr 26, 2023
f9c8573
Break out workspaces table
code-asher Apr 26, 2023
a83b0bc
Preserve selection when workspaces starts or stops
code-asher Apr 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/main/kotlin/com/coder/gateway/models/WorkspaceAgentModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
105 changes: 57 additions & 48 deletions src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspacesStepView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<WorkspaceAgentModel>(
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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -676,11 +669,9 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
}
}
withContext(Dispatchers.Main) {
val selectedWorkspace = tableOfWorkspaces.selectedObject?.name
listTableModelOfWorkspaces.items = ws.toList()
if (selectedWorkspace != null) {
tableOfWorkspaces.selectItem(selectedWorkspace)
}
val selectedWorkspace = tableOfWorkspaces.selectedObject
tableOfWorkspaces.listTableModel.items = ws.toList()
tableOfWorkspaces.selectItem(selectedWorkspace)
}
}

Expand Down Expand Up @@ -764,7 +755,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
Expand All @@ -776,6 +767,18 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
cs.cancel()
}

companion object {
val logger = Logger.getInstance(CoderWorkspacesStepView::class.java.simpleName)
}
}

class WorkspacesTableModel : ListTableModel<WorkspaceAgentModel>(
WorkspaceIconColumnInfo(""),
WorkspaceNameColumnInfo("Name"),
WorkspaceTemplateNameColumnInfo("Template"),
WorkspaceVersionColumnInfo("Version"),
WorkspaceStatusColumnInfo("Status")
) {
private class WorkspaceIconColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentModel, String>(columnName) {
override fun valueOf(workspace: WorkspaceAgentModel?): String? {
return workspace?.templateName
Expand Down Expand Up @@ -803,7 +806,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
}
}

private inner class WorkspaceNameColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentModel, String>(columnName) {
private class WorkspaceNameColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentModel, String>(columnName) {
override fun valueOf(workspace: WorkspaceAgentModel?): String? {
return workspace?.name
}
Expand All @@ -822,15 +825,16 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
text = value
}

font = RelativeFont.BOLD.derive([email protected].tableHeader.font)
font = RelativeFont.BOLD.derive(table.tableHeader.font)
border = JBUI.Borders.empty(0, 8)
return this
}
}
}
}

private inner class WorkspaceTemplateNameColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentModel, String>(columnName) {
private class WorkspaceTemplateNameColumnInfo(columnName: String) :
ColumnInfo<WorkspaceAgentModel, String>(columnName) {
override fun valueOf(workspace: WorkspaceAgentModel?): String? {
return workspace?.templateName
}
Expand All @@ -842,11 +846,6 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
}

override fun getRenderer(item: WorkspaceAgentModel?): TableCellRenderer {
val simpleH3 = [email protected]

val h3AttributesWithUnderlining = simpleH3.attributes as MutableMap<TextAttribute, Any>
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)
Expand All @@ -855,10 +854,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<TextAttribute, Any>
h3AttributesWithUnderlining[TextAttribute.UNDERLINE] = UNDERLINE_ON
font = JBFont.h3().deriveFont(h3AttributesWithUnderlining)
return this
}
}
Expand All @@ -869,7 +871,7 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
}
}

private inner class WorkspaceVersionColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentModel, String>(columnName) {
private class WorkspaceVersionColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentModel, String>(columnName) {
override fun valueOf(workspace: WorkspaceAgentModel?): String? {
return workspace?.status?.label
}
Expand All @@ -881,15 +883,15 @@ class CoderWorkspacesStepView(val setNextButtonEnabled: (Boolean) -> Unit) : Cod
if (value is String) {
text = value
}
font = [email protected].tableHeader.font
font = table.tableHeader.font
border = JBUI.Borders.empty(0, 8)
return this
}
}
}
}

private inner class WorkspaceStatusColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentModel, String>(columnName) {
private class WorkspaceStatusColumnInfo(columnName: String) : ColumnInfo<WorkspaceAgentModel, String>(columnName) {
override fun valueOf(workspace: WorkspaceAgentModel?): String? {
return workspace?.agentStatus?.label
}
Expand All @@ -902,41 +904,48 @@ 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 = [email protected].tableHeader.font
font = table.tableHeader.font
border = JBUI.Borders.empty(0, 8)
return this
}
}
}
}
}

private fun TableView<WorkspaceAgentModel>.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)
}
}
class WorkspacesTable : TableView<WorkspaceAgentModel>(WorkspacesTableModel()) {
/**
* 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)
}
}

companion object {
val logger = Logger.getInstance(CoderWorkspacesStepView::class.java.simpleName)
private fun getNewSelection(oldSelection: WorkspaceAgentModel?): Int {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming that returning -1 when not found is an explicit thing you're meant to do with a UI table?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is more related to lists in general than the table per se; we use an index to select an item and the various indexOf functions return -1 for no match so I did the same. But I also explicitly check for -1 and avoid selecting in that case so really we could return null for example if we wanted. I am not sure what would happen if we passed -1 for the selection index directly to the table.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and it errors with invalid index.

It does have some other functions that will return a -1 (for example convertRowIndexToView returns a -1 if the row is not visible) so I think using -1 is generally the preferred pattern.

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 }
}

}
26 changes: 1 addition & 25 deletions src/test/groovy/CoderCLIManagerTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand Down
54 changes: 54 additions & 0 deletions src/test/groovy/CoderWorkspacesStepViewTest.groovy
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice test. I'm not a huge Groovy fan, but I do like the table-driven Spock DSL.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same!

Original file line number Diff line number Diff line change
@@ -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.
}
}
26 changes: 26 additions & 0 deletions src/test/groovy/DataGen.groovy
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice

Original file line number Diff line number Diff line change
@@ -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
)
}
}