fix(dev-portal): address role activator review feedback

Author: eikeland

.changeset/dev-portal_add-role-activation.md

@@ -4,4 +4,4 @@
 
 Add claimable and permanent role tabs to the person side sheet, including activation and deactivation controls for claimable roles.
 
-Related isue #5230
+Related issue #5230

packages/dev-portal/package.json

@@ -53,11 +53,13 @@
     "@equinor/fusion-react-styles": "^2.1.0",
     "@equinor/fusion-wc-chip": "^1.2.2",
     "@equinor/fusion-wc-person": "^3.5.5",
+    "@testing-library/react": "^16.0.0",
     "@types/react": "^19.2.7",
     "@types/react-dom": "^19.2.3",
     "@types/semver": "^7.7.1",
     "@vitejs/plugin-react": "^6.0.1",
     "dotenv": "^17.3.1",
+    "happy-dom": "^20.8.4",
     "react": "^19.2.1",
     "react-dom": "^19.2.1",
     "rxjs": "^7.8.1",

packages/dev-portal/src/PersonSideSheet/sheets/roles/ClaimableRole.test.tsx

@@ -0,0 +1,83 @@
+import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { ClaimableRole } from './ClaimableRole';
+import type { ClaimableRoleAssignment } from './RolesApi';
+
+const mocks = vi.hoisted(() => ({
+  createClient: vi.fn(),
+  currentUser: { localAccountId: 'account-id' },
+}));
+
+vi.mock('@equinor/fusion-framework-react', () => ({
+  useFramework: () => ({
+    modules: { serviceDiscovery: { createClient: mocks.createClient } },
+  }),
+}));
+
+vi.mock('@equinor/fusion-framework-react/hooks', () => ({
+  useCurrentUser: () => mocks.currentUser,
+}));
+
+const assignment: ClaimableRoleAssignment = {
+  id: 'assignment-id',
+  claimableRole: {
+    name: 'Developer',
+    displayName: 'Developer role',
+    description: 'Temporary development access',
+  },
+  isActive: false,
+  activeTo: null,
+};
+
+describe('ClaimableRole', () => {
+  beforeEach(() => {
+    mocks.createClient.mockReset();
+  });
+
+  afterEach(() => {
+    cleanup();
+  });
+
+  it('requires a reason before activating a role', async () => {
+    const json = vi.fn();
+    mocks.createClient.mockResolvedValue({ json });
+
+    render(<ClaimableRole assignment={assignment} onChange={vi.fn()} />);
+
+    fireEvent.click(screen.getByLabelText('Activate Developer role'));
+    fireEvent.click(screen.getByRole('button', { name: 'Activate' }));
+
+    expect(await screen.findByText('Reason is required.')).toBeTruthy();
+    expect(json).not.toHaveBeenCalled();
+  });
+
+  it('activates a role and reports its updated state', async () => {
+    const json = vi.fn().mockResolvedValue({ activeToDate: '2026-08-07T16:00:00Z' });
+    const onChange = vi.fn();
+    mocks.createClient.mockResolvedValue({ json });
+
+    render(<ClaimableRole assignment={assignment} onChange={onChange} />);
+
+    fireEvent.click(screen.getByLabelText('Activate Developer role'));
+    fireEvent.change(screen.getByLabelText('Reason for activation'), {
+      target: { value: 'Testing role-dependent behavior' },
+    });
+    fireEvent.click(screen.getByRole('button', { name: 'Activate' }));
+
+    await waitFor(() =>
+      expect(onChange).toHaveBeenCalledWith({
+        ...assignment,
+        isActive: true,
+        activeTo: '2026-08-07T16:00:00Z',
+      }),
+    );
+    expect(json).toHaveBeenCalledWith(
+      '/accounts/account-id/claimable-role-assignments/assignment-id/activate',
+      {
+        method: 'POST',
+        body: JSON.stringify({ reason: 'Testing role-dependent behavior', hours: 2 }),
+      },
+    );
+  });
+});

packages/dev-portal/src/PersonSideSheet/sheets/roles/ClaimableRole.tsx

@@ -10,7 +10,7 @@ import {
 } from '@equinor/eds-core-react';
 import { useFramework } from '@equinor/fusion-framework-react';
 import { useCurrentUser } from '@equinor/fusion-framework-react/hooks';
-import type { ChangeEvent } from 'react';
+import type { ChangeEvent, ReactElement } from 'react';
 import { useState } from 'react';
 import styled from 'styled-components';
 
@@ -63,7 +63,7 @@ const Styled = {
  * @param props.onChange - Reports successful activation state changes to the parent tab.
  * @returns A role row with activation controls.
  */
-export const ClaimableRole = ({ assignment, onChange }: ClaimableRoleProps) => {
+export const ClaimableRole = ({ assignment, onChange }: ClaimableRoleProps): ReactElement => {
   const framework = useFramework();
   const user = useCurrentUser();
   const [isClaiming, setIsClaiming] = useState(false);

packages/dev-portal/src/PersonSideSheet/sheets/roles/RolesSheetContent.test.tsx

@@ -0,0 +1,64 @@
+import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { RolesSheetContent } from './RolesSheetContent';
+
+const mocks = vi.hoisted(() => ({
+  createClient: vi.fn(),
+  currentUser: { localAccountId: 'account-id' },
+  framework: {
+    modules: { serviceDiscovery: { createClient: vi.fn() } },
+  },
+}));
+
+mocks.framework.modules.serviceDiscovery.createClient = mocks.createClient;
+
+vi.mock('@equinor/fusion-framework-react', () => ({
+  useFramework: () => mocks.framework,
+}));
+
+vi.mock('@equinor/fusion-framework-react/hooks', () => ({
+  useCurrentUser: () => mocks.currentUser,
+}));
+
+vi.mock('./ClaimableRole', () => ({
+  ClaimableRole: () => <div>Claimable role</div>,
+}));
+
+describe('RolesSheetContent', () => {
+  beforeEach(() => {
+    mocks.createClient.mockReset();
+  });
+
+  afterEach(() => {
+    cleanup();
+  });
+
+  it('renders loading and empty states', async () => {
+    const json = vi.fn().mockResolvedValue([]);
+    mocks.createClient.mockResolvedValue({ json });
+
+    render(<RolesSheetContent navigate={vi.fn()} />);
+
+    expect(screen.getByLabelText('Loading roles')).toBeTruthy();
+    expect(await screen.findByText('You have no available roles')).toBeTruthy();
+    expect(json).toHaveBeenCalledTimes(2);
+  });
+
+  it('retries role retrieval after a failure', async () => {
+    const json = vi
+      .fn()
+      .mockRejectedValueOnce(new Error('Roles service unavailable'))
+      .mockResolvedValue([]);
+    mocks.createClient.mockResolvedValue({ json });
+
+    render(<RolesSheetContent navigate={vi.fn()} />);
+
+    expect(await screen.findByText('Roles service unavailable')).toBeTruthy();
+    fireEvent.click(screen.getByRole('button', { name: 'Retry' }));
+
+    expect(screen.getByLabelText('Loading roles')).toBeTruthy();
+    await waitFor(() => expect(mocks.createClient).toHaveBeenCalledTimes(2));
+    expect(await screen.findByText('You have no available roles')).toBeTruthy();
+  });
+});

packages/dev-portal/src/PersonSideSheet/sheets/roles/RolesSheetContent.tsx

@@ -10,7 +10,7 @@ import {
 import { arrow_back, verified_user } from '@equinor/eds-icons';
 import { useFramework } from '@equinor/fusion-framework-react';
 import { useCurrentUser } from '@equinor/fusion-framework-react/hooks';
-import { useEffect, useState } from 'react';
+import { type ReactElement, useEffect, useRef, useState } from 'react';
 import styled from 'styled-components';
 
 import type { SheetContentProps } from '../types';
@@ -47,14 +47,24 @@ const Styled = {
  * @param props.navigate - Navigates back to the person side sheet landing page.
  * @returns A tabbed role overview with loading, error, and empty states.
  */
-export const RolesSheetContent = ({ navigate }: SheetContentProps) => {
+export const RolesSheetContent = ({ navigate }: SheetContentProps): ReactElement => {
   const framework = useFramework();
   const user = useCurrentUser();
   const [tab, setTab] = useState(0);
   const [claimableRoles, setClaimableRoles] = useState<ClaimableRoleAssignment[]>([]);
   const [permanentRoles, setPermanentRoles] = useState<PermanentRoleAssignment[]>([]);
   const [isLoading, setIsLoading] = useState(true);
   const [error, setError] = useState<string>();
+  const [loadAttempt, setLoadAttempt] = useState(0);
+  const latestLoadAttempt = useRef(loadAttempt);
+  latestLoadAttempt.current = loadAttempt;
+
+  /** Starts a fresh role request after a retrieval failure. */
+  const handleRetry = (): void => {
+    setError(undefined);
+    setIsLoading(true);
+    setLoadAttempt((attempt) => attempt + 1);
+  };
 
   /**
    * Replaces a changed assignment after activation or deactivation succeeds.
@@ -71,6 +81,7 @@ export const RolesSheetContent = ({ navigate }: SheetContentProps) => {
 
   useEffect(() => {
     let isActive = true;
+    const currentLoadAttempt = loadAttempt;
 
     /** Loads both role collections together so the tabs represent one consistent snapshot. */
     const loadRoles = async (): Promise<void> => {
@@ -90,19 +101,19 @@ export const RolesSheetContent = ({ navigate }: SheetContentProps) => {
         ]);
 
         // Ignore a completed request after the side sheet content has unmounted.
-        if (isActive) {
+        if (isActive && latestLoadAttempt.current === currentLoadAttempt) {
           setClaimableRoles(claimable);
           setPermanentRoles(permanent);
           setError(undefined);
         }
       } catch (cause) {
         // Keep transport details out of the side sheet while preserving a useful retry direction.
-        if (isActive) {
+        if (isActive && latestLoadAttempt.current === currentLoadAttempt) {
           setError(cause instanceof Error ? cause.message : 'Failed to load roles.');
         }
       } finally {
         // Avoid updating state when navigation unmounts this sheet during a request.
-        if (isActive) {
+        if (isActive && latestLoadAttempt.current === currentLoadAttempt) {
           setIsLoading(false);
         }
       }
@@ -113,7 +124,7 @@ export const RolesSheetContent = ({ navigate }: SheetContentProps) => {
     return () => {
       isActive = false;
     };
-  }, [framework, user?.localAccountId]);
+  }, [framework, user?.localAccountId, loadAttempt]);
 
   // Prepare role rows before markup so the tab panels only render presentation state.
   const claimableItems = claimableRoles.map((assignment) => (
@@ -154,9 +165,14 @@ export const RolesSheetContent = ({ navigate }: SheetContentProps) => {
         {isLoading ? (
           <CircularProgress aria-label="Loading roles" />
         ) : error ? (
-          <Banner>
-            <Banner.Message>{error}</Banner.Message>
-          </Banner>
+          <>
+            <Banner>
+              <Banner.Message>{error}</Banner.Message>
+            </Banner>
+            <Button variant="outlined" onClick={handleRetry}>
+              Retry
+            </Button>
+          </>
         ) : (
           <Tabs activeTab={tab} onChange={(index) => setTab(Number(index))}>
             <Tabs.List>

packages/dev-portal/vitest.config.ts

@@ -4,9 +4,9 @@ import { name, version } from './package.json';
 
 export default defineProject({
   test: {
-    include: ['src/**/*.test.ts'],
+    include: ['src/**/*.test.{ts,tsx}'],
     name: `${name}@${version}`,
-    environment: 'node',
+    environment: 'happy-dom',
     globals: true,
   },
 });