From e9a7167fa32fad6e89527785166b66da4ce1d345 Mon Sep 17 00:00:00 2001 From: Roland Date: Sun, 25 Oct 2026 16:25:37 -0600 Subject: [PATCH] Renaming a Tag renames it in Download settings update_tag never renamed settings.tag_names, and its channel_tag_name edit mutated the JSON in place, so it was never saved. The old name stayed, and a Download that later found a new Channel failed (UnknownTag). Co-Authored-By: Claude Opus 5.5 --- wrolpi/tags.py | 25 ++++++++++++++++--------- wrolpi/test/test_tags.py | 24 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/wrolpi/tags.py b/wrolpi/tags.py index 2d38c5c9..e02bc3f6 100644 --- a/wrolpi/tags.py +++ b/wrolpi/tags.py @@ -187,15 +187,9 @@ class Tag(ModelHelper, Base): from wrolpi.downloader import Download to_flush = list() for download in session.query(Download): - if download.tag_names: - download.tag_names = [i if i != old_name else name for i in download.tag_names] - to_flush.append(download) - if download.settings and 'channel_tag_name' in download.settings: - settings = download.settings - tag_name = settings.pop('channel_tag_name') - tag_name = [i for i in tag_name if i != old_name] - settings['channel_tag_name'] = tag_name - download.settings = settings + tag_names, settings = with_tag_name_renamed(download.tag_names, download.settings, old_name, name) + if tag_names != download.tag_names or settings != download.settings: + download.tag_names, download.settings = tag_names, settings to_flush.append(download) if to_flush: @@ -319,6 +313,19 @@ def without_tag_names(download_tag_names: Optional[List[str]], settings: Optiona return download_tag_names, settings +def with_tag_name_renamed(download_tag_names: Optional[List[str]], settings: Optional[dict], old_name: str, + new_name: str) -> Tuple[Optional[List[str]], Optional[dict]]: + """A Download's tag_names and settings with a Tag's old name replaced, in the settings' tag_names and + channel_tag_name too. New objects, so an ORM change to the JSON is seen.""" + if download_tag_names: + download_tag_names = [new_name if i == old_name else i for i in download_tag_names] + if settings: + settings = {key: [new_name if i == old_name else i for i in value] + if key in ('tag_names', 'channel_tag_name') and value else value + for key, value in settings.items()} + return download_tag_names, settings + + def remove_tag_names_from_downloads(session: Session, names: Set[str]): """Remove these Tag names from every Download that would apply them; applying a Tag that does not exist fails the download. (Changes download_manager.yaml.)""" diff --git a/wrolpi/test/test_tags.py b/wrolpi/test/test_tags.py index 2ca7d715..e9e7bd3c 100644 --- a/wrolpi/test/test_tags.py +++ b/wrolpi/test/test_tags.py @@ -569,6 +569,30 @@ async def test_tag_rename_with_channel(test_session, test_directory, video_facto tag.delete() +@pytest.mark.asyncio +async def test_tag_rename_downloads(test_session, tag_factory, await_background_tasks): + """Renaming a Tag renames it everywhere a Download applies it, including the settings passed on to the + Downloads (and Channels) it creates.""" + from wrolpi.downloader import Download + tag, other = await tag_factory('one'), await tag_factory('two') + download = Download(url='https://example.com/playlist', downloader='video_channel', tag_names=['one', 'two'], + settings=dict(tag_names=['two', 'one'], channel_tag_name=['one'], video_resolutions=['720p'])) + untouched = Download(url='https://example.com/other', downloader='video_channel', tag_names=['two'], + settings=dict(tag_names=['two'], channel_tag_name=['two'])) + test_session.add_all([download, untouched]) + test_session.commit() + + await tag.update_tag(test_session, 'New Name', None) + await await_background_tasks() + test_session.commit() + + assert download.tag_names == ['New Name', 'two'] + assert download.settings == dict(tag_names=['two', 'New Name'], channel_tag_name=['New Name'], + video_resolutions=['720p']) + assert untouched.tag_names == ['two'] + assert untouched.settings == dict(tag_names=['two'], channel_tag_name=['two']) + + @pytest.mark.asyncio async def test_tags_directory_file_replaced(test_session, test_directory, tag_factory, video_bytes, await_switches): """Tags directory hardlinks should update when source file is replaced."""