diff --git a/app/src/api.js b/app/src/api.js index 183474ce..b8fc7255 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -267,6 +267,7 @@ export async function deleteVideos(videoIds) { description: message, time: 5000, }); + throw Error(message); } } @@ -1033,6 +1034,22 @@ export async function getBatchReorganizationStatus(batchJobId) { } } +export async function fetchCollectionConflicts(collectionId) { + const response = await apiGet(`${COLLECTIONS_API}/${collectionId}/reorganize/conflicts`); + if (response.ok) { + return await response.json(); + } else { + const message = await getErrorMessage(response, 'Unable to fetch conflicts. See server logs.'); + toast({ + type: 'error', + title: 'Conflict Fetch Error', + description: message, + time: 5000, + }); + throw new Error(message); + } +} + export async function getArchive(archiveId) { const response = await apiGet(`${ARCHIVES_API}/${archiveId}`); if (response.ok) { diff --git a/app/src/components/collections/BatchReorganizeModal.js b/app/src/components/collections/BatchReorganizeModal.js index c6455736..a7d52863 100644 --- a/app/src/components/collections/BatchReorganizeModal.js +++ b/app/src/components/collections/BatchReorganizeModal.js @@ -1,5 +1,6 @@ import React, {useCallback, useEffect, useState} from 'react'; -import {Grid, Icon, Label, Progress} from 'semantic-ui-react'; +import {Link} from 'react-router'; +import {Grid, Icon, Label, Progress, Button as SButton} from 'semantic-ui-react'; import Message from 'semantic-ui-react/dist/commonjs/collections/Message'; import {Button, Modal, Table} from '../Theme'; import {APIButton} from '../Common'; @@ -180,7 +181,6 @@ export function BatchReorganizeModal({ completed_collections: 0, current_collection: null, overall_percent: 0, - completed: [], failed_collection: null, error: null, }); @@ -230,7 +230,7 @@ export function BatchReorganizeModal({ {collection.collection_name} - {collection.files_needing_move} / {collection.total_files} + {collection.total_files} {collection.sample_move ? ( @@ -313,21 +313,6 @@ export function BatchReorganizeModal({ )} - {status.completed && status.completed.length > 0 && ( - - - Completed: -
- {status.completed.map((c, idx) => ( - - ))} -
- - - )} - {status.status === 'complete' && ( @@ -347,7 +332,19 @@ export function BatchReorganizeModal({

Failed on {kindLabel.toLowerCase()}: {status.failed_collection.name}

- {status.error &&

Error: {status.error}

} + + View {kindLabel} + + {status.error && ( +

Error: {status.error}

+ )} diff --git a/app/src/components/collections/BatchReorganizeModal.test.js b/app/src/components/collections/BatchReorganizeModal.test.js index 53904828..df344796 100644 --- a/app/src/components/collections/BatchReorganizeModal.test.js +++ b/app/src/components/collections/BatchReorganizeModal.test.js @@ -159,7 +159,6 @@ describe('BatchReorganizeModal', () => { completed_collections: 1, current_collection: null, overall_percent: 100, - completed: [{id: 1, name: 'Test', status: 'complete'}], failed_collection: null, error: null, }); @@ -199,7 +198,6 @@ describe('BatchReorganizeModal', () => { completed_collections: 0, current_collection: {id: 1, name: 'Test', status: 'running', total: 5, completed: 2, percent: 40}, overall_percent: 40, - completed: [], failed_collection: null, error: null, }); @@ -237,7 +235,6 @@ describe('BatchReorganizeModal', () => { completed_collections: 0, current_collection: {id: 1, name: 'Test Channel', status: 'running', total: 5, completed: 2, percent: 40}, overall_percent: 20, - completed: [], failed_collection: null, error: null, }); @@ -277,7 +274,6 @@ describe('BatchReorganizeModal', () => { completed_collections: 1, current_collection: null, overall_percent: 100, - completed: [{id: 1, name: 'Test', status: 'complete'}], failed_collection: null, error: null, }); diff --git a/app/src/components/collections/CollectionReorganizeModal.js b/app/src/components/collections/CollectionReorganizeModal.js index 03d11ccb..628a24e0 100644 --- a/app/src/components/collections/CollectionReorganizeModal.js +++ b/app/src/components/collections/CollectionReorganizeModal.js @@ -1,14 +1,16 @@ import React, {useCallback, useEffect, useState} from 'react'; -import {Grid, Icon, Label, Progress} from 'semantic-ui-react'; +import {Button as SButton, Grid, Icon, Label, Progress} from 'semantic-ui-react'; import Message from 'semantic-ui-react/dist/commonjs/collections/Message'; import {Button, Modal, Table} from '../Theme'; import {APIButton} from '../Common'; import { executeCollectionReorganization, + fetchCollectionConflicts, getReorganizationStatus, previewCollectionReorganization } from '../../api'; import {useReorganizationStatus} from '../../contexts/FileWorkerStatusContext'; +import {ConflictResolutionModal} from './ConflictResolutionModal'; /** * Modal component for previewing and executing collection file reorganization. @@ -22,6 +24,7 @@ import {useReorganizationStatus} from '../../contexts/FileWorkerStatusContext'; * @param {string} collectionName - Name of the collection for display * @param {Function} onComplete - Optional callback when reorganization completes * @param {boolean} needsReorganization - Whether the collection.needs_reorganization flag is true + * @param {string} collectionKind - 'channel' or 'domain' for conflict resolution modal */ export function CollectionReorganizeModal({ open, @@ -30,6 +33,7 @@ export function CollectionReorganizeModal({ collectionName, onComplete, needsReorganization, + collectionKind, }) { const [loading, setLoading] = useState(false); const [preview, setPreview] = useState(null); @@ -37,6 +41,8 @@ export function CollectionReorganizeModal({ const [jobId, setJobId] = useState(null); const [status, setStatus] = useState(null); const [polling, setPolling] = useState(false); + const [showConflictModal, setShowConflictModal] = useState(false); + const [loadingConflicts, setLoadingConflicts] = useState(false); // Check if this collection is currently being reorganized const { @@ -51,21 +57,22 @@ export function CollectionReorganizeModal({ if (open && collectionId) { // Check if THIS collection is currently being reorganized const isThisActive = isReorganizing && - taskType === 'reorganize' && - activeCollectionId === collectionId; + taskType === 'reorganize' && + activeCollectionId === collectionId; if (isThisActive) { // Resume: show current progress from worker status + // Don't set a fake job ID - we'll watch context instead setLoading(false); setPreview(null); - setJobId('active'); // Marker for active job + setJobId(null); setStatus({ status: workerStatus.status, total: workerStatus.operation_total, completed: workerStatus.operation_processed, percent: workerStatus.operation_percent, }); - setPolling(true); + setPolling(false); // Don't poll job endpoint - watch context instead } else if (!polling && !jobId) { // Normal: fetch preview (only if not already polling/resuming) setLoading(true); @@ -137,6 +144,57 @@ export function CollectionReorganizeModal({ }; }, [jobId, polling, collectionId, onComplete]); + // Watch context for status updates when resuming an active reorganization + useEffect(() => { + const isThisActive = isReorganizing && + taskType === 'reorganize' && + activeCollectionId === collectionId; + + // Only watch when: modal is open, no real job ID, and this collection is active + if (!open || jobId || !isThisActive) return; + + // Update status from context (only when values actually change) + const newStatus = workerStatus?.status || 'running'; + const newTotal = workerStatus?.operation_total || 0; + const newCompleted = workerStatus?.operation_processed || 0; + const newPercent = workerStatus?.operation_percent || 0; + + setStatus(prev => { + // Only update if values actually changed to prevent infinite loops + if (prev && + prev.status === newStatus && + prev.total === newTotal && + prev.completed === newCompleted && + prev.percent === newPercent) { + return prev; // Return same object to prevent re-render + } + return { + status: newStatus, + total: newTotal, + completed: newCompleted, + percent: newPercent, + }; + }); + }, [open, jobId, isReorganizing, taskType, activeCollectionId, workerStatus, collectionId]); + + // Detect completion when context indicates reorganization stopped for this collection + useEffect(() => { + // Only relevant when we have status but no job ID (resumed from context) + if (!status || jobId) return; + + const isThisActive = isReorganizing && + taskType === 'reorganize' && + activeCollectionId === collectionId; + + if (!isThisActive && status.status !== 'complete') { + // Reorganization completed + setStatus(prev => ({...prev, status: 'complete', percent: 100})); + if (onComplete) { + onComplete(); + } + } + }, [isReorganizing, taskType, activeCollectionId, collectionId, status, jobId, onComplete]); + const handleClose = useCallback(() => { // Allow closing - user can reopen to resume if needed setPreview(null); @@ -144,9 +202,51 @@ export function CollectionReorganizeModal({ setJobId(null); setStatus(null); setPolling(false); + setShowConflictModal(false); onClose(); }, [onClose]); + // Reload preview (used after conflict resolution) + const reloadPreview = useCallback(() => { + setLoading(true); + setError(null); + + previewCollectionReorganization(collectionId) + .then(data => { + setPreview(data); + setLoading(false); + }) + .catch(err => { + setError(err.message); + setLoading(false); + }); + }, [collectionId]); + + const handleConflictResolved = useCallback(() => { + // Reload preview to get updated conflicts + // Note: The conflict modal calls onClose() after onResolved(), which handles closing + reloadPreview(); + }, [reloadPreview]); + + const handleResolveConflicts = useCallback(async () => { + setLoadingConflicts(true); + setError(null); + try { + const data = await fetchCollectionConflicts(collectionId); + // Update preview with fresh conflicts (including updated quality rankings) + setPreview(prev => ({ + ...prev, + conflicts: data.conflicts, + has_conflicts: data.conflicts.length > 0, + })); + setShowConflictModal(true); + } catch (err) { + setError(err.message); + } finally { + setLoadingConflicts(false); + } + }, [collectionId]); + const handleReorganize = useCallback(async () => { try { setError(null); @@ -180,7 +280,8 @@ export function CollectionReorganizeModal({ Collection Appears Organized

This collection's format matches the current configuration, but - {' '}{preview.files_needing_move} files don't match the expected layout. + {' '}{preview.files_needing_move} files don't match the expected + layout. This can happen if a previous reorganization was interrupted.

@@ -204,6 +305,33 @@ export function CollectionReorganizeModal({ + {/* Conflict warning */} + {preview.has_conflicts && ( + + + + + Conflicts Detected + +

+ {preview.conflicts.length} destination{' '} + {preview.conflicts.length === 1 ? 'path has' : 'paths have'}{' '} + multiple files that would collide. + You must resolve these conflicts before reorganizing. +

+ + Resolve Conflicts + + + + + )} + {preview.sample_moves && preview.sample_moves.length > 0 && ( @@ -292,54 +420,67 @@ export function CollectionReorganizeModal({ }; const isInProgress = polling || (status && status.status !== 'complete' && status.status !== 'error'); - const canReorganize = preview && preview.files_needing_move > 0 && !isInProgress && !jobId; + const hasConflicts = preview && preview.has_conflicts; + const canReorganize = preview && preview.files_needing_move > 0 && !isInProgress && !jobId && !hasConflicts; return ( - - - Reorganize Files: {collectionName} - - - {loading && ( - - - - Loading Preview - Analyzing files... - - - )} + <> + + + Reorganize Files: {collectionName} + + + {loading && ( + + + + Loading Preview + Analyzing files... + + + )} - {error && ( - - Error -

{error}

- - )} + {error && ( + + Error +

{error}

+ + )} - {!loading && !jobId && renderPreview()} - {jobId && renderProgress()} - - - - {canReorganize && ( - - Reorganize Files - - )} - - + {!loading && !status && renderPreview()} + {status && renderProgress()} + + + + {canReorganize && ( + + Reorganize Files + + )} + + + + {/* Conflict Resolution Modal */} + setShowConflictModal(false)} + conflicts={preview?.conflicts || []} + collectionKind={collectionKind} + onResolved={handleConflictResolved} + /> + ); } diff --git a/app/src/components/collections/CollectionReorganizeModal.test.js b/app/src/components/collections/CollectionReorganizeModal.test.js new file mode 100644 index 00000000..3e6e825b --- /dev/null +++ b/app/src/components/collections/CollectionReorganizeModal.test.js @@ -0,0 +1,465 @@ +import React from 'react'; +import {screen, waitFor, fireEvent} from '@testing-library/react'; +import {renderWithProviders} from '../../test-utils'; +import {CollectionReorganizeModal} from './CollectionReorganizeModal'; +import * as api from '../../api'; + +// Mock the API functions +jest.mock('../../api', () => ({ + previewCollectionReorganization: jest.fn(), + executeCollectionReorganization: jest.fn(), + getReorganizationStatus: jest.fn(), +})); + +// Mock react-semantic-toasts-2 +jest.mock('react-semantic-toasts-2', () => ({ + toast: jest.fn(), +})); + +// Mock the FileWorkerStatusContext - default mock returns inactive state +jest.mock('../../contexts/FileWorkerStatusContext', () => ({ + useReorganizationStatus: () => ({ + isReorganizing: false, + taskType: null, + collectionId: null, + collectionKind: null, + batchStatus: null, + workerStatus: null, + refresh: jest.fn(), + }), + useFileWorkerStatus: () => ({ + status: null, + error: null, + refresh: jest.fn(), + setFastPolling: jest.fn(), + }), +})); + +describe('CollectionReorganizeModal', () => { + const defaultProps = { + open: true, + onClose: jest.fn(), + collectionId: 1, + collectionName: 'Test Channel', + onComplete: jest.fn(), + needsReorganization: true, + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('loads preview when opened', async () => { + const mockPreview = { + files_needing_move: 45, + total_files: 50, + current_file_format: '%(title)s.%(ext)s', + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [ + {old_path: 'video.mp4', new_path: '2024-01-15/video.mp4'}, + ], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + + renderWithProviders(); + + // Should show loading initially + expect(screen.getByText(/Loading Preview/i)).toBeInTheDocument(); + + // Wait for preview to load + await waitFor(() => { + expect(screen.getByText(/45/)).toBeInTheDocument(); + expect(screen.getByText(/50/)).toBeInTheDocument(); + }); + + expect(api.previewCollectionReorganization).toHaveBeenCalledWith(1); + }); + + it('shows sample moves in preview', async () => { + const mockPreview = { + files_needing_move: 10, + total_files: 20, + current_file_format: '%(title)s.%(ext)s', + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [ + {old_path: 'old/video1.mp4', new_path: 'new/video1.mp4'}, + {old_path: 'old/video2.mp4', new_path: 'new/video2.mp4'}, + ], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText('old/video1.mp4')).toBeInTheDocument(); + expect(screen.getByText('new/video1.mp4')).toBeInTheDocument(); + }); + }); + + it('shows "No Files Need Reorganization" message when no files need moving', async () => { + const mockPreview = { + files_needing_move: 0, + total_files: 50, + current_file_format: '%(upload_date)s/%(title)s.%(ext)s', + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText(/No Files Need Reorganization/i)).toBeInTheDocument(); + }); + }); + + it('shows warning when collection appears organized but has files to move', async () => { + const mockPreview = { + files_needing_move: 5, + total_files: 50, + current_file_format: '%(upload_date)s/%(title)s.%(ext)s', + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [{old_path: 'a.mp4', new_path: 'b.mp4'}], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + + // needsReorganization is false, but there are files to move + renderWithProviders( + + ); + + await waitFor(() => { + expect(screen.getByText(/Collection Appears Organized/i)).toBeInTheDocument(); + }); + }); + + it('executes reorganization on button click', async () => { + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + api.executeCollectionReorganization.mockResolvedValue({ + job_id: 'reorganize-abc123', + message: 'Reorganization started', + }); + api.getReorganizationStatus.mockResolvedValue({ + status: 'complete', + total: 10, + completed: 10, + percent: 100, + }); + + renderWithProviders(); + + // Wait for preview to load + await waitFor(() => { + expect(screen.getByRole('button', {name: /Reorganize Files/i})).toBeInTheDocument(); + }); + + // Click the reorganize button + fireEvent.click(screen.getByRole('button', {name: /Reorganize Files/i})); + + await waitFor(() => { + expect(api.executeCollectionReorganization).toHaveBeenCalledWith(1); + }); + }); + + it('shows progress bar during execution', async () => { + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + api.executeCollectionReorganization.mockResolvedValue({ + job_id: 'reorganize-abc123', + message: 'Reorganization started', + }); + api.getReorganizationStatus.mockResolvedValue({ + status: 'running', + total: 10, + completed: 5, + percent: 50, + }); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByRole('button', {name: /Reorganize Files/i})).toBeInTheDocument(); + }); + + fireEvent.click(screen.getByRole('button', {name: /Reorganize Files/i})); + + await waitFor(() => { + expect(screen.getByText(/5 of 10 files processed/i)).toBeInTheDocument(); + }); + }); + + it('calls onComplete when reorganization finishes', async () => { + const mockOnComplete = jest.fn(); + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + api.executeCollectionReorganization.mockResolvedValue({ + job_id: 'reorganize-abc123', + message: 'Reorganization started', + }); + api.getReorganizationStatus.mockResolvedValue({ + status: 'complete', + total: 10, + completed: 10, + percent: 100, + }); + + renderWithProviders( + + ); + + await waitFor(() => { + expect(screen.getByRole('button', {name: /Reorganize Files/i})).toBeInTheDocument(); + }); + + fireEvent.click(screen.getByRole('button', {name: /Reorganize Files/i})); + + await waitFor(() => { + expect(mockOnComplete).toHaveBeenCalled(); + }); + }); + + it('displays error message on API failure', async () => { + api.previewCollectionReorganization.mockRejectedValue(new Error('Network error')); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByText(/Network error/i)).toBeInTheDocument(); + }); + }); + + it('calls onClose when cancel button is clicked', async () => { + const mockOnClose = jest.fn(); + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + + renderWithProviders( + + ); + + await waitFor(() => { + expect(screen.getByRole('button', {name: /Cancel/i})).toBeInTheDocument(); + }); + + fireEvent.click(screen.getByRole('button', {name: /Cancel/i})); + + expect(mockOnClose).toHaveBeenCalled(); + }); + + it('shows completion message when reorganization completes', async () => { + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + api.executeCollectionReorganization.mockResolvedValue({ + job_id: 'reorganize-abc123', + message: 'Reorganization started', + }); + api.getReorganizationStatus.mockResolvedValue({ + status: 'complete', + total: 10, + completed: 10, + percent: 100, + }); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByRole('button', {name: /Reorganize Files/i})).toBeInTheDocument(); + }); + + fireEvent.click(screen.getByRole('button', {name: /Reorganize Files/i})); + + await waitFor(() => { + expect(screen.getByText(/Reorganization Complete/i)).toBeInTheDocument(); + }); + }); + + it('handles execution error gracefully', async () => { + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + api.executeCollectionReorganization.mockRejectedValue(new Error('Permission denied')); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByRole('button', {name: /Reorganize Files/i})).toBeInTheDocument(); + }); + + fireEvent.click(screen.getByRole('button', {name: /Reorganize Files/i})); + + await waitFor(() => { + expect(screen.getByText(/Permission denied/i)).toBeInTheDocument(); + }); + }); + + it('does not fetch preview when modal is closed', () => { + renderWithProviders(); + + expect(api.previewCollectionReorganization).not.toHaveBeenCalled(); + }); + + it('changes button text based on state', async () => { + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + api.executeCollectionReorganization.mockResolvedValue({ + job_id: 'reorganize-abc123', + message: 'Reorganization started', + }); + api.getReorganizationStatus.mockResolvedValue({ + status: 'complete', + total: 10, + completed: 10, + percent: 100, + }); + + renderWithProviders(); + + // Wait for preview to load and button to appear + await waitFor(() => { + expect(screen.getByRole('button', {name: /Reorganize Files/i})).toBeInTheDocument(); + }); + + // Should show "Cancel" initially (when not in progress) + expect(screen.getByRole('button', {name: /Cancel/i})).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', {name: /Reorganize Files/i})); + + // Should show "Close" after completion + await waitFor(() => { + expect(screen.getByRole('button', {name: /Close/i})).toBeInTheDocument(); + }); + }); + + it('polls for status with real job ID, not with "active"', async () => { + // This test verifies the bug fix: the component should poll using + // the real job_id returned from executeCollectionReorganization, + // not a hardcoded string like 'active' + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + api.executeCollectionReorganization.mockResolvedValue({ + job_id: 'reorganize-real-job-id-123', + message: 'Reorganization started', + }); + api.getReorganizationStatus.mockResolvedValue({ + status: 'running', + total: 10, + completed: 3, + percent: 30, + }); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByRole('button', {name: /Reorganize Files/i})).toBeInTheDocument(); + }); + + fireEvent.click(screen.getByRole('button', {name: /Reorganize Files/i})); + + // Wait for status polling to start + await waitFor(() => { + expect(api.getReorganizationStatus).toHaveBeenCalled(); + }); + + // CRITICAL: Verify that getReorganizationStatus was called with the real job ID + // NOT with 'active' which was the bug + expect(api.getReorganizationStatus).toHaveBeenCalledWith(1, 'reorganize-real-job-id-123'); + expect(api.getReorganizationStatus).not.toHaveBeenCalledWith(1, 'active'); + }); + + it('handles status polling error and retries', async () => { + const mockPreview = { + files_needing_move: 10, + total_files: 20, + new_file_format: '%(upload_date)s/%(title)s.%(ext)s', + sample_moves: [], + }; + + api.previewCollectionReorganization.mockResolvedValue(mockPreview); + api.executeCollectionReorganization.mockResolvedValue({ + job_id: 'reorganize-abc123', + message: 'Reorganization started', + }); + + // First call fails, second call succeeds + api.getReorganizationStatus + .mockRejectedValueOnce(new Error('Network error')) + .mockResolvedValue({ + status: 'complete', + total: 10, + completed: 10, + percent: 100, + }); + + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {}); + + renderWithProviders(); + + await waitFor(() => { + expect(screen.getByRole('button', {name: /Reorganize Files/i})).toBeInTheDocument(); + }); + + fireEvent.click(screen.getByRole('button', {name: /Reorganize Files/i})); + + // Wait for retry and completion + await waitFor(() => { + expect(screen.getByText(/Reorganization Complete/i)).toBeInTheDocument(); + }, {timeout: 3000}); + + // Should have retried after initial error + expect(api.getReorganizationStatus.mock.calls.length).toBeGreaterThanOrEqual(2); + + consoleSpy.mockRestore(); + }); +}); diff --git a/app/src/components/collections/ConflictResolutionModal.js b/app/src/components/collections/ConflictResolutionModal.js new file mode 100644 index 00000000..3c5da129 --- /dev/null +++ b/app/src/components/collections/ConflictResolutionModal.js @@ -0,0 +1,279 @@ +import React, {useEffect, useState} from 'react'; +import {Card, Grid, Icon, Image, Label, Segment, Button as SButton} from 'semantic-ui-react'; +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'; + +/** + * Card component for a single conflicting file. + * + * Manages its own loading and error state for deletion. + * Only calls onDelete when deletion succeeds. + * + * @param {Object} file - File info object from backend containing: + * - file_group_id: number - ID of the FileGroup + * - current_path: string - Current file path + * - title: string - File title + * - model_type: 'video'|'archive' - Type of content + * - size: number - File size in bytes + * - video_id: number|null - Video ID if model_type is 'video' + * - archive_id: number|null - Archive ID if model_type is 'archive' + * - poster_path: string|null - Path to thumbnail/poster image + * - published_datetime: string|null - ISO datetime string + * - source_id: string|null - External source identifier + * - quality_rank: number|null - Quality score for recommendations + * @param {Function} onDelete - Called with (file) when deletion succeeds + * @param {boolean} isRecommended - Whether this file is recommended to keep + */ +function ConflictFileCard({file, onDelete, isRecommended}) { + const [isDeleting, setIsDeleting] = useState(false); + const [localError, setLocalError] = useState(null); + + const handleDelete = async () => { + setIsDeleting(true); + setLocalError(null); + + try { + if (file.model_type === 'video' && file.video_id) { + await deleteVideos([file.video_id]); + } else if (file.model_type === 'archive' && file.archive_id) { + await deleteArchives([file.archive_id]); + } + onDelete(file); + } catch (err) { + setLocalError(err.message || 'Failed to delete'); + } finally { + setIsDeleting(false); + } + }; + + return ( + + + {localError && ( + + {localError} + + )} + + + {/* Poster/Thumbnail column */} + + {file.poster_path ? ( + + ) : ( + + + + )} + + + {/* File info column */} + + + {file.title || 'Untitled'} + {isRecommended && ( + + )} + + + {file.current_path} + + +
+ {file.size > 0 && ( + + )} + {file.published_datetime && ( + + )} + {file.source_id && ( + + )} + {file.quality_rank !== null && file.quality_rank !== undefined && ( + + )} + +
+ + + + {/* Delete button column */} + + + + + + + + + + ); +} + +/** + * Modal component for resolving reorganization conflicts. + * + * Shows files that would have the same destination path and allows users + * to delete duplicates before proceeding with reorganization. + * + * @param {boolean} open - Whether the modal is open + * @param {Function} onClose - Callback when modal is closed + * @param {Array} conflicts - Array of ConflictDetail objects from the backend + * @param {string} collectionKind - 'channel' or 'domain' + * @param {Function} onResolved - Callback when a file is deleted (to refresh preview) + */ +export function ConflictResolutionModal({ + open, + onClose, + conflicts = [], + collectionKind, + onResolved, + }) { + const [localConflicts, setLocalConflicts] = useState(conflicts); + const [hasChanges, setHasChanges] = useState(false); + + // Sync local conflicts with props when they change (e.g., when modal opens with new data) + useEffect(() => { + setLocalConflicts(conflicts); + setHasChanges(false); + }, [conflicts]); + + /** + * Handle successful file deletion from a ConflictFileCard. + * Only called when deletion succeeds (errors are handled in the card). + * @param {Object} file - The file that was deleted + */ + const handleFileDelete = (file) => { + // Update local conflicts: remove the deleted file + setLocalConflicts(prev => { + const updated = prev.map(conflict => ({ + ...conflict, + conflicting_files: conflict.conflicting_files.filter( + f => f.file_group_id !== file.file_group_id + ), + })).filter(conflict => conflict.conflicting_files.length > 1); + // Keep conflicts that still have 2+ files + return updated; + }); + setHasChanges(true); + }; + + const handleClose = () => { + if (hasChanges && onResolved) { + onResolved(); + } + onClose(); + }; + + const totalConflicts = localConflicts.length; + const totalFiles = localConflicts.reduce((sum, c) => sum + c.conflicting_files.length, 0); + + return ( + + + Resolve Conflicts Before Reorganizing + + + + Destination Path Conflicts Detected +

+ {totalConflicts} destination {totalConflicts === 1 ? 'path has' : 'paths have'}{' '} + multiple files that would be moved there. This typically happens when the same{' '} + {collectionKind === 'channel' ? 'video' : 'page'} was downloaded multiple times. +

+

+ Delete the duplicates you don't want to keep before reorganizing. + For each conflict, keep only one file. +

+ + + {localConflicts.map((conflict, idx) => { + // Find the highest quality rank in this conflict (for video recommendations) + const hasRanks = conflict.conflicting_files.some( + f => f.quality_rank !== null && f.quality_rank !== undefined + ); + const highestRank = hasRanks + ? Math.max(...conflict.conflicting_files.map(f => f.quality_rank || 0)) + : null; + // Check if there are multiple files with the same highest rank + const highestRankCount = hasRanks + ? conflict.conflicting_files.filter(f => f.quality_rank === highestRank).length + : 0; + + return ( + + + +
+ {conflict.conflicting_files.map((file, fileIdx) => { + // Only show "Recommended" if there's a clear winner (one file with highest rank) + const isRecommended = hasRanks && + highestRankCount === 1 && + file.quality_rank === highestRank; + + return ( + + ); + })} +
+ + ); + })} + + {localConflicts.length === 0 && ( + + All Conflicts Resolved +

You can now proceed with the reorganization.

+ + )} + + + + + + ); +} diff --git a/app/src/components/collections/ConflictResolutionModal.test.js b/app/src/components/collections/ConflictResolutionModal.test.js new file mode 100644 index 00000000..4a608077 --- /dev/null +++ b/app/src/components/collections/ConflictResolutionModal.test.js @@ -0,0 +1,360 @@ +import React from 'react'; +import {screen, waitFor, fireEvent} from '@testing-library/react'; +import {renderWithProviders} from '../../test-utils'; +import {ConflictResolutionModal} from './ConflictResolutionModal'; +import * as api from '../../api'; + +// Mock the API functions +jest.mock('../../api', () => ({ + deleteVideos: jest.fn(), + deleteArchives: jest.fn(), +})); + +// Mock react-semantic-toasts-2 +jest.mock('react-semantic-toasts-2', () => ({ + toast: jest.fn(), +})); + +describe('ConflictResolutionModal', () => { + const mockConflicts = [ + { + destination_path: 'videos/2024-01-15/My Video.mp4', + conflicting_files: [ + { + file_group_id: 1, + current_path: 'videos/old/My Video.mp4', + title: 'My Video', + model_type: 'video', + size: 1024000, + video_id: 101, + archive_id: null, + poster_path: 'videos/old/My Video.jpg', + published_datetime: '2024-01-15T10:00:00Z', + source_id: 'abc123', + }, + { + file_group_id: 2, + current_path: 'videos/new/My Video.mp4', + title: 'My Video', + model_type: 'video', + size: 2048000, + video_id: 102, + archive_id: null, + poster_path: null, + published_datetime: '2024-01-15T10:00:00Z', + source_id: 'abc123', + }, + ], + }, + ]; + + const defaultProps = { + open: true, + onClose: jest.fn(), + conflicts: mockConflicts, + collectionKind: 'channel', + onResolved: jest.fn(), + }; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders the modal with conflicts', () => { + renderWithProviders(); + + expect(screen.getByText(/Resolve Conflicts Before Reorganizing/i)).toBeInTheDocument(); + expect(screen.getByText(/Destination Path Conflicts Detected/i)).toBeInTheDocument(); + expect(screen.getByText(/1 destination path has/i)).toBeInTheDocument(); + }); + + it('shows conflict destination path', () => { + renderWithProviders(); + + expect(screen.getByText(/videos\/2024-01-15\/My Video.mp4/)).toBeInTheDocument(); + }); + + it('shows file details for conflicting files', () => { + renderWithProviders(); + + // Should show both file paths + expect(screen.getByText('videos/old/My Video.mp4')).toBeInTheDocument(); + expect(screen.getByText('videos/new/My Video.mp4')).toBeInTheDocument(); + + // Should show title + const titles = screen.getAllByText('My Video'); + expect(titles.length).toBeGreaterThanOrEqual(2); + + // Should show source_id + const sourceIds = screen.getAllByText('abc123'); + expect(sourceIds.length).toBe(2); + }); + + it('shows file size in human-readable format', () => { + renderWithProviders(); + + // 1024000 bytes = ~1000 kB (uses humanFileSize from Common.js) + expect(screen.getByText('1000.0 kB')).toBeInTheDocument(); + // 2048000 bytes = ~2 MB + expect(screen.getByText('2.0 MB')).toBeInTheDocument(); + }); + + it('calls deleteVideos when delete button is clicked for video', async () => { + api.deleteVideos.mockResolvedValue({}); + + renderWithProviders(); + + // Find and click the first delete button + const deleteButtons = screen.getAllByTitle('Delete this file'); + fireEvent.click(deleteButtons[0]); + + await waitFor(() => { + expect(api.deleteVideos).toHaveBeenCalledWith([101]); + }); + + // File should be removed from UI (conflict resolved since only 1 file remains) + await waitFor(() => { + expect(screen.getByText(/All Conflicts Resolved/i)).toBeInTheDocument(); + }); + + // onResolved is NOT called until modal is closed + expect(defaultProps.onResolved).not.toHaveBeenCalled(); + + // Click Close/Done button - now onResolved should be called + fireEvent.click(screen.getByRole('button', {name: /Done/i})); + expect(defaultProps.onResolved).toHaveBeenCalled(); + }); + + it('calls deleteArchives when delete button is clicked for archive', async () => { + const archiveConflicts = [ + { + destination_path: 'archives/example.com/page.html', + conflicting_files: [ + { + file_group_id: 10, + current_path: 'archives/old/page.html', + title: 'Example Page', + model_type: 'archive', + size: 50000, + video_id: null, + archive_id: 201, + poster_path: null, + published_datetime: null, + source_id: null, + }, + ], + }, + ]; + + api.deleteArchives.mockResolvedValue({}); + + renderWithProviders( + + ); + + const deleteButton = screen.getByTitle('Delete this file'); + fireEvent.click(deleteButton); + + await waitFor(() => { + expect(api.deleteArchives).toHaveBeenCalledWith([201]); + }); + }); + + it('shows success message when no conflicts remain', () => { + renderWithProviders( + + ); + + expect(screen.getByText(/All Conflicts Resolved/i)).toBeInTheDocument(); + expect(screen.getByText(/You can now proceed with the reorganization/i)).toBeInTheDocument(); + }); + + it('shows Done button when no conflicts', () => { + renderWithProviders( + + ); + + expect(screen.getByRole('button', {name: /Done/i})).toBeInTheDocument(); + }); + + it('shows Close button when conflicts exist', () => { + renderWithProviders(); + + expect(screen.getByRole('button', {name: /Close/i})).toBeInTheDocument(); + }); + + it('calls onClose when Close button is clicked (without changes)', () => { + renderWithProviders(); + + fireEvent.click(screen.getByRole('button', {name: /Close/i})); + + // onClose is called, but onResolved is NOT called since no changes were made + expect(defaultProps.onClose).toHaveBeenCalled(); + expect(defaultProps.onResolved).not.toHaveBeenCalled(); + }); + + it('displays multiple conflicts', () => { + const multipleConflicts = [ + { + destination_path: 'videos/path1.mp4', + conflicting_files: [ + {file_group_id: 1, current_path: 'a.mp4', title: 'Video 1', model_type: 'video', video_id: 1}, + {file_group_id: 2, current_path: 'b.mp4', title: 'Video 1', model_type: 'video', video_id: 2}, + ], + }, + { + destination_path: 'videos/path2.mp4', + conflicting_files: [ + {file_group_id: 3, current_path: 'c.mp4', title: 'Video 2', model_type: 'video', video_id: 3}, + {file_group_id: 4, current_path: 'd.mp4', title: 'Video 2', model_type: 'video', video_id: 4}, + ], + }, + ]; + + renderWithProviders( + + ); + + expect(screen.getByText(/2 destination paths have/i)).toBeInTheDocument(); + expect(screen.getByText(/videos\/path1.mp4/)).toBeInTheDocument(); + expect(screen.getByText(/videos\/path2.mp4/)).toBeInTheDocument(); + }); + + it('shows video type label for videos', () => { + renderWithProviders(); + + const videoLabels = screen.getAllByText('video'); + expect(videoLabels.length).toBe(2); + }); + + it('shows archive type label for archives', () => { + const archiveConflicts = [ + { + destination_path: 'archives/page.html', + conflicting_files: [ + {file_group_id: 1, current_path: 'old.html', title: 'Page', model_type: 'archive', archive_id: 1}, + ], + }, + ]; + + renderWithProviders( + + ); + + expect(screen.getByText('archive')).toBeInTheDocument(); + }); + + it('does not render when closed', () => { + renderWithProviders( + + ); + + expect(screen.queryByText(/Resolve Conflicts Before Reorganizing/i)).not.toBeInTheDocument(); + }); + + it('displays quality rank for videos', () => { + const conflictsWithRank = [ + { + destination_path: 'videos/2024-01-15/My Video.mp4', + conflicting_files: [ + { + file_group_id: 1, + current_path: 'videos/old/My Video.mp4', + title: 'My Video', + model_type: 'video', + size: 1024000, + video_id: 101, + quality_rank: 12, + }, + { + file_group_id: 2, + current_path: 'videos/new/My Video.mp4', + title: 'My Video', + model_type: 'video', + size: 2048000, + video_id: 102, + quality_rank: 5, + }, + ], + }, + ]; + + renderWithProviders( + + ); + + // Should show quality ranks + expect(screen.getByText('12')).toBeInTheDocument(); + expect(screen.getByText('5')).toBeInTheDocument(); + }); + + it('shows recommended badge on highest ranked video', () => { + const conflictsWithRank = [ + { + destination_path: 'videos/2024-01-15/My Video.mp4', + conflicting_files: [ + { + file_group_id: 1, + current_path: 'videos/old/My Video.mp4', + title: 'My Video', + model_type: 'video', + video_id: 101, + quality_rank: 15, + }, + { + file_group_id: 2, + current_path: 'videos/new/My Video.mp4', + title: 'My Video', + model_type: 'video', + video_id: 102, + quality_rank: 8, + }, + ], + }, + ]; + + renderWithProviders( + + ); + + // Should show "Recommended to Keep" badge (only on the first/highest ranked) + expect(screen.getByText(/Recommended to Keep/i)).toBeInTheDocument(); + }); + + it('does not show rank display for archives', () => { + const archiveConflicts = [ + { + destination_path: 'archives/page.html', + conflicting_files: [ + { + file_group_id: 1, + current_path: 'old.html', + title: 'Page', + model_type: 'archive', + archive_id: 1, + quality_rank: null, + }, + { + file_group_id: 2, + current_path: 'new.html', + title: 'Page', + model_type: 'archive', + archive_id: 2, + quality_rank: null, + }, + ], + }, + ]; + + renderWithProviders( + + ); + + // Should NOT show "Recommended to Keep" badge for archives + expect(screen.queryByText(/Recommended to Keep/i)).not.toBeInTheDocument(); + }); +}); diff --git a/wrolpi/collections/api.py b/wrolpi/collections/api.py index 7c497ca8..b8f27667 100644 --- a/wrolpi/collections/api.py +++ b/wrolpi/collections/api.py @@ -275,10 +275,11 @@ async def search_collections_endpoint(request: Request, body: schema.CollectionS @openapi.response(HTTPStatus.BAD_REQUEST, JSONErrorResponse) async def preview_reorganization_endpoint(request: Request, collection_id: int): """ - Get preview with sample of files to be renamed. + Get preview with sample of files to be renamed and conflict detection. - Returns a preview of what would happen if the collection is reorganized. - This is useful for showing users what files would move before they commit. + Returns a preview of what would happen if the collection is reorganized, + including any destination conflicts that need to be resolved before + reorganization can proceed. Returns: - collection_id: The collection ID @@ -288,13 +289,14 @@ async def preview_reorganization_endpoint(request: Request, collection_id: int): - sample_moves: ~10 example moves [{old_path, new_path}, ...] - new_file_format: The format template that will be applied - current_file_format: The format currently stored on collection + - conflicts: List of destination conflicts with file details + - has_conflicts: Boolean flag indicating if conflicts exist """ from wrolpi.collections.reorganize import get_reorganization_preview session = request.ctx.session try: - # Use exact_count=True for individual collection preview to get accurate count - preview = get_reorganization_preview(collection_id, session, exact_count=True) + preview = get_reorganization_preview(collection_id, session) return json_response({ 'collection_id': preview.collection_id, 'collection_name': preview.collection_name, @@ -303,6 +305,8 @@ async def preview_reorganization_endpoint(request: Request, collection_id: int): 'sample_moves': preview.sample_moves, 'new_file_format': preview.new_file_format, 'current_file_format': preview.current_file_format, + 'conflicts': preview.conflicts or [], + 'has_conflicts': preview.has_conflicts, }) except UnknownCollection as e: return json_response({'error': str(e)}, status=HTTPStatus.NOT_FOUND) @@ -310,6 +314,34 @@ async def preview_reorganization_endpoint(request: Request, collection_id: int): return json_response({'error': str(e)}, status=HTTPStatus.BAD_REQUEST) +@collection_bp.get('//reorganize/conflicts') +@openapi.summary('Get conflict details with ffprobe processing') +@openapi.response(HTTPStatus.OK, description="Conflict details for resolution UI") +@openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse) +@openapi.response(HTTPStatus.BAD_REQUEST, JSONErrorResponse) +async def get_conflicts_endpoint(request: Request, collection_id: int): + """ + Get detailed conflict information for resolution UI. + + Processes ffprobe for any videos missing it to ensure accurate + quality rankings (including audio stream bonus). + + Returns: + - conflicts: List of conflict details with updated quality rankings + """ + from wrolpi.collections.reorganize import get_conflict_details_with_ffprobe + + session = request.ctx.session + try: + conflicts = await get_conflict_details_with_ffprobe(collection_id, session) + session.commit() + return json_response({'conflicts': conflicts}) + except UnknownCollection as e: + return json_response({'error': str(e)}, status=HTTPStatus.NOT_FOUND) + except ValueError as e: + return json_response({'error': str(e)}, status=HTTPStatus.BAD_REQUEST) + + @collection_bp.post('//reorganize') @openapi.summary('Execute collection reorganization') @openapi.response(HTTPStatus.OK, schema.ReorganizationExecuteResponse, description="Reorganization started") diff --git a/wrolpi/collections/reorganize.py b/wrolpi/collections/reorganize.py index ec8130ee..8c5b4576 100644 --- a/wrolpi/collections/reorganize.py +++ b/wrolpi/collections/reorganize.py @@ -24,14 +24,23 @@ from .models import Collection logger = logger.getChild(__name__) __all__ = [ + 'BatchReorganizationPreview', 'ReorganizationPreview', - 'get_reorganization_preview', - 'execute_reorganization', - 'get_collections_needing_reorganization', 'execute_batch_reorganization', + 'execute_reorganization', + 'get_batch_reorganization_preview', 'get_batch_reorganization_status', + 'get_collections_needing_reorganization', + 'get_conflict_details_with_ffprobe', + 'get_reorganization_preview', ] +# Quality ranking weights for conflict resolution +# High-value metadata indicators (more rare/valuable) +RANK_HIGH_VALUE = 2 +# Standard metadata indicators (more common) +RANK_STANDARD = 1 + @dataclass class ReorganizationPreview: @@ -48,6 +57,28 @@ class ReorganizationPreview: sample_moves: List[dict] # Sample of moves: [{old_path, new_path}, ...] new_file_format: str # The format that will be applied current_file_format: Optional[str] # The format currently stored on collection + conflicts: List[dict] = None # Destination conflicts with file details + has_conflicts: bool = False # Quick flag indicating if conflicts exist + + +@dataclass +class BatchReorganizationPreview: + """Lightweight preview for batch overview - no conflict detection. + + Used by get_collections_needing_reorganization() for fast batch previews. + For individual collection modals where conflict details are needed, + use ReorganizationPreview via get_reorganization_preview() instead. + + Includes one sample_move to show users what the reorganization will do, + but skips expensive full-mapping computation and conflict detection. + """ + collection_id: int + collection_name: str + total_files: int + files_needing_move: int + new_file_format: str + current_file_format: Optional[str] = None + sample_move: Optional[dict] = None # Single sample: {old_path, new_path} def _get_format_required_variables(file_format: str) -> set: @@ -186,28 +217,25 @@ def get_reorganization_preview( collection_id: int, session: Session = None, sample_size: int = 10, - exact_count: bool = False, ) -> ReorganizationPreview: - """Get preview with sample of files to be renamed. + """Get preview with sample of files to be renamed and detect conflicts. - Does NOT compute all moves by default - only samples for preview display. + Builds full move mappings to detect destination conflicts. Args: collection_id: The collection to preview reorganization for session: Database session (optional, will create one if not provided) sample_size: Number of sample moves to include in preview - exact_count: If True, compute the actual count of files needing moves - (more expensive but accurate for individual collection preview) Returns: - ReorganizationPreview with counts and sample moves + ReorganizationPreview with counts, sample moves, and conflict details Raises: UnknownCollection: If collection not found """ if session is None: with get_db_session() as session: - return get_reorganization_preview(collection_id, session, sample_size, exact_count) + return get_reorganization_preview(collection_id, session, sample_size) collection = session.query(Collection).filter_by(id=collection_id).one_or_none() if not collection: @@ -216,10 +244,6 @@ def get_reorganization_preview( if not collection.directory: raise ValueError(f"Collection '{collection.name}' has no directory. Cannot reorganize.") - collection_directory = pathlib.Path(collection.directory) - if not collection_directory.is_absolute(): - collection_directory = get_media_directory() / collection_directory - # Get the current file format from config current_config_format = collection._get_current_file_format() @@ -227,6 +251,8 @@ def get_reorganization_preview( files_needing_move = 0 total_files = 0 + move_mappings = [] + if collection.kind == 'channel': from modules.videos.models import Video, Channel @@ -241,49 +267,33 @@ def get_reorganization_preview( sample_moves=[], new_file_format=current_config_format or '', current_file_format=collection.file_format, + conflicts=[], + has_conflicts=False, ) # Use COUNT for total_files (fast) total_files = session.query(func.count(Video.id)).filter_by(channel_id=channel.id).scalar() - if exact_count: - # Compute actual count by building full move mappings - move_mappings = _build_move_mappings_for_channel(collection, session) - files_needing_move = len(move_mappings) - else: - # Assume all files need moving (computing actual count is expensive) - files_needing_move = total_files + # Always build full move mappings (needed for conflict detection) + move_mappings = _build_move_mappings_for_channel(collection, session) + files_needing_move = len(move_mappings) - # Only load limited videos for sample search - videos = session.query(Video).options( - joinedload(Video.file_group) - ).filter_by(channel_id=channel.id).limit(10).all() + # Build sample moves from the first N mappings + media_dir = get_media_directory() + for source_path, dest_path in move_mappings[:sample_size]: + try: + old_relative = str(source_path.relative_to(media_dir)) + except ValueError: + old_relative = str(source_path) + try: + new_relative = str(dest_path.relative_to(media_dir)) + except ValueError: + new_relative = str(dest_path) - for video in videos: - if not video.file_group or not video.file_group.primary_path: - continue - - current_path = pathlib.Path(video.file_group.primary_path) - new_path = _compute_new_path_for_video(video, collection_directory) - - if new_path and current_path != new_path: - # Get relative paths for display - media_dir = get_media_directory() - try: - old_relative = str(current_path.relative_to(media_dir)) - except ValueError: - old_relative = str(current_path) - try: - new_relative = str(new_path.relative_to(media_dir)) - except ValueError: - new_relative = str(new_path) - - sample_moves.append({ - 'old_path': old_relative, - 'new_path': new_relative, - }) - if len(sample_moves) >= sample_size: - break + sample_moves.append({ + 'old_path': old_relative, + 'new_path': new_relative, + }) elif collection.kind == 'domain': from modules.archive.models import Archive @@ -291,44 +301,29 @@ def get_reorganization_preview( # Use COUNT for total_files (fast) total_files = session.query(func.count(Archive.id)).filter_by(collection_id=collection_id).scalar() - if exact_count: - # Compute actual count by building full move mappings - move_mappings = _build_move_mappings_for_domain(collection, session) - files_needing_move = len(move_mappings) - else: - # Assume all files need moving (computing actual count is expensive) - files_needing_move = total_files + # Always build full move mappings (needed for conflict detection) + move_mappings = _build_move_mappings_for_domain(collection, session) + files_needing_move = len(move_mappings) - # Only load limited archives for sample search - archives = session.query(Archive).options( - joinedload(Archive.file_group) - ).filter_by(collection_id=collection_id).limit(10).all() + # Build sample moves from the first N mappings + media_dir = get_media_directory() + for source_path, dest_path in move_mappings[:sample_size]: + try: + old_relative = str(source_path.relative_to(media_dir)) + except ValueError: + old_relative = str(source_path) + try: + new_relative = str(dest_path.relative_to(media_dir)) + except ValueError: + new_relative = str(dest_path) - for archive in archives: - if not archive.file_group or not archive.file_group.primary_path: - continue + sample_moves.append({ + 'old_path': old_relative, + 'new_path': new_relative, + }) - current_path = pathlib.Path(archive.file_group.primary_path) - new_path = _compute_new_path_for_archive(archive, collection_directory) - - if new_path and current_path != new_path: - # Get relative paths for display - media_dir = get_media_directory() - try: - old_relative = str(current_path.relative_to(media_dir)) - except ValueError: - old_relative = str(current_path) - try: - new_relative = str(new_path.relative_to(media_dir)) - except ValueError: - new_relative = str(new_path) - - sample_moves.append({ - 'old_path': old_relative, - 'new_path': new_relative, - }) - if len(sample_moves) >= sample_size: - break + # Detect conflicts + conflicts = _detect_conflicts_with_details(move_mappings, collection, session) return ReorganizationPreview( collection_id=collection_id, @@ -338,6 +333,116 @@ def get_reorganization_preview( sample_moves=sample_moves, new_file_format=current_config_format or '', current_file_format=collection.file_format, + conflicts=conflicts, + has_conflicts=len(conflicts) > 0, + ) + + +def get_batch_reorganization_preview( + collection_id: int, + session: Session, +) -> BatchReorganizationPreview: + """Get a lightweight preview for batch context - skips expensive conflict detection. + + Use this for the batch overview page where we need fast counts for many collections. + For individual collection modals where conflict details are needed, + use get_reorganization_preview() instead. + + Computes ONE sample move to show users what will happen, but avoids computing + moves for ALL files (which is the expensive part). + + Args: + collection_id: The collection to preview + session: Database session + + Returns: + BatchReorganizationPreview with counts and one sample move (no conflicts) + + Raises: + UnknownCollection: If collection not found + """ + from sqlalchemy.orm import joinedload + + collection = session.query(Collection).filter_by(id=collection_id).one_or_none() + if not collection: + raise UnknownCollection(f"Collection with ID {collection_id} not found") + + # Get the current file format from config + current_config_format = collection._get_current_file_format() + + collection_directory = pathlib.Path(collection.directory) + if not collection_directory.is_absolute(): + collection_directory = get_media_directory() / collection_directory + + total_files = 0 + sample_move = None + media_dir = get_media_directory() + + if collection.kind == 'channel': + from modules.videos.models import Video, Channel + + # Get the channel for this collection + channel = session.query(Channel).filter_by(collection_id=collection_id).one_or_none() + if channel: + # Fast COUNT query - no joins, no loading of related objects + total_files = session.query(func.count(Video.id)).filter_by(channel_id=channel.id).scalar() or 0 + + # Get ONE video for sample move (cheap - just one query with limit 1) + sample_video = session.query(Video).options( + joinedload(Video.file_group) + ).filter_by(channel_id=channel.id).first() + + if sample_video and sample_video.file_group and sample_video.file_group.primary_path: + current_path = pathlib.Path(sample_video.file_group.primary_path) + new_path = _compute_new_path_for_video(sample_video, collection_directory) + if new_path and current_path != new_path: + try: + old_relative = str(current_path.relative_to(media_dir)) + except ValueError: + old_relative = str(current_path) + try: + new_relative = str(new_path.relative_to(media_dir)) + except ValueError: + new_relative = str(new_path) + sample_move = {'old_path': old_relative, 'new_path': new_relative} + + elif collection.kind == 'domain': + from modules.archive.models import Archive + + # Fast COUNT query + total_files = session.query(func.count(Archive.id)).filter_by(collection_id=collection_id).scalar() or 0 + + # Get ONE archive for sample move (cheap - just one query with limit 1) + sample_archive = session.query(Archive).options( + joinedload(Archive.file_group) + ).filter_by(collection_id=collection_id).first() + + if sample_archive and sample_archive.file_group and sample_archive.file_group.primary_path: + current_path = pathlib.Path(sample_archive.file_group.primary_path) + new_path = _compute_new_path_for_archive(sample_archive, collection_directory) + if new_path and current_path != new_path: + try: + old_relative = str(current_path.relative_to(media_dir)) + except ValueError: + old_relative = str(current_path) + try: + new_relative = str(new_path.relative_to(media_dir)) + except ValueError: + new_relative = str(new_path) + sample_move = {'old_path': old_relative, 'new_path': new_relative} + + # For batch preview, assume all files need moving if collection needs reorganization + # This avoids expensive _build_move_mappings calls + files_needing_move = total_files if collection.needs_reorganization else 0 + + return BatchReorganizationPreview( + collection_id=collection_id, + collection_name=collection.name, + total_files=total_files, + files_needing_move=files_needing_move, + new_file_format=current_config_format or '', + current_file_format=collection.file_format, + sample_move=sample_move, ) @@ -403,6 +508,163 @@ def _build_move_mappings_for_domain( return move_mappings +def _get_largest_sizes_by_source_id(videos: list) -> dict: + """Get the largest file size for each source_id among a set of videos. + + Videos that are larger are probably better quality, so larger videos get a rank bonus. + """ + sizes = {} + for video in videos: + source_id = video.source_id + if not source_id: + continue + old_size = sizes.get(source_id, 0) + file_size = video.file_group.size if video.file_group else 0 + sizes[source_id] = max(old_size, file_size) + return sizes + + +def _rank_video_quality(video, sizes_by_source_id: dict) -> int: + """Returns a higher integer the more data and metadata the Video record is. + + A video with info_json is more valuable than one without, etc. + Adapted from scripts/delete_similarly_named_videos.py. + + Uses RANK_HIGH_VALUE for rare/valuable metadata and RANK_STANDARD for common metadata. + """ + from modules.videos.lib import parse_video_file_name + + rank = 0 + file_group = video.file_group + + # High-value metadata (rare/valuable indicators) + if file_group and file_group.url: + rank += RANK_HIGH_VALUE + if video.source_id: + rank += RANK_HIGH_VALUE + if video.info_json_path and video.info_json_path.is_file(): + try: + if video.info_json_path.stat().st_size > 0: + rank += RANK_HIGH_VALUE + except (OSError, IOError): + pass + # Check if video file matches channel info_json + if video.source_id and video.channel: + try: + if entry := video.get_channel_entry(): + if entry.get('title') and entry['title'] in str(video.video_path): + rank += RANK_HIGH_VALUE + except Exception: + pass + if video.get_comments(): + rank += RANK_HIGH_VALUE + + # Standard metadata (more common indicators) + if video.channel_id: + rank += RANK_STANDARD + if video.caption_paths: + rank += RANK_STANDARD + if video.poster_path and video.poster_path.is_file(): + try: + if video.poster_path.stat().st_size > 0: + rank += RANK_STANDARD + except (OSError, IOError): + pass + if video.ffprobe_json: + rank += RANK_STANDARD + + # Largest file for source_id gets a bonus + if video.source_id: + largest_size = sizes_by_source_id.get(video.source_id) + if largest_size and file_group and file_group.size == largest_size: + rank += RANK_STANDARD + + # Title from JSON matches filename + if video.video_path: + try: + _, _, _, video_title = parse_video_file_name(video.video_path) + video_title = video_title.strip() if video_title else '' + if file_group and file_group.title and file_group.title == video_title: + rank += RANK_STANDARD + except Exception: + pass + + # Channel name in video path + if video.channel_id and video.channel: + try: + channel_name = None + if video.channel.info_json: + channel_name = video.channel.info_json.get('channel') + if channel_name and channel_name in str(video.video_path): + rank += RANK_HIGH_VALUE + except Exception: + pass + + # Precise published datetime (not midnight) indicates better metadata + if file_group and file_group.published_datetime: + dt = file_group.published_datetime + # Check if time is not midnight (has actual time precision) + if dt.hour != 0 or dt.minute != 0 or dt.second != 0: + rank += RANK_STANDARD + + # Bonus for videos with audio + if video.ffprobe_json: + if video.get_streams_by_codec_type('audio'): + rank += RANK_HIGH_VALUE + + return rank + + +def _rank_archive_quality(archive) -> int: + """Returns a higher integer the more quality the Archive record is. + + Archives with more complete metadata (readability files, timestamps, + extracted content) receive higher ranks. + + Uses RANK_HIGH_VALUE for rare/valuable metadata and RANK_STANDARD for common metadata. + """ + rank = 0 + file_group = archive.file_group + + # High-value metadata (rare/valuable indicators) + if archive.readability_json_path and archive.readability_json_path.is_file(): + rank += RANK_HIGH_VALUE + if file_group and file_group.url: + rank += RANK_HIGH_VALUE + + # Standard metadata (more common indicators) + if archive.readability_path and archive.readability_path.is_file(): + rank += RANK_STANDARD + if archive.readability_txt_path and archive.readability_txt_path.is_file(): + rank += RANK_STANDARD + if archive.screenshot_path and archive.screenshot_path.is_file(): + rank += RANK_STANDARD + if file_group: + if file_group.title: + rank += RANK_STANDARD + if file_group.author: + rank += RANK_STANDARD + if file_group.published_datetime: + rank += RANK_STANDARD + if file_group.download_datetime: + rank += RANK_STANDARD + if file_group.published_modified_datetime: + rank += RANK_STANDARD + if file_group.b_text: # description + rank += RANK_STANDARD + if file_group.d_text: # extracted article text + rank += RANK_STANDARD + if file_group.length and file_group.length > 0: + rank += RANK_STANDARD + # Precise published datetime (not midnight) + if file_group.published_datetime: + dt = file_group.published_datetime + if dt.hour != 0 or dt.minute != 0 or dt.second != 0: + rank += RANK_STANDARD + + return rank + + def _check_for_destination_conflicts( move_mappings: List[Tuple[pathlib.Path, pathlib.Path]], collection_name: str, @@ -435,6 +697,176 @@ def _check_for_destination_conflicts( ) +def _detect_conflicts_with_details( + move_mappings: List[Tuple[pathlib.Path, pathlib.Path]], + collection: Collection, + session: Session, +) -> List[dict]: + """Detect destination conflicts and return detailed info for UI display. + + Returns a list of conflict details, where each conflict contains: + - destination_path: The path where multiple files would end up + - conflicting_files: List of file info dicts with metadata for comparison + + For video conflicts, includes quality_rank based on metadata completeness + and sorts files by rank (highest first). + + Args: + move_mappings: List of (source_path, dest_path) tuples + collection: The collection being reorganized + session: Database session + + Returns: + List of conflict detail dicts (empty if no conflicts) + """ + from wrolpi.files.models import FileGroup + + # Build destination -> source paths mapping + dest_to_sources = {} + for source, dest in move_mappings: + if dest in dest_to_sources: + dest_to_sources[dest].append(source) + else: + dest_to_sources[dest] = [source] + + # Find conflicts (destinations with multiple sources) + conflicts = {dest: sources for dest, sources in dest_to_sources.items() if len(sources) > 1} + + if not conflicts: + return [] + + media_dir = get_media_directory() + conflict_details = [] + + for dest_path, source_paths in conflicts.items(): + # Get relative destination path for display + try: + dest_relative = str(dest_path.relative_to(media_dir)) + except ValueError: + dest_relative = str(dest_path) + + conflicting_files = [] + # Collect videos for ranking (only for channel collections) + videos_for_ranking = [] + # Collect archives for ranking (only for domain collections) + archives_for_ranking = [] + + for source_path in source_paths: + # Look up the FileGroup by its primary_path + try: + source_relative = str(source_path.relative_to(media_dir)) + except ValueError: + source_relative = str(source_path) + + file_group = session.query(FileGroup).filter( + FileGroup.primary_path == str(source_path) + ).one_or_none() + + if not file_group: + # FileGroup not found, add minimal info + conflicting_files.append({ + 'file_group_id': None, + 'current_path': source_relative, + 'title': source_path.stem, + 'model_type': collection.kind, + 'size': 0, + 'video_id': None, + 'archive_id': None, + 'poster_path': None, + 'published_datetime': None, + 'source_id': None, + 'quality_rank': None, + }) + continue + + # Get model-specific metadata + file_info = { + 'file_group_id': file_group.id, + 'current_path': source_relative, + 'title': file_group.title or source_path.stem, + 'model_type': file_group.model or collection.kind, + 'size': file_group.size or 0, + 'video_id': None, + 'archive_id': None, + 'poster_path': None, + 'published_datetime': None, + 'source_id': None, + 'quality_rank': None, + } + + # Add model-specific details + if collection.kind == 'channel': + from modules.videos.models import Video + video = session.query(Video).filter_by(file_group_id=file_group.id).one_or_none() + if video: + file_info['video_id'] = video.id + file_info['source_id'] = video.source_id + # Date is stored on file_group + if file_group.published_datetime: + file_info['published_datetime'] = file_group.published_datetime.isoformat() + # Get poster path + if video.poster_path: + try: + poster_relative = str(pathlib.Path(video.poster_path).relative_to(media_dir)) + except ValueError: + poster_relative = video.poster_path + file_info['poster_path'] = poster_relative + # Collect video for ranking calculation + videos_for_ranking.append((file_info, video)) + + elif collection.kind == 'domain': + from modules.archive.models import Archive + archive = session.query(Archive).filter_by(file_group_id=file_group.id).one_or_none() + if archive: + file_info['archive_id'] = archive.id + # Archives don't have source_id, use file_group.url + if file_group.url: + file_info['source_id'] = file_group.url + if file_group.download_datetime: + file_info['published_datetime'] = file_group.download_datetime.isoformat() + # Get screenshot path for archives + if archive.screenshot_path: + try: + screenshot_relative = str(pathlib.Path(archive.screenshot_path).relative_to(media_dir)) + except ValueError: + screenshot_relative = archive.screenshot_path + file_info['poster_path'] = screenshot_relative + # Collect archive for ranking calculation + archives_for_ranking.append((file_info, archive)) + + conflicting_files.append(file_info) + + # Calculate quality rankings for videos + if collection.kind == 'channel' and videos_for_ranking: + # Get largest sizes by source_id for ranking bonus + videos_only = [v for _, v in videos_for_ranking] + sizes_by_source_id = _get_largest_sizes_by_source_id(videos_only) + + # Calculate rank for each video + for file_info, video in videos_for_ranking: + file_info['quality_rank'] = _rank_video_quality(video, sizes_by_source_id) + + # Calculate quality rankings for archives + elif collection.kind == 'domain' and archives_for_ranking: + # Calculate rank for each archive + for file_info, archive in archives_for_ranking: + file_info['quality_rank'] = _rank_archive_quality(archive) + + # Sort by quality_rank (highest first), then by size (works for both videos and archives) + if videos_for_ranking or archives_for_ranking: + conflicting_files.sort( + key=lambda f: (f.get('quality_rank') or 0, f.get('size') or 0), + reverse=True + ) + + conflict_details.append({ + 'destination_path': dest_relative, + 'conflicting_files': conflicting_files, + }) + + return conflict_details + + def execute_reorganization( collection_id: int, session: Session = None, @@ -572,14 +1004,15 @@ def get_reorganization_status(job_id: str) -> dict: def get_collections_needing_reorganization( kind: str, session: Session = None, - sample_size: int = 1, ) -> dict: - """Get all collections of a kind that need reorganization with sample previews. + """Get all collections of a kind that need reorganization with lightweight previews. + + Uses get_batch_reorganization_preview() for fast counts without expensive + conflict detection or sample move generation. Args: kind: 'channel' or 'domain' session: Database session (optional) - sample_size: Number of sample moves per collection (default 1) Returns: Dict with: @@ -590,7 +1023,7 @@ def get_collections_needing_reorganization( """ if session is None: with get_db_session() as session: - return get_collections_needing_reorganization(kind, session, sample_size) + return get_collections_needing_reorganization(kind, session) # Query all collections of this kind with directories collections = session.query(Collection).filter( @@ -614,16 +1047,15 @@ def get_collections_needing_reorganization( total_files_needing_move = 0 for collection in collections_needing_reorg: - # Get preview for each collection + # Use lightweight batch preview (skips expensive conflict detection) try: - preview = get_reorganization_preview(collection.id, session, sample_size=sample_size) - sample_move = preview.sample_moves[0] if preview.sample_moves else None + preview = get_batch_reorganization_preview(collection.id, session) result_collections.append({ - 'collection_id': collection.id, - 'collection_name': collection.name, + 'collection_id': preview.collection_id, + 'collection_name': preview.collection_name, 'total_files': preview.total_files, 'files_needing_move': preview.files_needing_move, - 'sample_move': sample_move, + 'sample_move': preview.sample_move, }) total_files_needing_move += preview.files_needing_move except Exception as e: @@ -668,7 +1100,7 @@ def execute_batch_reorganization(kind: str, session: Session = None) -> dict: return execute_batch_reorganization(kind, session) # Get collections needing reorganization - reorg_info = get_collections_needing_reorganization(kind, session, sample_size=0) + reorg_info = get_collections_needing_reorganization(kind, session) collections_to_process = reorg_info['collections'] if not collections_to_process: @@ -796,3 +1228,52 @@ def get_batch_reorganization_status(batch_job_id: str) -> dict: 'failed_collection': None, 'error': None, } + + +async def get_conflict_details_with_ffprobe( + collection_id: int, + session: Session, +) -> List[dict]: + """Get conflict details, processing ffprobe for any videos missing it. + + This ensures accurate quality rankings (including audio stream bonus) + when resolving conflicts. The +2 audio bonus in _rank_video_quality() + only applies if video.ffprobe_json exists. + + Args: + collection_id: The collection to get conflicts for + session: Database session + + Returns: + List of conflict detail dicts (empty if no conflicts) + + Raises: + UnknownCollection: If collection not found + """ + preview = get_reorganization_preview(collection_id, session) + + if not preview.conflicts: + return [] + + # Process ffprobe for videos missing it (only for channel collections) + from modules.videos.models import Video + + videos_processed = 0 + for conflict in preview.conflicts: + for file_info in conflict['conflicting_files']: + if file_info.get('video_id'): + video = session.query(Video).get(file_info['video_id']) + if video and not video.ffprobe_json: + try: + await video.get_ffprobe_json() + videos_processed += 1 + except Exception as e: + logger.warning(f'Failed to process ffprobe for video {video.id}: {e}') + + # Flush ffprobe data if any was processed (preserves caller's transaction control) + if videos_processed > 0: + session.flush() + # Re-fetch preview to get updated quality rankings + preview = get_reorganization_preview(collection_id, session) + + return preview.conflicts diff --git a/wrolpi/collections/schema.py b/wrolpi/collections/schema.py index 56c3f10d..9a20a487 100644 --- a/wrolpi/collections/schema.py +++ b/wrolpi/collections/schema.py @@ -54,6 +54,29 @@ class CollectionTagInfoResponse: conflict_message: Optional[str] = None +@dataclass +class ConflictFileInfo: + """Info about a single file in a reorganization conflict.""" + file_group_id: int + current_path: str + title: str + model_type: str # 'video' or 'archive' + size: int = 0 + video_id: Optional[int] = None + archive_id: Optional[int] = None + poster_path: Optional[str] = None + published_datetime: Optional[str] = None + source_id: Optional[str] = None + quality_rank: Optional[int] = None # Metadata quality score (videos only) + + +@dataclass +class ConflictDetail: + """Details about a single destination path conflict.""" + destination_path: str + conflicting_files: List[ConflictFileInfo] + + @dataclass class ReorganizationPreviewResponse: """Response for reorganization preview.""" @@ -64,6 +87,8 @@ class ReorganizationPreviewResponse: sample_moves: List[dict] new_file_format: str current_file_format: Optional[str] = None + conflicts: List[ConflictDetail] = field(default_factory=list) + has_conflicts: bool = False @dataclass diff --git a/wrolpi/collections/test/test_reorganize.py b/wrolpi/collections/test/test_reorganize.py index f39c6c38..434a282e 100644 --- a/wrolpi/collections/test/test_reorganize.py +++ b/wrolpi/collections/test/test_reorganize.py @@ -468,7 +468,7 @@ def test_get_collections_needing_reorganization_includes_sample_move( # Create a video in this channel using factory video = video_factory(channel_id=channel.id) - result = get_collections_needing_reorganization('channel', test_session, sample_size=1) + result = get_collections_needing_reorganization('channel', test_session) assert len(result['collections']) == 1 collection_info = result['collections'][0] @@ -1729,9 +1729,9 @@ async def test_domain_reorganization_moves_files_from_root_to_year_subdirectory( assert old_path.parent == domain_dir, \ f"Archive should be in root, not year subdir. Got: {old_path.parent}" - # Get preview with exact_count - should detect files need moving + # Get preview - should detect files need moving with get_db_session() as session: - preview = get_reorganization_preview(collection_id, session, exact_count=True) + preview = get_reorganization_preview(collection_id, session) assert preview.files_needing_move > 0, \ f"Should detect files need moving. Got: {preview.files_needing_move}" @@ -1814,9 +1814,9 @@ async def test_channel_reorganization_moves_files_from_root_to_year_subdirectory assert old_path.parent == channel_dir, \ f"Video should be in root, not year subdir. Got: {old_path.parent}" - # Get preview with exact_count - should detect files need moving + # Get preview - should detect files need moving with get_db_session() as session: - preview = get_reorganization_preview(collection_id, session, exact_count=True) + preview = get_reorganization_preview(collection_id, session) assert preview.files_needing_move > 0, \ f"Should detect files need moving. Got: {preview.files_needing_move}" @@ -1940,12 +1940,12 @@ async def test_channel_reorganization_preserves_root_level_files( config._config['yt_dlp_options'] = yt_dlp_options -def test_preview_exact_count_computes_actual_moves(test_session, test_directory, archive_factory): - """With exact_count=True, preview should compute actual files needing move, not total files. +def test_preview_computes_actual_moves(test_session, test_directory, archive_factory): + """Preview should compute actual files needing move by building move mappings. This test verifies that: - - exact_count=False returns total_files as files_needing_move (estimate) - - exact_count=True computes the actual count by building move mappings + - Preview correctly identifies files in wrong location + - After reorganization, preview shows 0 files needing move """ # Create domain collection with a year-based format (different from default) domain_dir = test_directory / 'archive' / 'exactcount.com' @@ -1977,19 +1977,13 @@ def test_preview_exact_count_computes_actual_moves(test_session, test_directory, archive_obj.collection_id = collection_id session.commit() - # Without exact_count, preview assumes all files need moving + # Preview should detect that the file needs moving (in wrong location) with get_db_session() as session: - preview_estimate = get_reorganization_preview(collection_id, session, exact_count=False) - # This is the current behavior - assumes all need moving - assert preview_estimate.files_needing_move == preview_estimate.total_files - assert preview_estimate.files_needing_move == 1 - - # With exact_count, preview also detects that files need moving (they're in wrong location) - with get_db_session() as session: - preview_exact = get_reorganization_preview(collection_id, session, exact_count=True) + preview = get_reorganization_preview(collection_id, session) # Files are in root, but should be in year subdir, so 1 file needs moving - assert preview_exact.files_needing_move == 1, \ - f"File in wrong location should need moving. Got: {preview_exact.files_needing_move}" + assert preview.files_needing_move == 1, \ + f"File in wrong location should need moving. Got: {preview.files_needing_move}" + assert preview.total_files == 1 # Now execute the reorganization to move files to correct location from wrolpi.files.worker import file_worker @@ -2001,9 +1995,9 @@ def test_preview_exact_count_computes_actual_moves(test_session, test_directory, import asyncio asyncio.get_event_loop().run_until_complete(file_worker.process_queue()) - # After reorganization, with exact_count should show 0 files needing move + # After reorganization, should show 0 files needing move with get_db_session() as session: - preview_after = get_reorganization_preview(collection_id, session, exact_count=True) + preview_after = get_reorganization_preview(collection_id, session) assert preview_after.files_needing_move == 0, \ f"After reorganization, no files should need moving. Got: {preview_after.files_needing_move}" finally: @@ -2076,6 +2070,225 @@ async def test_reorganize_fails_on_filename_conflicts(async_client, test_directo assert 'Duplicate Title' in str(exc_info.value) +@pytest.mark.asyncio +async def test_reorganize_preview_returns_conflict_details(async_client, test_directory, video_factory): + """Preview should return detailed conflict information for UI display.""" + from datetime import datetime, timezone + + # Create channel collection + channel_dir = test_directory / 'videos' / 'ConflictPreviewChannel' + channel_dir.mkdir(parents=True, exist_ok=True) + + with get_db_session(commit=True) as session: + collection = Collection( + name='ConflictPreviewChannel', + kind='channel', + directory=channel_dir, + file_format='%(old_format)s.%(ext)s' # Needs reorganization + ) + session.add(collection) + session.flush() + channel = Channel(name='ConflictPreviewChannel', collection_id=collection.id, directory=channel_dir) + session.add(channel) + session.commit() + channel_id = channel.id + collection_id = collection.id + + # Create two videos with DIFFERENT initial filenames but IDENTICAL destination metadata + same_date = datetime(2024, 1, 15, tzinfo=timezone.utc) + same_source_id = 'preview_duplicate_id' + + video1 = video_factory( + channel_id=channel_id, + title='video_one', + upload_date=same_date, + with_video_file=True, + with_info_json={'uploader': 'TestUploader', 'id': same_source_id} + ) + + video2 = video_factory( + channel_id=channel_id, + title='video_two', + upload_date=same_date, + with_video_file=True, + with_info_json={'uploader': 'TestUploader', 'id': same_source_id} + ) + + # Update both videos to have the same title and source_id + with get_db_session(commit=True) as session: + v1 = session.query(Video).get(video1.id) + v2 = session.query(Video).get(video2.id) + v1.file_group.title = 'Duplicate Preview Title' + v2.file_group.title = 'Duplicate Preview Title' + v1.source_id = same_source_id + v2.source_id = same_source_id + + # Get preview - should show conflicts instead of raising an exception + with get_db_session() as session: + preview = get_reorganization_preview(collection_id, session) + + # Verify preview has conflict information + assert preview.has_conflicts is True, "Preview should indicate conflicts exist" + assert len(preview.conflicts) == 1, f"Should have 1 conflict, got {len(preview.conflicts)}" + + conflict = preview.conflicts[0] + assert 'destination_path' in conflict + assert 'conflicting_files' in conflict + assert len(conflict['conflicting_files']) == 2, "Should have 2 conflicting files" + + # Verify file details are included + for file_info in conflict['conflicting_files']: + assert 'file_group_id' in file_info + assert 'video_id' in file_info + assert 'current_path' in file_info + assert 'title' in file_info + assert 'model_type' in file_info + assert file_info['model_type'] == 'video' + assert file_info['source_id'] == same_source_id + + +@pytest.mark.asyncio +async def test_reorganize_preview_no_conflicts(async_client, test_directory, video_factory): + """Preview should show no conflicts when files have unique destinations.""" + from datetime import datetime, timezone + + # Create channel collection + channel_dir = test_directory / 'videos' / 'NoConflictChannel' + channel_dir.mkdir(parents=True, exist_ok=True) + + with get_db_session(commit=True) as session: + collection = Collection( + name='NoConflictChannel', + kind='channel', + directory=channel_dir, + file_format='%(old_format)s.%(ext)s' + ) + session.add(collection) + session.flush() + channel = Channel(name='NoConflictChannel', collection_id=collection.id, directory=channel_dir) + session.add(channel) + session.commit() + channel_id = channel.id + collection_id = collection.id + + # Create two videos with DIFFERENT titles (unique destinations) + video1 = video_factory( + channel_id=channel_id, + title='Unique Title One', + upload_date=datetime(2024, 1, 15, tzinfo=timezone.utc), + with_video_file=True, + with_info_json={'uploader': 'TestUploader', 'id': 'unique_id_1'} + ) + + video2 = video_factory( + channel_id=channel_id, + title='Unique Title Two', + upload_date=datetime(2024, 1, 16, tzinfo=timezone.utc), + with_video_file=True, + with_info_json={'uploader': 'TestUploader', 'id': 'unique_id_2'} + ) + + # Get preview - should show no conflicts + with get_db_session() as session: + preview = get_reorganization_preview(collection_id, session) + + assert preview.has_conflicts is False, "Preview should indicate no conflicts" + assert preview.conflicts == [], "Conflicts list should be empty" + + +@pytest.mark.asyncio +async def test_conflict_preview_includes_quality_rank(async_client, test_directory, video_factory): + """Conflict preview should include quality_rank for videos based on metadata completeness. + + Videos with more metadata (info_json, captions, poster) should rank higher. + Conflicting files should be sorted by quality_rank (highest first). + """ + from datetime import datetime, timezone + + # Create channel collection + channel_dir = test_directory / 'videos' / 'QualityRankChannel' + channel_dir.mkdir(parents=True, exist_ok=True) + + with get_db_session(commit=True) as session: + collection = Collection( + name='QualityRankChannel', + kind='channel', + directory=channel_dir, + file_format='%(old_format)s.%(ext)s' # Needs reorganization + ) + session.add(collection) + session.flush() + channel = Channel(name='QualityRankChannel', collection_id=collection.id, directory=channel_dir) + session.add(channel) + session.commit() + channel_id = channel.id + collection_id = collection.id + + same_date = datetime(2024, 1, 15, tzinfo=timezone.utc) + same_source_id = 'quality_rank_test_id' + + # video1 - MINIMAL metadata (lower rank expected) + video1 = video_factory( + channel_id=channel_id, + title='minimal_video', + upload_date=same_date, + with_video_file=True, + # No info_json, no poster, no captions + ) + + # video2 - RICH metadata (higher rank expected) + video2 = video_factory( + channel_id=channel_id, + title='rich_video', + upload_date=same_date, + with_video_file=True, + with_info_json={'uploader': 'TestUploader', 'id': same_source_id, 'title': 'Rich Video Title'}, + with_poster_ext='jpg', + with_caption_file=True, + ) + + # Make both videos conflict (same title, same source_id → same destination) + with get_db_session(commit=True) as session: + v1 = session.query(Video).get(video1.id) + v2 = session.query(Video).get(video2.id) + v1.file_group.title = 'Same Title' + v2.file_group.title = 'Same Title' + v1.source_id = same_source_id + v2.source_id = same_source_id + # Give v1 a URL so it has SOME data, but still less than v2 + v1.file_group.url = 'https://example.com/video1' + + # Get preview - should detect conflicts with quality_rank + with get_db_session() as session: + preview = get_reorganization_preview(collection_id, session) + + assert preview.has_conflicts is True, "Should detect conflicts" + assert len(preview.conflicts) == 1, "Should have 1 conflict" + + conflict = preview.conflicts[0] + files = conflict['conflicting_files'] + assert len(files) == 2, "Should have 2 conflicting files" + + # Verify quality_rank is present on all video files + for file_info in files: + assert 'quality_rank' in file_info, f"quality_rank missing from {file_info}" + assert file_info['quality_rank'] is not None, f"quality_rank should not be None for videos" + + # Verify files are sorted by quality_rank (highest first) + ranks = [f['quality_rank'] for f in files] + assert ranks == sorted(ranks, reverse=True), \ + f"Files should be sorted by quality_rank descending. Got: {ranks}" + + # The video with more metadata (video2) should rank higher + video2_info = next(f for f in files if f['video_id'] == video2.id) + video1_info = next(f for f in files if f['video_id'] == video1.id) + assert video2_info['quality_rank'] > video1_info['quality_rank'], \ + f"Video with more metadata should rank higher. v2={video2_info['quality_rank']}, v1={video1_info['quality_rank']}" + + # The highest ranked video should be first + assert files[0]['video_id'] == video2.id, "Highest ranked video should be first" + + # ============================================================================ # Filename Parsing Fallback Tests # ============================================================================ diff --git a/wrolpi/files/worker.py b/wrolpi/files/worker.py index c489dacf..2846f1b0 100644 --- a/wrolpi/files/worker.py +++ b/wrolpi/files/worker.py @@ -2322,9 +2322,18 @@ class FileWorker: except Exception as e: # Collection failed - stop batch processing logger.error(f'Batch reorganize: {collection_name} failed: {e}') + + # Get the actual channel ID for the UI link (collection_id != channel_id) + channel_id = None + if kind == 'channel': + from modules.videos.models import Channel + channel = session.query(Channel).filter_by(collection_id=collection_id).one_or_none() + channel_id = channel.id if channel else None + batch_status['failed_collection'] = { 'id': collection_id, 'name': collection_name, + 'channel_id': channel_id, } batch_status['error'] = str(e) batch_status['current_collection'] = None