Skip to content

refactor(site): update agent status to include the lifecycle #5835

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 4 commits into from
Jan 24, 2023
Merged

refactor(site): update agent status to include the lifecycle #5835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0368d98
refactor: update agent status to include the lifecycle
BrunoQuaresma Jan 24, 2023
d6f3456
Update site/src/i18n/en/agent.json
BrunoQuaresma Jan 24, 2023
2f3a7f4
Remove unused test
BrunoQuaresma Jan 24, 2023
443dd8e
Merge branch 'bq/update-agent-status' of github.com:coder/coder into …
BrunoQuaresma Jan 24, 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
27 changes: 27 additions & 0 deletions site/src/components/Resources/AgentRow.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import {
MockWorkspace,
MockWorkspaceAgent,
MockWorkspaceAgentConnecting,
MockWorkspaceAgentStartError,
MockWorkspaceAgentStarting,
MockWorkspaceAgentStartTimeout,
MockWorkspaceAgentTimeout,
MockWorkspaceApp,
} from "testHelpers/entities"
Expand Down Expand Up @@ -86,6 +89,30 @@ Timeout.args = {
showApps: true,
}

export const Starting = Template.bind({})
Starting.args = {
agent: MockWorkspaceAgentStarting,
workspace: MockWorkspace,
applicationsHost: "",
showApps: true,
}

export const StartTimeout = Template.bind({})
StartTimeout.args = {
agent: MockWorkspaceAgentStartTimeout,
workspace: MockWorkspace,
applicationsHost: "",
showApps: true,
}

export const StartError = Template.bind({})
StartError.args = {
agent: MockWorkspaceAgentStartError,
workspace: MockWorkspace,
applicationsHost: "",
showApps: true,
}

export const ShowingPortForward = Template.bind({})
ShowingPortForward.args = {
agent: MockWorkspaceAgent,
Expand Down
141 changes: 138 additions & 3 deletions site/src/components/Resources/AgentStatus.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,146 @@ import {
import { useRef, useState } from "react"
import Link from "@material-ui/core/Link"

const ConnectedStatus: React.FC = () => {
// If we think in the agent status and lifecycle into a single enum/state I’d
// say we would have: connecting, timeout, disconnected, connected:created,
// connected:starting, connected:start_timeout, connected:start_error,
// connected:ready

const ReadyLifeCycle: React.FC = () => {
const styles = useStyles()
const { t } = useTranslation("workspacePage")

return (
<div
role="status"
aria-label={t("agentStatus.connected")}
aria-label={t("agentStatus.connected.ready")}
className={combineClasses([styles.status, styles.connected])}
/>
)
}

const StartingLifecycle: React.FC = () => {
const styles = useStyles()
const { t } = useTranslation("workspacePage")

return (
<Tooltip title={t("agentStatus.connected.starting")}>
<div
role="status"
aria-label={t("agentStatus.connected.starting")}
className={combineClasses([styles.status, styles.connecting])}
/>
</Tooltip>
)
}

const StartTimeoutLifecycle: React.FC<{
agent: WorkspaceAgent
}> = ({ agent }) => {
const { t } = useTranslation("agent")
const styles = useStyles()
const anchorRef = useRef<SVGSVGElement>(null)
const [isOpen, setIsOpen] = useState(false)
const id = isOpen ? "timeout-popover" : undefined

return (
<>
<WarningRounded
ref={anchorRef}
onMouseEnter={() => setIsOpen(true)}
onMouseLeave={() => setIsOpen(false)}
role="status"
aria-label={t("status.startTimeout")}
className={styles.timeoutWarning}
/>
<HelpPopover
id={id}
open={isOpen}
anchorEl={anchorRef.current}
onOpen={() => setIsOpen(true)}
onClose={() => setIsOpen(false)}
>
<HelpTooltipTitle>{t("startTimeoutTooltip.title")}</HelpTooltipTitle>
<HelpTooltipText>
{t("startTimeoutTooltip.message")}{" "}
<Link
target="_blank"
rel="noreferrer"
href={agent.troubleshooting_url}
>
{t("startTimeoutTooltip.link")}
</Link>
.
</HelpTooltipText>
</HelpPopover>
</>
)
}

const StartErrorLifecycle: React.FC<{
agent: WorkspaceAgent
}> = ({ agent }) => {
const { t } = useTranslation("agent")
const styles = useStyles()
const anchorRef = useRef<SVGSVGElement>(null)
const [isOpen, setIsOpen] = useState(false)
const id = isOpen ? "timeout-popover" : undefined

return (
<>
<WarningRounded
ref={anchorRef}
onMouseEnter={() => setIsOpen(true)}
onMouseLeave={() => setIsOpen(false)}
role="status"
aria-label={t("status.error")}
className={styles.errorWarning}
/>
<HelpPopover
id={id}
open={isOpen}
anchorEl={anchorRef.current}
onOpen={() => setIsOpen(true)}
onClose={() => setIsOpen(false)}
>
<HelpTooltipTitle>{t("startErrorTooltip.title")}</HelpTooltipTitle>
<HelpTooltipText>
{t("startErrorTooltip.message")}{" "}
<Link
target="_blank"
rel="noreferrer"
href={agent.troubleshooting_url}
>
{t("startErrorTooltip.link")}
</Link>
.
</HelpTooltipText>
</HelpPopover>
</>
)
}

const ConnectedStatus: React.FC<{
agent: WorkspaceAgent
}> = ({ agent }) => {
return (
<ChooseOne>
<Cond condition={agent.lifecycle_state === "ready"}>
<ReadyLifeCycle />
</Cond>
<Cond condition={agent.lifecycle_state === "start_timeout"}>
<StartTimeoutLifecycle agent={agent} />
</Cond>
<Cond condition={agent.lifecycle_state === "start_error"}>
<StartErrorLifecycle agent={agent} />
</Cond>
<Cond>
<StartingLifecycle />
Copy link
Member

Choose a reason for hiding this comment

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

This will catch both early created and starting states, so this is fine! In the future we'll have states for shutting down as well.

</Cond>
</ChooseOne>
)
}

const DisconnectedStatus: React.FC = () => {
const styles = useStyles()
const { t } = useTranslation("workspacePage")
Expand Down Expand Up @@ -105,7 +232,7 @@ export const AgentStatus: React.FC<{
return (
<ChooseOne>
<Cond condition={agent.status === "connected"}>
<ConnectedStatus />
<ConnectedStatus agent={agent} />
</Cond>
<Cond condition={agent.status === "disconnected"}>
<DisconnectedStatus />
Expand Down Expand Up @@ -160,4 +287,12 @@ const useStyles = makeStyles((theme) => ({
position: "relative",
top: theme.spacing(1),
},

errorWarning: {
color: theme.palette.error.main,
width: theme.spacing(2.5),
height: theme.spacing(2.5),
position: "relative",
top: theme.spacing(1),
},
}))
14 changes: 13 additions & 1 deletion site/src/i18n/en/agent.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,24 @@
"noApps": "None"
},
"status": {
"timeout": "Timeout"
"timeout": "Timeout",
"startTimeout": "Start Timeout",
"startError": "Error"
},
"timeoutTooltip": {
"title": "Agent is taking too long to connect",
"message": "We noticed this agent is taking longer than expected to connect.",
"link": "Troubleshoot"
},
"startTimeoutTooltip": {
"title": "Agent is taking too long to start",
"message": "We noticed this agent is taking longer than expected to start.",
"link": "Troubleshoot"
},
"startErrorTooltip": {
"title": "Error starting agent",
"message": "Something went wrong during the agent start.",
"link": "Troubleshoot"
},
"unableToConnect": "Unable to connect"
}
5 changes: 4 additions & 1 deletion site/src/i18n/en/workspacePage.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@
"pending": "Pending"
},
"agentStatus": {
"connected": "Connected",
"connected": {
"ready": "Ready",
"starting": "Starting..."
},
"connecting": "Connecting...",
"disconnected": "Disconnected",
"timeout": "Timeout"
Expand Down
65 changes: 0 additions & 65 deletions site/src/pages/WorkspacePage/WorkspacePage.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,7 @@ import {
MockStoppingWorkspace,
MockTemplate,
MockWorkspace,
MockWorkspaceAgent,
MockWorkspaceAgentConnecting,
MockWorkspaceAgentDisconnected,
MockWorkspaceBuild,
MockWorkspaceResource2,
renderWithAuth,
waitForLoaderToBeRemoved,
} from "../../testHelpers/renderHelpers"
Expand Down Expand Up @@ -284,65 +280,4 @@ describe("WorkspacePage", () => {
})
})
})

describe("Resources", () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I really think tests like these should live in Storybook. Integration tests should be used for user interactions IMO.

it("shows the status of each agent in each resource", async () => {
const getTemplateMock = jest
.spyOn(api, "getTemplate")
.mockResolvedValueOnce(MockTemplate)

const workspaceWithResources = {
...MockWorkspace,
latest_build: {
...MockWorkspaceBuild,
resources: [
{
...MockWorkspaceResource2,
agents: [
MockWorkspaceAgent,
MockWorkspaceAgentDisconnected,
MockWorkspaceAgentConnecting,
],
},
],
},
}

server.use(
rest.get(
`/api/v2/users/:username/workspace/:workspaceName`,
(req, res, ctx) => {
return res(ctx.status(200), ctx.json(workspaceWithResources))
},
),
)

await renderWorkspacePage()
const agent1Names = await screen.findAllByText(MockWorkspaceAgent.name)
expect(agent1Names.length).toEqual(1)
const agent2Names = await screen.findAllByText(
MockWorkspaceAgentDisconnected.name,
)
expect(agent2Names.length).toEqual(2)
const agent1Status = await screen.findAllByLabelText(
t<string>(`agentStatus.${MockWorkspaceAgent.status}`, {
ns: "workspacePage",
}),
)
expect(agent1Status.length).toEqual(1)
const agentDisconnected = await screen.findAllByLabelText(
t<string>(`agentStatus.${MockWorkspaceAgentDisconnected.status}`, {
ns: "workspacePage",
}),
)
expect(agentDisconnected.length).toEqual(1)
const agentConnecting = await screen.findAllByLabelText(
t<string>(`agentStatus.${MockWorkspaceAgentConnecting.status}`, {
ns: "workspacePage",
}),
)
expect(agentConnecting.length).toEqual(1)
expect(getTemplateMock).toBeCalled()
})
})
})