diff --git a/app/src/api.js b/app/src/api.js index b25dfc24..884e9b38 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -247,7 +247,7 @@ export async function getVideoCaptions(fileGroupId) { } } -export async function deleteVideos(fileGroupIds) { +export async function deleteVideos(fileGroupIds, force = false) { if (!fileGroupIds || fileGroupIds.length === 0) { toast({ type: 'error', @@ -258,17 +258,27 @@ export async function deleteVideos(fileGroupIds) { } console.info(`Deleting Videos: ${fileGroupIds}`); const i = fileGroupIds.join(','); - const response = await apiDelete(`${VIDEOS_API}/${i}`); - if (response.status !== 204) { - const message = await getErrorMessage(response, 'Failed to delete videos.'); - toast({ - type: 'error', - title: 'Deleting Videos failed', - description: message, - time: 5000, - }); - throw Error(message); + const url = force ? `${VIDEOS_API}/${i}?force=true` : `${VIDEOS_API}/${i}`; + const response = await apiDelete(url); + if (response.ok) { + return response; } + try { + const json = await response.clone().json(); + if (json.code === 'FILE_GROUP_IS_TAGGED' && json.file_groups) { + return {tagged: true, file_groups: json.file_groups}; + } + } catch (e) { + // Not JSON, fall through to generic error. + } + const message = await getErrorMessage(response, 'Failed to delete videos.'); + toast({ + type: 'error', + title: 'Deleting Videos failed', + description: message, + time: 5000, + }); + throw Error(message); } export async function downloadVideoMetadata(videoUrl) { @@ -741,7 +751,7 @@ export async function deleteItems(itemIds) { return response } -export async function deleteArchives(archiveIds) { +export async function deleteArchives(archiveIds, force = false) { if (!archiveIds || archiveIds.length === 0) { toast({ type: 'error', @@ -752,15 +762,24 @@ export async function deleteArchives(archiveIds) { } console.log(`Deleting Archives: ${archiveIds}`); let i = archiveIds.join(','); - const response = await apiDelete(`${ARCHIVES_API}/${i}`); - if (!response.ok) { - const message = await getErrorMessage(response, 'Failed to delete archives'); - toast({ - type: 'error', title: 'Unexpected server response', description: message, time: 5000, - }); - throw Error('Failed to delete archives'); + const url = force ? `${ARCHIVES_API}/${i}?force=true` : `${ARCHIVES_API}/${i}`; + const response = await apiDelete(url); + if (response.ok) { + return response; } - return response + try { + const json = await response.clone().json(); + if (json.code === 'FILE_GROUP_IS_TAGGED' && json.file_groups) { + return {tagged: true, file_groups: json.file_groups}; + } + } catch (e) { + // Not JSON, fall through to generic error. + } + const message = await getErrorMessage(response, 'Failed to delete archives'); + toast({ + type: 'error', title: 'Unexpected server response', description: message, time: 5000, + }); + throw Error('Failed to delete archives'); } export async function searchArchives(offset, limit, domain, searchStr, order, tagNames, headline) { @@ -2234,9 +2253,29 @@ export async function getDoc(fileGroupId) { return await apiGet(`${DOCS_API}/${fileGroupId}`); } -export async function deleteDocs(docIds) { +export async function deleteDocs(docIds, force = false) { const ids = Array.isArray(docIds) ? docIds.join(',') : docIds; - return await apiDelete(`${DOCS_API}/${ids}`); + const url = force ? `${DOCS_API}/${ids}?force=true` : `${DOCS_API}/${ids}`; + const response = await apiDelete(url); + if (response.ok) { + return response; + } + try { + const json = await response.clone().json(); + if (json.code === 'FILE_GROUP_IS_TAGGED' && json.file_groups) { + return {tagged: true, file_groups: json.file_groups}; + } + } catch (e) { + // Not JSON, fall through to generic error. + } + const message = await getErrorMessage(response, 'Failed to delete documents.'); + toast({ + type: 'error', + title: 'Deleting Documents failed', + description: message, + time: 5000, + }); + throw Error(message); } export async function searchDocs(offset, limit, searchStr, order, tagNames, author, subject, mimetype) { diff --git a/app/src/components/Archive.js b/app/src/components/Archive.js index f726ddd4..90d450c7 100644 --- a/app/src/components/Archive.js +++ b/app/src/components/Archive.js @@ -60,6 +60,7 @@ import { updateArchiveDownloaderConfig, } from "../api"; import {InputForm, useForm} from "../hooks/useForm"; +import {TaggedDeleteConfirmModal} from "./TaggedDeleteConfirmModal"; import {CollectionTagModal} from "./collections/CollectionTagModal"; import {CollectionReorganizeModal} from "./collections/CollectionReorganizeModal"; import {BatchReorganizeModal} from "./collections/BatchReorganizeModal"; @@ -96,6 +97,7 @@ function ArchivePage() { const {fileGroupId} = useParams(); const {archiveFile, history, fetchArchive} = useArchive(fileGroupId); const {theme} = useContext(ThemeContext); + const [taggedFileGroups, setTaggedFileGroups] = useState(null); let title = archiveFile ? archiveFile.title ? archiveFile.title : archiveFile.name : null; useTitle(title); @@ -132,8 +134,12 @@ function ArchivePage() { : null; - const localDeleteArchive = async () => { - await deleteArchives([archiveFile.id]); + const localDeleteArchive = async (force = false) => { + const result = await deleteArchives([archiveFile.id], force); + if (result && result.tagged) { + setTaggedFileGroups(result.file_groups); + return; + } toast({ type: 'success', title: 'Archive Deleted', @@ -180,7 +186,7 @@ function ArchivePage() { color='red' confirmContent='Are you sure you want to delete this archive? All files will be deleted' confirmButton='Delete' - onClick={localDeleteArchive} + onClick={() => localDeleteArchive(false)} obeyWROLMode={true} > Delete @@ -356,6 +362,16 @@ function ArchivePage() { /> {historyList} + + setTaggedFileGroups(null)} + onConfirm={async () => { + setTaggedFileGroups(null); + await localDeleteArchive(true); + }} + /> > } @@ -1082,6 +1098,7 @@ function ArchiveSettingsPage() { function ArchivesPage() { const [selectedArchives, setSelectedArchives] = useState([]); const [bulkTagOpen, setBulkTagOpen] = useState(false); + const [taggedFileGroups, setTaggedFileGroups] = useState(null); const searchInputRef = React.useRef(); useHotkeys('f', (e) => { @@ -1132,10 +1149,17 @@ function ArchivesPage() { } } - const onDelete = async () => { - const archiveIds = archives.filter(i => selectedArchives.indexOf(i['primary_path']) >= 0) - .map(i => i['id']); - await deleteArchives(archiveIds); + const archiveIdsForSelection = () => archives + .filter(i => selectedArchives.indexOf(i['primary_path']) >= 0) + .map(i => i['id']); + + const onDelete = async (force = false) => { + const archiveIds = archiveIdsForSelection(); + const result = await deleteArchives(archiveIds, force); + if (result && result.tagged) { + setTaggedFileGroups(result.file_groups); + return; + } await fetchArchives(); setSelectedArchives([]); } @@ -1166,7 +1190,7 @@ function ArchivesPage() { disabled={_.isEmpty(selectedArchives)} confirmButton='Delete' confirmContent='Are you sure you want to delete these archives files? This cannot be undone.' - onClick={onDelete} + onClick={() => onDelete(false)} >Delete {body} {paginator} + setTaggedFileGroups(null)} + onConfirm={async () => { + setTaggedFileGroups(null); + await onDelete(true); + }} + /> > } diff --git a/app/src/components/Docs.js b/app/src/components/Docs.js index 4e7a309a..21b90dd1 100644 --- a/app/src/components/Docs.js +++ b/app/src/components/Docs.js @@ -21,6 +21,7 @@ import { } from "./Common"; import {Button, darkTheme, Header, Icon, Segment, Statistic, Tab} from "./Theme"; import {BulkTagModal} from "./BulkTagModal"; +import {TaggedDeleteConfirmModal} from "./TaggedDeleteConfirmModal"; import {DocSearchFilterButton, FilesView} from "./Files"; import {useAuthors, useDoc, useOneQuery, useSearchDocs, useSubjects} from "../hooks/customHooks"; import {TagsSelector} from "../Tags"; @@ -47,6 +48,7 @@ function DocsPage() { const [selectedDocs, setSelectedDocs] = useState([]); const [bulkTagOpen, setBulkTagOpen] = useState(false); + const [taggedFileGroups, setTaggedFileGroups] = useState(null); const onSelect = (path, checked) => { if (checked && path) { @@ -56,9 +58,13 @@ function DocsPage() { } } - const onDelete = async () => { + const onDelete = async (force = false) => { const docIds = docs.filter(i => selectedDocs.indexOf(i['primary_path']) >= 0).map(i => i['id']); - await deleteDocs(docIds); + const result = await deleteDocs(docIds, force); + if (result && result.tagged) { + setTaggedFileGroups(result.file_groups); + return; + } await fetchDocs(); setSelectedDocs([]); } @@ -86,7 +92,7 @@ function DocsPage() { disabled={_.isEmpty(selectedDocs)} confirmButton='Delete' confirmContent='Are you sure you want to delete these documents? This cannot be undone.' - onClick={onDelete} + onClick={() => onDelete(false)} obeyWROLMode={true} >Delete Invert @@ -162,6 +168,15 @@ function DocsPage() { {body} {paginator} + setTaggedFileGroups(null)} + onConfirm={async () => { + setTaggedFileGroups(null); + await onDelete(true); + }} + /> >; } @@ -171,6 +186,7 @@ function DocPage() { const {docFile, doc, fetchDoc} = useDoc(parseInt(fileGroupId)); const {theme} = useContext(ThemeContext); const [searchParams] = useSearchParams(); + const [taggedFileGroups, setTaggedFileGroups] = useState(null); // Deep-link params: `loc` = EPUB spine index, `page` = PDF page, `q` = search term. const locParam = searchParams.get('loc'); const pageParam = searchParams.get('page'); @@ -244,9 +260,13 @@ function DocPage() { : null; const embedUrl = isEpub ? viewerUrl : pdfSrc; - const handleDelete = async () => { + const handleDelete = async (force = false) => { if (doc && doc.id) { - await deleteDocs([docFile.id]); + const result = await deleteDocs([docFile.id], force); + if (result && result.tagged) { + setTaggedFileGroups(result.file_groups); + return; + } toast({ type: 'success', title: 'Document Deleted', @@ -415,7 +435,7 @@ function DocPage() { color='red' confirmContent='Are you sure you want to delete this document? All files will be deleted.' confirmButton='Delete' - onClick={handleDelete} + onClick={() => handleDelete(false)} obeyWROLMode={true} >Delete >; @@ -442,6 +462,16 @@ function DocPage() { + + setTaggedFileGroups(null)} + onConfirm={async () => { + setTaggedFileGroups(null); + await handleDelete(true); + }} + /> >; } diff --git a/app/src/components/FileBrowser.js b/app/src/components/FileBrowser.js index f77aa791..a35158f6 100644 --- a/app/src/components/FileBrowser.js +++ b/app/src/components/FileBrowser.js @@ -33,6 +33,7 @@ import {FilePreviewContext} from "./FilePreview"; import {DragSelectionContext, DragSelectionProvider, SettingsContext, ThemeContext} from "../contexts/contexts"; import {BulkTagModal} from "./BulkTagModal"; import {InlineErrorBoundary} from "./ErrorBoundary"; +import {TaggedDeleteConfirmModal} from "./TaggedDeleteConfirmModal"; function depthIndentation(path) { // Repeated spaces for every folder a path is in. @@ -431,38 +432,12 @@ export function FileBrowser() { paths={selectedPaths} onComplete={reset} /> - setTaggedFileGroups(null)} - closeIcon - > - Tagged Files Will Be Deleted - - The following tagged files will be deleted: - - - - File - Tags - - - - {taggedFileGroups && taggedFileGroups.map((fg, idx) => ( - - {fg.primary_path || fg.name} - {(fg.tags || []).join(', ')} - - ))} - - - - - setTaggedFileGroups(null)}>Cancel - - Delete - - - + taggedFileGroups={taggedFileGroups} + onCancel={() => setTaggedFileGroups(null)} + onConfirm={onForceDelete} + /> ; const onFolderClick = async (folder) => { diff --git a/app/src/components/TaggedDeleteConfirmModal.js b/app/src/components/TaggedDeleteConfirmModal.js new file mode 100644 index 00000000..0c161e3d --- /dev/null +++ b/app/src/components/TaggedDeleteConfirmModal.js @@ -0,0 +1,34 @@ +import React from "react"; +import {Button, Icon, Modal, Table} from "./Theme"; +import {TableBody, TableCell, TableHeader, TableHeaderCell, TableRow} from "semantic-ui-react"; + +export function TaggedDeleteConfirmModal({open, taggedFileGroups, onConfirm, onCancel}) { + return + Tagged Files Will Be Deleted + + The following tagged files will be deleted: + + + + File + Tags + + + + {(taggedFileGroups || []).map((fg, idx) => ( + + {fg.primary_path || fg.name} + {(fg.tags || []).join(', ')} + + ))} + + + + + Cancel + + Delete + + + ; +} diff --git a/app/src/components/TaggedDeleteConfirmModal.test.js b/app/src/components/TaggedDeleteConfirmModal.test.js new file mode 100644 index 00000000..c4854c85 --- /dev/null +++ b/app/src/components/TaggedDeleteConfirmModal.test.js @@ -0,0 +1,93 @@ +import React from 'react'; +import {screen, fireEvent} from '@testing-library/react'; +import {renderWithProviders} from '../test-utils'; +import {TaggedDeleteConfirmModal} from './TaggedDeleteConfirmModal'; + +describe('TaggedDeleteConfirmModal', () => { + const sampleGroups = [ + {id: 1, primary_path: 'videos/foo.mp4', tags: ['favorite', 'review']}, + {id: 2, primary_path: 'archives/bar.html', tags: ['important']}, + ]; + + it('renders nothing when closed', () => { + renderWithProviders( + + ); + expect(screen.queryByText(/Tagged Files Will Be Deleted/i)).not.toBeInTheDocument(); + }); + + it('renders rows for each tagged file group', () => { + renderWithProviders( + + ); + expect(screen.getAllByText(/Tagged Files Will Be Deleted/i).length).toBeGreaterThan(0); + expect(screen.getByText('videos/foo.mp4')).toBeInTheDocument(); + expect(screen.getByText('archives/bar.html')).toBeInTheDocument(); + expect(screen.getByText('favorite, review')).toBeInTheDocument(); + expect(screen.getByText('important')).toBeInTheDocument(); + }); + + it('calls onCancel when Cancel button clicked', () => { + const onCancel = jest.fn(); + renderWithProviders( + + ); + fireEvent.click(screen.getByRole('button', {name: /Cancel/i})); + expect(onCancel).toHaveBeenCalled(); + }); + + it('calls onConfirm when Delete button clicked', () => { + const onConfirm = jest.fn(); + renderWithProviders( + + ); + const deleteButton = screen.getAllByRole('button', {name: /Delete/i}) + .find(b => b.textContent.includes('Delete')); + fireEvent.click(deleteButton); + expect(onConfirm).toHaveBeenCalled(); + }); + + it('handles empty/null taggedFileGroups gracefully', () => { + renderWithProviders( + + ); + expect(screen.getAllByText(/Tagged Files Will Be Deleted/i).length).toBeGreaterThan(0); + }); + + it('falls back to name when primary_path is missing', () => { + renderWithProviders( + + ); + expect(screen.getByText('fallback-name')).toBeInTheDocument(); + }); +}); diff --git a/app/src/components/VideoPlayer.js b/app/src/components/VideoPlayer.js index 24616867..cd98591f 100644 --- a/app/src/components/VideoPlayer.js +++ b/app/src/components/VideoPlayer.js @@ -26,6 +26,7 @@ import {Button, darkTheme, Header, Icon, Loader, Segment, Tab} from "./Theme"; import {VideoCard} from "./Videos"; import {TagsSelector} from "../Tags"; import {Comment, CommentGroup, Input, Label, Transition} from "semantic-ui-react"; +import {TaggedDeleteConfirmModal} from "./TaggedDeleteConfirmModal"; const MEDIA_PATH = '/media'; @@ -171,6 +172,7 @@ function VideoPage({videoFile, prevFile, nextFile, fetchVideo, ...props}) { const navigate = useNavigate(); const videoRef = React.useRef(); + const [taggedFileGroups, setTaggedFileGroups] = useState(null); useTitle(videoFile ? videoFile.title ? videoFile.title : videoFile.name : null); let video; @@ -276,8 +278,12 @@ function VideoPage({videoFile, prevFile, nextFile, fetchVideo, ...props}) { return Could not find video; } - const handleDeleteVideo = async (video_id) => { - await deleteVideos([video_id]) + const handleDeleteVideo = async (video_id, force = false) => { + const result = await deleteVideos([video_id], force); + if (result && result.tagged) { + setTaggedFileGroups(result.file_groups); + return; + } navigate(-1); } @@ -575,6 +581,15 @@ function VideoPage({videoFile, prevFile, nextFile, fetchVideo, ...props}) { {prevNextVideosSegment} + setTaggedFileGroups(null)} + onConfirm={async () => { + setTaggedFileGroups(null); + await handleDeleteVideo(videoFile.id, true); + }} + /> > } diff --git a/app/src/components/Videos.js b/app/src/components/Videos.js index d05ea6d8..8283dc44 100644 --- a/app/src/components/Videos.js +++ b/app/src/components/Videos.js @@ -52,6 +52,7 @@ import Grid from "semantic-ui-react/dist/commonjs/collections/Grid"; import Icon from "semantic-ui-react/dist/commonjs/elements/Icon"; import {Button, Card, Header, Loader, Modal, Placeholder, Popup, Segment, Statistic} from "./Theme"; import {BulkTagModal} from "./BulkTagModal"; +import {TaggedDeleteConfirmModal} from "./TaggedDeleteConfirmModal"; import { deleteVideos, deleteCookies, @@ -88,6 +89,7 @@ export function VideosPage() { const {searchParams} = React.useContext(QueryContext); const [selectedVideos, setSelectedVideos] = useState([]); const [bulkTagOpen, setBulkTagOpen] = useState(false); + const [taggedFileGroups, setTaggedFileGroups] = useState(null); const searchInputRef = React.useRef(); useHotkeys('f', (e) => { @@ -156,9 +158,13 @@ export function VideosPage() { setSelectedVideos(newSelectedVideos); } - const onDelete = async () => { + const onDelete = async (force = false) => { const fileGroupIds = videos.filter(i => selectedVideos.indexOf(i['primary_path']) >= 0).map(i => i['id']); - await deleteVideos(fileGroupIds); + const result = await deleteVideos(fileGroupIds, force); + if (result && result.tagged) { + setTaggedFileGroups(result.file_groups); + return; + } await fetchVideos(); setSelectedVideos([]); } @@ -189,7 +195,7 @@ export function VideosPage() { disabled={_.isEmpty(selectedVideos)} confirmButton='Delete' confirmContent='Are you sure you want to delete these video files? This cannot be undone.' - onClick={onDelete} + onClick={() => onDelete(false)} >Delete {body} {paginator} + setTaggedFileGroups(null)} + onConfirm={async () => { + setTaggedFileGroups(null); + await onDelete(true); + }} + /> > } diff --git a/app/src/components/collections/ConflictResolutionModal.js b/app/src/components/collections/ConflictResolutionModal.js index e9341009..f55c875c 100644 --- a/app/src/components/collections/ConflictResolutionModal.js +++ b/app/src/components/collections/ConflictResolutionModal.js @@ -4,6 +4,7 @@ import Message from 'semantic-ui-react/dist/commonjs/collections/Message'; import {Button, Modal} from '../Theme'; import {deleteArchives, deleteVideos} from '../../api'; import {humanFileSize, isoDatetimeToString} from '../Common'; +import {TaggedDeleteConfirmModal} from '../TaggedDeleteConfirmModal'; /** * Card component for a single conflicting file. @@ -28,16 +29,26 @@ import {humanFileSize, isoDatetimeToString} from '../Common'; function ConflictFileCard({file, onDelete, isRecommended}) { const [isDeleting, setIsDeleting] = useState(false); const [localError, setLocalError] = useState(null); + const [taggedFileGroups, setTaggedFileGroups] = useState(null); - const handleDelete = async () => { + const performDelete = async (force) => { + if (file.model_type === 'video' && file.file_group_id) { + return await deleteVideos([file.file_group_id], force); + } else if (file.model_type === 'archive' && file.archive_id) { + return await deleteArchives([file.archive_id], force); + } + return null; + }; + + const handleDelete = async (force = false) => { setIsDeleting(true); setLocalError(null); try { - if (file.model_type === 'video' && file.file_group_id) { - await deleteVideos([file.file_group_id]); - } else if (file.model_type === 'archive' && file.archive_id) { - await deleteArchives([file.archive_id]); + const result = await performDelete(force); + if (result && result.tagged) { + setTaggedFileGroups(result.file_groups); + return; } onDelete(file); } catch (err) { @@ -125,7 +136,7 @@ function ConflictFileCard({file, onDelete, isRecommended}) { icon loading={isDeleting} disabled={isDeleting} - onClick={handleDelete} + onClick={() => handleDelete(false)} title='Delete this file' > @@ -134,6 +145,15 @@ function ConflictFileCard({file, onDelete, isRecommended}) { + setTaggedFileGroups(null)} + onConfirm={async () => { + setTaggedFileGroups(null); + await handleDelete(true); + }} + /> ); } diff --git a/app/src/components/collections/ConflictResolutionModal.test.js b/app/src/components/collections/ConflictResolutionModal.test.js index c90a3c1e..1c68afea 100644 --- a/app/src/components/collections/ConflictResolutionModal.test.js +++ b/app/src/components/collections/ConflictResolutionModal.test.js @@ -107,7 +107,7 @@ describe('ConflictResolutionModal', () => { fireEvent.click(deleteButtons[0]); await waitFor(() => { - expect(api.deleteVideos).toHaveBeenCalledWith([1]); + expect(api.deleteVideos).toHaveBeenCalledWith([1], false); }); // File should be removed from UI (conflict resolved since only 1 file remains) @@ -157,7 +157,7 @@ describe('ConflictResolutionModal', () => { fireEvent.click(deleteButton); await waitFor(() => { - expect(api.deleteArchives).toHaveBeenCalledWith([201]); + expect(api.deleteArchives).toHaveBeenCalledWith([201], false); }); }); @@ -318,6 +318,39 @@ describe('ConflictResolutionModal', () => { expect(screen.getByText(/Recommended to Keep/i)).toBeInTheDocument(); }); + it('shows tagged-delete confirmation modal when API returns FILE_GROUP_IS_TAGGED', async () => { + api.deleteVideos.mockResolvedValue({ + tagged: true, + file_groups: [ + {id: 1, primary_path: 'unique-tagged-path-12345.mp4', tags: ['unique-tag-zxcv']}, + ], + }); + + renderWithProviders(); + + const deleteButtons = screen.getAllByTitle('Delete this file'); + fireEvent.click(deleteButtons[0]); + + // Wait for the secondary confirmation modal to appear (use unique path/tag to avoid + // matching the parent ConflictResolutionModal's "Tagged" label). + await waitFor(() => { + expect(screen.getByText('unique-tagged-path-12345.mp4')).toBeInTheDocument(); + }); + expect(screen.getByText('unique-tag-zxcv')).toBeInTheDocument(); + + // Confirming should re-call deleteVideos with force=true. + api.deleteVideos.mockResolvedValueOnce({}); + // The TaggedDeleteConfirmModal's Delete button has the trash icon. + const confirmButton = screen.getAllByRole('button') + .find(b => b.classList.contains('red') && b.textContent.includes('Delete') + && b.querySelector('.trash.icon')); + fireEvent.click(confirmButton); + + await waitFor(() => { + expect(api.deleteVideos).toHaveBeenLastCalledWith([1], true); + }); + }); + it('does not show rank display for archives', () => { const archiveConflicts = [ { diff --git a/modules/archive/api.py b/modules/archive/api.py index 1793b243..706d01cb 100644 --- a/modules/archive/api.py +++ b/modules/archive/api.py @@ -46,15 +46,21 @@ async def get_archive(request: Request, file_group_id: int): @archive_bp.delete('/', name='archive_delete_one') @archive_bp.delete('/', name='archive_delete_many') -@openapi.description('Delete Archives by FileGroup IDs') +@openapi.description('Delete Archives by FileGroup IDs. Pass ?force=true to delete even tagged FileGroups.') @openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse) @wrol_mode_check -async def delete_archive(_: Request, file_group_ids: str): +async def delete_archive(request: Request, file_group_ids: str): try: file_group_ids = [int(i) for i in str(file_group_ids).split(',')] except ValueError: raise ValidationError('Could not parse file group ids') - lib.delete_archives_by_file_group_ids(*file_group_ids) + force = request.args.get('force', '').lower() == 'true' + tagged = lib.delete_archives_by_file_group_ids(*file_group_ids, force=force) + if tagged: + return json_response( + {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged}, + HTTPStatus.CONFLICT, + ) return response.empty() diff --git a/modules/archive/lib.py b/modules/archive/lib.py index 50ec046d..91c0ec55 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -27,10 +27,10 @@ from wrolpi.dates import now, Seconds from wrolpi.db import get_db_session, get_db_curs from wrolpi.errors import UnknownArchive, InvalidOrderBy, InvalidDatetime from wrolpi.events import Events -from wrolpi.files.lib import handle_file_group_search_results +from wrolpi.files.lib import handle_file_group_search_results, get_tagged_file_groups_by_ids from wrolpi.files.models import FileGroup from wrolpi.switches import register_switch_handler, ActivateSwitchMethod -from wrolpi.tags import tag_append_sub_select_where +from wrolpi.tags import tag_append_sub_select_where, save_tags_config from wrolpi.vars import PYTEST, DOCKERIZED logger = logger.getChild(__name__) @@ -1125,15 +1125,28 @@ def get_archive_by_file_group_id(session: Session, file_group_id: int, skip_view return archive -def delete_archives_by_file_group_ids(*file_group_ids: List[int]): - """Delete Archives by their FileGroup IDs and all of their files.""" +def delete_archives_by_file_group_ids(*file_group_ids: int, force: bool = False) -> Optional[List[dict]]: + """Delete Archives by their FileGroup IDs and all of their files. + + If any FileGroup is tagged and force is False, returns the list of serialized tagged FileGroups + without deleting anything. If force is True, deletes even tagged files and re-saves the tags + config so `tags.yaml` reflects the removed TagFiles. + """ with get_db_session(commit=True) as session: archives: List[Archive] = list(session.query(Archive).filter(Archive.file_group_id.in_(file_group_ids))) if not archives: raise UnknownArchive(f'Unknown Archives with FileGroup IDs: {", ".join(map(str, file_group_ids))}') + tagged = get_tagged_file_groups_by_ids(session, [a.file_group_id for a in archives]) + if tagged and not force: + return tagged + for archive in archives: - archive.delete() + archive.delete(force=force) + + if tagged: + save_tags_config.activate_switch() + return None def archive_strftime(dt: datetime) -> str: diff --git a/modules/archive/models.py b/modules/archive/models.py index 96495626..ef25b356 100644 --- a/modules/archive/models.py +++ b/modules/archive/models.py @@ -140,8 +140,8 @@ class Archive(Base, ModelHelper): return self.file_group.published_datetime > other.file_group.published_datetime - def delete(self): - self.file_group.delete() + def delete(self, force: bool = False): + self.file_group.delete(force=force) session = Session.object_session(self) collection = self.collection diff --git a/modules/archive/test/test_api.py b/modules/archive/test/test_api.py index 4de7ba77..f6f0a710 100644 --- a/modules/archive/test/test_api.py +++ b/modules/archive/test/test_api.py @@ -510,3 +510,68 @@ async def test_archive_browsers_endpoint_dockerized(async_client): assert response.json['browsers'] == [] assert response.json['available'] is False assert 'message' in response.json + + +@pytest.mark.asyncio +async def test_delete_archive_untagged(async_client, test_session, archive_factory): + """An untagged Archive can be deleted via DELETE /api/archive/ without force.""" + archive = archive_factory('example.com', 'https://example.com/1') + test_session.commit() + paths = list(archive.my_paths()) + assert paths and all(p.is_file() for p in paths) + + request, response = await async_client.delete(f'/api/archive/{archive.file_group_id}') + assert response.status_code == HTTPStatus.NO_CONTENT + assert all(not p.exists() for p in paths) + assert test_session.query(Archive).count() == 0 + + +@pytest.mark.asyncio +async def test_delete_archive_tagged_requires_force(async_client, test_session, archive_factory, tag_factory, + await_switches): + """A tagged Archive returns 409 with FILE_GROUP_IS_TAGGED until force=true is set.""" + tag = await tag_factory() + archive = archive_factory('example.com', 'https://example.com/1', tag_names=[tag.name]) + test_session.commit() + paths = list(archive.my_paths()) + + request, response = await async_client.delete(f'/api/archive/{archive.file_group_id}') + assert response.status_code == HTTPStatus.CONFLICT + assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' + assert len(response.json['file_groups']) == 1 + returned = response.json['file_groups'][0] + assert returned['id'] == archive.file_group_id + assert tag.name in returned['tags'] + # Nothing was deleted. + assert all(p.is_file() for p in paths) + assert test_session.query(Archive).count() == 1 + + # With force=true, the Archive is deleted. + request, response = await async_client.delete(f'/api/archive/{archive.file_group_id}?force=true') + assert response.status_code == HTTPStatus.NO_CONTENT + assert all(not p.exists() for p in paths) + assert test_session.query(Archive).count() == 0 + await await_switches() + + +@pytest.mark.asyncio +async def test_delete_archive_atomic_when_some_tagged(async_client, test_session, archive_factory, tag_factory): + """When deleting multiple Archives and any are tagged, NONE are deleted until force=true.""" + tag = await tag_factory() + untagged = archive_factory('example.com', 'https://example.com/1') + tagged = archive_factory('example.com', 'https://example.com/2', tag_names=[tag.name]) + test_session.commit() + untagged_paths = list(untagged.my_paths()) + tagged_paths = list(tagged.my_paths()) + + ids = f'{untagged.file_group_id},{tagged.file_group_id}' + request, response = await async_client.delete(f'/api/archive/{ids}') + assert response.status_code == HTTPStatus.CONFLICT + assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' + # Only the tagged FileGroup is in the response. + returned_ids = {fg['id'] for fg in response.json['file_groups']} + assert returned_ids == {tagged.file_group_id} + # Nothing was deleted. + assert all(p.is_file() for p in untagged_paths) + assert all(p.is_file() for p in tagged_paths) + assert test_session.query(Archive).count() == 2 diff --git a/modules/docs/api.py b/modules/docs/api.py index ce53ab18..646a55aa 100644 --- a/modules/docs/api.py +++ b/modules/docs/api.py @@ -36,15 +36,21 @@ async def get_doc(request: Request, file_group_id: int): @docs_bp.delete('/', name='doc_delete_one') @docs_bp.delete('/', name='doc_delete_many') -@openapi.description('Delete Docs by FileGroup IDs') +@openapi.description('Delete Docs by FileGroup IDs. Pass ?force=true to delete even tagged FileGroups.') @openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse) @wrol_mode_check -async def delete_docs(_: Request, file_group_ids: str): +async def delete_docs(request: Request, file_group_ids: str): try: file_group_ids = [int(i) for i in str(file_group_ids).split(',')] except ValueError: raise ValidationError('Could not parse file group ids') - _delete_docs(*file_group_ids) + force = request.args.get('force', '').lower() == 'true' + tagged = _delete_docs(*file_group_ids, force=force) + if tagged: + return json_response( + {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged}, + HTTPStatus.CONFLICT, + ) return response.empty() diff --git a/modules/docs/lib.py b/modules/docs/lib.py index e6a9be4d..384306dc 100644 --- a/modules/docs/lib.py +++ b/modules/docs/lib.py @@ -11,7 +11,8 @@ from wrolpi.collections.models import Collection from wrolpi.collections.types import collection_type_registry from wrolpi.db import get_db_session from wrolpi.errors import ValidationError -from wrolpi.files.lib import split_path_stem_and_suffix, get_mimetype +from wrolpi.files.lib import split_path_stem_and_suffix, get_mimetype, get_tagged_file_groups_by_ids +from wrolpi.tags import save_tags_config logger = logging.getLogger(__name__) @@ -346,13 +347,33 @@ def _get_doc(session, file_group_id: int): return doc -def _delete_docs(*file_group_ids): +def _delete_docs(*file_group_ids, force: bool = False) -> Optional[List[dict]]: + """Delete Docs by their FileGroup IDs and all of their files. + + If any FileGroup is tagged and force is False, returns the list of serialized tagged FileGroups + without deleting anything. If force is True, deletes even tagged files and re-saves the tags + config so `tags.yaml` reflects the removed TagFiles. + """ from .models import Doc with get_db_session(commit=True) as session: - for file_group_id in file_group_ids: - doc = session.query(Doc).filter_by(file_group_id=file_group_id).one_or_none() - if doc: - session.delete(doc) + docs = list(session.query(Doc).filter(Doc.file_group_id.in_(file_group_ids))) + if not docs: + raise ValidationError( + f'Could not find Docs to delete with file_group_ids: {", ".join(map(str, file_group_ids))}' + ) + + tagged = get_tagged_file_groups_by_ids(session, [d.file_group_id for d in docs]) + if tagged and not force: + return tagged + + # Delete the FileGroup, which removes the files on disk and cascades the Doc row via + # the ON DELETE CASCADE foreign key on Doc.file_group_id. + for doc in docs: + doc.file_group.delete(force=force) + + if tagged: + save_tags_config.activate_switch() + return None def _search_docs(search_str=None, author=None, subject=None, language=None, mimetype=None, diff --git a/modules/docs/test/test_api.py b/modules/docs/test/test_api.py index 30fd4cb1..34a1aa88 100644 --- a/modules/docs/test/test_api.py +++ b/modules/docs/test/test_api.py @@ -77,14 +77,74 @@ async def test_docs_search_by_tag(async_client, test_session, doc_factory, tag_f @pytest.mark.asyncio async def test_delete_doc(async_client, test_session, test_directory, example_epub, refresh_files): - """Docs can be deleted.""" + """Docs can be deleted, including their on-disk files and FileGroup row.""" + from wrolpi.files.models import FileGroup await refresh_files() doc = test_session.query(Doc).first() assert doc file_group_id = doc.file_group_id + paths = list(doc.file_group.my_paths()) + assert paths and all(p.is_file() for p in paths) request, response = await async_client.delete(f'/api/docs/{file_group_id}') assert response.status_code == HTTPStatus.NO_CONTENT + # Doc, FileGroup, and on-disk files all removed. assert test_session.query(Doc).filter_by(file_group_id=file_group_id).one_or_none() is None + assert test_session.query(FileGroup).filter_by(id=file_group_id).one_or_none() is None + assert all(not p.exists() for p in paths) + + +@pytest.mark.asyncio +async def test_delete_doc_tagged_requires_force(async_client, test_session, doc_factory, tag_factory, + await_switches): + """DELETE /api/docs/ returns 409 for tagged docs until ?force=true is passed.""" + from wrolpi.files.models import FileGroup + tag = await tag_factory() + doc = doc_factory() + doc.file_group.add_tag(test_session, tag.name) + test_session.commit() + file_group_id = doc.file_group_id + paths = list(doc.file_group.my_paths()) + + request, response = await async_client.delete(f'/api/docs/{file_group_id}') + assert response.status_code == HTTPStatus.CONFLICT + assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' + assert len(response.json['file_groups']) == 1 + assert response.json['file_groups'][0]['id'] == file_group_id + assert tag.name in response.json['file_groups'][0]['tags'] + # Nothing was deleted. + assert all(p.is_file() for p in paths) + assert test_session.query(Doc).count() == 1 + + # With force=true, the Doc is deleted. + request, response = await async_client.delete(f'/api/docs/{file_group_id}?force=true') + assert response.status_code == HTTPStatus.NO_CONTENT + assert all(not p.exists() for p in paths) + assert test_session.query(Doc).count() == 0 + assert test_session.query(FileGroup).filter_by(id=file_group_id).one_or_none() is None + await await_switches() + + +@pytest.mark.asyncio +async def test_delete_doc_atomic_when_some_tagged(async_client, test_session, doc_factory, tag_factory): + """When deleting multiple Docs and any are tagged, NONE are deleted until ?force=true.""" + untagged = doc_factory() + tagged = doc_factory() + tag = await tag_factory() + tagged.file_group.add_tag(test_session, tag.name) + test_session.commit() + untagged_paths = list(untagged.file_group.my_paths()) + tagged_paths = list(tagged.file_group.my_paths()) + + ids = f'{untagged.file_group_id},{tagged.file_group_id}' + request, response = await async_client.delete(f'/api/docs/{ids}') + assert response.status_code == HTTPStatus.CONFLICT + assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' + returned_ids = {fg['id'] for fg in response.json['file_groups']} + assert returned_ids == {tagged.file_group_id} + # Nothing was deleted. + assert all(p.is_file() for p in untagged_paths) + assert all(p.is_file() for p in tagged_paths) + assert test_session.query(Doc).count() == 2 diff --git a/modules/videos/models.py b/modules/videos/models.py index 2371074e..83064a9b 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -90,10 +90,10 @@ class Video(ModelHelper, Base): ) return d - def delete(self, add_to_skip_list: bool = True): + def delete(self, add_to_skip_list: bool = True, force: bool = False): """Remove all files and File records related to this video. Delete this Video record. Add it to it's Channel's skip list.""" - self.file_group.delete(add_to_skip_list=add_to_skip_list) + self.file_group.delete(add_to_skip_list=add_to_skip_list, force=force) session = Session.object_session(self) session.delete(self) diff --git a/modules/videos/test/test_video.py b/modules/videos/test/test_video.py index 34fa9b22..ff41e5ac 100644 --- a/modules/videos/test/test_video.py +++ b/modules/videos/test/test_video.py @@ -1,4 +1,5 @@ from datetime import datetime, timezone +from http import HTTPStatus import pytest @@ -45,6 +46,59 @@ async def test_delete_video_with_tag(async_client, test_session, video_factory, assert test_session.query(FileGroup).count() == 1 +@pytest.mark.asyncio +async def test_delete_video_api_tagged_requires_force(async_client, test_session, video_factory, tag_factory, + await_switches): + """DELETE /api/videos/ returns 409 for tagged videos until ?force=true is passed.""" + video = video_factory(with_video_file=True, with_poster_ext='png') + tag = await tag_factory() + video.add_tag(test_session, tag.name) + test_session.commit() + file_group_id = video.file_group_id + paths = list(video.file_group.my_paths()) + + request, response = await async_client.delete(f'/api/videos/{file_group_id}') + assert response.status_code == HTTPStatus.CONFLICT + assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' + assert len(response.json['file_groups']) == 1 + assert response.json['file_groups'][0]['id'] == file_group_id + assert tag.name in response.json['file_groups'][0]['tags'] + # Nothing was deleted. + assert all(p.is_file() for p in paths) + assert test_session.query(Video).count() == 1 + + # With force=true, the Video is deleted. + request, response = await async_client.delete(f'/api/videos/{file_group_id}?force=true') + assert response.status_code == HTTPStatus.NO_CONTENT + assert all(not p.exists() for p in paths) + assert test_session.query(Video).count() == 0 + assert test_session.query(FileGroup).count() == 0 + await await_switches() + + +@pytest.mark.asyncio +async def test_delete_video_api_atomic_when_some_tagged(async_client, test_session, video_factory, tag_factory): + """When deleting multiple Videos and any are tagged, NONE are deleted until ?force=true.""" + untagged = video_factory(with_video_file=True) + tagged = video_factory(with_video_file=True) + tag = await tag_factory() + tagged.add_tag(test_session, tag.name) + test_session.commit() + untagged_paths = list(untagged.file_group.my_paths()) + tagged_paths = list(tagged.file_group.my_paths()) + + ids = f'{untagged.file_group_id},{tagged.file_group_id}' + request, response = await async_client.delete(f'/api/videos/{ids}') + assert response.status_code == HTTPStatus.CONFLICT + assert response.json['code'] == 'FILE_GROUP_IS_TAGGED' + returned_ids = {fg['id'] for fg in response.json['file_groups']} + assert returned_ids == {tagged.file_group_id} + # Nothing was deleted. + assert all(p.is_file() for p in untagged_paths) + assert all(p.is_file() for p in tagged_paths) + assert test_session.query(Video).count() == 2 + + @pytest.mark.asyncio async def test_video_channel_refresh(async_client, test_session, test_directory, channel_factory, video_factory, refresh_files): diff --git a/modules/videos/video/api.py b/modules/videos/video/api.py index 459c8a52..3aa9278d 100644 --- a/modules/videos/video/api.py +++ b/modules/videos/video/api.py @@ -75,7 +75,7 @@ async def search_videos(_: Request, body: schema.VideoSearchRequest): @video_bp.delete('/', name='Video Delete Many') @video_bp.delete('/', name='Video Delete One') -@openapi.description('Delete videos.') +@openapi.description('Delete videos. Pass ?force=true to delete even tagged FileGroups.') @openapi.response(HTTPStatus.NO_CONTENT) @openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse) @wrol_mode_check @@ -86,7 +86,13 @@ async def video_delete(request: Request, file_group_ids: str): raise ValidationError('Unable to parse file group ids') session = request.ctx.session - lib.delete_videos(session, *file_group_ids) + force = request.args.get('force', '').lower() == 'true' + tagged = lib.delete_videos(session, *file_group_ids, force=force) + if tagged: + return json_response( + {'code': 'FILE_GROUP_IS_TAGGED', 'file_groups': tagged}, + HTTPStatus.CONFLICT, + ) save_channels_config.activate_switch() Events.send_deleted(f'Deleted {len(file_group_ids)} videos') diff --git a/modules/videos/video/lib.py b/modules/videos/video/lib.py index 00d50cf7..e82cb29d 100644 --- a/modules/videos/video/lib.py +++ b/modules/videos/video/lib.py @@ -12,9 +12,9 @@ from wrolpi.common import logger, limit_concurrent, wrol_mode_check from wrolpi.dates import now from wrolpi.db import get_db_session from wrolpi.downloader import download_manager -from wrolpi.files.lib import handle_file_group_search_results +from wrolpi.files.lib import handle_file_group_search_results, get_tagged_file_groups_by_ids from wrolpi.files.models import FileGroup -from wrolpi.tags import tag_append_sub_select_where +from wrolpi.tags import tag_append_sub_select_where, save_tags_config from wrolpi.vars import VIDEO_COMMENTS_FETCH_COUNT, YTDLP_CACHE_DIR from ..cookies import cookies_unlocked, cookies_for_download from ..errors import UnknownVideo @@ -162,19 +162,33 @@ def search_videos( return results, total -def delete_videos(session: Session, *file_group_ids: int): +def delete_videos(session: Session, *file_group_ids: int, force: bool = False) -> Optional[List[dict]]: + """Delete Videos by their FileGroup IDs and all of their files. + + If any FileGroup is tagged and force is False, returns the list of serialized tagged FileGroups + without deleting anything. If force is True, deletes even tagged files and re-saves the tags + config so `tags.yaml` reflects the removed TagFiles. + """ videos = list(session.query(Video).filter(Video.file_group_id.in_(file_group_ids))) if not videos: raise UnknownVideo('Could not find videos to delete') + tagged = get_tagged_file_groups_by_ids(session, [v.file_group_id for v in videos]) + if tagged and not force: + return tagged + logger.warning(f'Deleting {len(videos)} videos') # Get all URLs, skip them once (so config isn't saved multiple times). urls = list(filter(None, [i.file_group.url for i in videos])) download_manager.add_to_skip_list(*urls) for video in videos: - video.delete() + video.delete(force=force) session.commit() + if tagged: + save_tags_config.activate_switch() + return None + def download_video_info_json(url: str) -> dict: """Download video info JSON, using encrypted cookies for authentication if available.""" diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index 54875495..c7498b26 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -55,7 +55,8 @@ logger = logger.getChild(__name__) __all__ = ['list_directories_contents', 'delete', 'split_path_stem_and_suffix', 'search_files', 'get_mimetype', 'split_file_name_words', 'get_primary_file', 'get_file_statistics', 'search_file_suggestion_count', 'glob_shared_stem', 'upsert_file', 'get_unique_files_by_stem', - 'rename', 'delete_directory', 'handle_file_group_search_results', 'get_file_location_href'] + 'rename', 'delete_directory', 'handle_file_group_search_results', 'get_file_location_href', + 'get_tagged_file_groups_by_ids'] def get_file_tag_names(session: Session, file: pathlib.Path) -> List[str]: @@ -215,6 +216,20 @@ def list_directories_contents(session: Session, directories_: List[str]) -> Dict return paths +def get_tagged_file_groups_by_ids(session: Session, file_group_ids: List[int]) -> List[dict]: + """Return serialized FileGroups (subset of `file_group_ids`) that have at least one TagFile. + + Used by Archive/Video/Doc deletion endpoints to require force-confirmation before deleting tagged files.""" + if not file_group_ids: + return [] + rows = session.query(FileGroup) \ + .join(TagFile, TagFile.file_group_id == FileGroup.id) \ + .filter(FileGroup.id.in_(file_group_ids)) \ + .distinct() \ + .all() + return [fg.__json__() for fg in rows] + + @wrol_mode_check async def delete(*paths: Union[str, pathlib.Path], force: bool = False): """ diff --git a/wrolpi/files/test/test_lib.py b/wrolpi/files/test/test_lib.py index e874842a..b21f8a94 100644 --- a/wrolpi/files/test/test_lib.py +++ b/wrolpi/files/test/test_lib.py @@ -143,6 +143,39 @@ async def test_delete_tagged(await_switches, test_session, test_directory, make_ assert not (test_directory / 'foo').exists() +@pytest.mark.asyncio +async def test_get_tagged_file_groups_by_ids(test_session, test_directory, make_files_structure, tag_factory, + refresh_files, await_switches): + """`get_tagged_file_groups_by_ids` returns only the FileGroups with at least one TagFile.""" + tag = await tag_factory() + make_files_structure({'a.txt': 'a', 'b.txt': 'b', 'c.txt': 'c'}) + await refresh_files() + + fgs = test_session.query(FileGroup).order_by(FileGroup.primary_path).all() + assert len(fgs) == 3 + a_fg, b_fg, c_fg = fgs + + # Tag two of the three FileGroups. + a_fg.add_tag(test_session, tag.id) + c_fg.add_tag(test_session, tag.id) + await await_switches() + test_session.commit() + + # Empty input → empty list. + assert lib.get_tagged_file_groups_by_ids(test_session, []) == [] + + # Untagged FileGroup is excluded. + assert lib.get_tagged_file_groups_by_ids(test_session, [b_fg.id]) == [] + + # All-untagged batch is empty. + result = lib.get_tagged_file_groups_by_ids(test_session, [a_fg.id, b_fg.id, c_fg.id]) + returned_ids = {fg['id'] for fg in result} + assert returned_ids == {a_fg.id, c_fg.id} + # Each returned FileGroup is serialized and includes its tag names. + for fg in result: + assert tag.name in fg.get('tags', []) + + @pytest.mark.asyncio async def test_delete_nested(test_session, make_files_structure): """Refuse to delete nested files in case user mis-clicks."""
The following tagged files will be deleted: