diff --git a/site/src/app.tsx b/site/src/app.tsx index 11e3223ed7..9c7ece503b 100644 --- a/site/src/app.tsx +++ b/site/src/app.tsx @@ -1,7 +1,5 @@ import CssBaseline from "@material-ui/core/CssBaseline" import ThemeProvider from "@material-ui/styles/ThemeProvider" -import { LicenseBanner } from "components/LicenseBanner/LicenseBanner" -import { ServiceBanner } from "components/ServiceBanner/ServiceBanner" import { FC } from "react" import { HelmetProvider } from "react-helmet-async" import { BrowserRouter as Router } from "react-router-dom" @@ -20,8 +18,6 @@ export const App: FC = () => { - - diff --git a/site/src/components/DashboardLayout/DashboardLayout.tsx b/site/src/components/DashboardLayout/DashboardLayout.tsx index b55ad39978..bd20eaaed0 100644 --- a/site/src/components/DashboardLayout/DashboardLayout.tsx +++ b/site/src/components/DashboardLayout/DashboardLayout.tsx @@ -1,49 +1,56 @@ import { makeStyles } from "@material-ui/core/styles" -import { useActor } from "@xstate/react" +import { useMachine } from "@xstate/react" import { Loader } from "components/Loader/Loader" -import { FC, Suspense, useContext, useEffect } from "react" -import { XServiceContext } from "../../xServices/StateContext" +import { FC, Suspense } from "react" import { Navbar } from "../Navbar/Navbar" import { UpdateCheckBanner } from "components/UpdateCheckBanner/UpdateCheckBanner" import { Margins } from "components/Margins/Margins" import { Outlet } from "react-router-dom" +import { LicenseBanner } from "components/LicenseBanner/LicenseBanner" +import { ServiceBanner } from "components/ServiceBanner/ServiceBanner" +import { updateCheckMachine } from "xServices/updateCheck/updateCheckXService" +import { usePermissions } from "hooks/usePermissions" +import { UpdateCheckResponse } from "api/typesGenerated" export const DashboardLayout: FC = () => { const styles = useStyles() - const xServices = useContext(XServiceContext) - const [authState] = useActor(xServices.authXService) - const [updateCheckState, updateCheckSend] = useActor( - xServices.updateCheckXService, - ) - - useEffect(() => { - if (authState.matches("signedIn")) { - updateCheckSend("CHECK") - } else { - updateCheckSend("CLEAR") - } - }, [authState, updateCheckSend]) + const permissions = usePermissions() + const [updateCheckState, updateCheckSend] = useMachine(updateCheckMachine, { + context: { + permissions, + }, + }) + const { error: updateCheckError, updateCheck } = updateCheckState.context return ( -
- - {updateCheckState.context.show && ( -
- - updateCheckSend("DISMISS")} - /> - + <> + + + +
+ + + {updateCheckState.matches("show") && ( +
+ + updateCheckSend("DISMISS")} + /> + +
+ )} + +
+ }> + +
- )} -
- }> - -
-
+ ) } diff --git a/site/src/components/UpdateCheckBanner/UpdateCheckBanner.test.tsx b/site/src/components/UpdateCheckBanner/UpdateCheckBanner.test.tsx deleted file mode 100644 index d376cda381..0000000000 --- a/site/src/components/UpdateCheckBanner/UpdateCheckBanner.test.tsx +++ /dev/null @@ -1,51 +0,0 @@ -import { fireEvent, screen, waitFor } from "@testing-library/react" -import i18next from "i18next" -import { MockUpdateCheck, render } from "testHelpers/renderHelpers" -import { UpdateCheckBanner } from "./UpdateCheckBanner" - -describe("UpdateCheckBanner", () => { - it("shows an update notification when one is available", () => { - const { t } = i18next - render( - , - ) - - const updateText = t("updateCheck.message", { - ns: "common", - version: MockUpdateCheck.version, - }) - // Message contatins HTML elements so we check it in parts. - for (const text of updateText.split(/<\/?[0-9]+>/)) { - expect(screen.getByText(text, { exact: false })).toBeInTheDocument() - } - - expect(screen.getAllByRole("link")[0]).toHaveAttribute( - "href", - MockUpdateCheck.url, - ) - }) - - it("is hidden when dismissed", async () => { - const dismiss = jest.fn() - const { container } = render( - , - ) - - fireEvent.click(screen.getByRole("button")) - await waitFor(() => expect(dismiss).toBeCalledTimes(1), { timeout: 2000 }) - - expect(container.firstChild).toBeNull() - }) - - it("does not show when up-to-date", async () => { - const { container } = render( - , - ) - expect(container.firstChild).toBeNull() - }) -}) diff --git a/site/src/components/UpdateCheckBanner/UpdateCheckBanner.tsx b/site/src/components/UpdateCheckBanner/UpdateCheckBanner.tsx index 88ecce5a15..3fc692bb75 100644 --- a/site/src/components/UpdateCheckBanner/UpdateCheckBanner.tsx +++ b/site/src/components/UpdateCheckBanner/UpdateCheckBanner.tsx @@ -2,12 +2,12 @@ import Link from "@material-ui/core/Link" import { AlertBanner } from "components/AlertBanner/AlertBanner" import { Trans, useTranslation } from "react-i18next" import * as TypesGen from "api/typesGenerated" -import { FC, useState } from "react" +import { FC } from "react" export interface UpdateCheckBannerProps { - updateCheck?: TypesGen.UpdateCheckResponse - error?: Error | unknown - onDismiss?: () => void + updateCheck: TypesGen.UpdateCheckResponse + error?: unknown + onDismiss: () => void } export const UpdateCheckBanner: FC< @@ -15,45 +15,33 @@ export const UpdateCheckBanner: FC< > = ({ updateCheck, error, onDismiss }) => { const { t } = useTranslation("common") - const isOutdated = updateCheck && !updateCheck.current - - const [show, setShow] = useState(error || isOutdated) - - const dismiss = () => { - onDismiss && onDismiss() - setShow(false) - } - return ( - <> - {show && ( - - <> - {error && <>{t("updateCheck.error")} } - {isOutdated && ( -
- - Coder {"{{version}}"} is now available. View the{" "} - release notes and{" "} - - upgrade instructions - {" "} - for more information. - -
- )} - -
- )} - + + <> + {error ? ( + t("updateCheck.error") + ) : ( +
+ + Coder {"{{version}}"} is now available. View the{" "} + release notes and{" "} + + upgrade instructions + {" "} + for more information. + +
+ )} + +
) } diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index fbaa5c034c..5dc194dc24 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1096,6 +1096,7 @@ export const MockPermissions: Permissions = { updateUsers: true, viewAuditLog: true, viewDeploymentConfig: true, + viewUpdateCheck: true, } export const MockAppearance: TypesGen.AppearanceConfig = { diff --git a/site/src/xServices/StateContext.tsx b/site/src/xServices/StateContext.tsx index c94e078d8f..d77c94defb 100644 --- a/site/src/xServices/StateContext.tsx +++ b/site/src/xServices/StateContext.tsx @@ -3,7 +3,6 @@ import { createContext, FC, ReactNode } from "react" import { ActorRefFrom } from "xstate" import { authMachine } from "./auth/authXService" import { buildInfoMachine } from "./buildInfo/buildInfoXService" -import { updateCheckMachine } from "./updateCheck/updateCheckXService" import { entitlementsMachine } from "./entitlements/entitlementsXService" import { appearanceMachine } from "./appearance/appearanceXService" @@ -12,7 +11,6 @@ interface XServiceContextType { buildInfoXService: ActorRefFrom entitlementsXService: ActorRefFrom appearanceXService: ActorRefFrom - updateCheckXService: ActorRefFrom } /** @@ -33,7 +31,6 @@ export const XServiceProvider: FC<{ children: ReactNode }> = ({ children }) => { buildInfoXService: useInterpret(buildInfoMachine), entitlementsXService: useInterpret(entitlementsMachine), appearanceXService: useInterpret(appearanceMachine), - updateCheckXService: useInterpret(updateCheckMachine), }} > {children} diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 5ea51e6e7c..e1c663fb31 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -16,6 +16,7 @@ export const checks = { viewAuditLog: "viewAuditLog", viewDeploymentConfig: "viewDeploymentConfig", createGroup: "createGroup", + viewUpdateCheck: "viewUpdateCheck", } as const export const permissionsToCheck = { @@ -67,6 +68,12 @@ export const permissionsToCheck = { }, action: "create", }, + [checks.viewUpdateCheck]: { + object: { + resource_type: "update_check", + }, + action: "read", + }, } as const export type Permissions = Record diff --git a/site/src/xServices/updateCheck/updateCheckXService.test.ts b/site/src/xServices/updateCheck/updateCheckXService.test.ts new file mode 100644 index 0000000000..a0338f4dea --- /dev/null +++ b/site/src/xServices/updateCheck/updateCheckXService.test.ts @@ -0,0 +1,138 @@ +import { waitFor } from "@testing-library/react" +import { MockPermissions, MockUpdateCheck } from "testHelpers/entities" +import { interpret } from "xstate" +import { + clearDismissedVersionOnLocal, + getDismissedVersionOnLocal, + saveDismissedVersionOnLocal, + updateCheckMachine, +} from "./updateCheckXService" + +describe("updateCheckMachine", () => { + beforeEach(() => { + clearDismissedVersionOnLocal() + }) + + it("is dismissed when does not have permission to see it", () => { + const machine = updateCheckMachine.withContext({ + permissions: { + ...MockPermissions, + viewUpdateCheck: false, + }, + }) + + const updateCheckService = interpret(machine) + updateCheckService.start() + expect(updateCheckService.state.matches("dismissed")).toBeTruthy() + }) + + it("is dismissed when it is already using current version", async () => { + const machine = updateCheckMachine + .withContext({ + permissions: { + ...MockPermissions, + viewUpdateCheck: true, + }, + }) + .withConfig({ + services: { + getUpdateCheck: () => + Promise.resolve({ + ...MockUpdateCheck, + current: true, + }), + }, + }) + + const updateCheckService = interpret(machine) + updateCheckService.start() + + await waitFor(() => { + expect(updateCheckService.state.matches("dismissed")).toBeTruthy() + }) + }) + + it("is dismissed when it was dismissed previously", async () => { + const machine = updateCheckMachine + .withContext({ + permissions: { + ...MockPermissions, + viewUpdateCheck: true, + }, + }) + .withConfig({ + services: { + getUpdateCheck: () => + Promise.resolve({ + ...MockUpdateCheck, + current: false, + }), + }, + }) + + saveDismissedVersionOnLocal(MockUpdateCheck.version) + const updateCheckService = interpret(machine) + updateCheckService.start() + + await waitFor(() => { + expect(updateCheckService.state.matches("dismissed")).toBeTruthy() + }) + }) + + it("shows when has permission and is outdated", async () => { + const machine = updateCheckMachine + .withContext({ + permissions: { + ...MockPermissions, + viewUpdateCheck: true, + }, + }) + .withConfig({ + services: { + getUpdateCheck: () => + Promise.resolve({ + ...MockUpdateCheck, + current: false, + }), + }, + }) + + const updateCheckService = interpret(machine) + updateCheckService.start() + + await waitFor(() => { + expect(updateCheckService.state.matches("show")).toBeTruthy() + }) + }) + + it("it is dismissed when the DISMISS event happens", async () => { + const machine = updateCheckMachine + .withContext({ + permissions: { + ...MockPermissions, + viewUpdateCheck: true, + }, + }) + .withConfig({ + services: { + getUpdateCheck: () => + Promise.resolve({ + ...MockUpdateCheck, + current: false, + }), + }, + }) + + const updateCheckService = interpret(machine) + updateCheckService.start() + await waitFor(() => { + expect(updateCheckService.state.matches("show")).toBeTruthy() + }) + + updateCheckService.send("DISMISS") + await waitFor(() => { + expect(updateCheckService.state.matches("dismissed")).toBeTruthy() + expect(getDismissedVersionOnLocal()).toEqual(MockUpdateCheck.version) + }) + }) +}) diff --git a/site/src/xServices/updateCheck/updateCheckXService.ts b/site/src/xServices/updateCheck/updateCheckXService.ts index dddce422a4..e90d73c2ed 100644 --- a/site/src/xServices/updateCheck/updateCheckXService.ts +++ b/site/src/xServices/updateCheck/updateCheckXService.ts @@ -1,33 +1,15 @@ import { assign, createMachine } from "xstate" -import { checkAuthorization, getUpdateCheck } from "api/api" +import { getUpdateCheck } from "api/api" import { AuthorizationResponse, UpdateCheckResponse } from "api/typesGenerated" - -export const checks = { - viewUpdateCheck: "viewUpdateCheck", -} - -export const permissionsToCheck = { - [checks.viewUpdateCheck]: { - object: { - resource_type: "update_check", - }, - action: "read", - }, -} - -export type Permissions = Record +import { checks, Permissions } from "xServices/auth/authXService" export interface UpdateCheckContext { - show: boolean + permissions: Permissions updateCheck?: UpdateCheckResponse - permissions?: Permissions error?: Error | unknown } -export type UpdateCheckEvent = - | { type: "CHECK" } - | { type: "CLEAR" } - | { type: "DISMISS" } +export type UpdateCheckEvent = { type: "DISMISS" } export const updateCheckMachine = createMachine( { @@ -46,36 +28,8 @@ export const updateCheckMachine = createMachine( } }, }, - context: { - show: false, - }, - initial: "idle", + initial: "checkingPermissions", states: { - idle: { - on: { - CHECK: { - target: "fetchingPermissions", - }, - }, - }, - fetchingPermissions: { - invoke: { - src: "checkPermissions", - id: "checkPermissions", - onDone: [ - { - actions: ["assignPermissions"], - target: "checkingPermissions", - }, - ], - onError: [ - { - actions: ["assignError"], - target: "show", - }, - ], - }, - }, checkingPermissions: { always: [ { @@ -83,8 +37,7 @@ export const updateCheckMachine = createMachine( cond: "canViewUpdateCheck", }, { - target: "dismissOrClear", - cond: "canNotViewUpdateCheck", + target: "dismissed", }, ], }, @@ -94,36 +47,28 @@ export const updateCheckMachine = createMachine( id: "getUpdateCheck", onDone: [ { - actions: ["assignUpdateCheck", "clearError"], + actions: ["assignUpdateCheck"], target: "show", + cond: "shouldShowUpdateCheck", + }, + { + target: "dismissed", }, ], onError: [ { - actions: ["assignError", "clearUpdateCheck"], - target: "show", + actions: ["assignError"], + target: "dismissed", }, ], }, }, show: { - entry: "assignShow", - always: [ - { - target: "dismissOrClear", - }, - ], - }, - dismissOrClear: { on: { DISMISS: { - actions: ["assignHide", "setDismissedVersion"], + actions: ["setDismissedVersion"], target: "dismissed", }, - CLEAR: { - actions: ["clearUpdateCheck", "clearError", "assignHide"], - target: "idle", - }, }, }, dismissed: { @@ -133,48 +78,45 @@ export const updateCheckMachine = createMachine( }, { services: { - checkPermissions: async () => - checkAuthorization({ checks: permissionsToCheck }), - getUpdateCheck: getUpdateCheck, + getUpdateCheck, }, actions: { - assignPermissions: assign({ - permissions: (_, event) => event.data as Permissions, - }), - assignShow: assign((context) => ({ - show: - localStorage.getItem("dismissedVersion") !== - context.updateCheck?.version, - })), - assignHide: assign({ - show: false, - }), assignUpdateCheck: assign({ updateCheck: (_, event) => event.data, }), - clearUpdateCheck: assign((context) => ({ - ...context, - updateCheck: undefined, - })), assignError: assign({ error: (_, event) => event.data, }), - clearError: assign((context) => ({ - ...context, - error: undefined, - })), - setDismissedVersion: (context) => { - if (context.updateCheck?.version) { - // We use localStorage to ensure users who have dismissed the UpdateCheckBanner are not plagued by its reappearance on page reload - localStorage.setItem("dismissedVersion", context.updateCheck.version) + setDismissedVersion: ({ updateCheck }) => { + if (!updateCheck) { + throw new Error("Update check is not set") } + + saveDismissedVersionOnLocal(updateCheck.version) }, }, guards: { - canViewUpdateCheck: (context) => - context.permissions?.[checks.viewUpdateCheck] || false, - canNotViewUpdateCheck: (context) => - !context.permissions?.[checks.viewUpdateCheck], + canViewUpdateCheck: ({ permissions }) => + permissions[checks.viewUpdateCheck] || false, + shouldShowUpdateCheck: (_, { data }) => { + const isNotDismissed = getDismissedVersionOnLocal() !== data.version + const isOutdated = !data.current + + return isNotDismissed && isOutdated + }, }, }, ) + +// Exporting to be used in the tests +export const saveDismissedVersionOnLocal = (version: string): void => { + window.localStorage.setItem("dismissedVersion", version) +} + +export const getDismissedVersionOnLocal = (): string | undefined => { + return localStorage.getItem("dismissedVersion") ?? undefined +} + +export const clearDismissedVersionOnLocal = (): void => { + localStorage.removeItem("dismissedVersion") +}