Skip to content

chore: Update file url to use baseUrl and file key #4755

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
chore: Update file url to use baseUrl and file key
  • Loading branch information
apburnes committed Mar 19, 2025
commit 082a30513e3ddd6b700a696cce110f4eae04dbcf
2 changes: 1 addition & 1 deletion docs/DEVELOPMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ In our Docker Compose environment, `app` is the name of the container where the

For example:

- Use `docker compose --env-file ./services/local/docker.env build run --rm app yarn test` to run local testing on the app.
- Use `docker compose --env-file ./services/local/docker.env run --rm app yarn test` to run local testing on the app.
- Use `docker compose run --rm app yarn lint` to check that your local changes meet our linting standards.
- Use `docker compose run --rm app yarn format` to format your local changes based on our standards.

Expand Down
17 changes: 9 additions & 8 deletions frontend/pages/sites/$siteId/storage/FileList.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ const SORT_KEY_LAST_MODIFIED = 'updatedAt';

const FileListRow = ({
item,
storageRoot,
baseUrl,
path,
currentSortKey,
onNavigate,
onDelete,
onViewDetails,
highlight = false,
}) => {
const copyUrl = `${storageRoot}${path}${item.name}`;
const copyUrl = `${baseUrl}/${item.key}`;
// handle copying the file's URL
const [copySuccess, setCopySuccess] = useState(false);
const handleCopy = async (url) => {
Expand Down Expand Up @@ -116,7 +116,7 @@ const FileListRow = ({

const FileList = ({
path,
storageRoot,
baseUrl,
data,
onDelete,
onNavigate,
Expand All @@ -128,7 +128,7 @@ const FileList = ({
children,
}) => {
const TABLE_CAPTION = `
Listing all contents for the current folder, sorted by ${currentSortKey} in
Listing all contents for the current folder, sorted by ${currentSortKey} in
${ariaFormatSort(currentSortOrder)} order
`; // TODO: Create and update an aria live region to announce all changes

Expand Down Expand Up @@ -200,10 +200,10 @@ const FileList = ({
{children}
{data.map((item) => (
<FileListRow
key={item.name}
key={item.key}
item={item}
path={path}
storageRoot={storageRoot}
baseUrl={baseUrl}
currentSortKey={currentSortKey}
onNavigate={onNavigate}
onDelete={onDelete}
Expand Down Expand Up @@ -243,7 +243,7 @@ const SortIcon = ({ sort = '' }) => (

FileList.propTypes = {
path: PropTypes.string.isRequired,
storageRoot: PropTypes.string.isRequired,
baseUrl: PropTypes.string.isRequired,
data: PropTypes.arrayOf(
PropTypes.shape({
name: PropTypes.string.isRequired,
Expand All @@ -263,9 +263,10 @@ FileList.propTypes = {

FileListRow.propTypes = {
path: PropTypes.string.isRequired,
storageRoot: PropTypes.string.isRequired,
baseUrl: PropTypes.string.isRequired,
item: PropTypes.shape({
name: PropTypes.string.isRequired,
key: PropTypes.string.isRequired,
type: PropTypes.string.isRequired,
updatedAt: PropTypes.string.isRequired,
}).isRequired,
Expand Down
125 changes: 33 additions & 92 deletions frontend/pages/sites/$siteId/storage/FileList.test.jsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import React from 'react';
import { render, screen, within, fireEvent, waitFor } from '@testing-library/react';
import { act, render, screen, within, fireEvent, waitFor } from '@testing-library/react';
import '@testing-library/jest-dom';
import { MemoryRouter } from 'react-router-dom';
import FileList from './FileList.jsx';

const mockFiles = [
{ id: 20, name: 'Documents', type: 'directory', updatedAt: '2024-02-10T12:30:00Z' },
{
id: 20,
name: 'Documents',
key: '~assets/Documents',
type: 'directory',
updatedAt: '2024-02-10T12:30:00Z',
},
{
id: 21,
name: 'report.pdf',
key: '~assets/report.pdf',
type: 'application/pdf',
updatedAt: '2025-01-09T15:45:00Z',
updatedBy: '[email protected]',
Expand All @@ -17,6 +23,7 @@ const mockFiles = [
{
id: 22,
name: 'presentation.ppt',
key: '~assets/presentation.ppt',
type: 'application/vnd.ms-powerpoint',
updatedAt: '2024-02-08T09:15:00Z',
updatedBy: '[email protected]',
Expand All @@ -26,7 +33,7 @@ const mockFiles = [

const mockProps = {
path: '/',
storageRoot: 'https://custom.domain.gov/~assets',
baseUrl: 'https://custom.domain.gov',
data: mockFiles,
onDelete: jest.fn(),
onNavigate: jest.fn(),
Expand All @@ -42,62 +49,38 @@ describe('FileList', () => {
});

it('renders correctly with file and folder names', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
expect(screen.getByRole('link', { name: 'Documents' })).toBeInTheDocument();
expect(screen.getByRole('link', { name: 'report.pdf' })).toBeInTheDocument();
expect(screen.getByRole('link', { name: 'presentation.ppt' })).toBeInTheDocument();
});

it('does not trigger onViewDetails when clicking a folder', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
fireEvent.click(screen.getByRole('link', { name: 'Documents' }));
expect(mockProps.onViewDetails).not.toHaveBeenCalled();
});

it('calls onNavigate when a folder is clicked', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
fireEvent.click(screen.getByRole('link', { name: 'Documents' }));
expect(mockProps.onNavigate).toHaveBeenCalledWith('/Documents/');
});

it('does not trigger onNavigate when clicking a file', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
fireEvent.click(screen.getByRole('link', { name: 'report.pdf' }));
expect(mockProps.onNavigate).not.toHaveBeenCalled();
});

it('calls onViewDetails when a file name is clicked', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
fireEvent.click(screen.getByRole('link', { name: 'report.pdf' }));
expect(mockProps.onViewDetails).toHaveBeenCalledWith('report.pdf');
});

it('calls onSort and reverses sort when a sortable header is clicked', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);

fireEvent.click(screen.getByLabelText('Sort by name'));
expect(mockProps.onSort).toHaveBeenCalledWith('name');
Expand All @@ -108,11 +91,7 @@ describe('FileList', () => {
});

it('calls onSort with updatedAt for last modified header', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);

const sortButton = screen.getByLabelText('Sort by last modified');
fireEvent.click(sortButton);
Expand All @@ -121,44 +100,28 @@ describe('FileList', () => {
});

it('shows the ascending icon if currentSortOrder is asc', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
const sortButton = screen.getByLabelText('Sort by name');
const ascendingIcon = within(sortButton).getByLabelText('ascending sort icon');
expect(ascendingIcon).toBeInTheDocument();
});

it('shows the descending icon if currentSortOrder is desc', () => {
render(
<MemoryRouter>
<FileList {...mockProps} currentSortOrder="desc" />
</MemoryRouter>,
);
render(<FileList {...mockProps} currentSortOrder="desc" />);
const sortButton = screen.getByLabelText('Sort by name');
const descendingIcon = within(sortButton).getByLabelText('descending sort icon');
expect(descendingIcon).toBeInTheDocument();
});

it('shows the unsorted icon on headers that are not currently sorted', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
const sortButton = screen.getByLabelText('Sort by last modified');
const unsortedIcon = within(sortButton).getByLabelText('unsorted icon');
expect(unsortedIcon).toBeInTheDocument();
});

it('calls onSort with name for file name header', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);

const sortButton = screen.getByLabelText('Sort by name');
fireEvent.click(sortButton);
Expand All @@ -167,11 +130,7 @@ describe('FileList', () => {
});

it('calls onDelete when a folder delete button is clicked', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
fireEvent.click(screen.getAllByRole('button', { name: 'Delete' })[0]);
expect(mockProps.onDelete).toHaveBeenCalledWith({
...mockFiles[0],
Expand All @@ -180,21 +139,13 @@ describe('FileList', () => {
});

it('calls onDelete when a file delete button is clicked', () => {
render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);
fireEvent.click(screen.getAllByRole('button', { name: 'Delete' })[1]);
expect(mockProps.onDelete).toHaveBeenCalledWith({ ...mockFiles[1] });
});

it('renders no rows when no files are present', () => {
render(
<MemoryRouter>
<FileList {...mockProps} data={[]} />
</MemoryRouter>,
);
render(<FileList {...mockProps} data={[]} />);
expect(screen.getAllByRole('row').length).toBe(1);
});

Expand All @@ -207,11 +158,7 @@ describe('FileList', () => {

jest.useFakeTimers();

render(
<MemoryRouter>
<FileList {...mockProps} />
</MemoryRouter>,
);
render(<FileList {...mockProps} />);

const copyButton = screen.getAllByText('Copy link')[0];

Expand All @@ -222,7 +169,7 @@ describe('FileList', () => {
);

await screen.findByText('Copied!');
jest.advanceTimersByTime(5000);
act(() => jest.advanceTimersByTime(5000));
await waitFor(() => expect(screen.queryByText('Copied!')).not.toBeInTheDocument());
expect(screen.getAllByText('Copy link')).toHaveLength(2);

Expand All @@ -231,24 +178,18 @@ describe('FileList', () => {

it('renders children if provided', () => {
render(
<MemoryRouter>
<FileList {...mockProps}>
<tr data-testid="child-row">
<td>Child Row</td>
</tr>
</FileList>
</MemoryRouter>,
<FileList {...mockProps}>
<tr data-testid="child-row">
<td>Child Row</td>
</tr>
</FileList>,
);
expect(screen.getByTestId('child-row')).toBeInTheDocument();
});

it('applies the highlight class when highlightItem matches the row name', () => {
const highlightProps = { ...mockProps, highlightItem: 'report.pdf' };
render(
<MemoryRouter>
<FileList {...highlightProps} />
</MemoryRouter>,
);
render(<FileList {...highlightProps} />);

const cell = screen.getByText('report.pdf');
// eslint-disable-next-line testing-library/no-node-access
Expand Down
2 changes: 1 addition & 1 deletion frontend/pages/sites/$siteId/storage/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ function FileStoragePage() {
)}
<FileList
path={path}
storageRoot={storageRoot}
baseUrl={site.siteOrigin}
data={fetchedPublicFiles || []}
onDelete={handleDelete}
onNavigate={handleNavigate}
Expand Down
Loading
Loading