rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-26 19:01:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

Ensuring domain config saves when tagging/untagging a domain.

Browse source
This commit is contained in:
Roland 2025-12-06 09:27:40 -07:00 • committed by lrnselfreliance
parent 16d24a588d
commit 47044cdd1f
17 changed files with 651 additions and 114 deletions
Download patch file Download diff file Expand all files Collapse all files

1
app/src/components/Archive.js
View file

@ -597,6 +597,7 @@ export function DomainEditPage() {
getTagInfo={handleGetTagInfo}
onSave={handleTagSave}
collectionName="Domain"
hasDirectory={!!domain?.directory}
/>
{/* Downloads Segment */}

1
app/src/components/Channels.js
View file

@ -280,6 +280,7 @@ export function ChannelEditPage() {
getTagInfo={handleGetTagInfo}
onSave={handleTagSave}
collectionName="Channel"
hasDirectory={!!channel.directory}
/>
{/* Downloads Segment */}

56
app/src/components/collections/CollectionTagModal.js
View file

@ -16,6 +16,8 @@ import {APIButton, Toggle} from '../Common';
* @param {Function} getTagInfo - Async function to fetch tag info: (tagName) => Promise<{suggested_directory, conflict, conflict_message}>
* @param {Function} onSave - Async function called when saving: (tagName, directory) => Promise<void>
* @param {string} collectionName - Name of the collection for toast messages (e.g., "Domain", "Channel")
* @param {boolean} hasDirectory - Whether the collection has a directory (default true for backward compatibility)
* When false, hides directory toggle and input, always passes null for directory
*/
export function CollectionTagModal({
open,
@ -25,6 +27,7 @@ export function CollectionTagModal({
getTagInfo,
onSave,
collectionName = 'Collection',
hasDirectory = true,
}) {
const [newTagName, setNewTagName] = useState(currentTagName || null);
const [moveToTagDirectory, setMoveToTagDirectory] = useState(true);
@ -86,12 +89,15 @@ export function CollectionTagModal({
// Handle save
const handleSave = useCallback(async () => {
await onSave(newTagName, moveToTagDirectory ? newTagDirectory : null);
// When hasDirectory is false, always pass null for directory
const directoryToSave = hasDirectory && moveToTagDirectory ? newTagDirectory : null;
await onSave(newTagName, directoryToSave);
handleClose();
}, [newTagName, moveToTagDirectory, newTagDirectory, onSave, handleClose]);
}, [newTagName, hasDirectory, moveToTagDirectory, newTagDirectory, onSave, handleClose]);
const modalTitle = currentTagName ? 'Modify Tag' : 'Add Tag';
const saveButtonText = moveToTagDirectory ? 'Move' : 'Save';
// Show "Move" button only when hasDirectory is true and move toggle is on
const saveButtonText = hasDirectory && moveToTagDirectory ? 'Move' : 'Save';
return (
<Modal
@ -112,26 +118,30 @@ export function CollectionTagModal({
/>
</Grid.Column>
</Grid.Row>
<Grid.Row>
<Grid.Column>
<Toggle
label='Move to directory: '
checked={moveToTagDirectory}
onChange={setMoveToTagDirectory}
/>
</Grid.Column>
</Grid.Row>
<Grid.Row>
<Grid.Column>
<Input
fluid
value={newTagDirectory}
onChange={(e, {value}) => setNewTagDirectory(value)}
disabled={!moveToTagDirectory}
/>
</Grid.Column>
</Grid.Row>
{conflictMessage && (
{hasDirectory && (
<Grid.Row>
<Grid.Column>
<Toggle
label='Move to directory: '
checked={moveToTagDirectory}
onChange={setMoveToTagDirectory}
/>
</Grid.Column>
</Grid.Row>
)}
{hasDirectory && (
<Grid.Row>
<Grid.Column>
<Input
fluid
value={newTagDirectory}
onChange={(e, {value}) => setNewTagDirectory(value)}
disabled={!moveToTagDirectory}
/>
</Grid.Column>
</Grid.Row>
)}
{hasDirectory && conflictMessage && (
<Grid.Row>
<Grid.Column>
<Message warning>

235
app/src/components/collections/CollectionTagModal.test.js
View file

@ -439,4 +439,239 @@ describe('CollectionTagModal', () => {
expect(mockOnClose).toHaveBeenCalled();
});
});
describe('hasDirectory prop', () => {
it('hides move toggle and directory input when hasDirectory is false', () => {
// TDD test - should FAIL initially because hasDirectory prop doesn't exist yet
render(
<CollectionTagModal
{...defaultProps}
hasDirectory={false}
originalDirectory={null}
/>
);
// Toggle and directory input should be hidden
expect(screen.queryByTestId('toggle')).not.toBeInTheDocument();
expect(screen.queryByRole('textbox')).not.toBeInTheDocument();
});
it('shows move toggle and directory input when hasDirectory is true', () => {
render(
<CollectionTagModal
{...defaultProps}
hasDirectory={true}
originalDirectory="/some/directory"
/>
);
// Toggle and directory input should be visible
expect(screen.getByTestId('toggle')).toBeInTheDocument();
expect(screen.getByRole('textbox')).toBeInTheDocument();
});
it('defaults hasDirectory to true for backward compatibility', () => {
// When hasDirectory is not specified, it should default to true (show directory UI)
render(
<CollectionTagModal
{...defaultProps}
originalDirectory="/some/directory"
/>
);
// Toggle and directory input should be visible by default
expect(screen.getByTestId('toggle')).toBeInTheDocument();
expect(screen.getByRole('textbox')).toBeInTheDocument();
});
it('calls onSave with null directory when hasDirectory is false', async () => {
const mockOnClose = jest.fn();
const mockOnSave = jest.fn().mockResolvedValue(undefined);
const mockGetTagInfo = jest.fn().mockResolvedValue({
suggested_directory: null,
conflict: false,
});
render(
<CollectionTagModal
{...defaultProps}
hasDirectory={false}
originalDirectory={null}
onSave={mockOnSave}
onClose={mockOnClose}
getTagInfo={mockGetTagInfo}
/>
);
// Add a tag
const addTagButton = screen.getByTestId('add-tag');
await userEvent.click(addTagButton);
// Wait for getTagInfo to be called
await waitFor(() => {
expect(mockGetTagInfo).toHaveBeenCalled();
});
// Click save button
await act(async () => {
await userEvent.click(screen.getByTestId('api-button-Save'));
});
// Verify onSave was called with null directory
expect(mockOnSave).toHaveBeenCalledWith('test-tag', null);
expect(mockOnClose).toHaveBeenCalled();
});
});
describe('Tag Removal Save Behavior', () => {
it('calls onSave with updated directory after removing tag', async () => {
const mockOnSave = jest.fn().mockResolvedValue(undefined);
const mockOnClose = jest.fn();
const mockGetTagInfo = jest.fn().mockResolvedValue({
suggested_directory: '/videos/wrolpi.org', // Untagged directory
conflict: false,
conflict_message: null,
});
render(
<CollectionTagModal
open={true}
onClose={mockOnClose}
currentTagName="WROL" // Currently has a tag
originalDirectory='/videos/WROL/wrolpi.org' // Current tagged directory
getTagInfo={mockGetTagInfo}
onSave={mockOnSave}
collectionName="Channel"
hasDirectory={true}
/>
);
// Remove the tag - this triggers getTagInfo(null) and should update directory
await userEvent.click(screen.getByTestId('remove-tag'));
// Wait for directory to be updated with the untagged directory
await waitFor(() => {
const input = screen.getByRole('textbox');
expect(input).toHaveValue('/videos/wrolpi.org');
});
// Click Move button to save
await act(async () => {
await userEvent.click(screen.getByTestId('api-button-Move'));
});
// Verify onSave was called with null tag and NEW directory
expect(mockOnSave).toHaveBeenCalledWith(null, '/videos/wrolpi.org');
});
it('calls onSave with updated directory even when clicking save quickly after removing tag', async () => {
// This test simulates a potential race condition where user clicks save
// very quickly after removing the tag, before the async getTagInfo completes
const mockOnSave = jest.fn().mockResolvedValue(undefined);
const mockOnClose = jest.fn();
// Simulate a slow API call
const mockGetTagInfo = jest.fn().mockImplementation(() => {
return new Promise(resolve => {
setTimeout(() => {
resolve({
suggested_directory: '/videos/wrolpi.org',
conflict: false,
conflict_message: null,
});
}, 100); // 100ms delay
});
});
render(
<CollectionTagModal
open={true}
onClose={mockOnClose}
currentTagName="WROL"
originalDirectory='/videos/WROL/wrolpi.org'
getTagInfo={mockGetTagInfo}
onSave={mockOnSave}
collectionName="Channel"
hasDirectory={true}
/>
);
// Remove the tag
await userEvent.click(screen.getByTestId('remove-tag'));
// Wait for the directory to update (ensures async completed)
await waitFor(() => {
const input = screen.getByRole('textbox');
expect(input).toHaveValue('/videos/wrolpi.org');
}, { timeout: 500 });
// Click Move button
await act(async () => {
await userEvent.click(screen.getByTestId('api-button-Move'));
});
// Verify onSave was called with null tag and NEW directory
expect(mockOnSave).toHaveBeenCalledWith(null, '/videos/wrolpi.org');
});
it('does NOT send stale directory if user clicks save before async completes', async () => {
// This test checks what happens if user manages to click save BEFORE getTagInfo resolves
// This would be the actual bug scenario
const mockOnSave = jest.fn().mockResolvedValue(undefined);
const mockOnClose = jest.fn();
// Create a promise that we can control
let resolveGetTagInfo;
const mockGetTagInfo = jest.fn().mockImplementation(() => {
return new Promise(resolve => {
resolveGetTagInfo = resolve;
});
});
render(
<CollectionTagModal
open={true}
onClose={mockOnClose}
currentTagName="WROL"
originalDirectory='/videos/WROL/wrolpi.org'
getTagInfo={mockGetTagInfo}
onSave={mockOnSave}
collectionName="Channel"
hasDirectory={true}
/>
);
// Remove the tag - triggers getTagInfo but doesn't resolve yet
await userEvent.click(screen.getByTestId('remove-tag'));
// getTagInfo should have been called
expect(mockGetTagInfo).toHaveBeenCalledWith(null);
// At this point, directory is still the old value
const input = screen.getByRole('textbox');
expect(input).toHaveValue('/videos/WROL/wrolpi.org');
// Now resolve the getTagInfo promise
await act(async () => {
resolveGetTagInfo({
suggested_directory: '/videos/wrolpi.org',
conflict: false,
conflict_message: null,
});
});
// Wait for directory to update
await waitFor(() => {
expect(input).toHaveValue('/videos/wrolpi.org');
});
// NOW click save
await act(async () => {
await userEvent.click(screen.getByTestId('api-button-Move'));
});
// Verify onSave was called with the CORRECT directory
expect(mockOnSave).toHaveBeenCalledWith(null, '/videos/wrolpi.org');
});
});
});

11
modules/archive/lib.py
View file

@ -123,15 +123,8 @@ class DomainsConfig(ConfigFile):
collection_data = collection_data.copy()
collection_data['kind'] = 'domain'
# Warn if tag_name provided without directory
tag_name = collection_data.get('tag_name')
directory = collection_data.get('directory')
if tag_name and not directory:
logger.warning(
f"Domain collection '{name}' has tag_name '{tag_name}' "
f"but no directory - tags require a directory. Tag will be ignored."
)
collection_data.pop('tag_name', None)
# Note: Collections can now be tagged even without a directory
# Tags enable UI search/filtering for directory-less collections
valid_data_list.append(collection_data)
except Exception as e:

106
modules/archive/test/test_domain_tagging.py
View file

@ -20,22 +20,6 @@ async def test_create_domain_collection_with_directory(test_session, test_direct
test_session.commit()
assert collection.directory == domain_dir
assert collection.can_be_tagged is True
@pytest.mark.asyncio
async def test_create_unrestricted_domain_collection(test_session):
"""Unrestricted domain collection (no directory) cannot be tagged."""
collection = Collection(
name='example.com',
kind='domain',
directory=None # Unrestricted
)
test_session.add(collection)
test_session.commit()
assert collection.directory is None
assert collection.can_be_tagged is False
@pytest.mark.asyncio
@ -63,8 +47,8 @@ async def test_tag_domain_collection_with_directory(async_client, test_session,
@pytest.mark.asyncio
async def test_cannot_tag_unrestricted_domain_collection(async_client, test_session, tag_factory):
"""Domain collection without directory cannot be tagged."""
async def test_tag_unrestricted_domain_collection(async_client, test_session, tag_factory):
"""Domain collection without directory can be tagged for UI searchability."""
collection = Collection(
name='example.com',
kind='domain',
@ -74,10 +58,14 @@ async def test_cannot_tag_unrestricted_domain_collection(async_client, test_sess
tag = await tag_factory(name='News')
test_session.commit()
assert collection.can_be_tagged is False
# Collections can now be tagged even without a directory
collection.set_tag('News')
test_session.commit()
with pytest.raises(ValueError, match='Cannot tag domain collection.*without a directory'):
collection.set_tag('News')
assert collection.tag is not None
assert collection.tag.name == 'News'
# Directory should still be None
assert collection.directory is None
@pytest.mark.asyncio
@ -169,13 +157,11 @@ collections:
assert 'example.com' in str(collection.directory)
assert collection.tag is not None
assert collection.tag.name == 'News'
assert collection.can_be_tagged is True
@pytest.mark.asyncio
async def test_domain_config_warns_tag_without_directory(test_session, test_directory, tag_factory, caplog,
async_client):
"""Config warns when tag_name provided without directory."""
async def test_domain_config_with_tag_without_directory(test_session, test_directory, tag_factory, async_client):
"""Config can have tag_name without directory - tag enables UI search."""
from modules.archive.lib import DomainsConfig
# Create config file with tag but no directory
@ -195,18 +181,16 @@ collections:
domains_config = DomainsConfig()
domains_config.import_config(config_file)
# Verify collection created without tag
# Verify collection created with tag but no directory
collection = test_session.query(Collection).filter_by(
name='example.com',
kind='domain'
).one()
assert collection.directory is None
assert collection.tag is None # Tag ignored
assert collection.can_be_tagged is False
# Check warning was logged
assert "tags require a directory" in caplog.text
# Tag is now applied even without directory - enables UI search
assert collection.tag is not None
assert collection.tag.name == 'News'
@pytest.mark.asyncio
@ -268,7 +252,6 @@ async def test_get_or_create_domain_collection_with_directory(test_session, test
assert collection.name == 'test.com'
assert collection.kind == 'domain'
assert collection.directory == domain_dir
assert collection.can_be_tagged is True
@pytest.mark.asyncio
@ -311,3 +294,62 @@ async def test_get_archive_destination_unrestricted(test_session):
expected = get_archive_directory() / 'example.com'
assert destination == expected
@pytest.mark.asyncio
async def test_tag_domain_without_directory_saves_config(test_session, test_directory, tag_factory, await_switches):
"""Tagging domain collection without directory should still save config."""
from modules.archive.lib import domains_config
from wrolpi.collections.lib import tag_collection
import yaml
# Create domain collection without directory
collection = Collection(name='example.com', kind='domain', directory=None)
test_session.add(collection)
await tag_factory(name='News')
test_session.commit()
# Tag using library function (no directory)
tag_collection(collection.id, tag_name='News', directory=None, session=test_session)
test_session.commit()
# Wait for switches to process
await await_switches()
# Verify config was saved (file created by switch, just read it)
config_file = domains_config.get_file()
data = yaml.safe_load(config_file.read_text())
domain_entry = next((c for c in data.get('collections', []) if c['name'] == 'example.com'), None)
assert domain_entry is not None, "Domain collection not found in config"
assert domain_entry.get('tag_name') == 'News', "Tag name not saved in config"
@pytest.mark.asyncio
async def test_untag_domain_without_directory_saves_config(test_session, test_directory, tag_factory, await_switches):
"""Untagging domain collection without directory should still save config."""
from modules.archive.lib import domains_config
from wrolpi.collections.lib import tag_collection
import yaml
# Create domain collection without directory, already tagged
collection = Collection(name='example.com', kind='domain', directory=None)
test_session.add(collection)
tag = await tag_factory(name='News')
collection.tag = tag
test_session.commit()
# Untag using library function (tag_name=None)
tag_collection(collection.id, tag_name=None, directory=None, session=test_session)
test_session.commit()
# Wait for switches to process
await await_switches()
# Verify config was saved without tag (file created by switch, just read it)
config_file = domains_config.get_file()
data = yaml.safe_load(config_file.read_text())
domain_entry = next((c for c in data.get('collections', []) if c['name'] == 'example.com'), None)
assert domain_entry is not None, "Domain collection not found in config"
assert domain_entry.get('tag_name') is None, "Tag should be removed from config"

2
modules/videos/channel/lib.py
View file

@ -283,6 +283,8 @@ async def tag_channel(tag_name: str | None, directory: pathlib.Path | None, chan
save_channels_config.activate_switch()
else:
logger.info(f'Tagging {channel} with {tag_name}')
# Always save config when tag changes, even without directory
save_channels_config.activate_switch()
@optional_session

53
modules/videos/channel/test/test_lib.py
View file

@ -146,3 +146,56 @@ async def test_tag_channel_existing(async_client, test_session, test_directory,
with test_channels_config.open() as fh:
config = yaml.load(fh, Loader=yaml.Loader)
assert config['channels'][0]['tag_name'] == tag.name
@pytest.mark.asyncio
async def test_tag_channel_without_directory_saves_config(
test_session, test_directory, channel_factory, tag_factory, test_channels_config, await_switches
):
"""Tagging channel without providing directory should still save config."""
channel_directory = test_directory / 'videos' / 'Channel Name'
channel = channel_factory(name='Channel Name', directory=channel_directory)
test_session.commit()
await await_switches()
tag = await tag_factory(name='Tech')
# Tag without providing directory (directory=None)
await lib.tag_channel(tag.name, None, channel.id, test_session)
test_session.commit()
await await_switches()
# Channel's tag should be saved to the config even without directory
with test_channels_config.open() as fh:
config = yaml.load(fh, Loader=yaml.Loader)
channel_entry = next((c for c in config.get('channels', []) if c['name'] == 'Channel Name'), None)
assert channel_entry is not None, "Channel not found in config"
assert channel_entry.get('tag_name') == 'Tech', "Tag name not saved in config"
@pytest.mark.asyncio
async def test_untag_channel_without_directory_saves_config(
test_session, test_directory, channel_factory, tag_factory, test_channels_config, await_switches
):
"""Untagging channel without providing directory should still save config."""
channel_directory = test_directory / 'videos' / 'Channel Name'
channel = channel_factory(name='Channel Name', directory=channel_directory)
tag = await tag_factory(name='Tech')
channel.set_tag(tag.name)
test_session.commit()
await await_switches()
# Verify tag is set
assert channel.tag is not None
# Untag without providing directory (tag_name=None, directory=None)
await lib.tag_channel(None, None, channel.id, test_session)
test_session.commit()
await await_switches()
# Channel's tag should be removed in the config
with test_channels_config.open() as fh:
config = yaml.load(fh, Loader=yaml.Loader)
channel_entry = next((c for c in config.get('channels', []) if c['name'] == 'Channel Name'), None)
assert channel_entry is not None, "Channel not found in config"
assert channel_entry.get('tag_name') is None, "Tag should be removed from config"

58
wrolpi/collections/lib.py
View file

@ -302,7 +302,7 @@ def refresh_collection(collection_id: int, send_events: bool = True) -> None:
Events.send_directory_refresh(f'Refreshing: {relative_dir}')
@optional_session
@optional_session(commit=True)
def tag_collection(
collection_id: int,
tag_name: Optional[str] = None,
@ -333,22 +333,35 @@ def tag_collection(
# If tag_name is None, remove the tag from the collection
if tag_name is None:
collection.tag_id = None
# Update directory if provided (user may want to move files when removing tag)
old_directory = collection.directory
if directory:
target_directory = validate_collection_directory(directory)
collection.directory = target_directory
else:
target_directory = collection.directory
session.flush()
# Trigger domain config save if this is a domain collection
# Trigger config save based on collection kind
if collection.kind == 'domain':
# Local import to avoid circular import: collections -> archive -> collections
from modules.archive.lib import save_domains_config
save_domains_config.activate_switch()
elif collection.kind == 'channel':
# Local import to avoid circular import: collections -> videos -> collections
from modules.videos.lib import save_channels_config
save_channels_config.activate_switch()
# Return info about the un-tagging operation
relative_dir = get_relative_to_media_directory(collection.directory) if collection.directory else None
relative_dir = get_relative_to_media_directory(target_directory) if target_directory else None
return {
'collection_id': collection.id,
'collection_name': collection.name,
'tag_name': None,
'directory': str(relative_dir) if relative_dir else None,
'will_move_files': False,
'will_move_files': target_directory is not None and old_directory != target_directory,
}
# Get or create the tag
@ -366,36 +379,36 @@ def tag_collection(
# Use existing directory
target_directory = collection.directory
else:
# Suggest a directory based on collection type and name
if collection.kind == 'domain':
# Local import to avoid circular import: collections -> archive -> collections
from modules.archive.lib import get_archive_directory
base_dir = get_archive_directory()
else:
base_dir = get_media_directory()
target_directory = base_dir / collection.name
# No directory provided and collection has none - keep it that way
# Collections can be tagged without a directory for UI search/filtering
target_directory = None
# Apply the tag
collection.tag_id = tag.id
collection.directory = target_directory
# Only update directory if we have a target (don't auto-generate for directory-less collections)
if target_directory is not None:
collection.directory = target_directory
session.flush()
# Trigger domain config save if this is a domain collection
# Trigger config save based on collection kind
if collection.kind == 'domain':
# Local import to avoid circular import: collections -> archive -> collections
from modules.archive.lib import save_domains_config
save_domains_config.activate_switch()
elif collection.kind == 'channel':
# Local import to avoid circular import: collections -> videos -> collections
from modules.videos.lib import save_channels_config
save_channels_config.activate_switch()
# Return info about the tagging operation
relative_dir = get_relative_to_media_directory(target_directory)
relative_dir = get_relative_to_media_directory(target_directory) if target_directory else None
return {
'collection_id': collection.id,
'collection_name': collection.name,
'tag_name': tag_name,
'directory': str(relative_dir),
'will_move_files': collection.directory != target_directory,
'directory': str(relative_dir) if relative_dir else None,
'will_move_files': target_directory is not None and collection.directory != target_directory,
}
@ -409,6 +422,7 @@ def get_tag_info(
Get information about tagging a collection with a specific tag.
Returns the suggested directory and checks for conflicts with existing collections.
For collections without a directory, returns suggested_directory=None.
Args:
collection_id: The collection ID
@ -426,6 +440,14 @@ def get_tag_info(
if not collection:
raise UnknownCollection(f"Collection with ID {collection_id} not found")
# For collections without a directory, return None - no directory suggestions needed
if collection.directory is None:
return {
'suggested_directory': None,
'conflict': False,
'conflict_message': None,
}
# Use the collection's format_directory method to get the suggested directory
suggested_directory = collection.format_directory(tag_name)

35
wrolpi/collections/models.py
View file

@ -349,8 +349,9 @@ class Collection(ModelHelper, Base):
).all()
existing_by_directory = {str(c.directory): c for c in existing}
# Step 3: Pre-fetch existing collections by (name, kind) for directory-less collections
name_kind_pairs = [(d['name'], d.get('kind', 'channel')) for d in data_list if not d.get('directory')]
# Step 3: Pre-fetch existing collections by (name, kind) for ALL collections
# This handles cases where directory changed or directory-less collections
name_kind_pairs = [(d['name'], d.get('kind', 'channel')) for d in data_list]
existing_by_name_kind = {}
if name_kind_pairs:
from sqlalchemy import and_, or_
@ -400,10 +401,12 @@ class Collection(ModelHelper, Base):
directory_str = None
# Find existing collection from pre-fetched data
# Priority: 1) by directory (if provided), 2) by (name, kind) as fallback
collection = None
if directory_str:
collection = existing_by_directory.get(directory_str)
else:
# Always check by (name, kind) as fallback - handles directory changes
if not collection:
collection = existing_by_name_kind.get((name, kind))
if collection:
@ -783,16 +786,9 @@ class Collection(ModelHelper, Base):
def set_tag(self, tag_id_or_name: int | str | None) -> Optional[Tag]:
"""Assign or clear the Tag on this Collection. Mirrors Channel.set_tag semantics.
Raises:
ValueError: If attempting to tag a domain collection without a directory.
All collections can be tagged regardless of directory status. Tags enable
UI search/filtering even for directory-less collections.
"""
# Validate domain collections can only be tagged if they have a directory
if self.kind == 'domain' and self.directory is None and tag_id_or_name is not None:
raise ValueError(
f"Cannot tag domain collection '{self.name}' without a directory. "
"Set collection.directory first or use an unrestricted domain collection."
)
session = Session.object_session(self)
if tag_id_or_name is None:
self.tag = None
@ -803,20 +799,6 @@ class Collection(ModelHelper, Base):
self.tag_id = self.tag.id if self.tag else None
return self.tag
@property
def can_be_tagged(self) -> bool:
"""Check if this Collection can be tagged.
Domain collections require a directory to be tagged.
Other collection types can always be tagged.
Returns:
True if this collection can be tagged, False otherwise.
"""
if self.kind == 'domain':
return self.directory is not None
return True
def __json__(self) -> dict:
"""Return JSON-serializable dict for API responses.
@ -838,7 +820,6 @@ class Collection(ModelHelper, Base):
'directory': directory_str,
'description': self.description,
'tag_name': self.tag.name if self.tag else None,
'can_be_tagged': self.can_be_tagged,
'item_count': self.item_count,
'total_size': self.total_size,
'downloads': self.downloads,

70
wrolpi/collections/test/test_api.py
View file

@ -427,3 +427,73 @@ class TestCollectionTagging:
# Verify the tag has been removed
assert collection.tag_id is None
assert collection.tag_name is None
@pytest.mark.asyncio
async def test_tag_collection_without_directory(
self, async_client, test_session, tag_factory
):
"""Test that tagging a collection without a directory works."""
# Create a tag
tag = await tag_factory('TestTag')
test_session.flush()
# Create a domain collection WITHOUT a directory
collection = Collection(
name='example.com',
kind='domain',
directory=None # No directory!
)
test_session.add(collection)
test_session.commit()
# Verify collection has no directory
assert collection.directory is None
# Send POST request to tag the collection
request, response = await async_client.post(
f'/api/collections/{collection.id}/tag',
json={'tag_name': 'TestTag'}
)
# Check response - should succeed
assert response.status_code == HTTPStatus.OK
# Refresh the collection from database
test_session.refresh(collection)
# Verify the tag was set
assert collection.tag_id == tag.id
assert collection.tag_name == 'TestTag'
# Directory should still be None - not auto-generated
assert collection.directory is None
@pytest.mark.asyncio
async def test_get_tag_info_without_directory(
self, async_client, test_session
):
"""Test that get_tag_info returns None for suggested_directory when collection has no directory."""
# Create a domain collection WITHOUT a directory
collection = Collection(
name='example.com',
kind='domain',
directory=None # No directory!
)
test_session.add(collection)
test_session.commit()
# Request tag info
request, response = await async_client.post(
f'/api/collections/{collection.id}/tag_info',
json={'tag_name': 'TestTag'}
)
# Check response
assert response.status_code == HTTPStatus.OK
data = response.json
# For collections without a directory, suggested_directory should be None
# (Currently it always computes a directory path)
assert data['suggested_directory'] is None
# No conflict checking needed without a directory
assert data['conflict'] is False
assert data['conflict_message'] is None

76
wrolpi/collections/test/test_collection_tagging.py
View file

@ -143,3 +143,79 @@ async def test_tagging_unrestricted_collection_updates_config_only(
entry = next(c for c in dumped if c.get('name') == 'Loose Stuff')
assert entry.get('tag_name') == 'Favorites'
assert 'directory' not in entry
def test_set_tag_on_directory_less_domain_collection(test_session):
"""Test that set_tag works on domain collections without a directory."""
from wrolpi.tags import Tag
# Create a domain collection without a directory
collection = Collection(name='example.com', kind='domain', directory=None)
test_session.add(collection)
# Create a tag directly
tag = Tag(name='TestTag', color='#ff0000')
test_session.add(tag)
test_session.commit()
# This should NOT raise ValueError - it should successfully set the tag
# (Currently raises: ValueError: Cannot tag domain collection 'example.com' without a directory)
collection.set_tag('TestTag')
test_session.commit()
# Verify tag was set
assert collection.tag_id == tag.id
assert collection.tag_name == 'TestTag'
# Directory should still be None
assert collection.directory is None
@pytest.mark.asyncio
async def test_tag_collection_removes_tag_and_updates_directory(test_session, test_directory, tag_factory, await_switches):
"""When removing a tag via tag_collection, the directory should be updated if provided."""
from wrolpi.collections.lib import tag_collection
# Create a domain collection with a tag and tagged directory
tagged_dir = test_directory / 'archive' / 'News' / 'example.com'
tagged_dir.mkdir(parents=True, exist_ok=True)
collection = Collection(
name='example.com',
kind='domain',
directory=tagged_dir,
)
test_session.add(collection)
# Create tag using factory
tag = await tag_factory(name='News')
collection.tag = tag
test_session.commit()
# Verify initial state
assert collection.tag_name == 'News'
assert 'News' in str(collection.directory)
# New directory without the tag
untagged_dir = test_directory / 'archive' / 'example.com'
untagged_dir.mkdir(parents=True, exist_ok=True)
# Remove the tag AND update the directory
result = tag_collection(
collection_id=collection.id,
tag_name=None, # Remove tag
directory=str(untagged_dir), # New directory
session=test_session
)
test_session.commit()
await await_switches()
# Verify tag was removed
assert collection.tag_id is None
assert collection.tag_name is None
# Verify directory was updated (THIS IS THE BUG - directory is not updated)
assert collection.directory == untagged_dir, \
f"Expected directory to be {untagged_dir}, but got {collection.directory}"
# Verify result contains correct directory
assert result['directory'] == str(untagged_dir.relative_to(test_directory))

4
wrolpi/common.py
View file

@ -546,7 +546,9 @@ class ConfigFile:
raise RuntimeError(f'Refusing to save config because it was never successfully imported! {rel_path}')
version = self.read_config_file(file).get('version')
if version and version > self.version:
raise RuntimeError(f'Refusing to overwrite newer config ({rel_path}): {version} > {self.version}')
message = f'Refusing to overwrite newer config ({rel_path}): {version} > {self.version}'
Events.send_config_save_failed(message)
raise RuntimeError(message)
# Don't overwrite a real config while testing.
if PYTEST and not is_tempfile(file):

8
wrolpi/contexts.py
View file

@ -29,6 +29,7 @@ def attach_shared_contexts(app: Sanic):
app.shared_ctx.channels_config = manager.dict()
app.shared_ctx.download_manager_config = manager.dict()
app.shared_ctx.videos_downloader_config = manager.dict()
app.shared_ctx.domains_config = manager.dict()
# Shared dicts.
app.shared_ctx.refresh = manager.dict()
app.shared_ctx.uploaded_files = manager.dict()
@ -84,6 +85,7 @@ def reset_shared_contexts(app: Sanic):
app.shared_ctx.channels_config.clear()
app.shared_ctx.download_manager_config.clear()
app.shared_ctx.videos_downloader_config.clear()
app.shared_ctx.domains_config.clear()
# Shared dicts.
app.shared_ctx.refresh.clear()
app.shared_ctx.uploaded_files.clear()
@ -180,3 +182,9 @@ def initialize_configs_contexts(app: Sanic):
DOWNLOAD_MANAGER_CONFIG.initialize(app.shared_ctx.download_manager_config)
except Exception as e:
logger.error(f'Failed to initialize in-memory download manager config: {e}')
try:
from modules.archive.lib import domains_config
domains_config.initialize(app.shared_ctx.domains_config)
except Exception as e:
logger.error(f'Failed to initialize in-memory domains config: {e}')

4
wrolpi/db.py
View file

@ -157,10 +157,8 @@ def optional_session(commit: Union[callable, bool] = False) -> callable:
if commit:
session.commit()
else:
with get_db_session() as session:
with get_db_session(commit=commit) as session:
result = func(*args, session=session, **kwargs)
if commit:
session.commit()
return result
def wrapper(*w_args, **w_kwargs):

3
wrolpi/files/test/test_ebooks.py
View file

@ -113,7 +113,8 @@ async def test_search_ebooks(test_session, async_client, example_epub):
@pytest.mark.asyncio
async def test_discover_calibre_cover(test_session, test_directory, example_epub, example_mobi, image_file):
async def test_discover_calibre_cover(test_session, async_client, test_directory, example_epub, example_mobi,
image_file):
"""Calibre puts a cover near an ebook file, test if it can be found."""
# Create a Calibre metadata file.
metadata = test_directory / 'metadata.opf'

42
wrolpi/test/test_contexts.py Normal file
View file

@ -0,0 +1,42 @@
"""Tests for shared context initialization."""
import pytest
def test_domains_config_in_shared_context():
"""Verify domains_config is initialized in shared context.
This is critical for multi-process deployments. Without proper initialization,
domains_config.version won't be shared across processes, causing config save failures.
The bug this catches: domains_config was missing from attach_shared_contexts() and
initialize_configs_contexts() in wrolpi/contexts.py, causing version tracking to fail
in production multi-process environment.
"""
from sanic import Sanic
from wrolpi.contexts import attach_shared_contexts, initialize_configs_contexts
# Create a fresh Sanic app for this test to avoid polluting global state
test_app = Sanic(f"test_context_{id(test_domains_config_in_shared_context)}")
# Initialize shared contexts as done in production
attach_shared_contexts(test_app)
initialize_configs_contexts(test_app)
# Verify domains_config has a shared context
assert hasattr(test_app.shared_ctx, 'domains_config'), \
"domains_config not found in shared_ctx - add it to attach_shared_contexts() in contexts.py"
# Verify all other expected configs are also present
expected_configs = [
'wrolpi_config',
'tags_config',
'inventories_config',
'channels_config',
'download_manager_config',
'videos_downloader_config',
'domains_config',
]
for config_name in expected_configs:
assert hasattr(test_app.shared_ctx, config_name), \
f"{config_name} not found in shared_ctx"