From 1bc46dba951793eddf4f7aff716b7ad2524ab9f3 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 30 Jan 2024 13:37:22 +0000 Subject: [PATCH 1/2] feat(site): do not show popover on update deadline --- site/src/api/queries/workspaces.ts | 32 +- .../WorkspaceScheduleControls.test.tsx | 189 +++++++++++ .../WorkspaceScheduleControls.tsx | 302 +++++------------- 3 files changed, 276 insertions(+), 247 deletions(-) create mode 100644 site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx diff --git a/site/src/api/queries/workspaces.ts b/site/src/api/queries/workspaces.ts index 25b74984eb0af..b1252fe9af0f0 100644 --- a/site/src/api/queries/workspaces.ts +++ b/site/src/api/queries/workspaces.ts @@ -1,8 +1,11 @@ import * as API from "api/api"; -import { QueryClient, type QueryOptions } from "react-query"; +import { + QueryClient, + UseMutationOptions, + type QueryOptions, +} from "react-query"; import { putWorkspaceExtension } from "api/api"; -import dayjs from "dayjs"; -import { getDeadline, getMaxDeadline, getMinDeadline } from "utils/schedule"; +import { Dayjs } from "dayjs"; import { type WorkspaceBuildParameter, type Workspace, @@ -103,25 +106,12 @@ export function workspaces(config: WorkspacesRequest = {}) { } as const satisfies QueryOptions; } -export const decreaseDeadline = (workspace: Workspace) => { - return { - mutationFn: (hours: number) => { - const proposedDeadline = getDeadline(workspace).subtract(hours, "hours"); - const newDeadline = dayjs.max(proposedDeadline, getMinDeadline()); - return putWorkspaceExtension(workspace.id, newDeadline); - }, - }; -}; - -export const increaseDeadline = (workspace: Workspace) => { +export const updateDeadline = ( + workspace: Workspace, +): UseMutationOptions => { return { - mutationFn: (hours: number) => { - const proposedDeadline = getDeadline(workspace).add(hours, "hours"); - const newDeadline = dayjs.min( - proposedDeadline, - getMaxDeadline(workspace), - ); - return putWorkspaceExtension(workspace.id, newDeadline); + mutationFn: (deadline: Dayjs) => { + return putWorkspaceExtension(workspace.id, deadline); }, }; }; diff --git a/site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx b/site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx new file mode 100644 index 0000000000000..71997850858d5 --- /dev/null +++ b/site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx @@ -0,0 +1,189 @@ +import { render, screen } from "@testing-library/react"; +import { ThemeProvider } from "contexts/ThemeProvider"; +import { QueryClient, QueryClientProvider, useQuery } from "react-query"; +import { MockWorkspace } from "testHelpers/entities"; +import { WorkspaceScheduleControls } from "./WorkspaceScheduleControls"; +import { workspaceByOwnerAndName } from "api/queries/workspaces"; +import { RouterProvider, createMemoryRouter } from "react-router-dom"; +import userEvent from "@testing-library/user-event"; +import { server } from "testHelpers/server"; +import { rest } from "msw"; +import dayjs from "dayjs"; +import * as API from "api/api"; +import { GlobalSnackbar } from "components/GlobalSnackbar/GlobalSnackbar"; + +const Wrapper = () => { + const { data: workspace } = useQuery( + workspaceByOwnerAndName(MockWorkspace.owner_name, MockWorkspace.name), + ); + + if (!workspace) { + return null; + } + + return ; +}; + +test("add 3 hours to deadline", async () => { + const user = userEvent.setup(); + const baseDeadline = dayjs().add(3, "hour"); + const updateDeadlineSpy = jest + .spyOn(API, "putWorkspaceExtension") + .mockResolvedValue(); + server.use( + rest.get( + "/api/v2/users/:username/workspace/:workspaceName", + (req, res, ctx) => { + return res( + ctx.status(200), + ctx.json({ + ...MockWorkspace, + latest_build: { + ...MockWorkspace.latest_build, + deadline: baseDeadline.toISOString(), + }, + }), + ); + }, + ), + ); + render( + + + }])} + /> + + + , + ); + await screen.findByTestId("schedule-controls"); + + // Check if base deadline is displayed correctly + expect(screen.getByText("Stop in 3 hours")).toBeInTheDocument(); + + const addButton = screen.getByLabelText("Add hour"); + await user.click(addButton); + await user.click(addButton); + await user.click(addButton); + await screen.findByText( + "Workspace shutdown time has been successfully updated.", + ); + expect(updateDeadlineSpy).toHaveBeenCalledTimes(1); + expect(screen.getByText("Stop in 6 hours")).toBeInTheDocument(); + + // Mocks are used here because the 'usedDeadline' is a dayjs object, which + // can't be directly compared. + const usedWorkspaceId = updateDeadlineSpy.mock.calls[0][0]; + const usedDeadline = updateDeadlineSpy.mock.calls[0][1]; + expect(usedWorkspaceId).toEqual(MockWorkspace.id); + expect(usedDeadline.toISOString()).toEqual( + baseDeadline.add(3, "hour").toISOString(), + ); +}); + +test("remove 3 hours to deadline", async () => { + const user = userEvent.setup(); + const baseDeadline = dayjs().add(3, "hour"); + const updateDeadlineSpy = jest + .spyOn(API, "putWorkspaceExtension") + .mockResolvedValue(); + server.use( + rest.get( + "/api/v2/users/:username/workspace/:workspaceName", + (req, res, ctx) => { + return res( + ctx.status(200), + ctx.json({ + ...MockWorkspace, + latest_build: { + ...MockWorkspace.latest_build, + deadline: baseDeadline.toISOString(), + }, + }), + ); + }, + ), + ); + render( + + + }])} + /> + + + , + ); + await screen.findByTestId("schedule-controls"); + + // Check if base deadline is displayed correctly + expect(screen.getByText("Stop in 3 hours")).toBeInTheDocument(); + + const subButton = screen.getByLabelText("Subtract hour"); + await user.click(subButton); + await user.click(subButton); + await screen.findByText( + "Workspace shutdown time has been successfully updated.", + ); + expect(updateDeadlineSpy).toHaveBeenCalledTimes(1); + expect(screen.getByText("Stop in an hour")).toBeInTheDocument(); + + // Mocks are used here because the 'usedDeadline' is a dayjs object, which + // can't be directly compared. + const usedWorkspaceId = updateDeadlineSpy.mock.calls[0][0]; + const usedDeadline = updateDeadlineSpy.mock.calls[0][1]; + expect(usedWorkspaceId).toEqual(MockWorkspace.id); + expect(usedDeadline.toISOString()).toEqual( + baseDeadline.subtract(2, "hour").toISOString(), + ); +}); + +test("rollback to previous deadline on error", async () => { + const user = userEvent.setup(); + const baseDeadline = dayjs().add(3, "hour"); + const updateDeadlineSpy = jest + .spyOn(API, "putWorkspaceExtension") + .mockRejectedValue({}); + server.use( + rest.get( + "/api/v2/users/:username/workspace/:workspaceName", + (req, res, ctx) => { + return res( + ctx.status(200), + ctx.json({ + ...MockWorkspace, + latest_build: { + ...MockWorkspace.latest_build, + deadline: baseDeadline.toISOString(), + }, + }), + ); + }, + ), + ); + render( + + + }])} + /> + + + , + ); + await screen.findByTestId("schedule-controls"); + + // Check if base deadline is displayed correctly + expect(screen.getByText("Stop in 3 hours")).toBeInTheDocument(); + + const addButton = screen.getByLabelText("Add hour"); + await user.click(addButton); + await user.click(addButton); + await user.click(addButton); + await screen.findByText( + "We couldn't update your workspace shutdown time. Please try again.", + ); + expect(updateDeadlineSpy).toHaveBeenCalledTimes(1); + expect(screen.getByText("Stop in 3 hours")).toBeInTheDocument(); +}); diff --git a/site/src/pages/WorkspacePage/WorkspaceScheduleControls.tsx b/site/src/pages/WorkspacePage/WorkspaceScheduleControls.tsx index 3327c3035ada6..25997e16a19dc 100644 --- a/site/src/pages/WorkspacePage/WorkspaceScheduleControls.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceScheduleControls.tsx @@ -1,7 +1,6 @@ -import { css } from "@emotion/css"; import { type Interpolation, type Theme } from "@emotion/react"; import Link, { LinkProps } from "@mui/material/Link"; -import { forwardRef, type FC } from "react"; +import { forwardRef, type FC, useRef } from "react"; import { Link as RouterLink } from "react-router-dom"; import { isWorkspaceOn } from "utils/workspace"; import type { Workspace } from "api/typesGenerated"; @@ -16,20 +15,16 @@ import { import IconButton from "@mui/material/IconButton"; import RemoveIcon from "@mui/icons-material/RemoveOutlined"; import AddIcon from "@mui/icons-material/AddOutlined"; -import TextField from "@mui/material/TextField"; -import Button from "@mui/material/Button"; -import { - Popover, - PopoverContent, - PopoverTrigger, - usePopover, -} from "components/Popover/Popover"; import Tooltip from "@mui/material/Tooltip"; import _ from "lodash"; import { getErrorMessage } from "api/errors"; -import { decreaseDeadline, increaseDeadline } from "api/queries/workspaces"; -import { displaySuccess, displayError } from "components/GlobalSnackbar/utils"; -import { useMutation } from "react-query"; +import { + updateDeadline, + workspaceByOwnerAndNameKey, +} from "api/queries/workspaces"; +import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; +import { useMutation, useQueryClient } from "react-query"; +import { Dayjs } from "dayjs"; export interface WorkspaceScheduleControlsProps { workspace: Workspace; @@ -40,6 +35,7 @@ export const WorkspaceScheduleControls: FC = ({ workspace, canUpdateSchedule, }) => { + const queryClient = useQueryClient(); const deadline = getDeadline(workspace); const maxDeadlineDecrease = getMaxDeadlineChange(deadline, getMinDeadline()); const maxDeadlineIncrease = getMaxDeadlineChange( @@ -48,28 +44,50 @@ export const WorkspaceScheduleControls: FC = ({ ); const deadlinePlusEnabled = maxDeadlineIncrease >= 1; const deadlineMinusEnabled = maxDeadlineDecrease >= 1; - - const onDeadlineChangeSuccess = () => { - displaySuccess("Updated workspace shutdown time."); - }; - const onDeadlineChangeFails = (error: unknown) => { - displayError( - getErrorMessage(error, "Failed to update workspace shutdown time."), + const deadlineUpdateTimeout = useRef(); + const lastStableDeadline = useRef(deadline); + + const updateWorkspaceDeadlineQueryData = (deadline: Dayjs) => { + queryClient.setQueryData( + workspaceByOwnerAndNameKey(workspace.owner_name, workspace.name), + { + ...workspace, + latest_build: { + ...workspace.latest_build, + deadline: deadline.toISOString(), + }, + }, ); }; - const decreaseMutation = useMutation({ - ...decreaseDeadline(workspace), - onSuccess: onDeadlineChangeSuccess, - onError: onDeadlineChangeFails, - }); - const increaseMutation = useMutation({ - ...increaseDeadline(workspace), - onSuccess: onDeadlineChangeSuccess, - onError: onDeadlineChangeFails, + + const updateDeadlineMutation = useMutation({ + ...updateDeadline(workspace), + onSuccess: (_, updatedDeadline) => { + displaySuccess("Workspace shutdown time has been successfully updated."); + lastStableDeadline.current = updatedDeadline; + }, + onError: (error) => { + displayError( + getErrorMessage( + error, + "We couldn't update your workspace shutdown time. Please try again.", + ), + ); + updateWorkspaceDeadlineQueryData(lastStableDeadline.current); + }, }); + const handleDeadlineChange = (newDeadline: Dayjs) => { + clearTimeout(deadlineUpdateTimeout.current); + // Optimistic update + updateWorkspaceDeadlineQueryData(newDeadline); + deadlineUpdateTimeout.current = window.setTimeout(() => { + updateDeadlineMutation.mutate(newDeadline); + }, 500); + }; + return ( -
+
{isWorkspaceOn(workspace) ? ( ) : ( @@ -79,161 +97,40 @@ export const WorkspaceScheduleControls: FC = ({ )} {canUpdateSchedule && canEditDeadline(workspace) && ( - - - - - - - - + + { + handleDeadlineChange(deadline.subtract(1, "h")); + }} > - - - - - - - - - - + + + + { + handleDeadlineChange(deadline.add(1, "h")); + }} > - - - - + + + +
)}
); }; -interface AddTimeContentProps { - maxDeadlineIncrease: number; - onDeadlinePlus: (value: number) => void; -} - -const AddTimeContent: FC = ({ - maxDeadlineIncrease, - onDeadlinePlus, -}) => { - const popover = usePopover(); - - return ( - <> -

Add hours to deadline

-

- Delay the shutdown of this workspace for a few more hours. This is only - applied once. -

-
{ - e.preventDefault(); - const formData = new FormData(e.currentTarget); - const hours = Number(formData.get("hours")); - onDeadlinePlus(hours); - popover.setIsOpen(false); - }} - > - - - - - - ); -}; - -interface DecreaseTimeContentProps { - maxDeadlineDecrease: number; - onDeadlineMinus: (hours: number) => void; -} - -export const DecreaseTimeContent: FC = ({ - maxDeadlineDecrease, - onDeadlineMinus, -}) => { - const popover = usePopover(); - - return ( - <> -

Subtract hours to deadline

-

- Anticipate the shutdown of this workspace for a few more hours. This is - only applied once. -

-
{ - e.preventDefault(); - const formData = new FormData(e.currentTarget); - const hours = Number(formData.get("hours")); - onDeadlineMinus(hours); - popover.setIsOpen(false); - }} - > - - - - - - ); -}; - interface AutoStopDisplayProps { workspace: Workspace; } @@ -311,29 +208,12 @@ const isShutdownSoon = (workspace: Workspace): boolean => { return diff < oneHour; }; -const classNames = { - paper: css` - padding: 24px; - max-width: 288px; - margin-top: 8px; - border-radius: 4px; - display: flex; - flex-direction: column; - gap: 8px; - `, - - deadlineFormInput: css` - font-size: 14px; - padding: 0px; - border-radius: 4px; - `, -}; - const styles = { scheduleValue: { display: "flex", alignItems: "center", gap: 12, + fontVariantNumeric: "tabular-nums", }, scheduleControls: { @@ -353,34 +233,4 @@ const styles = { height: 12, }, }), - - timePopoverTitle: { - fontWeight: 600, - margin: 0, - marginBottom: 8, - }, - - timePopoverDescription: (theme) => ({ - color: theme.palette.text.secondary, - margin: 0, - }), - - timePopoverForm: { - display: "flex", - alignItems: "center", - gap: 8, - padding: "8px 0", - marginTop: 12, - }, - - timePopoverField: { - margin: 0, - }, - - timePopoverButton: { - borderRadius: 4, - paddingLeft: 16, - paddingRight: 16, - flexShrink: 0, - }, } satisfies Record>; From f7ac8a53c4387ea7df8e88a59699af2a5c5820dd Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 30 Jan 2024 16:56:59 +0000 Subject: [PATCH 2/2] Resolve PR suggestions --- .../WorkspaceScheduleControls.test.tsx | 129 +++++++----------- .../WorkspaceScheduleControls.tsx | 8 +- 2 files changed, 52 insertions(+), 85 deletions(-) diff --git a/site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx b/site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx index 71997850858d5..87599a49e89a0 100644 --- a/site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceScheduleControls.test.tsx @@ -24,12 +24,9 @@ const Wrapper = () => { return ; }; -test("add 3 hours to deadline", async () => { - const user = userEvent.setup(); - const baseDeadline = dayjs().add(3, "hour"); - const updateDeadlineSpy = jest - .spyOn(API, "putWorkspaceExtension") - .mockResolvedValue(); +const BASE_DEADLINE = dayjs().add(3, "hour"); + +const renderScheduleControls = async () => { server.use( rest.get( "/api/v2/users/:username/workspace/:workspaceName", @@ -40,7 +37,7 @@ test("add 3 hours to deadline", async () => { ...MockWorkspace, latest_build: { ...MockWorkspace.latest_build, - deadline: baseDeadline.toISOString(), + deadline: BASE_DEADLINE.toISOString(), }, }), ); @@ -58,18 +55,26 @@ test("add 3 hours to deadline", async () => { , ); await screen.findByTestId("schedule-controls"); - - // Check if base deadline is displayed correctly expect(screen.getByText("Stop in 3 hours")).toBeInTheDocument(); +}; + +test("add 3 hours to deadline", async () => { + const user = userEvent.setup(); + const updateDeadlineSpy = jest + .spyOn(API, "putWorkspaceExtension") + .mockResolvedValue(); - const addButton = screen.getByLabelText("Add hour"); + await renderScheduleControls(); + + const addButton = screen.getByRole("button", { + name: /add 1 hour to deadline/i, + }); await user.click(addButton); await user.click(addButton); await user.click(addButton); await screen.findByText( "Workspace shutdown time has been successfully updated.", ); - expect(updateDeadlineSpy).toHaveBeenCalledTimes(1); expect(screen.getByText("Stop in 6 hours")).toBeInTheDocument(); // Mocks are used here because the 'usedDeadline' is a dayjs object, which @@ -78,55 +83,26 @@ test("add 3 hours to deadline", async () => { const usedDeadline = updateDeadlineSpy.mock.calls[0][1]; expect(usedWorkspaceId).toEqual(MockWorkspace.id); expect(usedDeadline.toISOString()).toEqual( - baseDeadline.add(3, "hour").toISOString(), + BASE_DEADLINE.add(3, "hour").toISOString(), ); }); test("remove 3 hours to deadline", async () => { const user = userEvent.setup(); - const baseDeadline = dayjs().add(3, "hour"); const updateDeadlineSpy = jest .spyOn(API, "putWorkspaceExtension") .mockResolvedValue(); - server.use( - rest.get( - "/api/v2/users/:username/workspace/:workspaceName", - (req, res, ctx) => { - return res( - ctx.status(200), - ctx.json({ - ...MockWorkspace, - latest_build: { - ...MockWorkspace.latest_build, - deadline: baseDeadline.toISOString(), - }, - }), - ); - }, - ), - ); - render( - - - }])} - /> - - - , - ); - await screen.findByTestId("schedule-controls"); - // Check if base deadline is displayed correctly - expect(screen.getByText("Stop in 3 hours")).toBeInTheDocument(); + await renderScheduleControls(); - const subButton = screen.getByLabelText("Subtract hour"); + const subButton = screen.getByRole("button", { + name: /subtract 1 hour from deadline/i, + }); await user.click(subButton); await user.click(subButton); await screen.findByText( "Workspace shutdown time has been successfully updated.", ); - expect(updateDeadlineSpy).toHaveBeenCalledTimes(1); expect(screen.getByText("Stop in an hour")).toBeInTheDocument(); // Mocks are used here because the 'usedDeadline' is a dayjs object, which @@ -135,55 +111,46 @@ test("remove 3 hours to deadline", async () => { const usedDeadline = updateDeadlineSpy.mock.calls[0][1]; expect(usedWorkspaceId).toEqual(MockWorkspace.id); expect(usedDeadline.toISOString()).toEqual( - baseDeadline.subtract(2, "hour").toISOString(), + BASE_DEADLINE.subtract(2, "hour").toISOString(), ); }); test("rollback to previous deadline on error", async () => { const user = userEvent.setup(); - const baseDeadline = dayjs().add(3, "hour"); + const initialScheduleMessage = "Stop in 3 hours"; + jest.spyOn(API, "putWorkspaceExtension").mockRejectedValue({}); + + await renderScheduleControls(); + + const addButton = screen.getByRole("button", { + name: /add 1 hour to deadline/i, + }); + await user.click(addButton); + await user.click(addButton); + await user.click(addButton); + await screen.findByText( + "We couldn't update your workspace shutdown time. Please try again.", + ); + // In case of an error, the schedule message should remain unchanged + expect(screen.getByText(initialScheduleMessage)).toBeInTheDocument(); +}); + +test("request is only sent once when clicking multiple times", async () => { + const user = userEvent.setup(); const updateDeadlineSpy = jest .spyOn(API, "putWorkspaceExtension") - .mockRejectedValue({}); - server.use( - rest.get( - "/api/v2/users/:username/workspace/:workspaceName", - (req, res, ctx) => { - return res( - ctx.status(200), - ctx.json({ - ...MockWorkspace, - latest_build: { - ...MockWorkspace.latest_build, - deadline: baseDeadline.toISOString(), - }, - }), - ); - }, - ), - ); - render( - - - }])} - /> - - - , - ); - await screen.findByTestId("schedule-controls"); + .mockResolvedValue(); - // Check if base deadline is displayed correctly - expect(screen.getByText("Stop in 3 hours")).toBeInTheDocument(); + await renderScheduleControls(); - const addButton = screen.getByLabelText("Add hour"); + const addButton = screen.getByRole("button", { + name: /add 1 hour to deadline/i, + }); await user.click(addButton); await user.click(addButton); await user.click(addButton); await screen.findByText( - "We couldn't update your workspace shutdown time. Please try again.", + "Workspace shutdown time has been successfully updated.", ); expect(updateDeadlineSpy).toHaveBeenCalledTimes(1); - expect(screen.getByText("Stop in 3 hours")).toBeInTheDocument(); }); diff --git a/site/src/pages/WorkspacePage/WorkspaceScheduleControls.tsx b/site/src/pages/WorkspacePage/WorkspaceScheduleControls.tsx index 25997e16a19dc..4613f6d489c84 100644 --- a/site/src/pages/WorkspacePage/WorkspaceScheduleControls.tsx +++ b/site/src/pages/WorkspacePage/WorkspaceScheduleControls.tsx @@ -25,6 +25,7 @@ import { import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; import { useMutation, useQueryClient } from "react-query"; import { Dayjs } from "dayjs"; +import { visuallyHidden } from "@mui/utils"; export interface WorkspaceScheduleControlsProps { workspace: Workspace; @@ -100,21 +101,19 @@ export const WorkspaceScheduleControls: FC = ({
{ handleDeadlineChange(deadline.subtract(1, "h")); }} > + Subtract 1 hour - + = ({ }} > + Add 1 hour