Merge pull request #4779 from cloud-gov/fix-file-view-details-alert-flash

fix: Error alert flash when loading file details directly

Author: apburnes

api/serializers/site.js

@@ -11,6 +11,7 @@ const allowedAttributes = [
   'defaultBranch',
   'domain',
   'engine',
+  'liveDomain',
   'owner',
   'publishedAt',
   'repository',
@@ -58,6 +59,30 @@ const viewLinks = {
   viewLink: 'site',
 };
 
+function getLiveDomain(branchConfigs, domains) {
+  if (!branchConfigs || !domains) {
+    return '';
+  }
+
+  const config = branchConfigs.find((c) => c.context === 'site');
+
+  if (!config) {
+    return '';
+  }
+
+  const domain = domains
+    .filter((d) => d.state === 'provisioned')
+    .find((d) => d.siteBranchConfigId === config.id);
+
+  if (!domain || !domain?.names) {
+    return '';
+  }
+
+  const domainName = domain.names.split(',')[0];
+
+  return `https://${domainName}`;
+}
+
 // Eventually replace `serialize`
 function serializeNew(site, isSystemAdmin = false) {
   const object = site.get({
@@ -96,6 +121,9 @@ function serializeNew(site, isSystemAdmin = false) {
 const serializeObject = (site, isSystemAdmin) => {
   const json = serializeNew(site, isSystemAdmin);
 
+  const liveDomain = getLiveDomain(json.SiteBranchConfigs, json.Domains);
+  json.liveDomain = liveDomain;
+
   if (json.Domains) {
     json.domains = site.Domains.map((d) =>
       pick(

frontend/hooks/useFileStorage.js

@@ -60,10 +60,6 @@ export default function useFileStorage(
       enabled: !!fileStorageId,
       staleTime: 2000,
       placeholderData: previousData.current || INITIAL_DATA,
-      onError: (err) => {
-        // using an empty string so that we don't end up with "undefined" at the end
-        throw new Error('Failed to fetch public files ' + (err?.message || ''));
-      },
     });
   useEffect(() => {
     if (data !== undefined) {

frontend/pages/sites/$siteId/storage/FileDetails.jsx

@@ -8,8 +8,8 @@ const FileDetails = ({
   name,
   id,
   fullPath,
-  updatedBy,
-  updatedAt,
+  lastModifiedBy,
+  lastModifiedAt,
   size,
   mimeType,
   onDelete,
@@ -61,12 +61,12 @@ const FileDetails = ({
             </td>
           </tr>
           <tr>
-            <th scope="row">Uploaded by</th>
-            <td className="text-bold">{updatedBy}</td>
+            <th scope="row">Last modified by</th>
+            <td className="text-bold">{lastModifiedBy}</td>
           </tr>
           <tr>
-            <th scope="row">Uploaded at</th>
-            <td>{updatedAt && dateAndTimeSimple(updatedAt)}</td>
+            <th scope="row">Last modified at</th>
+            <td>{lastModifiedAt && dateAndTimeSimple(lastModifiedAt)}</td>
           </tr>
           <tr>
             <th scope="row">File size</th>
@@ -86,9 +86,10 @@ const FileDetails = ({
                 type="button"
                 title="Remove from public storage"
                 className="usa-button usa-button--outline delete-button"
-                onClick={() => {
+                onClick={(e) => {
+                  e.preventDefault();
+
                   onDelete(thisItem);
-                  onClose();
                 }}
               >
                 Delete
@@ -105,8 +106,8 @@ FileDetails.propTypes = {
   name: PropTypes.string.isRequired,
   id: PropTypes.number.isRequired,
   fullPath: PropTypes.string.isRequired,
-  updatedBy: PropTypes.string.isRequired,
-  updatedAt: PropTypes.string.isRequired,
+  lastModifiedBy: PropTypes.string.isRequired,
+  lastModifiedAt: PropTypes.string.isRequired,
   size: PropTypes.number.isRequired,
   mimeType: PropTypes.string.isRequired,
   onDelete: PropTypes.func.isRequired,

frontend/pages/sites/$siteId/storage/FileDetails.test.jsx

@@ -17,8 +17,8 @@ describe('FileDetails', () => {
     id: 20,
     name: 'test-document.pdf',
     fullPath: 'https://example.com/files/test-document.pdf',
-    updatedBy: '[email protected]',
-    updatedAt: '2025-02-19T12:00:00Z',
+    lastModifiedBy: '[email protected]',
+    lastModifiedAt: '2025-02-19T12:00:00Z',
     size: 1048576, // 1MB
     mimeType: 'application/pdf',
     onDelete: mockOnDelete,
@@ -47,7 +47,7 @@ describe('FileDetails', () => {
     );
 
     // Uploaded by
-    expect(screen.getByText(fileProps.updatedBy)).toBeInTheDocument();
+    expect(screen.getByText(fileProps.lastModifiedBy)).toBeInTheDocument();
 
     // Uploaded on (formatted date check)
     expect(screen.getByText(/Feb 19, 2025/i)).toBeInTheDocument();

frontend/pages/sites/$siteId/storage/LocationBar.jsx

@@ -2,20 +2,24 @@ import React, { useMemo } from 'react';
 import PropTypes from 'prop-types';
 import { IconGlobe } from '@shared/icons';
 
-const LocationBar = ({ path, storageRoot, onNavigate }) => {
+const LocationBar = ({ path, storageRoot, onNavigate, trailingSlash = true }) => {
   // Only trim leading slashes, keep trailing
   const cleanedPath = path.replace(/^\/+/, '');
   const segments = cleanedPath ? cleanedPath.split('/').filter(Boolean) : [];
   // Show storageRoot only if it's the current or parent folder
   const showDomain = segments.length <= 1;
-  const grandparentPath = segments.slice(0, segments.length - 1).join('/') + '/';
+  const grandparentPath = segments.slice(0, -2).join('/') + '/';
 
   const displayedBreadcrumbs = useMemo(() => {
+    const trailer = trailingSlash ? '/' : '';
     if (segments.length > 1) {
       // Ensure it only includes the last two segments
-      return segments.slice(-2).map((seg) => `${seg}/`);
+      return segments.slice(-2).map((seg, idx) => {
+        if (idx === 1) return `${seg}${trailer}`;
+        return `${seg}/`;
+      });
     }
-    return segments.length === 1 ? [`${segments[0]}/`] : [];
+    return segments.length === 1 ? [`${segments[0]}${trailer}`] : [];
   }, [segments]);
 
   return (
@@ -110,7 +114,8 @@ LocationBar.propTypes = {
   path: PropTypes.string.isRequired,
   siteId: PropTypes.string.isRequired,
   storageRoot: PropTypes.string.isRequired,
-  onNavigate: PropTypes.func,
+  onNavigate: PropTypes.func.isRequired,
+  trailingSlash: PropTypes.bool,
 };
 
 export default LocationBar;

frontend/pages/sites/$siteId/storage/LocationBar.test.jsx

@@ -1,7 +1,6 @@
 import React from 'react';
 import '@testing-library/jest-dom';
 import { render, screen, fireEvent } from '@testing-library/react';
-import { MemoryRouter } from 'react-router-dom';
 import LocationBar from './LocationBar';
 
 describe('LocationBar Component', () => {
@@ -16,14 +15,12 @@ describe('LocationBar Component', () => {
   it('renders root directory correctly, in text (not link) if current directory', () => {
     render(
       /* MemoryRouter allows us to render components that use react Link  */
-      <MemoryRouter>
-        <LocationBar
-          path=""
-          siteId={siteId}
-          storageRoot={storageRoot}
-          onNavigate={mockOnNavigate}
-        />
-      </MemoryRouter>,
+      <LocationBar
+        path=""
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+      />,
     );
 
     expect(screen.getByText(`${storageRoot}/`)).toBeInTheDocument();
@@ -35,65 +32,57 @@ describe('LocationBar Component', () => {
 
   it('renders single-level folder correctly, with parent as link', () => {
     render(
-      <MemoryRouter>
-        <LocationBar
-          path="foo"
-          siteId={siteId}
-          storageRoot={storageRoot}
-          onNavigate={mockOnNavigate}
-        />
-      </MemoryRouter>,
+      <LocationBar
+        path="foo"
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+      />,
     );
 
     expect(screen.getByRole('link', { name: `${storageRoot}/` })).toBeInTheDocument();
     expect(screen.getByText('foo/')).toBeInTheDocument();
   });
 
-  test('renders nested folders with ../ link', () => {
+  it('renders nested folders with ../ link', () => {
     render(
-      <MemoryRouter>
-        <LocationBar
-          path="foo/bar"
-          siteId={siteId}
-          storageRoot={storageRoot}
-          onNavigate={mockOnNavigate}
-        />
-      </MemoryRouter>,
+      <LocationBar
+        path="foo/bar"
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+      />,
     );
 
     expect(screen.getByRole('link', { name: '../' })).toBeInTheDocument();
     expect(screen.getByText('bar/')).toBeInTheDocument();
   });
 
-  it('clicking a breadcrumb calls onNavigate', () => {
+  it('clicking a breadcrumb calls on parent and grandparent path', () => {
     render(
-      <MemoryRouter>
-        <LocationBar
-          path="foo/bar"
-          siteId={siteId}
-          storageRoot={storageRoot}
-          onNavigate={mockOnNavigate}
-        />
-      </MemoryRouter>,
+      <LocationBar
+        path="baz/foo/bar"
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+      />,
     );
 
     fireEvent.click(screen.getByRole('link', { name: '../' }));
-    expect(mockOnNavigate).toHaveBeenCalledWith('foo/');
+    expect(mockOnNavigate).toHaveBeenCalledWith('baz/');
 
     fireEvent.click(screen.getByRole('link', { name: 'foo/' }));
-    expect(mockOnNavigate).toHaveBeenCalledWith('foo/');
+    expect(mockOnNavigate).toHaveBeenCalledWith('baz/foo/');
   });
 
   it('renders deep nested path correctly', () => {
     render(
-      <MemoryRouter>
-        <LocationBar
-          path="foo/bar/baz"
-          siteId={siteId}
-          storageRoot={storageRoot}
-          onNavigate={mockOnNavigate}
-        />
-      </MemoryRouter>,
+      <LocationBar
+        path="foo/bar/baz"
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+      />,
     );
 
     expect(screen.getByRole('link', { name: '../' })).toBeInTheDocument();
@@ -102,31 +91,78 @@ describe('LocationBar Component', () => {
 
   it('does not render ../ at root level', () => {
     render(
-      <MemoryRouter>
-        <LocationBar
-          path=""
-          siteId={siteId}
-          storageRoot={storageRoot}
-          onNavigate={mockOnNavigate}
-        />
-      </MemoryRouter>,
+      <LocationBar
+        path=""
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+      />,
     );
 
     expect(screen.queryByRole('link', { name: '../' })).not.toBeInTheDocument();
   });
 
-  test('clicking domain/~assets resets to root if it is a link', () => {
+  it('clicking domain/~assets resets to root if it is a link', () => {
     render(
-      <MemoryRouter>
-        <LocationBar
-          path="foo"
-          siteId={siteId}
-          storageRoot={storageRoot}
-          onNavigate={mockOnNavigate}
-        />
-      </MemoryRouter>,
+      <LocationBar
+        path="foo"
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+      />,
     );
     fireEvent.click(screen.getByRole('link', { name: `${storageRoot}/` }));
     expect(mockOnNavigate).toHaveBeenCalledWith('/');
   });
+
+  it('removes the trailing slash in deep nested node', () => {
+    const fileName = 'qux.txt';
+    const path = `foo/bar/bash/${fileName}`;
+    render(
+      <LocationBar
+        path={path}
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+        trailingSlash={false}
+      />,
+    );
+
+    const span = screen.getByTitle(fileName);
+    expect(span.textContent).toEqual(fileName);
+  });
+
+  it('removes the trailing slash in root', () => {
+    const fileName = 'qux.txt';
+    const path = `/${fileName}`;
+    render(
+      <LocationBar
+        path={path}
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+        trailingSlash={false}
+      />,
+    );
+
+    const span = screen.getByTitle(fileName);
+    expect(span.textContent).toEqual(fileName);
+  });
+
+  it('appends the trailing slash by default', () => {
+    const fileName = 'qux.txt';
+    const path = `foo/bar/bash/${fileName}`;
+    render(
+      <LocationBar
+        path={path}
+        siteId={siteId}
+        storageRoot={storageRoot}
+        onNavigate={mockOnNavigate}
+      />,
+    );
+
+    const elementName = `${fileName}/`;
+    const span = screen.getByTitle(elementName);
+    expect(span.textContent).toEqual(elementName);
+  });
 });