diff --git a/wrolpi/tags.py b/wrolpi/tags.py index e5181e79..dfd079d2 100644 --- a/wrolpi/tags.py +++ b/wrolpi/tags.py @@ -183,18 +183,19 @@ class Tag(ModelHelper, Base): self.flush() # Rename old name referenced in any Downloads, in the caller's transaction. (A background task's writes - # would not be committed with it.) - from wrolpi.downloader import Download - to_flush = list() - for download in session.query(Download): - 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: - session.flush(to_flush) - save_downloads_config.activate_switch() + # would not be committed with it.) The caller saves the Downloads config after it commits. + if old_name and old_name != name: + from wrolpi.downloader import Download + to_flush = list() + downloads = session.query(Download).options( + load_only(Download.id, Download.tag_names, Download.settings)) + for download in downloads: + 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: + session.flush(to_flush) from modules.videos.errors import ChannelDirectoryConflict @@ -1138,8 +1139,10 @@ def get_overlapping_tags(session: Session, tag_names: Union[str, List[str]], lim async def upsert_tag(session: Session, name: str, color: str, tag_id: int = None) -> Tag: + old_name = None if tag_id: tag = Tag.find_by_id(session, tag_id) + old_name = tag.name await tag.update_tag(session, name, color) else: tag = Tag() @@ -1157,6 +1160,9 @@ async def upsert_tag(session: Session, name: str, color: str, tag_id: int = None save_tags_config.activate_switch() sync_tags_directory.activate_switch() + if old_name and old_name != tag.name: + # Downloads were renamed; save them only once they are committed. + save_downloads_config.activate_switch() return tag diff --git a/wrolpi/test/test_tags.py b/wrolpi/test/test_tags.py index c6a06797..e2307292 100644 --- a/wrolpi/test/test_tags.py +++ b/wrolpi/test/test_tags.py @@ -596,10 +596,11 @@ async def test_tag_rename_with_channel(test_session, test_directory, video_facto @pytest.mark.asyncio -async def test_tag_rename_downloads(async_client, test_session, tag_factory, await_background_tasks): +async def test_tag_rename_downloads(async_client, test_session, tag_factory, await_background_tasks, await_switches, + test_download_manager_config): """Renaming a Tag renames it everywhere a Download applies it, including the settings passed on to the - Downloads (and Channels) it creates. The rename is committed with the request, and leaves the database - writable.""" + Downloads (and Channels) it creates. The rename is committed with the request, saved to the config, and leaves + the database writable.""" from wrolpi.db import get_db_session from wrolpi.downloader import Download tag, other = await tag_factory('one'), await tag_factory('two') @@ -626,6 +627,12 @@ async def test_tag_rename_downloads(async_client, test_session, tag_factory, awa 'https://example.com/other': (['two'], dict(tag_names=['two'], channel_tag_name=['two'])), } + await await_switches() + config = yaml.load(test_download_manager_config.read_text(), Loader=yaml.Loader) + playlist, = [i for i in config['downloads'] if i['url'] == 'https://example.com/playlist'] + assert playlist['tag_names'] == ['New Name', 'two'] + assert playlist['settings']['channel_tag_name'] == ['New Name'] + # Nothing holds the write lock. request, response = await async_client.post('/api/tag', content=json.dumps(dict(name='three', color='#000000'))) assert response.status_code == HTTPStatus.CREATED