diff --git a/codersdk/toolsdk/toolsdk_test.go b/codersdk/toolsdk/toolsdk_test.go index 0754514693a0e..fb321e90e7dee 100644 --- a/codersdk/toolsdk/toolsdk_test.go +++ b/codersdk/toolsdk/toolsdk_test.go @@ -665,6 +665,10 @@ func TestMain(m *testing.M) { var untested []string for _, tool := range toolsdk.All { if tested, ok := testedTools.Load(tool.Name); !ok || !tested.(bool) { + // Test is skipped on Windows + if runtime.GOOS == "windows" && tool.Name == "coder_workspace_bash" { + continue + } untested = append(untested, tool.Name) } } diff --git a/docs/user-guides/desktop/index.md b/docs/user-guides/desktop/index.md index 116f7d4d6de69..d5f5e5aabb3c2 100644 --- a/docs/user-guides/desktop/index.md +++ b/docs/user-guides/desktop/index.md @@ -8,12 +8,6 @@ Coder Desktop requires a Coder deployment running [v2.20.0](https://github.com/c ## Install Coder Desktop -> [!IMPORTANT] -> Coder Desktop can't connect through a corporate VPN. -> -> Due to a [known issue](#coder-desktop-cant-connect-through-another-vpn), -> if your Coder deployment requires that you connect through a corporate VPN, Desktop will timeout when it tries to connect. -
You can install Coder Desktop on macOS or Windows. @@ -144,9 +138,7 @@ To avoid system VPN configuration conflicts, only one copy of `Coder Desktop.app If the logged in Coder deployment requires a corporate VPN to connect, Coder Connect can't establish communication through the VPN, and will time out. -This is due to known issues with [macOS](https://github.com/coder/coder-desktop-macos/issues/201) and -[Windows](https://github.com/coder/coder-desktop-windows/issues/147) networking. -A resolution is in progress. +This issue has been fixed in Coder v2.24.3 and later. For macOS clients, Coder Desktop v0.8.0 or later is also required. ## Next Steps diff --git a/site/.storybook/preview.jsx b/site/.storybook/preview.jsx index f7a5407c457c1..c491428178f37 100644 --- a/site/.storybook/preview.jsx +++ b/site/.storybook/preview.jsx @@ -101,6 +101,13 @@ function withHelmet(Story) { ); } +/** + * This JSX file isn't part of the main project, so it doesn't get to use the + * ambient types defined in `storybook.d.ts` to provide extra type-safety. + * Extracting main key to avoid typos. + */ +const queryParametersKey = "queries"; + /** @type {Decorator} */ function withQuery(Story, { parameters }) { const queryClient = new QueryClient({ @@ -112,8 +119,8 @@ function withQuery(Story, { parameters }) { }, }); - if (parameters.queries) { - for (const query of parameters.queries) { + if (parameters[queryParametersKey]) { + for (const query of parameters[queryParametersKey]) { queryClient.setQueryData(query.key, query.data); } } diff --git a/site/src/api/queries/templates.ts b/site/src/api/queries/templates.ts index 5135f2304426e..9057179a8740c 100644 --- a/site/src/api/queries/templates.ts +++ b/site/src/api/queries/templates.ts @@ -48,21 +48,6 @@ export const templates = ( }; }; -const getTemplatesByOrganizationQueryKey = ( - organization: string, - options?: GetTemplatesOptions, -) => [organization, "templates", options?.deprecated]; - -const templatesByOrganization = ( - organization: string, - options: GetTemplatesOptions = {}, -) => { - return { - queryKey: getTemplatesByOrganizationQueryKey(organization, options), - queryFn: () => API.getTemplatesByOrganization(organization, options), - }; -}; - export const templateACL = (templateId: string) => { return { queryKey: ["templateAcl", templateId], @@ -121,9 +106,11 @@ export const templateExamples = () => { }; }; +export const templateVersionRoot: string = "templateVersion"; + export const templateVersion = (versionId: string) => { return { - queryKey: ["templateVersion", versionId], + queryKey: [templateVersionRoot, versionId], queryFn: () => API.getTemplateVersion(versionId), }; }; @@ -134,7 +121,7 @@ export const templateVersionByName = ( versionName: string, ) => { return { - queryKey: ["templateVersion", organizationId, templateName, versionName], + queryKey: [templateVersionRoot, organizationId, templateName, versionName], queryFn: () => API.getTemplateVersionByName(organizationId, templateName, versionName), }; @@ -153,7 +140,7 @@ export const templateVersions = (templateId: string) => { }; export const templateVersionVariablesKey = (versionId: string) => [ - "templateVersion", + templateVersionRoot, versionId, "variables", ]; @@ -216,7 +203,7 @@ export const templaceACLAvailable = ( }; const templateVersionExternalAuthKey = (versionId: string) => [ - "templateVersion", + templateVersionRoot, versionId, "externalAuth", ]; @@ -257,21 +244,21 @@ const createTemplateFn = async (options: CreateTemplateOptions) => { export const templateVersionLogs = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "logs"], + queryKey: [templateVersionRoot, versionId, "logs"], queryFn: () => API.getTemplateVersionLogs(versionId), }; }; export const richParameters = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "richParameters"], + queryKey: [templateVersionRoot, versionId, "richParameters"], queryFn: () => API.getTemplateVersionRichParameters(versionId), }; }; export const resources = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "resources"], + queryKey: [templateVersionRoot, versionId, "resources"], queryFn: () => API.getTemplateVersionResources(versionId), }; }; @@ -293,7 +280,7 @@ export const previousTemplateVersion = ( ) => { return { queryKey: [ - "templateVersion", + templateVersionRoot, organizationId, templateName, versionName, @@ -313,7 +300,7 @@ export const previousTemplateVersion = ( export const templateVersionPresets = (versionId: string) => { return { - queryKey: ["templateVersion", versionId, "presets"], + queryKey: [templateVersionRoot, versionId, "presets"], queryFn: () => API.getTemplateVersionPresets(versionId), }; }; diff --git a/site/src/components/Abbr/Abbr.stories.tsx b/site/src/components/Abbr/Abbr.stories.tsx index b32138ad54fd5..7d079e4ac7416 100644 --- a/site/src/components/Abbr/Abbr.stories.tsx +++ b/site/src/components/Abbr/Abbr.stories.tsx @@ -6,10 +6,10 @@ const meta: Meta = { component: Abbr, decorators: [ (Story) => ( - <> +

Try the following text out in a screen reader!

- +
), ], }; @@ -25,9 +25,9 @@ export const InlinedShorthand: Story = { }, decorators: [ (Story) => ( -

+

The physical pain of getting bonked on the head with a cartoon mallet - lasts precisely 593{" "} + lasts precisely 593 diff --git a/site/src/components/Abbr/Abbr.test.tsx b/site/src/components/Abbr/Abbr.test.tsx index 3ae76f071bdfb..b67406299685a 100644 --- a/site/src/components/Abbr/Abbr.test.tsx +++ b/site/src/components/Abbr/Abbr.test.tsx @@ -1,5 +1,5 @@ import { render, screen } from "@testing-library/react"; -import { Abbr, type Pronunciation } from "./Abbr"; +import { Abbr } from "./Abbr"; type AbbreviationData = { abbreviation: string; @@ -7,28 +7,8 @@ type AbbreviationData = { expectedLabel: string; }; -type AssertionInput = AbbreviationData & { - pronunciation: Pronunciation; -}; - -function assertAccessibleLabel({ - abbreviation, - title, - expectedLabel, - pronunciation, -}: AssertionInput) { - const { unmount } = render( - - {abbreviation} - , - ); - - screen.getByLabelText(expectedLabel, { selector: "abbr" }); - unmount(); -} - describe(Abbr.name, () => { - it("Has an aria-label that equals the title if the abbreviation is shorthand", () => { + it("Omits abbreviation from screen-reader output if it is shorthand", () => { const sampleShorthands: AbbreviationData[] = [ { abbreviation: "ms", @@ -43,11 +23,22 @@ describe(Abbr.name, () => { ]; for (const shorthand of sampleShorthands) { - assertAccessibleLabel({ ...shorthand, pronunciation: "shorthand" }); + const { unmount } = render( + + {shorthand.abbreviation} + , + ); + + // The element doesn't have any ARIA role semantics baked in, + // so we have to get a little bit more creative with making sure the + // expected content is on screen in an accessible way + const element = screen.getByTitle(shorthand.title); + expect(element).toHaveTextContent(shorthand.expectedLabel); + unmount(); } }); - it("Has an aria label with title and 'flattened' pronunciation if abbreviation is acronym", () => { + it("Adds title and 'flattened' pronunciation if abbreviation is acronym", () => { const sampleAcronyms: AbbreviationData[] = [ { abbreviation: "NASA", @@ -67,11 +58,19 @@ describe(Abbr.name, () => { ]; for (const acronym of sampleAcronyms) { - assertAccessibleLabel({ ...acronym, pronunciation: "acronym" }); + const { unmount } = render( + + {acronym.abbreviation} + , + ); + + const element = screen.getByTitle(acronym.title); + expect(element).toHaveTextContent(acronym.expectedLabel); + unmount(); } }); - it("Has an aria label with title and initialized pronunciation if abbreviation is initialism", () => { + it("Adds title and initialized pronunciation if abbreviation is initialism", () => { const sampleInitialisms: AbbreviationData[] = [ { abbreviation: "FBI", @@ -91,7 +90,15 @@ describe(Abbr.name, () => { ]; for (const initialism of sampleInitialisms) { - assertAccessibleLabel({ ...initialism, pronunciation: "initialism" }); + const { unmount } = render( + + {initialism.abbreviation} + , + ); + + const element = screen.getByTitle(initialism.title); + expect(element).toHaveTextContent(initialism.expectedLabel); + unmount(); } }); }); diff --git a/site/src/components/Abbr/Abbr.tsx b/site/src/components/Abbr/Abbr.tsx index b27141818efb3..0c08c33e111ce 100644 --- a/site/src/components/Abbr/Abbr.tsx +++ b/site/src/components/Abbr/Abbr.tsx @@ -1,12 +1,13 @@ import type { FC, HTMLAttributes } from "react"; import { cn } from "utils/cn"; -export type Pronunciation = "shorthand" | "acronym" | "initialism"; +type Pronunciation = "shorthand" | "acronym" | "initialism"; type AbbrProps = HTMLAttributes & { children: string; title: string; pronunciation?: Pronunciation; + className?: string; }; /** @@ -22,23 +23,26 @@ export const Abbr: FC = ({ children, title, pronunciation = "shorthand", + className, ...delegatedProps }) => { return ( {children} + + {getAccessibleLabel(children, title, pronunciation)} + ); }; diff --git a/site/src/components/Filter/Filter.tsx b/site/src/components/Filter/Filter.tsx index b253c947ca486..ea5e60ff0a7f9 100644 --- a/site/src/components/Filter/Filter.tsx +++ b/site/src/components/Filter/Filter.tsx @@ -16,7 +16,6 @@ import { useDebouncedFunction } from "hooks/debounce"; import { ExternalLinkIcon } from "lucide-react"; import { ChevronDownIcon } from "lucide-react"; import { type FC, type ReactNode, useEffect, useRef, useState } from "react"; -import type { useSearchParams } from "react-router"; type PresetFilter = { name: string; @@ -27,35 +26,55 @@ type FilterValues = Record; type UseFilterConfig = { /** - * The fallback value to use in the event that no filter params can be parsed - * from the search params object. This value is allowed to change on - * re-renders. + * The fallback value to use in the event that no filter params can be + * parsed from the search params object. */ fallbackFilter?: string; - searchParamsResult: ReturnType; + searchParams: URLSearchParams; + onSearchParamsChange: (newParams: URLSearchParams) => void; onUpdate?: (newValue: string) => void; }; +export type UseFilterResult = Readonly<{ + query: string; + values: FilterValues; + used: boolean; + update: (newValues: string | FilterValues) => void; + debounceUpdate: (newValues: string | FilterValues) => void; + cancelDebounce: () => void; +}>; + export const useFilterParamsKey = "filter"; export const useFilter = ({ fallbackFilter = "", - searchParamsResult, + searchParams, + onSearchParamsChange, onUpdate, -}: UseFilterConfig) => { - const [searchParams, setSearchParams] = searchParamsResult; +}: UseFilterConfig): UseFilterResult => { const query = searchParams.get(useFilterParamsKey) ?? fallbackFilter; const update = (newValues: string | FilterValues) => { const serialized = typeof newValues === "string" ? newValues : stringifyFilter(newValues); + const noUpdateNeeded = query === serialized; + if (noUpdateNeeded) { + return; + } + /** + * @todo 2025-07-15 - We have a slightly nasty bug here, where trying to + * update state via immutable state updates causes our code to break. + * + * In theory, it would be better to make a copy of the search params. We + * can then mutate and dispatch the copy instead of the original. Doing + * that causes other parts of our existing logic to break, though. + * That's a sign that our other code is slightly broken, and only just + * happens to work by chance right now. + */ searchParams.set(useFilterParamsKey, serialized); - setSearchParams(searchParams); - - if (onUpdate !== undefined) { - onUpdate(serialized); - } + onSearchParamsChange(searchParams); + onUpdate?.(serialized); }; const { debounced: debounceUpdate, cancelDebounce } = useDebouncedFunction( @@ -73,8 +92,6 @@ export const useFilter = ({ }; }; -export type UseFilterResult = ReturnType; - const parseFilterQuery = (filterQuery: string): FilterValues => { if (filterQuery === "") { return {}; diff --git a/site/src/hooks/usePagination.ts b/site/src/hooks/usePagination.ts index 146ad9707da25..235a1505033e1 100644 --- a/site/src/hooks/usePagination.ts +++ b/site/src/hooks/usePagination.ts @@ -1,25 +1,43 @@ import { DEFAULT_RECORDS_PER_PAGE } from "components/PaginationWidget/utils"; -import type { useSearchParams } from "react-router"; -export const usePagination = ({ - searchParamsResult, -}: { - searchParamsResult: ReturnType; -}) => { - const [searchParams, setSearchParams] = searchParamsResult; - const page = searchParams.get("page") ? Number(searchParams.get("page")) : 1; - const limit = DEFAULT_RECORDS_PER_PAGE; - const offset = page <= 0 ? 0 : (page - 1) * limit; +const paginationKey = "page"; - const goToPage = (page: number) => { - searchParams.set("page", page.toString()); - setSearchParams(searchParams); - }; +type UsePaginationOptions = Readonly<{ + searchParams: URLSearchParams; + onSearchParamsChange: (newParams: URLSearchParams) => void; +}>; + +type UsePaginationResult = Readonly<{ + page: number; + limit: number; + offset: number; + goToPage: (page: number) => void; +}>; + +export function usePagination( + options: UsePaginationOptions, +): UsePaginationResult { + const { searchParams, onSearchParamsChange } = options; + const limit = DEFAULT_RECORDS_PER_PAGE; + const rawPage = Number.parseInt(searchParams.get(paginationKey) || "1", 10); + const page = Number.isNaN(rawPage) || rawPage <= 0 ? 1 : rawPage; return { page, limit, - goToPage, - offset, + offset: Math.max(0, (page - 1) * limit), + goToPage: (newPage) => { + const abortNavigation = + page === newPage || + !Number.isFinite(newPage) || + !Number.isInteger(newPage); + if (abortNavigation) { + return; + } + + const copy = new URLSearchParams(searchParams); + copy.set("page", newPage.toString()); + onSearchParamsChange(copy); + }, }; -}; +} diff --git a/site/src/pages/AuditPage/AuditPage.test.tsx b/site/src/pages/AuditPage/AuditPage.test.tsx index bcbc40da8af5c..80f6e74ae1a26 100644 --- a/site/src/pages/AuditPage/AuditPage.test.tsx +++ b/site/src/pages/AuditPage/AuditPage.test.tsx @@ -1,6 +1,7 @@ import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { API } from "api/api"; +import type { AuditLogsRequest } from "api/typesGenerated"; import { DEFAULT_RECORDS_PER_PAGE } from "components/PaginationWidget/utils"; import { http, HttpResponse } from "msw"; import { @@ -106,7 +107,7 @@ describe("AuditPage", () => { await userEvent.type(filterField, query); await waitFor(() => - expect(getAuditLogsSpy).toBeCalledWith({ + expect(getAuditLogsSpy).toHaveBeenCalledWith<[AuditLogsRequest]>({ limit: DEFAULT_RECORDS_PER_PAGE, offset: 0, q: query, diff --git a/site/src/pages/AuditPage/AuditPage.tsx b/site/src/pages/AuditPage/AuditPage.tsx index 02adf36186999..6c8c52a679ada 100644 --- a/site/src/pages/AuditPage/AuditPage.tsx +++ b/site/src/pages/AuditPage/AuditPage.tsx @@ -33,7 +33,8 @@ const AuditPage: FC = () => { const [searchParams, setSearchParams] = useSearchParams(); const auditsQuery = usePaginatedQuery(paginatedAudits(searchParams)); const filter = useFilter({ - searchParamsResult: [searchParams, setSearchParams], + searchParams, + onSearchParamsChange: setSearchParams, onUpdate: auditsQuery.goToFirstPage, }); diff --git a/site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx b/site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx index aeb34d7bebd40..fd7fc12e38901 100644 --- a/site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx +++ b/site/src/pages/ConnectionLogPage/ConnectionLogPage.tsx @@ -29,7 +29,8 @@ const ConnectionLogPage: FC = () => { paginatedConnectionLogs(searchParams), ); const filter = useFilter({ - searchParamsResult: [searchParams, setSearchParams], + searchParams, + onSearchParamsChange: setSearchParams, onUpdate: connectionlogsQuery.goToFirstPage, }); diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 9b4bb9608e90b..99876bfdb534d 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -104,9 +104,7 @@ export const CreateWorkspacePageViewExperimental: FC< owner, setOwner, }) => { - const [suggestedName, setSuggestedName] = useState(() => - generateWorkspaceName(), - ); + const [suggestedName, setSuggestedName] = useState(generateWorkspaceName); const [showPresetParameters, setShowPresetParameters] = useState(false); const id = useId(); const workspaceNameInputRef = useRef(null); @@ -120,14 +118,8 @@ export const CreateWorkspacePageViewExperimental: FC< // Only touched fields are sent to the websocket // Autofilled parameters are marked as touched since they have been modified - const initialTouched = parameters.reduce( - (touched, parameter) => { - if (autofillByName[parameter.name] !== undefined) { - touched[parameter.name] = true; - } - return touched; - }, - {} as Record, + const initialTouched = Object.fromEntries( + parameters.filter((p) => autofillByName[p.name]).map((p) => [p, true]), ); // The form parameters values hold the working state of the parameters that will be submitted when creating a workspace diff --git a/site/src/pages/TemplatesPage/TemplatesPage.tsx b/site/src/pages/TemplatesPage/TemplatesPage.tsx index fb4d2a8454dc6..d03d29716b4c9 100644 --- a/site/src/pages/TemplatesPage/TemplatesPage.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPage.tsx @@ -14,10 +14,11 @@ const TemplatesPage: FC = () => { const { permissions, user: me } = useAuthenticated(); const { showOrganizations } = useDashboard(); - const searchParamsResult = useSearchParams(); + const [searchParams, setSearchParams] = useSearchParams(); const filter = useFilter({ fallbackFilter: "deprecated:false", - searchParamsResult, + searchParams, + onSearchParamsChange: setSearchParams, onUpdate: () => {}, // reset pagination }); diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 354e77aceac7e..aaa67e060d50b 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -39,9 +39,8 @@ type UserPageProps = { const UsersPage: FC = ({ defaultNewPassword }) => { const queryClient = useQueryClient(); const navigate = useNavigate(); - const searchParamsResult = useSearchParams(); + const [searchParams, setSearchParams] = useSearchParams(); const { entitlements } = useDashboard(); - const [searchParams] = searchParamsResult; const groupsByUserIdQuery = useQuery(groupsByUserId()); const authMethodsQuery = useQuery(authMethods()); @@ -58,9 +57,10 @@ const UsersPage: FC = ({ defaultNewPassword }) => { enabled: viewDeploymentConfig, }); - const usersQuery = usePaginatedQuery(paginatedUsers(searchParamsResult[0])); + const usersQuery = usePaginatedQuery(paginatedUsers(searchParams)); const useFilterResult = useFilter({ - searchParamsResult, + searchParams, + onSearchParamsChange: setSearchParams, onUpdate: usersQuery.goToFirstPage, }); diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx index 56f8fb34a32e8..678f0331331a6 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx @@ -1,7 +1,7 @@ import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { API } from "api/api"; -import type { Workspace } from "api/typesGenerated"; +import type { Workspace, WorkspacesResponse } from "api/typesGenerated"; import { http, HttpResponse } from "msw"; import { MockDormantOutdatedWorkspace, @@ -28,18 +28,17 @@ describe("WorkspacesPage", () => { }); it("renders an empty workspaces page", async () => { - // Given server.use( http.get("/api/v2/workspaces", async () => { - return HttpResponse.json({ workspaces: [], count: 0 }); + return HttpResponse.json({ + workspaces: [], + count: 0, + }); }), ); - // When renderWithAuth(); - - // Then - await screen.findByText("Create a workspace"); + await screen.findByRole("heading", { name: /Create a workspace/ }); }); it("renders a filled workspaces page", async () => { diff --git a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx index 22739d21dce89..2fec0c3975403 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPage.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPage.tsx @@ -1,8 +1,8 @@ import { getErrorDetail, getErrorMessage } from "api/errors"; import { workspacePermissionsByOrganization } from "api/queries/organizations"; -import { templates } from "api/queries/templates"; +import { templateVersionRoot, templates } from "api/queries/templates"; import { workspaces } from "api/queries/workspaces"; -import type { Workspace, WorkspaceStatus } from "api/typesGenerated"; +import type { WorkspaceStatus } from "api/typesGenerated"; import { useFilter } from "components/Filter/Filter"; import { useUserFilterMenu } from "components/Filter/UserFilter"; import { displayError } from "components/GlobalSnackbar/utils"; @@ -11,7 +11,7 @@ import { useEffectEvent } from "hooks/hookPolyfills"; import { usePagination } from "hooks/usePagination"; import { useDashboard } from "modules/dashboard/useDashboard"; import { useOrganizationsFilterMenu } from "modules/tableFiltering/options"; -import { type FC, useEffect, useMemo, useState } from "react"; +import { type FC, useMemo, useState } from "react"; import { Helmet } from "react-helmet-async"; import { useQuery, useQueryClient } from "react-query"; import { useSearchParams } from "react-router"; @@ -22,15 +22,21 @@ import { WorkspacesPageView } from "./WorkspacesPageView"; import { useBatchActions } from "./batchActions"; import { useStatusFilterMenu, useTemplateFilterMenu } from "./filter/menus"; -// To reduce the number of fetches, we reduce the fetch interval if there are no -// active workspace builds. -const ACTIVE_BUILD_STATUSES: WorkspaceStatus[] = [ +/** + * The set of all workspace statuses that indicate that the state for a + * workspace is in the middle of a transition and will eventually reach a more + * stable state/status. + */ +const ACTIVE_BUILD_STATUSES: readonly WorkspaceStatus[] = [ "canceling", "deleting", "pending", "starting", "stopping", ]; + +// To reduce the number of fetches, we reduce the fetch interval if there are no +// active workspace builds. const ACTIVE_BUILDS_REFRESH_INTERVAL = 5_000; const NO_ACTIVE_BUILDS_REFRESH_INTERVAL = 30_000; @@ -48,13 +54,35 @@ function useSafeSearchParams() { >; } +type BatchAction = "delete" | "update"; + const WorkspacesPage: FC = () => { const queryClient = useQueryClient(); - // If we use a useSearchParams for each hook, the values will not be in sync. - // So we have to use a single one, centralizing the values, and pass it to - // each hook. - const searchParamsResult = useSafeSearchParams(); - const pagination = usePagination({ searchParamsResult }); + // We have to be careful with how we use useSearchParams or any other + // derived hooks. The URL is global state, but each call to useSearchParams + // creates a different, contradictory source of truth for what the URL + // should look like. We need to make sure that we only mount the hook once + // per page + const [searchParams, setSearchParams] = useSafeSearchParams(); + // Always need to make sure that we reset the checked workspaces each time + // the filtering or pagination changes, as that will almost always change + // which workspaces are shown on screen and which can be interacted with + const [checkedWorkspaceIds, setCheckedWorkspaceIds] = useState( + new Set(), + ); + const resetChecked = () => { + if (checkedWorkspaceIds.size !== 0) { + setCheckedWorkspaceIds(new Set()); + } + }; + + const pagination = usePagination({ + searchParams, + onSearchParamsChange: (newParams) => { + setSearchParams(newParams); + resetChecked(); + }, + }); const { permissions, user: me } = useAuthenticated(); const { entitlements } = useDashboard(); const templatesQuery = useQuery(templates()); @@ -78,14 +106,18 @@ const WorkspacesPage: FC = () => { }); }, [templatesQuery.data, workspacePermissionsQuery.data]); - const filterProps = useWorkspacesFilter({ - searchParamsResult, - onFilterChange: () => pagination.goToPage(1), + const filterState = useWorkspacesFilter({ + searchParams, + onSearchParamsChange: setSearchParams, + onFilterChange: () => { + pagination.goToPage(1); + resetChecked(); + }, }); const workspacesQueryOptions = workspaces({ ...pagination, - q: filterProps.filter.query, + q: filterState.filter.query, }); const { data, error, refetch } = useQuery({ ...workspacesQueryOptions, @@ -109,28 +141,18 @@ const WorkspacesPage: FC = () => { refetchOnWindowFocus: "always", }); - const [checkedWorkspaces, setCheckedWorkspaces] = useState< - readonly Workspace[] - >([]); - const [confirmingBatchAction, setConfirmingBatchAction] = useState< - "delete" | "update" | null - >(null); - const [urlSearchParams] = searchParamsResult; + const [activeBatchAction, setActiveBatchAction] = useState(); const canCheckWorkspaces = entitlements.features.workspace_batch_actions.enabled; const batchActions = useBatchActions({ onSuccess: async () => { await refetch(); - setCheckedWorkspaces([]); + resetChecked(); }, }); - // We want to uncheck the selected workspaces always when the url changes - // because of filtering or pagination - // biome-ignore lint/correctness/useExhaustiveDependencies: consider refactoring - useEffect(() => { - setCheckedWorkspaces([]); - }, [urlSearchParams]); + const checkedWorkspaces = + data?.workspaces.filter((w) => checkedWorkspaceIds.has(w.id)) ?? []; return ( <> @@ -142,7 +164,18 @@ const WorkspacesPage: FC = () => { canCreateTemplate={permissions.createTemplates} canChangeVersions={permissions.updateTemplates} checkedWorkspaces={checkedWorkspaces} - onCheckChange={setCheckedWorkspaces} + onCheckChange={(newWorkspaces) => { + setCheckedWorkspaceIds((current) => { + const newIds = newWorkspaces.map((ws) => ws.id); + const sameContent = + current.size === newIds.length && + newIds.every((id) => current.has(id)); + if (sameContent) { + return current; + } + return new Set(newIds); + }); + }} canCheckWorkspaces={canCheckWorkspaces} templates={filteredTemplates} templatesFetchStatus={templatesQuery.status} @@ -152,12 +185,31 @@ const WorkspacesPage: FC = () => { page={pagination.page} limit={pagination.limit} onPageChange={pagination.goToPage} - filterProps={filterProps} - isRunningBatchAction={batchActions.isLoading} - onDeleteAll={() => setConfirmingBatchAction("delete")} - onUpdateAll={() => setConfirmingBatchAction("update")} - onStartAll={() => batchActions.startAll(checkedWorkspaces)} - onStopAll={() => batchActions.stopAll(checkedWorkspaces)} + filterState={filterState} + isRunningBatchAction={batchActions.isProcessing} + onBatchDeleteTransition={() => setActiveBatchAction("delete")} + onBatchStartTransition={() => batchActions.start(checkedWorkspaces)} + onBatchStopTransition={() => batchActions.stop(checkedWorkspaces)} + onBatchUpdateTransition={() => { + // Just because batch-updating can be really dangerous + // action for running workspaces, we're going to invalidate + // all relevant queries as a prefetch strategy before the + // modal content is even allowed to mount. + for (const ws of checkedWorkspaces) { + // Our data layer is a little messy right now, so + // there's no great way to invalidate a bunch of + // template version queries with a single function call, + // while also avoiding all other tangentially connected + // resources that use the same key pattern. Have to be + // super granular and make one call per workspace. + queryClient.invalidateQueries({ + queryKey: [templateVersionRoot, ws.template_active_version_id], + exact: true, + type: "all", + }); + } + setActiveBatchAction("update"); + }} onActionSuccess={async () => { await queryClient.invalidateQueries({ queryKey: workspacesQueryOptions.queryKey, @@ -172,31 +224,27 @@ const WorkspacesPage: FC = () => { /> setActiveBatchAction(undefined)} onConfirm={async () => { - await batchActions.deleteAll(checkedWorkspaces); - setConfirmingBatchAction(null); - }} - onClose={() => { - setConfirmingBatchAction(null); + await batchActions.delete(checkedWorkspaces); + setActiveBatchAction(undefined); }} /> setActiveBatchAction(undefined)} onConfirm={async () => { - await batchActions.updateAll({ + await batchActions.updateTemplateVersions({ workspaces: checkedWorkspaces, isDynamicParametersEnabled: false, }); - setConfirmingBatchAction(null); - }} - onClose={() => { - setConfirmingBatchAction(null); + setActiveBatchAction(undefined); }} /> @@ -206,17 +254,20 @@ const WorkspacesPage: FC = () => { export default WorkspacesPage; type UseWorkspacesFilterOptions = { - searchParamsResult: ReturnType; + searchParams: URLSearchParams; + onSearchParamsChange: (newParams: URLSearchParams) => void; onFilterChange: () => void; }; const useWorkspacesFilter = ({ - searchParamsResult, + searchParams, + onSearchParamsChange, onFilterChange, }: UseWorkspacesFilterOptions) => { const filter = useFilter({ fallbackFilter: "owner:me", - searchParamsResult, + searchParams, + onSearchParamsChange, onUpdate: onFilterChange, }); diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx index e0fa40027cfc7..5696721a089d6 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.stories.tsx @@ -11,7 +11,6 @@ import { import { DEFAULT_RECORDS_PER_PAGE } from "components/PaginationWidget/utils"; import dayjs from "dayjs"; import uniqueId from "lodash/uniqueId"; -import type { ComponentProps } from "react"; import { expect, within } from "storybook/test"; import { MockBuildInfo, @@ -31,6 +30,7 @@ import { withProxyProvider, } from "testHelpers/storybook"; import { WorkspacesPageView } from "./WorkspacesPageView"; +import type { WorkspaceFilterState } from "./filter/WorkspacesFilter"; const createWorkspace = ( name: string, @@ -134,9 +134,7 @@ const allWorkspaces = [ ...Object.values(additionalWorkspaces), ]; -type FilterProps = ComponentProps["filterProps"]; - -const defaultFilterProps = getDefaultFilterProps({ +const defaultFilterProps = getDefaultFilterProps({ query: "owner:me", menus: { user: MockMenu, @@ -169,7 +167,7 @@ const meta: Meta = { component: WorkspacesPageView, args: { limit: DEFAULT_RECORDS_PER_PAGE, - filterProps: defaultFilterProps, + filterState: defaultFilterProps, checkedWorkspaces: [], canCheckWorkspaces: true, templates: mockTemplates, @@ -266,7 +264,7 @@ export const UserHasNoWorkspacesAndNoTemplates: Story = { export const NoSearchResults: Story = { args: { workspaces: [], - filterProps: { + filterState: { ...defaultFilterProps, filter: { ...defaultFilterProps.filter, diff --git a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx index 6563533bc43da..4476211387871 100644 --- a/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx +++ b/site/src/pages/WorkspacesPage/WorkspacesPageView.tsx @@ -26,7 +26,7 @@ import { mustUpdateWorkspace } from "utils/workspace"; import { WorkspaceHelpTooltip } from "./WorkspaceHelpTooltip"; import { WorkspacesButton } from "./WorkspacesButton"; import { - type WorkspaceFilterProps, + type WorkspaceFilterState, WorkspacesFilter, } from "./filter/WorkspacesFilter"; @@ -45,16 +45,16 @@ interface WorkspacesPageViewProps { workspaces?: readonly Workspace[]; checkedWorkspaces: readonly Workspace[]; count?: number; - filterProps: WorkspaceFilterProps; + filterState: WorkspaceFilterState; page: number; limit: number; onPageChange: (page: number) => void; onCheckChange: (checkedWorkspaces: readonly Workspace[]) => void; isRunningBatchAction: boolean; - onDeleteAll: () => void; - onUpdateAll: () => void; - onStartAll: () => void; - onStopAll: () => void; + onBatchDeleteTransition: () => void; + onBatchUpdateTransition: () => void; + onBatchStartTransition: () => void; + onBatchStopTransition: () => void; canCheckWorkspaces: boolean; templatesFetchStatus: TemplateQuery["status"]; templates: TemplateQuery["data"]; @@ -69,15 +69,15 @@ export const WorkspacesPageView: FC = ({ error, limit, count, - filterProps, + filterState, onPageChange, page, checkedWorkspaces, onCheckChange, - onDeleteAll, - onUpdateAll, - onStopAll, - onStartAll, + onBatchDeleteTransition, + onBatchUpdateTransition, + onBatchStopTransition, + onBatchStartTransition, isRunningBatchAction, canCheckWorkspaces, templates, @@ -87,10 +87,10 @@ export const WorkspacesPageView: FC = ({ onActionSuccess, onActionError, }) => { - // Let's say the user has 5 workspaces, but tried to hit page 100, which does - // not exist. In this case, the page is not valid and we want to show a better - // error message. - const invalidPageNumber = page !== 1 && workspaces?.length === 0; + // Let's say the user has 5 workspaces, but tried to hit page 100, which + // does not exist. In this case, the page is not valid and we want to show a + // better error message. + const pageNumberIsInvalid = page !== 1 && workspaces?.length === 0; return ( @@ -117,9 +117,12 @@ export const WorkspacesPageView: FC = ({ )} @@ -155,7 +158,7 @@ export const WorkspacesPageView: FC = ({ !mustUpdateWorkspace(w, canChangeVersions), ) } - onClick={onStartAll} + onClick={onBatchStartTransition} > Start @@ -165,12 +168,12 @@ export const WorkspacesPageView: FC = ({ (w) => w.latest_build.status === "running", ) } - onClick={onStopAll} + onClick={onBatchStopTransition} > Stop - + = ({ Delete… @@ -187,7 +190,7 @@ export const WorkspacesPageView: FC = ({ ) : ( - !invalidPageNumber && ( + !pageNumberIsInvalid && ( = ({ )} - {invalidPageNumber ? ( + {pageNumberIsInvalid ? ( ({ border: `1px solid ${theme.palette.divider}`, @@ -221,7 +224,7 @@ export const WorkspacesPageView: FC = ({ Promise; } -export function useBatchActions(options: UseBatchActionsProps) { +type UpdateAllPayload = Readonly<{ + workspaces: readonly Workspace[]; + isDynamicParametersEnabled: boolean; +}>; + +type UseBatchActionsResult = Readonly<{ + isProcessing: boolean; + start: (workspaces: readonly Workspace[]) => Promise; + stop: (workspaces: readonly Workspace[]) => Promise; + delete: (workspaces: readonly Workspace[]) => Promise; + updateTemplateVersions: ( + payload: UpdateAllPayload, + ) => Promise; + favorite: (payload: readonly Workspace[]) => Promise; + unfavorite: (payload: readonly Workspace[]) => Promise; +}>; + +export function useBatchActions( + options: UseBatchActionsOptions, +): UseBatchActionsResult { const { onSuccess } = options; const startAllMutation = useMutation({ @@ -45,10 +64,7 @@ export function useBatchActions(options: UseBatchActionsProps) { }); const updateAllMutation = useMutation({ - mutationFn: (payload: { - workspaces: readonly Workspace[]; - isDynamicParametersEnabled: boolean; - }) => { + mutationFn: (payload: UpdateAllPayload) => { const { workspaces, isDynamicParametersEnabled } = payload; return Promise.all( workspaces @@ -63,8 +79,8 @@ export function useBatchActions(options: UseBatchActionsProps) { }); const favoriteAllMutation = useMutation({ - mutationFn: (workspaces: readonly Workspace[]) => { - return Promise.all( + mutationFn: async (workspaces: readonly Workspace[]): Promise => { + await Promise.all( workspaces .filter((w) => !w.favorite) .map((w) => API.putFavoriteWorkspace(w.id)), @@ -77,8 +93,8 @@ export function useBatchActions(options: UseBatchActionsProps) { }); const unfavoriteAllMutation = useMutation({ - mutationFn: (workspaces: readonly Workspace[]) => { - return Promise.all( + mutationFn: async (workspaces: readonly Workspace[]): Promise => { + await Promise.all( workspaces .filter((w) => w.favorite) .map((w) => API.deleteFavoriteWorkspace(w.id)), @@ -91,13 +107,13 @@ export function useBatchActions(options: UseBatchActionsProps) { }); return { - favoriteAll: favoriteAllMutation.mutateAsync, - unfavoriteAll: unfavoriteAllMutation.mutateAsync, - startAll: startAllMutation.mutateAsync, - stopAll: stopAllMutation.mutateAsync, - deleteAll: deleteAllMutation.mutateAsync, - updateAll: updateAllMutation.mutateAsync, - isLoading: + favorite: favoriteAllMutation.mutateAsync, + unfavorite: unfavoriteAllMutation.mutateAsync, + start: startAllMutation.mutateAsync, + stop: stopAllMutation.mutateAsync, + delete: deleteAllMutation.mutateAsync, + updateTemplateVersions: updateAllMutation.mutateAsync, + isProcessing: favoriteAllMutation.isPending || unfavoriteAllMutation.isPending || startAllMutation.isPending || diff --git a/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx b/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx index ea3081d84c7f3..caebfd04526d4 100644 --- a/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx +++ b/site/src/pages/WorkspacesPage/filter/WorkspacesFilter.tsx @@ -1,4 +1,8 @@ -import { Filter, MenuSkeleton, type useFilter } from "components/Filter/Filter"; +import { + Filter, + MenuSkeleton, + type UseFilterResult, +} from "components/Filter/Filter"; import { type UserFilterMenu, UserMenu } from "components/Filter/UserFilter"; import { useDashboard } from "modules/dashboard/useDashboard"; import { @@ -62,8 +66,8 @@ const PRESETS_WITH_DORMANT: FilterPreset[] = [ }, ]; -export type WorkspaceFilterProps = { - filter: ReturnType; +export type WorkspaceFilterState = { + filter: UseFilterResult; error?: unknown; menus: { user?: UserFilterMenu; @@ -73,21 +77,36 @@ export type WorkspaceFilterProps = { }; }; +type WorkspaceFilterProps = Readonly<{ + filter: UseFilterResult; + error: unknown; + templateMenu: TemplateFilterMenu; + statusMenu: StatusFilterMenu; + + userMenu?: UserFilterMenu; + organizationsMenu?: OrganizationsFilterMenu; +}>; + export const WorkspacesFilter: FC = ({ filter, error, - menus, + templateMenu, + statusMenu, + userMenu, + organizationsMenu, }) => { const { entitlements, showOrganizations } = useDashboard(); const width = showOrganizations ? 175 : undefined; const presets = entitlements.features.advanced_template_scheduling.enabled ? PRESETS_WITH_DORMANT : PRESET_FILTERS; + const organizationsActive = + showOrganizations && organizationsMenu !== undefined; return ( = ({ )} options={ <> - {menus.user && } - - - {showOrganizations && menus.organizations !== undefined && ( - + {userMenu && } + + + {organizationsActive && ( + )} } optionsSkeleton={ <> - {menus.user && } + {userMenu && } - {showOrganizations && } + {organizationsActive && } } />