From f1a58f043427a2cdf36271563cb4bdde81adcb6e Mon Sep 17 00:00:00 2001 From: Dan Ditomaso Date: Mon, 10 Mar 2025 20:30:04 -0400 Subject: [PATCH] refactor: fixed unsafe roles dialog and hook logic, added tests --- .../UnsafeRolesDialog.test.tsx | 131 +++++++++--------- .../UnsafeRolesDialog/UnsafeRolesDialog.tsx | 44 ++++-- .../UnsafeRolesDialog/useUnsafeRoles.test.ts | 102 -------------- .../UnsafeRolesDialog/useUnsafeRoles.ts | 40 ------ .../useUnsafeRolesDialog.test.tsx | 117 ++++++++++++++++ .../UnsafeRolesDialog/useUnsafeRolesDialog.ts | 39 ++++++ src/components/Form/FormSelect.tsx | 33 ++--- .../Config/Device/Device.test.tsx | 46 ++++++ .../PageComponents/Config/Device/index.tsx | 34 +---- src/components/UI/Dialog.tsx | 22 ++- src/components/UI/ErrorPage.tsx | 2 +- src/core/stores/deviceStore.ts | 8 ++ src/core/utils/eventBus.test.ts | 71 ++++++++++ src/core/utils/eventBus.ts | 44 ++++++ src/tests/setupTests.ts | 1 - 15 files changed, 460 insertions(+), 274 deletions(-) delete mode 100644 src/components/Dialog/UnsafeRolesDialog/useUnsafeRoles.test.ts delete mode 100644 src/components/Dialog/UnsafeRolesDialog/useUnsafeRoles.ts create mode 100644 src/components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog.test.tsx create mode 100644 src/components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog.ts create mode 100644 src/components/PageComponents/Config/Device/Device.test.tsx create mode 100644 src/core/utils/eventBus.test.ts create mode 100644 src/core/utils/eventBus.ts diff --git a/src/components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.test.tsx b/src/components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.test.tsx index 2b7b70c3..c6c8ff29 100644 --- a/src/components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.test.tsx +++ b/src/components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.test.tsx @@ -1,88 +1,91 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/react'; -import { UnsafeRolesDialog } from '@components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.tsx'; -import { useUnsafeRoles } from '@components/Dialog/UnsafeRolesDialog/useUnsafeRoles.ts'; - -vi.mock('@components/Dialog/UnsafeRolesDialog/useUnsafeRoles', () => ({ - useUnsafeRoles: vi.fn() -})); - -describe('UnsafeRolesDialog', () => { - const getConfirmStateMock = vi.fn(); - const toggleConfirmStateMock = vi.fn(); - const handleCloseDialogMock = vi.fn(); - const onOpenChangeMock = vi.fn(); - - beforeEach(() => { - vi.resetAllMocks(); - - getConfirmStateMock.mockReturnValue(false); - - (useUnsafeRoles as any).mockReturnValue({ - getConfirmState: getConfirmStateMock, - toggleConfirmState: toggleConfirmStateMock, - handleCloseDialog: handleCloseDialogMock - }); +// deno-lint-ignore-file +import { render, screen, fireEvent } from "@testing-library/react"; +import { describe, it, expect, vi } from "vitest"; +import { UnsafeRolesDialog } from "@components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.tsx"; +import { eventBus } from "@core/utils/eventBus.ts"; +import { DeviceWrapper } from "@app/DeviceWrapper.tsx"; + +describe("UnsafeRolesDialog", () => { + const mockDevice = { + setDialogOpen: vi.fn(), + }; + + const renderWithDeviceContext = (ui: any) => { + return render( + + {ui} + + ); + }; + + it("renders the dialog when open is true", () => { + renderWithDeviceContext(); + + const dialog = screen.getByRole('dialog'); + expect(dialog).toBeInTheDocument(); + + expect(screen.getByText(/I have read the/i)).toBeInTheDocument(); + expect(screen.getByText(/understand the implications/i)).toBeInTheDocument(); + + const links = screen.getAllByRole('link'); + expect(links).toHaveLength(2); + expect(links[0]).toHaveTextContent('Device Role Documentation'); + expect(links[1]).toHaveTextContent('Choosing The Right Device Role'); }); - it('should not render when open is false', () => { - render(); + it("displays the correct links", () => { + renderWithDeviceContext(); - expect(screen.queryByTestId('dialog')).not.toBeInTheDocument(); - }); + const docLink = screen.getByRole("link", { name: /Device Role Documentation/i }); + const blogLink = screen.getByRole("link", { name: /Choosing The Right Device Role/i }); - it('should render when open is true', () => { - render(); - - expect(screen.getByRole('dialog')).toBeInTheDocument(); - expect(screen.getByRole('heading')).toBeInTheDocument(); - expect(screen.getByText('Are you sure?')).toBeInTheDocument(); - expect(screen.getAllByRole('link')).length(2); - expect(screen.getByRole('checkbox')).toBeInTheDocument(); - expect(screen.getByText('Yes, I know what I\'m doing')).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /dismiss/i })).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /confirm/i })).toBeInTheDocument(); + expect(docLink).toHaveAttribute("href", "https://meshtastic.org/docs/configuration/radio/device/"); + expect(blogLink).toHaveAttribute("href", "https://meshtastic.org/blog/choosing-the-right-device-role/"); }); - it('should have disabled confirm button when checkbox is unchecked', () => { - getConfirmStateMock.mockReturnValue(false); - - render(); + it("does not allow confirmation until checkbox is checked", () => { + renderWithDeviceContext(); - expect(screen.getByRole('button', { name: /confirm/i })).toBeDisabled(); - }); + const confirmButton = screen.getByRole("button", { name: /confirm/i }); - it('should have enabled confirm button when checkbox is checked', () => { - getConfirmStateMock.mockReturnValue(true); + expect(confirmButton).toBeDisabled(); - render(); + const checkbox = screen.getByRole("checkbox"); + fireEvent.click(checkbox); - expect(screen.getByRole('button', { name: /confirm/i })).not.toBeDisabled(); + expect(confirmButton).toBeEnabled(); }); - it('should call toggleConfirmState when checkbox is clicked', () => { - render(); + it("emits the correct event when closing via close button", () => { + const eventSpy = vi.spyOn(eventBus, "emit"); + renderWithDeviceContext(); - fireEvent.click(screen.getByRole('checkbox')); + const dismissButton = screen.getByRole("button", { name: /close/i }); + fireEvent.click(dismissButton); - expect(toggleConfirmStateMock).toHaveBeenCalledTimes(1); + expect(eventSpy).toHaveBeenCalledWith("dialog:unsafeRoles", { action: "dismiss" }); }); - it('should call handleCloseDialog with "dismiss" when dismiss button is clicked', () => { - render(); + it("emits the correct event when dismissing", () => { + const eventSpy = vi.spyOn(eventBus, "emit"); + renderWithDeviceContext(); - fireEvent.click(screen.getByRole('button', { name: /dismiss/i })); + const dismissButton = screen.getByRole("button", { name: /dismiss/i }); + fireEvent.click(dismissButton); - expect(handleCloseDialogMock).toHaveBeenCalledWith('dismiss'); + expect(eventSpy).toHaveBeenCalledWith("dialog:unsafeRoles", { action: "dismiss" }); }); - it('should call handleCloseDialog with "confirm" when confirm button is clicked', () => { - getConfirmStateMock.mockReturnValue(true); + it("emits the correct event when confirming", () => { + const eventSpy = vi.spyOn(eventBus, "emit"); + renderWithDeviceContext(); - render(); + const checkbox = screen.getByRole("checkbox"); + const confirmButton = screen.getByRole("button", { name: /confirm/i }); - fireEvent.click(screen.getByRole('button', { name: /confirm/i })); + fireEvent.click(checkbox); + fireEvent.click(confirmButton); - expect(handleCloseDialogMock).toHaveBeenCalledWith("confirm"); + expect(eventSpy).toHaveBeenCalledWith("dialog:unsafeRoles", { action: "confirm" }); }); -}); \ No newline at end of file +}); diff --git a/src/components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.tsx b/src/components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.tsx index b2a79858..d3dc02dd 100644 --- a/src/components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.tsx +++ b/src/components/Dialog/UnsafeRolesDialog/UnsafeRolesDialog.tsx @@ -1,5 +1,6 @@ import { Dialog, + DialogClose, DialogContent, DialogDescription, DialogFooter, @@ -7,10 +8,11 @@ import { DialogTitle, } from "@components/UI/Dialog.tsx"; import { Link } from "@components/UI/Typography/Link.tsx"; -import { Checkbox } from "../../UI/Checkbox/index.tsx"; -import { Label } from "@components/UI/Label.tsx"; +import { Checkbox } from "@components/UI/Checkbox/index.tsx"; import { Button } from "@components/UI/Button.tsx"; -import { useUnsafeRoles } from "@components/Dialog/UnsafeRolesDialog/useUnsafeRoles.ts"; +import { useDevice } from "@core/stores/deviceStore.ts"; +import { useState } from "react"; +import { eventBus } from "@core/utils/eventBus.ts"; export interface RouterRoleDialogProps { open: boolean; @@ -18,34 +20,52 @@ export interface RouterRoleDialogProps { } export const UnsafeRolesDialog = ({ open, onOpenChange }: RouterRoleDialogProps) => { - const { getConfirmState, toggleConfirmState, handleCloseDialog } = useUnsafeRoles(); + const [confirmState, setConfirmState] = useState(false); + const { setDialogOpen } = useDevice(); - const deivceRoleLink = "https://meshtastic.org/docs/configuration/radio/device/"; + const deviceRoleLink = "https://meshtastic.org/docs/configuration/radio/device/"; const choosingTheRightDeviceRoleLink = "https://meshtastic.org/blog/choosing-the-right-device-role/"; + + const handleCloseDialog = (action: 'confirm' | 'dismiss') => { + setDialogOpen('unsafeRoles', false); + setConfirmState(false); + eventBus.emit('dialog:unsafeRoles', { action }); + } + return ( + handleCloseDialog('dismiss')} /> Are you sure? - I have read the Device Role Documentation{" "} + I have read the Device Role Documentation{" "} and the blog post about Choosing The Right Device Role and understand the implications of changing the role.
- + setConfirmState(!confirmState)} + > Yes, I know what I'm doing
- - - + ); }; diff --git a/src/components/Dialog/UnsafeRolesDialog/useUnsafeRoles.test.ts b/src/components/Dialog/UnsafeRolesDialog/useUnsafeRoles.test.ts deleted file mode 100644 index d575d42d..00000000 --- a/src/components/Dialog/UnsafeRolesDialog/useUnsafeRoles.test.ts +++ /dev/null @@ -1,102 +0,0 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { renderHook, act } from '@testing-library/react'; -import { useUnsafeRoles } from './useUnsafeRoles.ts'; -import { useDevice } from '@core/stores/deviceStore.ts'; -import useLocalStorage from '@core/hooks/useLocalStorage.ts'; - -vi.mock('@core/stores/deviceStore', () => ({ - useDevice: vi.fn() -})); - -vi.mock('@core/hooks/useLocalStorage', () => { - return { - default: vi.fn() - }; -}); - -describe('useUnsafeRoles', () => { - const setDialogOpenMock = vi.fn(); - const setAgreedToUnsafeRolesMock = vi.fn(); - - beforeEach(() => { - vi.resetAllMocks(); - - (useDevice as any).mockReturnValue({ - setDialogOpen: setDialogOpenMock - }); - - (useLocalStorage as any).mockReturnValue([ - false, - setAgreedToUnsafeRolesMock - ]); - }); - - it('should initialize with correct default values', () => { - const { result } = renderHook(() => useUnsafeRoles()); - - expect(result.current.agreedToUnSafeRoles).toBe(false); - expect(result.current.getConfirmState()).toBe(false); - }); - - it('should toggle confirm state correctly', () => { - const { result } = renderHook(() => useUnsafeRoles()); - - act(() => { - result.current.toggleConfirmState(); - }); - - expect(result.current.getConfirmState()).toBe(true); - - act(() => { - result.current.toggleConfirmState(); - }); - - expect(result.current.getConfirmState()).toBe(false); - }); - - it('should handle dialog close with dismiss state', () => { - const { result } = renderHook(() => useUnsafeRoles()); - - act(() => { - result.current.handleCloseDialog('dismiss'); - }); - - expect(setAgreedToUnsafeRolesMock).toHaveBeenCalledWith(false); - expect(setDialogOpenMock).toHaveBeenCalledWith('unsafeRoles', false); - }); - - it('should handle dialog close with confirm state', () => { - const { result } = renderHook(() => useUnsafeRoles()); - - act(() => { - result.current.handleCloseDialog('confirm'); - }); - - expect(setAgreedToUnsafeRolesMock).toHaveBeenCalledWith(true); - expect(setDialogOpenMock).toHaveBeenCalledWith('unsafeRoles', false); - }); - - it('should maintain state consistency across multiple operations', () => { - const { result } = renderHook(() => useUnsafeRoles()); - - act(() => { - result.current.toggleConfirmState(); - }); - expect(result.current.getConfirmState()).toBe(true); - - act(() => { - result.current.handleCloseDialog('confirm'); - }); - - expect(result.current.getConfirmState()).toBe(false); - expect(setAgreedToUnsafeRolesMock).toHaveBeenCalledWith(true); - - (useLocalStorage as any).mockReturnValue([ - true, - setAgreedToUnsafeRolesMock - ]); - - const { result: newResult } = renderHook(() => useUnsafeRoles()); - expect(newResult.current.agreedToUnSafeRoles).toBe(true); - }); -}); \ No newline at end of file diff --git a/src/components/Dialog/UnsafeRolesDialog/useUnsafeRoles.ts b/src/components/Dialog/UnsafeRolesDialog/useUnsafeRoles.ts deleted file mode 100644 index f7c4b6fa..00000000 --- a/src/components/Dialog/UnsafeRolesDialog/useUnsafeRoles.ts +++ /dev/null @@ -1,40 +0,0 @@ -import { useState, useCallback } from "react"; -import { useDevice } from "@core/stores/deviceStore.ts"; -import useLocalStorage from "@core/hooks/useLocalStorage.ts"; - -export const useUnsafeRoles = () => { - const [agreedToUnSafeRoles, setAgreedToUnsafeRoles] = useLocalStorage("agreeToUnsafeRole", false); - const [_confirmState, _setConfirmState] = useState(false); - const { setDialogOpen } = useDevice(); - - const toggleConfirmState = useCallback(() => { - setConfirmState(!_confirmState); - }, [_confirmState]); - - const setConfirmState = useCallback((state: boolean) => { - _setConfirmState(state); - }, [_setConfirmState]); - - const getConfirmState = useCallback(() => { - return _confirmState; - }, [_confirmState]); - - const handleCloseDialog = useCallback((closeState: "dismiss" | "confirm") => { - if (closeState === "dismiss") { - setAgreedToUnsafeRoles(false); - setConfirmState(false); - } - if (closeState === "confirm") { - setAgreedToUnsafeRoles(true); - setConfirmState(false); - } - setDialogOpen("unsafeRoles", false); - }, [setDialogOpen, setAgreedToUnsafeRoles]); - - return { - getConfirmState, - toggleConfirmState, - handleCloseDialog, - agreedToUnSafeRoles - }; -}; diff --git a/src/components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog.test.tsx b/src/components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog.test.tsx new file mode 100644 index 00000000..bc193646 --- /dev/null +++ b/src/components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog.test.tsx @@ -0,0 +1,117 @@ +import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { useUnsafeRolesDialog, UNSAFE_ROLES } from "@components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog"; +import { eventBus } from "@core/utils/eventBus"; + +vi.mock('@core/utils/eventBus', () => ({ + eventBus: { + on: vi.fn(), + off: vi.fn(), + emit: vi.fn(), + }, +})); + +const mockDevice = { + setDialogOpen: vi.fn(), +}; + +vi.mock('@core/stores/deviceStore', () => ({ + useDevice: () => ({ + setDialogOpen: mockDevice.setDialogOpen, + }), +})); + +describe('useUnsafeRolesDialog', () => { + beforeEach(() => { + vi.resetAllMocks(); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + const renderUnsafeRolesHook = () => { + return renderHook(() => useUnsafeRolesDialog()); + }; + + describe('handleCloseDialog', () => { + it('should call setDialogOpen with correct parameters when dialog is closed', () => { + const { result } = renderUnsafeRolesHook(); + + result.current.handleCloseDialog(); + + expect(mockDevice.setDialogOpen).toHaveBeenCalledWith('unsafeRoles', false); + }); + }); + + describe('validateRoleSelection', () => { + it('should resolve with true for safe roles without opening dialog', async () => { + const { result } = renderUnsafeRolesHook(); + const safeRole = 'SAFE_ROLE'; + + const validationResult = await result.current.validateRoleSelection(safeRole); + + expect(validationResult).toBe(true); + expect(mockDevice.setDialogOpen).not.toHaveBeenCalled(); + }); + + it('should open dialog for unsafe roles and resolve with true when confirmed', async () => { + const { result } = renderUnsafeRolesHook(); + + const validationPromise = result.current.validateRoleSelection(UNSAFE_ROLES[0]); + + expect(mockDevice.setDialogOpen).toHaveBeenCalledWith('unsafeRoles', true); + expect(eventBus.on).toHaveBeenCalledWith('dialog:unsafeRoles', expect.any(Function)); + + const onHandler = (eventBus.on as Mock).mock.calls[0][1]; + onHandler({ action: 'confirm' }); + const validationResult = await validationPromise; + + expect(validationResult).toBe(true); + expect(eventBus.off).toHaveBeenCalledWith('dialog:unsafeRoles', onHandler); + }); + + it('should resolve with false when user dismisses the dialog', async () => { + const { result } = renderUnsafeRolesHook(); + const validationPromise = result.current.validateRoleSelection(UNSAFE_ROLES[0]); + const onHandler = (eventBus.on as Mock).mock.calls[0][1]; + onHandler({ action: 'dismiss' }); + + const validationResult = await validationPromise; + expect(validationResult).toBe(false); + expect(eventBus.off).toHaveBeenCalledWith('dialog:unsafeRoles', onHandler); + }); + + it('should clean up event listener after response', async () => { + const { result } = renderUnsafeRolesHook(); + + const validationPromise = result.current.validateRoleSelection(UNSAFE_ROLES[1]); + const onHandler = (eventBus.on as Mock).mock.calls[0][1]; + + onHandler({ action: 'confirm' }); + await validationPromise; + + expect(eventBus.off).toHaveBeenCalledWith('dialog:unsafeRoles', onHandler); + }); + }); + + it('should work with all unsafe roles', async () => { + const { result } = renderUnsafeRolesHook(); + + for (const unsafeRole of UNSAFE_ROLES) { + mockDevice.setDialogOpen.mockClear(); + (eventBus.on as Mock).mockClear(); + + const validationPromise = result.current.validateRoleSelection(unsafeRole); + + expect(mockDevice.setDialogOpen).toHaveBeenCalledWith('unsafeRoles', true); + + const onHandler = (eventBus.on as Mock).mock.calls[0][1]; + onHandler({ action: 'confirm' }); + + const validationResult = await validationPromise; + + expect(validationResult).toBe(true); + } + }); +}); \ No newline at end of file diff --git a/src/components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog.ts b/src/components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog.ts new file mode 100644 index 00000000..5d417704 --- /dev/null +++ b/src/components/Dialog/UnsafeRolesDialog/useUnsafeRolesDialog.ts @@ -0,0 +1,39 @@ +import { useCallback } from "react"; +import { eventBus } from "@core/utils/eventBus.ts"; +import { useDevice } from "@core/stores/deviceStore.ts"; + +export const UNSAFE_ROLES = ["ROUTER", "REPEATER"]; +export type UnsafeRole = typeof UNSAFE_ROLES[number]; + +export const useUnsafeRolesDialog = () => { + const { setDialogOpen } = useDevice(); + + const handleCloseDialog = useCallback(() => { + setDialogOpen("unsafeRoles", false); + }, [setDialogOpen]); + + const validateRoleSelection = useCallback( + (newRoleKey: string): Promise => { + if (!UNSAFE_ROLES.includes(newRoleKey as UnsafeRole)) { + return Promise.resolve(true); + } + + setDialogOpen("unsafeRoles", true); + + return new Promise((resolve) => { + const handleResponse = ({ action }: { action: "confirm" | "dismiss" }) => { + eventBus.off("dialog:unsafeRoles", handleResponse); + resolve(action === "confirm"); + }; + + eventBus.on("dialog:unsafeRoles", handleResponse); + }); + }, + [setDialogOpen] + ); + + return { + handleCloseDialog, + validateRoleSelection, + }; +}; diff --git a/src/components/Form/FormSelect.tsx b/src/components/Form/FormSelect.tsx index ee533292..037b651c 100644 --- a/src/components/Form/FormSelect.tsx +++ b/src/components/Form/FormSelect.tsx @@ -10,11 +10,12 @@ import { SelectValue, } from "@components/UI/Select.tsx"; import { useController, type FieldValues } from "react-hook-form"; +import { computeHeadingLevel } from "@core/utils/test.tsx"; export interface SelectFieldProps extends BaseFormBuilderProps { type: "select"; selectChange?: (e: string, name: string) => void; - onBeforeChange?: (newValue: string, prevValue: string) => Promise; + validate?: (newValue: string) => Promise; properties: BaseFormBuilderProps["properties"] & { enumValue: { [s: string]: string | number; @@ -23,8 +24,6 @@ export interface SelectFieldProps extends BaseFormBuilderProps { }; } - - const formatEnumDisplay = (name: string): string => { return name .replace(/_/g, " ") @@ -48,14 +47,12 @@ export function SelectInput({ const { enumValue, formatEnumName, ...remainingProperties } = field.properties; const valueToKeyMap: Record = {}; - const keyToValueMap: Record = {}; const optionsEnumValues: [string, number][] = []; if (enumValue) { Object.entries(enumValue).forEach(([key, val]) => { if (typeof val === "number") { - valueToKeyMap[val.toString()] = key; // Map enum value to key - keyToValueMap[key] = val; // Map key to enum value + valueToKeyMap[val.toString()] = key; optionsEnumValues.push([key, val]); } }); @@ -63,27 +60,17 @@ export function SelectInput({ const handleValueChange = async (newValue: string) => { const selectedKey = valueToKeyMap[newValue]; - if (!selectedKey) return; - - if (field.onBeforeChange) { - try { - const result = await field.onBeforeChange(selectedKey, valueToKeyMap[value?.toString()]); - if (result === false) return; - const updatedValue = keyToValueMap[result]; - if (updatedValue !== undefined) { - if (field.selectChange) field.selectChange(updatedValue.toString(), result); - onChange(updatedValue); - } - } catch (error) { - console.error("Error in onBeforeChange function:", error); - } - } else { - if (field.selectChange) field.selectChange(newValue, selectedKey); - onChange(Number.parseInt(newValue)); + if (field.validate) { + const isValid = await field.validate(selectedKey); + if (!isValid) return; } + + if (field.selectChange) field.selectChange(newValue, selectedKey); + onChange(Number.parseInt(newValue)); }; + return (