From cc25e342d4c581be7ba12605fb77c2b0c2fcb3bc Mon Sep 17 00:00:00 2001 From: Roland Date: Mon, 14 Sep 2026 18:50:25 -0600 Subject: [PATCH] Video page: Edit > Edit to change a video's title PUT /api/videos/ writes the title (and fulltitle) into the Video's info json, creating the file when the video has none, then validates the Video again so its title and search text follow. Files stay the source of truth: the edit survives a refresh, and a re-download of the metadata replaces it (the modal says so). Co-Authored-By: Claude Fable 5.1 --- app/src/api.js | 12 ++++ app/src/components/VideoEditMenu.js | 88 +++++++++++++++++++++++- app/src/components/VideoEditMenu.test.js | 62 ++++++++++++++++- app/src/components/VideoPlayer.js | 1 + modules/videos/schema.py | 5 ++ modules/videos/test/test_video_edit.py | 77 +++++++++++++++++++++ modules/videos/video/api.py | 14 ++++ modules/videos/video/lib.py | 29 ++++++++ 8 files changed, 282 insertions(+), 6 deletions(-) create mode 100644 modules/videos/test/test_video_edit.py 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.