Skip to content

Commit 69e6014

Browse files
ikhoonclaude
andcommitted
Disable the per-file Delete button of a read-only repository
Motivation: The file list of the repository tree page offers a Delete button for every file, and it stayed enabled while the repository or its project was read-only. Pressing it opened the confirmation modal and the delete only failed once the server rejected it. Modifications: - Disable the Delete button of `FileList` when the repository is read-only, and explain why on hover, like the other write actions. - Keep the copy actions available, as they do not write. Result: - A read-only repository no longer offers a delete that is bound to fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent baee01f commit 69e6014

2 files changed

Lines changed: 108 additions & 9 deletions

File tree

webapp/src/dogma/features/file/FileList.tsx

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
MenuButton,
1010
MenuItem,
1111
MenuList,
12+
Tooltip,
1213
useDisclosure,
1314
} from '@chakra-ui/react';
1415
import { ColumnDef, createColumnHelper } from '@tanstack/react-table';
@@ -20,6 +21,7 @@ import { CopySupport } from 'dogma/features/file/CopySupport';
2021
import React, { useCallback, useMemo, useState } from 'react';
2122
import { MdDelete } from 'react-icons/md';
2223
import { DeleteFileModal } from 'dogma/common/components/editor/DeleteFileModal';
24+
import { useReadOnly } from 'dogma/features/repo/useReadOnly';
2325

2426
export type FileListProps<Data extends object> = {
2527
data: Data[];
@@ -42,6 +44,7 @@ const FileList = <Data extends object>({
4244
}: FileListProps<Data>) => {
4345
const columnHelper = createColumnHelper<FileDto>();
4446
const slug = `/app/projects/${projectName}/repos/${repoName}/files/${revision}${path}`;
47+
const [readOnly, readOnlyHint] = useReadOnly(projectName, repoName);
4548

4649
const { isOpen: isDeleteModalOpen, onOpen: onDeleteModalOpen, onClose: onDeleteModalClose } = useDisclosure();
4750
const [deletePath, setDeletePath] = useState('');
@@ -112,21 +115,36 @@ const FileList = <Data extends object>({
112115
</Menu>
113116
</WrapItem>
114117
</Wrap>
115-
<Button
116-
onClick={() => onClickDelete(info.row.original.path)}
117-
leftIcon={<MdDelete />}
118-
colorScheme="red"
119-
size="sm"
120-
>
121-
Delete
122-
</Button>
118+
<Tooltip label={readOnlyHint} isDisabled={!readOnly}>
119+
<Box>
120+
<Button
121+
onClick={() => onClickDelete(info.row.original.path)}
122+
isDisabled={readOnly}
123+
leftIcon={<MdDelete />}
124+
colorScheme="red"
125+
size="sm"
126+
>
127+
Delete
128+
</Button>
129+
</Box>
130+
</Tooltip>
123131
</HStack>
124132
),
125133
header: 'Actions',
126134
enableSorting: false,
127135
}),
128136
],
129-
[columnHelper, copySupport, directoryPath, projectName, repoName, slug, onClickDelete],
137+
[
138+
columnHelper,
139+
copySupport,
140+
directoryPath,
141+
projectName,
142+
repoName,
143+
slug,
144+
onClickDelete,
145+
readOnly,
146+
readOnlyHint,
147+
],
130148
);
131149
return (
132150
<Box>
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { screen, within } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import { renderWithProviders } from 'dogma/util/test-utils';
4+
import FileList from 'dogma/features/file/FileList';
5+
import { CopySupport } from 'dogma/features/file/CopySupport';
6+
import { PROJECT_READ_ONLY_HINT, REPO_READ_ONLY_HINT } from 'dogma/features/repo/useReadOnly';
7+
import { useGetProjectsQuery, useGetReposQuery } from 'dogma/features/api/apiSlice';
8+
9+
jest.mock('dogma/features/api/apiSlice', () => ({
10+
...jest.requireActual('dogma/features/api/apiSlice'),
11+
useGetProjectsQuery: jest.fn(),
12+
useGetReposQuery: jest.fn(),
13+
}));
14+
15+
const copySupport: CopySupport = {
16+
handleApiUrl: jest.fn(),
17+
handleWebUrl: jest.fn(),
18+
handleAsCliCommand: jest.fn(),
19+
handleAsCurlCommand: jest.fn(),
20+
};
21+
22+
const data = [
23+
{ revision: 6, path: '/hello.txt', type: 'TEXT', url: '/api/v1/projects/foo/repos/bar/contents/hello.txt' },
24+
];
25+
26+
const renderFileList = (projectStatus: string, repoStatus: string) => {
27+
(useGetProjectsQuery as jest.Mock).mockReturnValue({ data: [{ name: 'foo', status: projectStatus }] });
28+
(useGetReposQuery as jest.Mock).mockReturnValue({ data: [{ name: 'bar', status: repoStatus }] });
29+
return renderWithProviders(
30+
<FileList
31+
data={data}
32+
projectName="foo"
33+
repoName="bar"
34+
path=""
35+
directoryPath="/app/projects/foo/repos/bar/tree/head"
36+
revision="head"
37+
copySupport={copySupport}
38+
/>,
39+
);
40+
};
41+
42+
const deleteButton = () => screen.getByRole('button', { name: 'Delete' });
43+
44+
describe('FileList delete action', () => {
45+
beforeEach(() => jest.clearAllMocks());
46+
47+
it('offers Delete on a writable repository', () => {
48+
renderFileList('WRITABLE', 'WRITABLE');
49+
expect(deleteButton()).toBeEnabled();
50+
});
51+
52+
it('disables Delete when the repository is read-only', async () => {
53+
renderFileList('WRITABLE', 'READ_ONLY');
54+
expect(deleteButton()).toBeDisabled();
55+
56+
await userEvent.hover(deleteButton().parentElement);
57+
expect(await screen.findByRole('tooltip')).toHaveTextContent(REPO_READ_ONLY_HINT);
58+
});
59+
60+
it('disables Delete when the whole project is read-only', async () => {
61+
renderFileList('READ_ONLY', 'READ_ONLY');
62+
expect(deleteButton()).toBeDisabled();
63+
64+
await userEvent.hover(deleteButton().parentElement);
65+
expect(await screen.findByRole('tooltip')).toHaveTextContent(PROJECT_READ_ONLY_HINT);
66+
});
67+
68+
it('does not open the delete confirmation while read-only', async () => {
69+
renderFileList('WRITABLE', 'READ_ONLY');
70+
71+
await userEvent.click(deleteButton());
72+
73+
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
74+
});
75+
76+
it('keeps the copy actions available while read-only', () => {
77+
renderFileList('WRITABLE', 'READ_ONLY');
78+
const row = screen.getByText('hello.txt').closest('tr');
79+
expect(within(row).getByRole('button', { name: /Copy/ })).toBeEnabled();
80+
});
81+
});

0 commit comments

Comments
 (0)