diff --git a/app/src/api.js b/app/src/api.js index a08959fa..b56dfcae 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -236,6 +236,18 @@ export async function getVideo(fileGroupId) { return [data['file_group'], data['prev'], data['next']]; } +export async function updateVideo(fileGroupId, {title}) { + // Edits the Video's details (written to its info json). Returns the updated video file group. + const response = await apiPut(`${VIDEOS_API}/${fileGroupId}`, {title}); + if (!response.ok) { + const message = await getErrorMessage(response, 'Unable to save the video. See server logs.'); + toast({type: 'error', title: 'Failed to save', description: message, time: 5000}); + throw new Error(message); + } + const data = await response.json(); + return data['file_group']; +} + export async function transcodeVideo(fileGroupId, {video_codec = null, audio_codec = null, container = 'mp4'}) { // Queues a transcode Job; returns the job id. Watch it with `getJob`. const body = {video_codec, audio_codec, container}; diff --git a/app/src/components/VideoEditMenu.js b/app/src/components/VideoEditMenu.js index 4593bd5e..2020cf3f 100644 --- a/app/src/components/VideoEditMenu.js +++ b/app/src/components/VideoEditMenu.js @@ -1,7 +1,7 @@ import React, {useEffect, useRef, useState} from 'react'; -import {cancelJob, fetchVideoDownloadDefaults, getJob, transcodeVideo} from '../api'; +import {cancelJob, fetchVideoDownloadDefaults, getJob, transcodeVideo, updateVideo} from '../api'; import {useWROLMode} from '../hooks/customHooks'; -import {Button, Confirm, Group, Icon, Menu, Modal, Progress, Select, Stack, Status, Text, toast} from './ui'; +import {Button, Confirm, Group, Icon, Menu, Modal, Progress, Select, Stack, Status, Text, TextInput, toast} from './ui'; import { TRANSCODE_COPY, transcodeAudioCodecOptions, @@ -268,6 +268,73 @@ export function TranscodeModal({open, onClose, fileGroupId, video, onComplete}) } +/** + * Edit a Video's details. The change is written to the Video's info json, so it survives a + * refresh; a re-download of the Video's metadata will overwrite it. + */ +export function EditVideoModal({open, onClose, fileGroupId, videoFile, onSaved}) { + const [title, setTitle] = useState(''); + const [saving, setSaving] = useState(false); + + useEffect(() => { + if (open) { + // Start from the current details on every open, discarding an abandoned edit. + setTitle(videoFile?.title || ''); + } + }, [open, videoFile?.title]); + + const trimmed = title.trim(); + const unchanged = trimmed === (videoFile?.title || ''); + + const handleSave = async () => { + setSaving(true); + try { + const updated = await updateVideo(fileGroupId, {title: trimmed}); + toast({type: 'success', title: 'Saved', description: 'The video was updated.', time: 3000}); + if (onSaved) await onSaved(updated); + onClose(); + } catch (e) { + // updateVideo already toasted. + console.error(e); + } finally { + setSaving(false); + } + }; + + return + Edit Video + +
{ + e.preventDefault(); + if (trimmed && !unchanged && !saving) handleSave(); + }}> + setTitle(e.currentTarget.value)} + error={!trimmed ? 'A title is required' : null} + data-autofocus + /> + + Saved to the video's info json. Refreshing the video from its source will replace it. + + +
+ + + + + +} + /** * The "Edit" dropdown on the Video page: Refresh (re-download metadata), Transcode, and Delete. * @@ -275,8 +342,9 @@ export function TranscodeModal({open, onClose, fileGroupId, video, onComplete}) * page can fetch the Video again (its path and codecs changed). `onDelete` is awaited after the * user confirms; the page decides what happens next (navigate away, or the tagged-files prompt). */ -export function VideoEditMenu({videoFile, video, onRefresh, onTranscodeComplete, onDelete}) { +export function VideoEditMenu({videoFile, video, onRefresh, onTranscodeComplete, onDelete, onSaved}) { const wrolModeEnabled = useWROLMode(); + const [editOpen, setEditOpen] = useState(false); const [transcodeOpen, setTranscodeOpen] = useState(false); const [refreshing, setRefreshing] = useState(false); const [deleteOpen, setDeleteOpen] = useState(false); @@ -309,6 +377,13 @@ export function VideoEditMenu({videoFile, video, onRefresh, onTranscodeComplete, + } + onClick={() => setEditOpen(true)} + disabled={!!wrolModeEnabled} + > + Edit... + } onClick={handleRefresh} @@ -334,6 +409,13 @@ export function VideoEditMenu({videoFile, video, onRefresh, onTranscodeComplete, + setEditOpen(false)} + fileGroupId={videoFile?.id} + videoFile={videoFile} + onSaved={onSaved} + /> ({ @@ -9,16 +9,17 @@ jest.mock('../api', () => ({ fetchVideoDownloadDefaults: jest.fn(), getJob: jest.fn(), transcodeVideo: jest.fn(), + updateVideo: jest.fn(), })); -import {cancelJob, fetchVideoDownloadDefaults, getJob, transcodeVideo} from '../api'; +import {cancelJob, fetchVideoDownloadDefaults, getJob, transcodeVideo, updateVideo} from '../api'; const video = { codec_names: ['vp9', 'mjpeg', 'opus'], codec_types: ['video', 'video', 'audio'], video_path: 'videos/movie.webm', }; -const videoFile = {id: 7, url: 'https://example.com/watch?v=1', mimetype: 'video/webm', video}; +const videoFile = {id: 7, url: 'https://example.com/watch?v=1', mimetype: 'video/webm', title: 'Old Title', video}; // Mantine's Select renders a hidden input next to the visible one, both tied to the label. const selectInput = (label) => screen.getAllByLabelText(label).find(i => i.type !== 'hidden'); @@ -95,6 +96,52 @@ describe('TranscodeModal', () => { }); }); +describe('EditVideoModal', () => { + beforeEach(() => { + jest.clearAllMocks(); + updateVideo.mockResolvedValue({...videoFile, title: 'New Title'}); + }); + + test('starts from the current title and saves a changed one', async () => { + const onSaved = jest.fn().mockResolvedValue(undefined); + const onClose = jest.fn(); + renderWithProviders( + , + ); + + const input = screen.getByLabelText('Title'); + expect(input).toHaveValue('Old Title'); + // Nothing changed yet: Save is disabled. + expect(screen.getByRole('button', {name: /Save/})).toBeDisabled(); + + fireEvent.change(input, {target: {value: ' New Title '}}); + fireEvent.click(screen.getByRole('button', {name: /Save/})); + + await waitFor(() => expect(updateVideo).toHaveBeenCalledWith(7, {title: 'New Title'})); + await waitFor(() => expect(onSaved).toHaveBeenCalledTimes(1)); + expect(onClose).toHaveBeenCalledTimes(1); + }); + + test('an empty title cannot be saved', () => { + renderWithProviders(); + fireEvent.change(screen.getByLabelText('Title'), {target: {value: ' '}}); + expect(screen.getByText('A title is required')).toBeInTheDocument(); + expect(screen.getByRole('button', {name: /Save/})).toBeDisabled(); + expect(updateVideo).not.toHaveBeenCalled(); + }); + + test('a failed save keeps the modal open', async () => { + updateVideo.mockRejectedValue(new Error('boom')); + const onClose = jest.fn(); + renderWithProviders(); + fireEvent.change(screen.getByLabelText('Title'), {target: {value: 'Other'}}); + fireEvent.click(screen.getByRole('button', {name: /Save/})); + await waitFor(() => expect(updateVideo).toHaveBeenCalled()); + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByLabelText('Title')).toHaveValue('Other'); + }); +}); + describe('JobProgress', () => { beforeEach(() => { jest.clearAllMocks(); @@ -191,6 +238,15 @@ describe('VideoEditMenu', () => { expect(await screen.findByRole('menuitem', {name: /Refresh/})).toBeDisabled(); expect(screen.getByRole('menuitem', {name: /Transcode/})).toBeDisabled(); expect(screen.getByRole('menuitem', {name: /Delete/})).toBeDisabled(); + expect(screen.getByRole('menuitem', {name: /^Edit/})).toBeDisabled(); + }); + + test('Edit opens the details modal', async () => { + renderWithProviders(); + fireEvent.click(screen.getByRole('button', {name: /Edit/})); + fireEvent.click(await screen.findByRole('menuitem', {name: /^Edit/})); + expect(await screen.findByText('Edit Video')).toBeInTheDocument(); + expect(screen.getByLabelText('Title')).toHaveValue('Old Title'); }); test('Delete asks for confirmation before calling onDelete', async () => { diff --git a/app/src/components/VideoPlayer.js b/app/src/components/VideoPlayer.js index ba01fb7f..c2d9e9ab 100644 --- a/app/src/components/VideoPlayer.js +++ b/app/src/components/VideoPlayer.js @@ -495,6 +495,7 @@ function VideoPage({videoFile, prevFile, nextFile, fetchVideo, ...props}) { onRefresh={handleRefresh} onTranscodeComplete={fetchVideo} onDelete={async () => await handleDeleteVideo(videoFile.id)} + onSaved={fetchVideo} /> diff --git a/modules/videos/schema.py b/modules/videos/schema.py index 0a8423c1..21c0cdca 100644 --- a/modules/videos/schema.py +++ b/modules/videos/schema.py @@ -186,6 +186,11 @@ class VideoSearchRequest: deep: bool = False # When True, search all text including captions (d_text) +@dataclass +class VideoUpdateRequest: + title: str + + @dataclass class VideoTranscodeRequest: video_codec: Optional[str] = None # A key of TRANSCODE_VIDEO_TARGETS, or None to copy the stream. diff --git a/modules/videos/test/test_video_edit.py b/modules/videos/test/test_video_edit.py new file mode 100644 index 00000000..fa8b3805 --- /dev/null +++ b/modules/videos/test/test_video_edit.py @@ -0,0 +1,77 @@ +"""Editing a Video's details from the video page.""" +import json + +import pytest + +from modules.videos.models import Video +from modules.videos.video.lib import update_video +from wrolpi.errors import ValidationError + + +def test_update_video_title(test_session, test_directory, video_factory): + """The new title is written to the info json (title and fulltitle) and the Video re-validated, + so its title and search text change too. Other info json keys are kept.""" + video = video_factory(with_info_json={'title': 'Old Title', 'fulltitle': 'Old Title', 'duration': 7, + 'formats': ['kept, not cleaned']}) + test_session.commit() + assert video.file_group.title == 'Old Title' + + update_video(video.file_group_id, ' New Title ') + + test_session.expire_all() + video = Video.find_by_file_group_id(test_session, video.file_group_id) + assert video.file_group.title == 'New Title' + assert video.file_group.a_text == 'New Title' + assert video.file_group.length == 7, 'Other details from the info json are unchanged' + info_json = json.loads(video.info_json_path.read_text()) + assert info_json['title'] == 'New Title' and info_json['fulltitle'] == 'New Title' + assert info_json['formats'] == ['kept, not cleaned'] + + +def test_update_video_title_creates_info_json(test_session, test_directory, video_factory): + """A Video without an info json gets one holding the title.""" + video = video_factory() + test_session.commit() + assert video.info_json_path is None + + update_video(video.file_group_id, 'Brand New') + + test_session.expire_all() + video = Video.find_by_file_group_id(test_session, video.file_group_id) + assert video.file_group.title == 'Brand New' + assert video.info_json_path and video.info_json_path.is_file() + assert json.loads(video.info_json_path.read_text())['title'] == 'Brand New' + assert video.info_json_path.name in [i['path'] for i in video.file_group.files], 'The new file is tracked' + + +def test_update_video_title_empty(test_session, video_factory): + video = video_factory(with_info_json={'title': 'Old'}) + test_session.commit() + with pytest.raises(ValidationError): + update_video(video.file_group_id, ' ') + test_session.expire_all() + assert Video.find_by_file_group_id(test_session, video.file_group_id).file_group.title == 'Old' + + +@pytest.mark.asyncio +async def test_video_update_api(async_client, test_session, video_factory, wrol_mode_fixture): + video = video_factory(with_info_json={'title': 'Old'}) + test_session.commit() + + request, response = await async_client.put(f'/api/videos/{video.file_group_id}', + content=json.dumps({'title': 'Edited'})) + assert response.status == 200, response.json + assert response.json['file_group']['title'] == 'Edited' + assert response.json['file_group']['id'] == video.file_group_id + + request, response = await async_client.put(f'/api/videos/{video.file_group_id}', content=json.dumps({'title': ''})) + assert response.status == 400 + + request, response = await async_client.put('/api/videos/999999', content=json.dumps({'title': 'x'})) + assert response.status == 404 + + await wrol_mode_fixture(True) + request, response = await async_client.put(f'/api/videos/{video.file_group_id}', + content=json.dumps({'title': 'Nope'})) + assert response.status == 403 + await wrol_mode_fixture(False) diff --git a/modules/videos/video/api.py b/modules/videos/video/api.py index 03c0a5ed..c7e584c0 100644 --- a/modules/videos/video/api.py +++ b/modules/videos/video/api.py @@ -29,6 +29,20 @@ def video_get(request: Request, file_group_id: int): return json_response({'file_group': video, 'prev': previous_video, 'next': next_video}) +@video_bp.put('/') +@openapi.description('Edit a Video\'s details. Written to its info json; the Video is validated again.') +@openapi.body({'application/json': schema.VideoUpdateRequest}) +@openapi.response(HTTPStatus.OK, schema.VideoResponse) +@openapi.response(HTTPStatus.BAD_REQUEST, JSONErrorResponse) +@openapi.response(HTTPStatus.NOT_FOUND, JSONErrorResponse) +@validate(schema.VideoUpdateRequest) +@wrol_mode_check +async def video_update(_: Request, file_group_id: int, body: schema.VideoUpdateRequest): + lib.update_video(file_group_id, body.title) + video, previous_video, next_video = lib.get_video_for_app(file_group_id) + return json_response({'file_group': video, 'prev': previous_video, 'next': next_video}) + + @video_bp.get('//comments') @openapi.description('Get Video comments') @openapi.response(HTTPStatus.OK, schema.VideoCommentsResponse) diff --git a/modules/videos/video/lib.py b/modules/videos/video/lib.py index b7424835..174d230a 100644 --- a/modules/videos/video/lib.py +++ b/modules/videos/video/lib.py @@ -50,6 +50,35 @@ def get_video(file_group_id: int) -> Video: return video +def update_video(file_group_id: int, title: str) -> Video: + """Edit a Video's details. The files are the source of truth, so the change is written to the + Video's .info.json (created if missing) and the Video is validated again from it. + + A later re-download of the Video's metadata will overwrite the info json, and this edit with + it. + + @raise UnknownVideo: if the Video can not be found + @raise ValidationError: when the title is empty + """ + from wrolpi.errors import ValidationError + title = (title or '').strip() + if not title: + raise ValidationError('Video title cannot be empty') + + with get_db_session(commit=True) as session: + video = Video.find_by_file_group_id(session, file_group_id) + info_json = video.get_info_json() or dict() + # `extract_video_info_json` prefers fulltitle over title; keep the two in agreement. + info_json['title'] = title + info_json['fulltitle'] = title + # Keep the file as it was, other than the title. + video.replace_info_json(info_json, clean=False) + # Re-derive the title (and search text) from the file just written. + video.validate(session) + video.flush() + return video + + VIDEO_ORDERS = { # fg.id (the rowid) is the pagination tiebreaker: it is present in every index, so these # orders stay index-only. LOWER(fg.primary_path) forced SQLite to read every (large) row.