Skip to content

UI/UX Refinements, Test Suite Migration to Vitest, Security Hardening, and Infrastructure Updates- #70 - #71

Open
PaulJouvanceau wants to merge 93 commits into
opensvc:mainfrom
PaulJouvanceau:dev
Open

UI/UX Refinements, Test Suite Migration to Vitest, Security Hardening, and Infrastructure Updates- #70 #71
PaulJouvanceau wants to merge 93 commits into
opensvc:mainfrom
PaulJouvanceau:dev

Conversation

@PaulJouvanceau

Copy link
Copy Markdown
Contributor

Summary
This PR introduces a wide range of improvements across the application, including UI fixes, new features, test refactoring, and bug fixes related to SSE connections, event handling, and authentication.

Changes
UI & UX
Safari compatibility: Fixed badge overflow and aligned badges to the bottom of stat cards

Action menus: Increased z-index of all action menus to appear above EventLogs panel

Destructive actions: Highlighted destructive actions in red and moved them to the bottom of action lists

StatCard: Added dynamic height with configurable max subtitle height

Mobile/scroll: Resolved iOS scroll issues by centralizing scroll in the main container; fixed logs panel resize on iOS and close button visibility

Navigation: Replaced user icon with tag icon for Kinds route; fixed duplicate navigation on Nodes grid by stopping event propagation

Grid2: Replaced undefined Grid2 with Grid for proper mobile rendering

Auth: Show spinner in AuthChoice while authInfo is loading

Features
Added replace mode to setInstanceStatuses and used it on initial fetch to prevent stale data accumulation

Added bottomSpacing prop to LogsViewer for configurable bottom spacing

Added fallback fetch and dynamic configNode switching in ObjectDetails

Reactive config refresh with fallback loading state

Instances in ObjectDetail are now sorted alphabetically

Bug Fixes
SSE/Events: Added objectName parameter to startEventReception for path fix; fixed objectName usage; closed SSE connection on ClusterOverview unmount to prevent memory leaks

OIDC: Use stable base path for OIDC callback URLs instead of deriving from window.location

Auth: Debounced auth check on page resume to prevent redundant token validation

Events: Debounced localStorage persistence and avoided O(n) instance scanning for performance

Heartbeats: Improved GridHeartbeats click handling

Buffer: Reduced debug logging verbosity

Config: Added useConfig and improved eventSourceManager configuration

Test Refactoring & Coverage
Refactored and optimized numerous test suites for clarity, maintainability, and execution speed without reducing coverage:

ObjectDetails.test

eventSourceManager

Objects tests

App.test.jsx

useEventStore (branch coverage for buffer and Safari)

Heartbeats (consolidated, reduced duplication)

Login test file

LogsViewer test suite

NodesTable tests

NavBar tests

Namespaces unit tests

AuthProvider (fixed timing/isolation issues)

WhoAmI tests

ClusterOverview tests (fixed mock alignment)

DarkModeContext (added comprehensive unit tests)

ConfigSection (improved coverage and structure)

Additional test improvements:

Added shared helpers and parameterization for KeysSection tests

Used real MUI RadioGroup/FormControlLabel in KeysSection tests

Fixed flaky action tests in Objects component

Misc
Deleted patch action (cleanup)

Improved GridNodes click handling

…from current page path

Previously, the base URL was computed by stripping the last segment of
window.location.pathname, which caused redirect_uri and other callback
URLs to vary depending on the current page (e.g., /objects/auth-callback
when starting from an object detail page). This led to invalid callback
routes and "Not Found" errors from the OIDC provider.

Now the base path is dynamically extracted by matching the "/ui" prefix
in the pathname. This ensures all OIDC URIs are consistently rooted at
the application's sub-path, regardless of the page the user is on.
- Implement fallback data fetching via HTTP when no SSE events arrive within 5 seconds
- Auto-select initial configNode based on encap resources and handle switching when the node disappears
- Encode URI components in API URLs to prevent issues with special characters
- Subscribe to instanceConfig store changes and warn if unsubscribe is not a function
- Add tests: fallback fetch (success and failure), configNode switching, drawer max width, console action without rid, improved lifecycle assertions
- Added tests for removePendingDelete action
- Added tests for removeInstanceFromObject covering all three state keys and partial/no-op cases
- Added tests for setInstanceStatuses with replace=true, including replace with empty nodes, inherited properties, and shallow equal rejection
- Added tests for setConfigUpdated with missing namespace, cluster kind, and missing path
- Fixed existing assertions to expect empty objects instead of undefined after deleting the last instance from an object path
…ri optimizations

- Add tests for pending deletes filtering buffered instanceStatus and instanceConfig
- Add test for configUpdated event with missing name/node (fallback to warning)
- Add Safari-specific isolated module tests (setTimeout vs requestAnimationFrame, BATCH_SIZE=150)
- Fix mocking of default exports in Safari test suite
- Correct expected buffer state when pending delete removes buffered instanceStatus
…ed tests

Extract reusable mock factory (buildFetchMock), rendering and dialog open
helpers, and replace repetitive tests with test.each for auth token errors,
dialog close (cancel/Escape), and CRUD success/error paths.
…ght and custom scrollbars

- Introduce `maxSubtitleHeight` prop (default 160) to limit the subtitle area when `dynamicHeight` is enabled.
- Adjust flex layout so dynamic cards have a smaller min-height (120px) and non-flex subtitle area.
- Add thin custom scrollbar styling for the subtitle container to improve overflow handling.
- Apply `dynamicHeight` and `maxSubtitleHeight={220}` to GridNamespaces and GridKinds components for better space utilisation.
…overage loss

Merge multiple isolated test cases into comprehensive scenarios (all states,
filtering, sorting loops), introduce a createStream helper, and reduce
the number of render cycles. This significantly improves test suite performance
while keeping full coverage.
Replace mocked RadioGroup and FormControlLabel with actual MUI components
to fix interaction issues with radio inputs. Simplify other mocked components
and refactor helpers for clarity and reliability.
…rity

- Introduce `buildStatus` helper to centralize mock data construction
- Replace repetitive test blocks with `test.each` for state icons, URL filters, and sorting columns
- Consolidate mock implementations and clean up unused imports
- Add new tests for state column sorting and responsive filter visibility
- Improve test structure with clear section separators and better assertion patterns
- Simplify auth/cleanup tests and remove obsolete mocking of logger functions
- Introduce setupLogin and fillForm helpers to reduce boilerplate
- Merge input change and submit button disable checks into a single test
- Combine invalid token decode cases into one test
- Keep all code paths covered with fewer lines of code
…ertions

- Condense test file while preserving full code coverage
- Fix 404 error handling tests to use rejected fetch, matching real error paths
- Fix malformed JSON test to correctly match displayed substring
- Reduce test file size by ~50% using parametrized tests and cleanup
- Maintain all branch coverage for the store logic
- Extract common helpers (confirmDialog, withConsoleAction) to reduce duplication
- Consolidate parameterized tests for drawer resize and instanceConfig error handling
- Reorganize code with clear section comments for better navigation
- Introduce BASE_FNS factory to avoid repetitive mock function definitions
- Improve test descriptions and structure for clarity
- Remove redundant test code
- Added DEBUG_BUFFERS flag (default false) to suppress verbose buffer
  debug logs via debugLog function, keeping only essential logs.
- Updated test to verify needsFlush behavior without relying on
  disabled debug log messages.
…pagation on Frozen chip

Clicking the "Frozen" chip inside the Nodes grid triggered both the chip's onClick and the parent StatCard's onClick, causing two concurrent navigation attempts and requiring multiple clicks to open the page. Added stopPropagation in the chip's handler to ensure only a single navigation call is made.
Removed the onClick stopPropagation handler from the subtitle container
in StatCard. Previously, clicking the central area of the card (between
chips) was blocked because the container captured the event and stopped
propagation, requiring multiple clicks to navigate. Now clicks on empty
areas propagate to the parent Paper and trigger navigation as intended.
Chips still prevent propagation via their own handlers.
…y leak

The SSE connection opened by startEventReception was not being closed when
the ClusterOverview component unmounted, causing the connection to persist
and continue updating the Zustand store unnecessarily. Added closeEventSource()
in the useEffect cleanup to properly terminate the connection.
…lback

- Prevent duplicate navigation on Nodes grid by stopping event propagation on Frozen chip click
- Refactor GridHeartbeats chip handler to use direct useCallback with parameters instead of curried function, reducing unnecessary re-renders
…lidation

Added a debounce mechanism to the visibility/focus resume handler in OidcInitializer.
This prevents multiple rapid calls to isTokenValid and signinSilent when switching tabs
or refocusing the window, reducing unnecessary network requests and improving performance.
…e status merge on flush

ClusterOverview (the default landing route) felt unresponsive for a
moment right after mount, especially on first login when the SSE
connection replays the full cached cluster state.

Two compounding issues in the event pipeline caused this:

- eventSourceManager.flushBuffers() rebuilt the *entire*
  objectInstanceStatus map (existing state + delta) before calling
  setInstanceStatuses(), which then re-iterated every path/node in
  the cluster on every single flush instead of only the changed ones.
  During the initial cache replay this turned into O(total instances)
  work per flush instead of O(delta).

- useEventStore's zustand `persist` middleware wrote the full
  partialized state (objectStatus, objectInstanceStatus,
  instanceMonitor, instanceConfig) to localStorage synchronously on
  every set(). With several flushes happening within the first
  second after connecting, this meant repeated synchronous
  JSON.stringify + localStorage.setItem calls on potentially large
  objects, blocking the main thread and delaying click handling.

Changes:
- eventSourceManager.js: pass only the per-flush delta
  (buffersToFlush.instanceStatus) to setInstanceStatuses() instead of
  a pre-merged full copy of the store, letting it do the incremental
  per-path/per-node merge it was already designed for.
- useEventStore.js: wrap localStorage in a debounced storage adapter
  (800ms) for the persist middleware, coalescing bursts of writes
  into a single disk write. Pending writes are flushed immediately on
  beforeunload/visibilitychange so no data is lost on navigation away.

No behavior change for consumers of the store; only write frequency
and per-flush merge cost are affected.
- Consolidate mocks into a reusable setupMocks helper
- Use parameterized tests for breadcrumb paths
- Add test for /nodes/.../objects breadcrumb with plain text node
- Cover retry logic, fetchNodes error handling, and cluster name display
- Ensure all branches are covered
- Extract setup and render functions to reduce duplication
- Add CircularProgress mock for infinite scroll tests
- Simplify mock implementations and remove redundant test cases
- Organize infinite scroll tests into a nested describe block
- Improve test readability and consistency
- Remove edge case tests that are covered by existing tests
- Remove useEffect that forced overflow:hidden on html, body, and #root,
  which blocked natural scrolling on iOS.
- Replace height:100vh with height:100% in page components so they
  inherit height from the new scrollable main container.
- Create a fixed app-root-container with flex layout, using height:100%
  for stable viewport sizing, and set overflow-y:auto on main for
  smooth iOS scrolling (WebkitOverflowScrolling: touch).
- This ensures ClusterOverview, Objects, and other pages can be fully
  scrolled on iPhone without content being cut off or scroll blocked.
Replace MUI Drawer with custom fixed panels in ObjectInstanceView,
ObjectDetails, and NodesTable to avoid iOS touch event conflicts.

Implement resize using global pointer/touch listeners with
passive:false, pointer capture, and body overflow locking, matching
the proven approach from EventLogger.

Offset the panel below the AppBar (including safe-area-inset-top)
so the close button is no longer hidden on iOS.

Also restore document.body.style.cursor = 'default' on resize end
to keep existing tests passing, and add role="complementary" with
data-width attribute to the logs panel for test compatibility.
The mobile layout used `Grid2` which was not imported, causing a
ReferenceError on iOS and other mobile devices. Replaced with the
already imported `Grid` from @mui/material to restore functionality.
- Extracted dispatch actions into a configuration array for cleaner components
- Separated "unknown action" test to verify it does not alter initial state
- Grouped related tests using describe blocks for better organization
Consolidate repetitive mocks and setup, use parameterized tests for missing fields,
and inline helper functions to reduce code size without sacrificing test depth.
Consolidate repetitive mocks and setup, use parameterized tests for missing fields,
and inline helper functions to reduce code size without sacrificing test depth.
Previously, navigating to /auth-choice before the authInfo fetch completed
(expired session, direct URL access, slow network) rendered an empty dialog
with no buttons and no feedback, requiring a manual page refresh.

Changes:
- AuthChoice: accept optional authInfo prop; fall back to local useAuthInfo()
  if prop is absent (direct access, unit tests); show CircularProgress while
  authInfo is undefined instead of rendering an empty dialog
- App: call useAuthInfo() at App level so the fetch starts as early as
  possible, then pass the result down to <AuthChoice> and <OidcInitializer>
  via props, eliminating the double-fetch and reducing the window where
  authInfo is unavailable
- AuthInfo: revert to original implementation (no module-level or
  sessionStorage cache) to keep all existing unit tests green
Reduce test file size without lowering coverage
by merging near-identical test cases into parameterized test.each
blocks and extracting shared setup into helpers.

- Merge the 3 handleDialogConfirm rejection tests (batch node,
  individual node, object action) into a single test.each covering
  postNodeAction/postObjectAction error logging.
- Merge the 2 instanceConfig subscription tests (fires vs. does not
  fire snackbar) into one test.each.
- Merge the 2 batch-menu freeze visibility tests (mixed vs. fully
  frozen nodes) into one test.each.
- Merge the 2 best-effort resource-console-trigger tests (action
  button vs. tooltip title) into one test.each, extracting a shared
  findAndOpenResourceConsoleDialog helper.
- Add setStoreState() helper to replace the repeated
  useEventStore.mockImplementation/getState.mockReturnValue pairing
  used throughout the file.
- Add emptyState() factory to remove duplicated empty-state object
  literals (fallback fetch suite and others).

No test cases were removed; all assertions, error paths, and
conditional branches previously covered are preserved.
Add tests for previously uncovered lines:
- decodeToken returns null for invalid base64 payload (atob failure)
- decodeToken returns null for valid base64 but non-JSON payload
- Login form submission via Enter with empty fields shows error
- refreshToken handles missing refreshTokenExpiration (no expiration set)
- refreshToken stores new refresh token and its expiration when provided
- refreshToken clears refreshTokenExpiration when new refresh token has no exp
- Login when tokens have no exp: verify removeItem calls for tokenExpiration and refreshTokenExpiration
- refreshToken when access token has no exp: verify removeItem call for tokenExpiration

Also adds spy on localStorage.removeItem to assert cleanup calls.
- Export areStatusDotPropsEqual to allow direct unit testing of the memo comparison function.
- Add tests for navigation with down, warn, n/a status clicks.
- Add sorting tests for all columns (Up, Down, Warn, N/A, Total) in ascending order.
- Fix Namespace descending sort test to work with useDeferredValue by using a single click and waitFor.
- Add unit tests for areStatusDotPropsEqual with equal, different status, and different count.
- Ensure all sorting tests wait for deferred updates to reflect in the DOM.
- Remove isolated NamespaceTableRow test (coverage achieved via Namespaces component tests).
- Add istanbul ignore comments for defensive/unreachable code paths
  in ConfigSection.jsx
- Update Autocomplete mock to always call getOptionLabel
- Add test for network error in handleAddParams
…ns/suffixes when adding parameters

- Allow selecting existing sections or indexed section suffixes in the "Add parameters" flow
  using free-solo Autocomplete fields, while still enabling free text input for new sections.
…ents

- Add test cases for:
  * ObjectDeleted filtering with non-matching _rawEvent
  * CONNECTION events bypassing objectName filter
  * Subscription dialog updating when eventTypes change
  * Page events subscribe action with existing subscriptions
  * Rapid resize moves and timeout cleanup
  * Early unmount cleanup of initialLoading timer
  * Infinite scroll loading more logs
- Insert `/* istanbul ignore next */` comments to exclude branches that are
  difficult or impractical to cover in unit tests (syntax highlighting,
  resize scroll logic, Drawer slotProps style, and non-page chip style).
…router 8.3.0

- Replace all Jest test files with Vitest equivalents (vi.fn, vi.mock, vi.hoisted)
- Update package.json: add vitest, remove jest/babel-jest/ts-jest/etc, update scripts
- Add setupTests.js with vitest globals, jest alias, vitest-axe, polyfills
- Fix hoisted variable patterns for vi.mock factories
- Correct mock exports (default logger, eventSourceManager DEFAULT_FILTERS)
- Fix useNavigate mock pattern with vi.hoisted
- Add missing useMediaQuery import in several test files
- Adapt Objects test suite to current UI (no node columns, no filters toggle)
- Extend TextField mock in ActionDialogManager test for data-testid generation
- Upgrade to React 19 and react-router 8.3.0 to address security vulnerabilities
- Replace jest-axe with vitest-axe and update axe setup
- Misc test fixes: async timer flushing, console spy cleanup, dialog selectors
- Replace '@v1' with the commit hash for security (pinning).
- Correct 'files' syntax to properly attach dist/index.html to releases.
Add a safety check before fetching the .well-known/openid-configuration
to block requests to internal or malicious endpoints.

- Validate issuer URL scheme (HTTPS only, except localhost for dev)
- Reject URLs with embedded credentials
- Block IP addresses (IPv4 and IPv6) to prevent internal network access
- Disable HTTP redirects in fetch (`redirect: 'error'`)
- Keep the try/catch flow intact so existing tests for malformed URLs
  still pass

This closes the SSRF vector reported by SAST.
- Cover rendering of table headers, row counts, and status icon colors
- Verify navigation on row click and individual status cell clicks (up, down, warn, unprovisioned)
- Test event source lifecycle (start on mount, close on unmount, skip without token)
- Validate autocomplete filter behavior and URL parameter synchronization
- Check sorting by all columns (kind, up, down, warn, unprovisioned, total) in both directions
- Test infinite scroll (load more items, prevent duplicate loads, cleanup)
- Ensure correct messages for empty state and filter mismatch
- Fix sort test assertions for equal values by sorting expected subsets
Add unit tests for isSafeIssuerUrl edge cases (HTTP, credentials,
IPv4, private IPs, IPv6, missing host) and /ui base path handling.
This covers previously uncovered branches and increases line/branch
coverage in the OIDC configuration module.
- Add index.test.jsx covering the app bootstrap logic:
  - logs an error and skips rendering when #root is missing
  - creates the React root and renders the app tree when #root exists
  - renders the tree wrapped in React.StrictMode
  - resolves the router basename to "/ui" or "/" based on pathname
- Mock react-dom/client, react-router-dom, App, MUI theming, colors,
  main.css, logger, and DarkModeContext to isolate index.jsx from its
  dependencies
- Export getDesignTokens from index.jsx so the light/dark palette
  logic can be unit tested directly, since production code only ever
  invokes it with 'light' at import time
- Add tests for getDesignTokens covering both the light and dark
  palette branches, including mode-specific keys (text for dark,
  contrastText for light)
…anceStatus

Add test cases for:
- Returning and caching default empty data when instanceStatus is missing
- Maintaining last known data when instanceStatus disappears (prevDataRef not null)
- Frozen logic for zero date, missing frozen_at, and valid frozen date
- Monitor state handling when state is 'idle' or monitor is missing entirely
…o fix clipped action menus on iOS

- Replaced custom Popper + ClickAwayListener with MUI Menu components for object, batch node, and individual node action menus.
- Removed Safari-specific disablePortal workaround that caused clipping inside overflow-hidden containers.
- Added fullScreen support for dialogs on mobile (useMediaQuery) in ObjectDetail and ActionDialogManager.
- Updated related tests (HeaderSection.test, ObjectDetails.test) to match new Menu-based implementation.
- Adjusted ActionDialogManager and ActionDialogs to accept and apply fullScreen prop.
- Ensured handleResizeStart and other missing handlers are defined in ObjectDetail.

Fixes the issue where action menus were cut off at the bottom empty zone on iPhone Safari.
Add missing test scenarios to increase function and branch coverage:
- Include `color: 'red'` in delete action mock to exercise error style branch
- Add close button inside Menu mock to simulate onClose callback
- Add test verifying menu closes when onClose is triggered
…dlers and resize edge case

- Mock MUI Menu/Dialog components to actually invoke their onClose prop
  via an explicit close trigger, instead of spreading it inertly onto a
  <div>/<button>. This makes the "Actions on selected nodes" menu,
  "Node actions" menu, and console Dialog onClose handlers reachable
  and testable, matching real backdrop/Escape close behavior.
- Add test for the batch actions menu onClose handler.
- Add test for the individual node actions menu onClose handler.
- Add test for the console Dialog onClose handler (distinct from the
  Cancel button's onClick).
- Add test for the touchcancel listener during logs drawer resize,
  which was attached but never exercised (only touchend/mouseup were).
- Add test covering the hasInstances short-circuit branch in
  fetchFallbackData: decouple the reactive store selector
  from useEventStore.getState() so the fallback fetch fires while
  getState() already has instance data, asserting the store is not
  overwritten in that case.

Note: postConsoleAction and its dependents (Copy URL / Open in New
Tab / consoleUrlDialog onClose / handleConsoleConfirm's catch) remain
uncovered. They require pendingAction.rid to be set while
consoleDialogOpen is true, but no reachable call path in
ObjectDetails.jsx currently passes rid to openActionDialog. Closing
that gap needs a source-side change, not more tests.
- Mock useMediaQuery from '@mui/material' instead of '@mui/material/useMediaQuery'
- Replace mockReturnValueOnce with mockImplementation inspecting query strings
  to reliably return isWideScreen/isMobile values
- Add tests for mobile filter toggle, node column sorting, and row menu
  when object data comes from daemon fallback
- Update setup calls to explicitly pass { isWideScreen, isMobile } where needed
- Add test for recreating UserManager with Log configuration error
- Add test for failure when setting window.oidcUserManager
- Add test for cleanup of previous UserManager on recreate
- Fix TypeScript issues by casting Log and adding missing redirect_uri
- Remove flaky test for deleting non-configurable window property
Add tests to cover remaining untested branches in NodesTable component,
particularly around sorting, missing data, and Safari menu positioning.

- Add descending sort tests for all sortable columns
- Add tests for missing stats/monitor values during sorting
- Add test for Safari-specific calculateMenuPosition logic
- Use waitFor to ensure async re-renders are captured in sorting tests
- Fix expected order in descending sort tests to match actual behavior
Added additional test cases to cover missing branches in sorting logic and retry handling, achieving 100% branch coverage. Specifically:
- Test sorting with null/undefined name, type, head, and volume_count
- Test usage sorting with size=0 (false branch)
- Test retry success with non-array items
- Adjusted sort fallback test to correctly verify ascending/descending toggles
Enhance the StatCard component's hover state by adding a more prominent box shadow, background color change, and primary-colored border, using the MUI theme. The transition includes box-shadow, background-color, and border-color for smooth visual feedback. No other files are modified.
- Add `hover` prop to TableRowComponent to enable MUI hover effect
- Update CSS to correctly target `.MuiTableRow-hover:hover` for dark mode
- Ensure table cells inherit hover background to make row highlight visible
- Implement WhoAmI page displaying user info, permissions, server information, dark mode toggle, and logout
- Fetch user info from API, app version from GitHub with local caching, and daemon status
- Support both OIDC and local authentication flows
- Responsive layout for mobile and desktop using MUI breakpoints
- Add full Vitest test suite covering loading, errors, data rendering, version caching, daemon fetch, dark mode, logout, and mobile layout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant