From a6314bb71c872eeb3eb7d69260afe5b4a431ba85 Mon Sep 17 00:00:00 2001 From: Roland Date: Fri, 5 Dec 2025 17:12:21 -0700 Subject: [PATCH] Removing collection metadata. --- .../migrate_download_channel_to_collection.py | 53 ++++ app/src/api.js | 4 +- app/src/components/Archive.js | 48 ++-- app/src/components/Channels.js | 21 +- app/src/components/DomainEditPage.test.js | 11 +- app/src/components/DomainTagging.test.js | 35 ++- app/src/components/DomainsPage.test.js | 38 +-- .../collections/CollectionEditForm.js | 29 +-- .../collections/CollectionEditForm.test.js | 120 ++++++--- .../components/collections/CollectionTable.js | 142 +++++------ .../collections/CollectionTable.test.js | 228 +++++++++++------- app/src/hooks/customHooks.js | 40 +-- app/src/test-utils.js | 31 --- wrolpi/collections/api.py | 4 - wrolpi/collections/lib.py | 107 -------- wrolpi/collections/test/test_api.py | 80 +----- wrolpi/test/test_downloader.py | 35 +++ 17 files changed, 493 insertions(+), 533 deletions(-) diff --git a/alembic/versions/migrate_download_channel_to_collection.py b/alembic/versions/migrate_download_channel_to_collection.py index 5e328232..8b5be1a8 100644 --- a/alembic/versions/migrate_download_channel_to_collection.py +++ b/alembic/versions/migrate_download_channel_to_collection.py @@ -71,6 +71,45 @@ def upgrade(): op.drop_column('download', 'channel_id') print("Done\n") + # Step 6: Fix the update_channel_minimum_frequency trigger function + # It was referencing channel_id which no longer exists + print("Step 6: Updating update_channel_minimum_frequency trigger function...") + op.execute(""" + CREATE OR REPLACE FUNCTION update_channel_minimum_frequency() + RETURNS TRIGGER AS + $$ + DECLARE + v_channel_id INTEGER; + BEGIN + -- Handle INSERT and UPDATE: use NEW record + IF (TG_OP = 'DELETE') THEN + -- For DELETE, we need to find the channel from OLD record + IF OLD.collection_id IS NOT NULL THEN + SELECT id INTO v_channel_id FROM channel WHERE collection_id = OLD.collection_id; + IF v_channel_id IS NOT NULL THEN + UPDATE channel + SET minimum_frequency = (SELECT MIN(frequency) FROM download WHERE collection_id = OLD.collection_id) + WHERE id = v_channel_id; + END IF; + END IF; + RETURN OLD; + ELSE + -- For INSERT and UPDATE, use NEW record + IF NEW.collection_id IS NOT NULL THEN + SELECT id INTO v_channel_id FROM channel WHERE collection_id = NEW.collection_id; + IF v_channel_id IS NOT NULL THEN + UPDATE channel + SET minimum_frequency = (SELECT MIN(frequency) FROM download WHERE collection_id = NEW.collection_id) + WHERE id = v_channel_id; + END IF; + END IF; + RETURN NEW; + END IF; + END; + $$ LANGUAGE plpgsql; + """) + print("Done\n") + print("=" * 60) print("Migration Complete") print("=" * 60 + "\n") @@ -83,6 +122,20 @@ def downgrade(): bind = op.get_bind() session = Session(bind=bind) + # Restore the old trigger function that uses channel_id + op.execute(""" + CREATE OR REPLACE FUNCTION update_channel_minimum_frequency() + RETURNS TRIGGER AS + $$ + BEGIN + UPDATE channel + SET minimum_frequency = (SELECT MIN(frequency) FROM download WHERE channel_id = NEW.channel_id) + WHERE id = NEW.channel_id; + RETURN NEW; + END; + $$ LANGUAGE plpgsql; + """) + # Add back channel_id column op.add_column('download', sa.Column('channel_id', sa.Integer(), nullable=True)) diff --git a/app/src/api.js b/app/src/api.js index a29922e1..fd38689c 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -722,7 +722,7 @@ export async function fetchDomains() { const response = await apiGet(`${COLLECTIONS_API}?kind=domain`); if (response.ok) { let data = await response.json(); - return [data['collections'], data['totals']['collections'], data['metadata']]; + return [data['collections'], data['totals']['collections']]; } else { const message = await getErrorMessage(response, 'Unable to fetch Domains. See server logs.'); toast({ @@ -738,7 +738,7 @@ export async function fetchChannels() { const response = await apiGet(`${COLLECTIONS_API}?kind=channel`); if (response.ok) { let data = await response.json(); - return [data['collections'], data['totals']['collections'], data['metadata']]; + return [data['collections'], data['totals']['collections']]; } else { const message = await getErrorMessage(response, 'Unable to fetch Channels. See server logs.'); toast({ diff --git a/app/src/components/Archive.js b/app/src/components/Archive.js index 6b25c825..1d10af19 100644 --- a/app/src/components/Archive.js +++ b/app/src/components/Archive.js @@ -38,7 +38,6 @@ import {Link, Route, Routes, useLocation, useNavigate, useParams} from "react-ro import Message from "semantic-ui-react/dist/commonjs/collections/Message"; import { useArchive, - useCollectionMetadata, useDomain, useDomains, useOneQuery, @@ -356,10 +355,27 @@ export function ArchiveCard({file}) { } +// Domain table column configuration +const DOMAIN_COLUMNS = [ + {key: 'domain', label: 'Domain', sortable: true, width: 7}, + {key: 'tag_name', label: 'Tag', sortable: true, width: 2}, + {key: 'archive_count', label: 'Archives', sortable: true, align: 'right', width: 2}, + {key: 'min_download_frequency', label: 'Download Frequency', sortable: true, format: 'frequency', width: 2, hideOnMobile: true}, + {key: 'size', label: 'Size', sortable: true, align: 'right', format: 'bytes', width: 2, hideOnMobile: true}, + {key: 'actions', label: 'Manage', sortable: false, type: 'actions', width: 1} +]; + +const DOMAIN_ROUTES = { + list: '/archive/domains', + edit: '/archive/domain/:id/edit', + search: '/archive', + searchParam: 'domain' +}; + export function DomainsPage() { useTitle('Archive Domains'); - const [domains, total, metadata] = useDomains(); + const [domains] = useDomains(); const [searchStr, setSearchStr] = useOneQuery('domain'); // Header section matching ChannelsPage pattern @@ -406,30 +422,33 @@ export function DomainsPage() { {header} ; } +// Domain form field configuration +const DOMAIN_FIELDS = [ + {key: 'directory', label: 'Directory', type: 'directory', placeholder: 'Optional directory path'}, + // tag_name field is handled via modal button, not inline in form + {key: 'description', label: 'Description', type: 'textarea', placeholder: 'Optional description'} +]; + +const DOMAIN_MESSAGES = { + no_directory: 'Set a directory to enable tagging', + tag_will_move: 'Tagging will move files to a new directory' +}; + export function DomainEditPage() { const {domainId} = useParams(); const navigate = useNavigate(); const {domain, form, fetchDomain} = useDomain(parseInt(domainId)); - const {metadata} = useCollectionMetadata('domain'); // Modal state for tagging const [tagEditModalOpen, setTagEditModalOpen] = useState(false); - // Filter out tag_name field - we use the modal button instead of inline selector - const filteredMetadata = React.useMemo(() => { - if (!metadata) return metadata; - return { - ...metadata, - fields: metadata.fields.filter(field => field.key !== 'tag_name') - }; - }, [metadata]); - useTitle(`Edit Domain: ${domain?.domain || '...'}`); // Wrap form.onSubmit to add toast and refresh domain data @@ -546,7 +565,8 @@ export function DomainEditPage() { ; diff --git a/app/src/components/DomainEditPage.test.js b/app/src/components/DomainEditPage.test.js index 5fd819b8..d6dc43ea 100644 --- a/app/src/components/DomainEditPage.test.js +++ b/app/src/components/DomainEditPage.test.js @@ -2,7 +2,7 @@ import React from 'react'; import {render, renderInDarkMode, screen, waitFor, createTestForm} from '../test-utils'; import userEvent from '@testing-library/user-event'; import {DomainEditPage} from './Archive'; -import {createMockDomain, createMockMetadata} from '../test-utils'; +import {createMockDomain} from '../test-utils'; // Mock useParams to return domain ID jest.mock('react-router-dom', () => ({ @@ -13,12 +13,10 @@ jest.mock('react-router-dom', () => ({ // Mock the useDomain hook const mockUseDomain = jest.fn(); -const mockUseCollectionMetadata = jest.fn(); jest.mock('../hooks/customHooks', () => ({ ...jest.requireActual('../hooks/customHooks'), useDomain: (...args) => mockUseDomain(...args), - useCollectionMetadata: (...args) => mockUseCollectionMetadata(...args), })); // Mock useTitle @@ -29,12 +27,12 @@ jest.mock('./Common', () => ({ // Mock CollectionEditForm jest.mock('./collections/CollectionEditForm', () => ({ - CollectionEditForm: ({form, metadata, title, actionButtons}) => ( + CollectionEditForm: ({form, fields, title, actionButtons}) => (
{title &&

{title}

} {form?.loading &&
Loading...
} {form?.formData &&
Collection data loaded
} - {metadata &&
Metadata loaded
} + {fields &&
Fields configured
} {actionButtons &&
{actionButtons}
}
), @@ -64,11 +62,8 @@ jest.mock('../api', () => ({ })); describe('DomainEditPage', () => { - const mockMetadata = createMockMetadata(); - beforeEach(() => { jest.clearAllMocks(); - mockUseCollectionMetadata.mockReturnValue({metadata: mockMetadata}); }); describe('Loading States', () => { diff --git a/app/src/components/DomainTagging.test.js b/app/src/components/DomainTagging.test.js index 220bf346..950d6ff3 100644 --- a/app/src/components/DomainTagging.test.js +++ b/app/src/components/DomainTagging.test.js @@ -2,7 +2,7 @@ import React from 'react'; import {render, screen, createTestForm} from '../test-utils'; import {CollectionEditForm} from './collections/CollectionEditForm'; import {DomainsPage} from './Archive'; -import {createMockDomain, createMockMetadata, createMockDomains} from '../test-utils'; +import {createMockDomain, createMockDomains} from '../test-utils'; // Mock the TagsSelector component and TagsContext jest.mock('../Tags', () => ({ @@ -90,9 +90,19 @@ jest.mock('./collections/CollectionTable', () => ({ ), })); -describe('Domain Tagging Logic', () => { - const mockMetadata = createMockMetadata(); +// Test field configurations +const DOMAIN_FIELDS = [ + {key: 'directory', label: 'Directory', type: 'directory', placeholder: 'Optional directory path'}, + {key: 'tag_name', label: 'Tag', type: 'tag', placeholder: 'Select or create tag', depends_on: 'directory'}, + {key: 'description', label: 'Description', type: 'textarea', placeholder: 'Optional description'} +]; +const DOMAIN_MESSAGES = { + no_directory: 'Set a directory to enable tagging', + tag_will_move: 'Tagging will move files to a new directory' +}; + +describe('Domain Tagging Logic', () => { describe('Tag Field Dependency on Directory', () => { it('prevents tagging domain without directory', () => { const domainWithoutDirectory = createMockDomain({ @@ -106,7 +116,8 @@ describe('Domain Tagging Logic', () => { render( ); @@ -133,7 +144,8 @@ describe('Domain Tagging Logic', () => { render( ); @@ -160,7 +172,8 @@ describe('Domain Tagging Logic', () => { render( ); @@ -205,7 +218,7 @@ describe('Domain Tagging Logic', () => { }), ]; - mockUseDomains.mockReturnValue([mockDomains, 2, mockMetadata]); + mockUseDomains.mockReturnValue([mockDomains, 2]); render(); @@ -241,7 +254,7 @@ describe('Domain Tagging Logic', () => { }), ]; - mockUseDomains.mockReturnValue([mockDomains, 3, mockMetadata]); + mockUseDomains.mockReturnValue([mockDomains, 3]); render(); @@ -265,7 +278,8 @@ describe('Domain Tagging Logic', () => { render( ); @@ -285,7 +299,8 @@ describe('Domain Tagging Logic', () => { render( ); diff --git a/app/src/components/DomainsPage.test.js b/app/src/components/DomainsPage.test.js index 5ee7c5a0..f07b4785 100644 --- a/app/src/components/DomainsPage.test.js +++ b/app/src/components/DomainsPage.test.js @@ -1,7 +1,7 @@ import React from 'react'; import {render, screen, waitFor} from '../test-utils'; import {DomainsPage} from './Archive'; -import {createMockDomains, createMockMetadata} from '../test-utils'; +import {createMockDomains} from '../test-utils'; // Mock the custom hooks jest.mock('../hooks/customHooks', () => ({ @@ -12,10 +12,11 @@ jest.mock('../hooks/customHooks', () => ({ // Mock CollectionTable component jest.mock('./collections/CollectionTable', () => ({ - CollectionTable: ({collections, metadata, searchStr}) => ( + CollectionTable: ({collections, columns, routes, searchStr}) => (
{collections?.length || 0}
{searchStr}
+
{columns?.length || 0}
{collections?.map((domain) => (
{domain.domain} @@ -47,7 +48,6 @@ jest.mock('./Common', () => ({ describe('DomainsPage', () => { const {useDomains, useOneQuery} = require('../hooks/customHooks'); const {useTitle} = require('./Common'); - const mockMetadata = createMockMetadata(); beforeEach(() => { // Reset mocks before each test @@ -61,7 +61,7 @@ describe('DomainsPage', () => { describe('Page Rendering', () => { it('displays domains page without errors', () => { const mockDomains = createMockDomains(3); - useDomains.mockReturnValue([mockDomains, 3, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 3]); render(); @@ -70,18 +70,20 @@ describe('DomainsPage', () => { expect(screen.getByTestId('collection-table')).toBeInTheDocument(); }); - it('renders CollectionTable component', () => { + it('renders CollectionTable component with columns config', () => { const mockDomains = createMockDomains(2); - useDomains.mockReturnValue([mockDomains, 2, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 2]); render(); expect(screen.getByTestId('collection-table')).toBeInTheDocument(); + // Should have 6 columns defined in DOMAIN_COLUMNS + expect(screen.getByTestId('columns-count')).toHaveTextContent('6'); }); it('shows search input', () => { const mockDomains = createMockDomains(1); - useDomains.mockReturnValue([mockDomains, 1, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 1]); render(); @@ -94,7 +96,7 @@ describe('DomainsPage', () => { describe('Domain Display', () => { it('shows all domains from API', () => { const mockDomains = createMockDomains(5); - useDomains.mockReturnValue([mockDomains, 5, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 5]); render(); @@ -112,7 +114,7 @@ describe('DomainsPage', () => { {id: 2, domain: 'example2.com', archive_count: 20, size: 2000}, {id: 3, domain: 'example3.com', archive_count: 30, size: 3000}, ]; - useDomains.mockReturnValue([mockDomains, 3, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 3]); render(); @@ -123,7 +125,7 @@ describe('DomainsPage', () => { it('displays Edit buttons in Manage column', () => { const mockDomains = createMockDomains(3); - useDomains.mockReturnValue([mockDomains, 3, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 3]); render(); @@ -135,7 +137,7 @@ describe('DomainsPage', () => { it('Edit button has correct styling', () => { const mockDomains = createMockDomains(1); - useDomains.mockReturnValue([mockDomains, 1, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 1]); render(); @@ -150,7 +152,7 @@ describe('DomainsPage', () => { describe('Empty and Error States', () => { it('shows "No items yet" message when no domains', () => { // Empty array indicates no domains - useDomains.mockReturnValue([[], 0, mockMetadata]); + useDomains.mockReturnValue([[], 0]); render(); @@ -164,7 +166,7 @@ describe('DomainsPage', () => { it('shows error message when fetch fails', () => { // undefined indicates error state - useDomains.mockReturnValue([undefined, 0, mockMetadata]); + useDomains.mockReturnValue([undefined, 0]); render(); @@ -178,7 +180,7 @@ describe('DomainsPage', () => { it('does not show "New Domain" button', () => { const mockDomains = createMockDomains(2); - useDomains.mockReturnValue([mockDomains, 2, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 2]); render(); @@ -191,7 +193,7 @@ describe('DomainsPage', () => { describe('Search Integration', () => { it('disables search when no domains', () => { - useDomains.mockReturnValue([[], 0, mockMetadata]); + useDomains.mockReturnValue([[], 0]); render(); @@ -201,7 +203,7 @@ describe('DomainsPage', () => { it('enables search when domains exist', () => { const mockDomains = createMockDomains(3); - useDomains.mockReturnValue([mockDomains, 3, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 3]); render(); @@ -211,7 +213,7 @@ describe('DomainsPage', () => { it('passes search string to CollectionTable', () => { const mockDomains = createMockDomains(2); - useDomains.mockReturnValue([mockDomains, 2, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 2]); const mockSetSearchStr = jest.fn(); useOneQuery.mockReturnValue(['example', mockSetSearchStr]); @@ -226,7 +228,7 @@ describe('DomainsPage', () => { describe('Page Title', () => { it('sets page title correctly', () => { const mockDomains = createMockDomains(1); - useDomains.mockReturnValue([mockDomains, 1, mockMetadata]); + useDomains.mockReturnValue([mockDomains, 1]); render(); diff --git a/app/src/components/collections/CollectionEditForm.js b/app/src/components/collections/CollectionEditForm.js index 03433cb4..fc90f99f 100644 --- a/app/src/components/collections/CollectionEditForm.js +++ b/app/src/components/collections/CollectionEditForm.js @@ -8,13 +8,12 @@ import {InputForm} from '../../hooks/useForm'; /** * Renders a single form field based on field configuration. - * Extracted outside the component to avoid recreation on every render. * - * @param {Object} field - Field configuration from metadata + * @param {Object} field - Field configuration * @param {Object} form - Form object from useForm hook - * @param {Object} metadata - Backend-provided metadata containing messages + * @param {Object} messages - Messages for UI hints (optional) */ -function renderField(field, form, metadata) { +function renderField(field, form, messages = {}) { const disabled = field.depends_on && !form.formData[field.depends_on]; switch (field.type) { @@ -59,7 +58,7 @@ function renderField(field, form, metadata) { {field.required && *} {disabled && - {metadata.messages?.no_directory || 'Set a directory to enable tagging'} + {messages.no_directory || 'Set a directory to enable tagging'} } - {!disabled && tagProps.value && metadata.messages?.tag_will_move && - {metadata.messages.tag_will_move} + {!disabled && tagProps.value && messages.tag_will_move && + {messages.tag_will_move} } ; @@ -100,7 +99,8 @@ function FormSkeleton({fieldCount = 3}) { * Reusable form component for editing collections (Domains, Channels, etc). * * @param {Object} form - Form object from useForm hook - * @param {Object} metadata - Backend-provided metadata containing fields configuration + * @param {Array} fields - Array of field configurations + * @param {Object} messages - Messages for UI hints (optional) * @param {Function} onCancel - Optional callback when cancel is clicked * @param {String} title - Page title to display in header * @param {String} wrolModeContent - Content to show in WROL mode message (optional) @@ -109,7 +109,8 @@ function FormSkeleton({fieldCount = 3}) { */ export function CollectionEditForm({ form, - metadata, + fields, + messages = {}, onCancel, title, wrolModeContent, @@ -118,9 +119,9 @@ export function CollectionEditForm({ }) { const {SingleTag} = React.useContext(TagsContext); - if (!metadata) { + if (!fields || fields.length === 0) { return - No metadata available + No fields configured ; } @@ -143,14 +144,14 @@ export function CollectionEditForm({ } {showSkeleton ? ( - + ) : (
- {metadata.fields.map(field => ( + {fields.map(field => ( - {renderField(field, form, metadata)} + {renderField(field, form, messages)} ))} diff --git a/app/src/components/collections/CollectionEditForm.test.js b/app/src/components/collections/CollectionEditForm.test.js index 6e3e21a2..4da54e51 100644 --- a/app/src/components/collections/CollectionEditForm.test.js +++ b/app/src/components/collections/CollectionEditForm.test.js @@ -1,7 +1,7 @@ import React from 'react'; import {render, renderInDarkMode, renderInLightMode, hasInvertedStyling, screen, waitFor, createTestForm} from '../../test-utils'; import {CollectionEditForm} from './CollectionEditForm'; -import {createMockMetadata, createMockDomain} from '../../test-utils'; +import {createMockDomain} from '../../test-utils'; // Mock the TagsSelector component and TagsContext jest.mock('../../Tags', () => ({ @@ -55,8 +55,19 @@ jest.mock('../../hooks/useForm', () => ({ ), })); +// Test field configurations +const DOMAIN_FIELDS = [ + {key: 'directory', label: 'Directory', type: 'directory', placeholder: 'Optional directory path'}, + {key: 'tag_name', label: 'Tag', type: 'tag', placeholder: 'Select or create tag', depends_on: 'directory'}, + {key: 'description', label: 'Description', type: 'textarea', placeholder: 'Optional description'} +]; + +const DOMAIN_MESSAGES = { + no_directory: 'Set a directory to enable tagging', + tag_will_move: 'Tagging will move files to a new directory' +}; + describe('CollectionEditForm', () => { - const mockMetadata = createMockMetadata(); const mockCollection = createMockDomain(); describe('Form Rendering', () => { @@ -66,11 +77,12 @@ describe('CollectionEditForm', () => { render( ); - // Check that all fields from metadata are rendered + // Check that all fields from config are rendered expect(screen.getByTestId('directory-search')).toBeInTheDocument(); expect(screen.getByTestId('tags-selector')).toBeInTheDocument(); expect(screen.getByPlaceholderText(/optional description/i)).toBeInTheDocument(); @@ -90,7 +102,8 @@ describe('CollectionEditForm', () => { render( ); @@ -110,7 +123,8 @@ describe('CollectionEditForm', () => { render( ); @@ -125,7 +139,8 @@ describe('CollectionEditForm', () => { render( ); @@ -139,7 +154,8 @@ describe('CollectionEditForm', () => { render( ); @@ -152,7 +168,8 @@ describe('CollectionEditForm', () => { render( ); @@ -172,7 +189,8 @@ describe('CollectionEditForm', () => { render( ); @@ -195,7 +213,8 @@ describe('CollectionEditForm', () => { render( ); @@ -219,7 +238,8 @@ describe('CollectionEditForm', () => { render( ); @@ -236,7 +256,8 @@ describe('CollectionEditForm', () => { render( ); @@ -252,7 +273,8 @@ describe('CollectionEditForm', () => { render( ); @@ -263,18 +285,34 @@ describe('CollectionEditForm', () => { }); describe('Error States', () => { - it('handles missing metadata gracefully', () => { + it('handles missing fields gracefully', () => { const form = createTestForm(mockCollection); render( ); - // Should show warning about missing metadata - expect(screen.getByText(/no metadata available/i)).toBeInTheDocument(); + // Should show warning about missing fields + expect(screen.getByText(/no fields configured/i)).toBeInTheDocument(); + }); + + it('handles empty fields array gracefully', () => { + const form = createTestForm(mockCollection); + + render( + + ); + + // Should show warning about missing fields + expect(screen.getByText(/no fields configured/i)).toBeInTheDocument(); }); it('displays form-level errors', () => { @@ -285,7 +323,8 @@ describe('CollectionEditForm', () => { render( ); @@ -303,7 +342,8 @@ describe('CollectionEditForm', () => { const {container} = render( ); @@ -320,7 +360,8 @@ describe('CollectionEditForm', () => { const {container} = render( ); @@ -330,7 +371,7 @@ describe('CollectionEditForm', () => { expect(screen.getByTestId('directory-search')).toBeInTheDocument(); }); - it('renders correct number of skeleton fields based on metadata', () => { + it('renders correct number of skeleton fields based on fields config', () => { const form = createTestForm({}, { overrides: {loading: true} }); @@ -338,13 +379,14 @@ describe('CollectionEditForm', () => { const {container} = render( ); - // Should have skeleton fields equal to metadata.fields.length (3) + // Should have skeleton fields equal to fields.length (3) const skeletonFields = container.querySelectorAll('.form-skeleton-field'); - expect(skeletonFields).toHaveLength(mockMetadata.fields.length); + expect(skeletonFields).toHaveLength(DOMAIN_FIELDS.length); }); }); @@ -355,7 +397,8 @@ describe('CollectionEditForm', () => { const {container} = renderInDarkMode( ); @@ -371,7 +414,8 @@ describe('CollectionEditForm', () => { const {container} = renderInLightMode( ); @@ -387,7 +431,8 @@ describe('CollectionEditForm', () => { const {container} = renderInDarkMode( ); @@ -404,7 +449,8 @@ describe('CollectionEditForm', () => { const {container} = render( ); @@ -417,19 +463,18 @@ describe('CollectionEditForm', () => { describe('CSS Classes', () => { it('uses required-indicator class for required fields', () => { - const metadataWithRequired = createMockMetadata('domain', { - fields: [ - {key: 'name', label: 'Name', type: 'text', required: true}, - {key: 'description', label: 'Description', type: 'textarea', required: true}, - ] - }); + const fieldsWithRequired = [ + {key: 'name', label: 'Name', type: 'text', required: true}, + {key: 'description', label: 'Description', type: 'textarea', required: true}, + ]; const form = createTestForm(mockCollection); const {container} = render( ); @@ -445,7 +490,8 @@ describe('CollectionEditForm', () => { const {container} = render( ); diff --git a/app/src/components/collections/CollectionTable.js b/app/src/components/collections/CollectionTable.js index 570308cb..8f93d44b 100644 --- a/app/src/components/collections/CollectionTable.js +++ b/app/src/components/collections/CollectionTable.js @@ -8,62 +8,36 @@ import {ThemeContext, Media} from '../../contexts/contexts'; import {TagsContext} from '../../Tags'; /** - * Get the ID field from metadata routes configuration. - * @param {Object} metadata - Collection metadata + * Get the ID field from routes configuration. + * @param {Object} routes - Routes configuration * @returns {string} The ID field name */ -function getIdField(metadata) { - return metadata?.routes?.id_field || 'id'; +function getIdField(routes) { + return routes?.id_field || 'id'; } /** - * Calculate column widths based on column metadata. - * Uses Semantic UI's 16-column grid system. - * - * @param {Array} columns - Array of column configurations - * @returns {Array} Array of width values (1-16) - */ -function calculateColumnWidths(columns) { - const TOTAL = 16; - - const widths = columns.map(col => { - if (col.type === 'actions') return 1; - if (col.format === 'bytes' || col.format === 'frequency') return 2; - if (col.align === 'right') return 2; // numeric columns - if (col.key === 'tag_name') return 2; - return null; // grow column - calculate later - }); - - // Distribute remaining space to null (grow) columns - const fixedWidth = widths.filter(w => w !== null).reduce((a, b) => a + b, 0); - const growColumns = widths.filter(w => w === null).length; - const growWidth = Math.floor((TOTAL - fixedWidth) / (growColumns || 1)); - - return widths.map(w => w === null ? growWidth : w); -} - -/** - * Generate a search link for a collection based on metadata routing configuration. + * Generate a search link for a collection based on routes configuration. * Supports both query parameter-based (e.g., ?domain=...) and route-based (e.g., /channel/:id/video) linking. * * @param {Object} collection - The collection object - * @param {Object} metadata - Collection metadata with routes configuration + * @param {Object} routes - Routes configuration * @param {string} primaryKey - The primary column key to use for the value * @returns {string|null} The generated link, or null if no link can be generated */ -function getCollectionSearchLink(collection, metadata, primaryKey) { - const searchRoute = metadata.routes?.search; +function getCollectionSearchLink(collection, routes, primaryKey) { + const searchRoute = routes?.search; if (!searchRoute) { return null; } - // Check if metadata specifies a query parameter to use - if (metadata.routes.searchParam) { + // Check if routes specifies a query parameter to use + if (routes.searchParam) { // Query parameter-based linking (e.g., /archive?domain=example.com) - return `${searchRoute}?${metadata.routes.searchParam}=${collection[primaryKey]}`; + return `${searchRoute}?${routes.searchParam}=${collection[primaryKey]}`; } else if (searchRoute.includes(':id')) { // Route parameter-based linking (e.g., /videos/channel/123/video) - const idField = getIdField(metadata); + const idField = getIdField(routes); return searchRoute.replace(':id', collection[idField]); } @@ -75,22 +49,22 @@ function getCollectionSearchLink(collection, metadata, primaryKey) { * Render a single table row for desktop view. * * @param {Object} collection - The collection data - * @param {Object} metadata - Collection metadata + * @param {Array} columns - Column configurations + * @param {Object} routes - Routes configuration * @param {string} inverted - Inverted theme class * @param {Function} SingleTag - Tag component from context * @param {Function} onRowClick - Optional click handler - * @param {Array} columnWidths - Calculated column widths */ -function renderRow(collection, metadata, inverted, SingleTag, onRowClick, columnWidths) { - const cells = metadata.columns.map((col, index) => { +function renderRow(collection, columns, routes, inverted, SingleTag, onRowClick) { + const cells = columns.map((col) => { let value = collection[col.key]; // Handle actions column - render action buttons if (col.type === 'actions') { - const idField = getIdField(metadata); - const editRoute = metadata.routes?.edit?.replace(':id', collection[idField]); + const idField = getIdField(routes); + const editRoute = routes?.edit?.replace(':id', collection[idField]); const buttonClass = `ui button secondary ${inverted}`; - return + return {editRoute && Edit} ; } @@ -108,14 +82,14 @@ function renderRow(collection, metadata, inverted, SingleTag, onRowClick, column } // Special handling for the primary column (usually domain/name) - if (col.key === metadata.columns[0].key) { - const searchLink = getCollectionSearchLink(collection, metadata, col.key); + if (col.key === columns[0].key) { + const searchLink = getCollectionSearchLink(collection, routes, col.key); if (searchLink) { value = {value}; } } - return + return {value || '-'} ; }); @@ -130,31 +104,26 @@ function renderRow(collection, metadata, inverted, SingleTag, onRowClick, column } /** - * Mobile row component for collections + * Mobile row component for collections - stacked layout for better wrapping of long names */ -function MobileCollectionRow({collection, metadata}) { +function MobileCollectionRow({collection, mobileColumns, routes}) { const {SingleTag} = useContext(TagsContext); - const primaryColumn = metadata.columns[0]; - const idField = getIdField(metadata); - const editRoute = metadata.routes?.edit?.replace(':id', collection[idField]); - const searchLink = getCollectionSearchLink(collection, metadata, primaryColumn.key); + const primaryColumn = mobileColumns[0]; + const idField = getIdField(routes); + const editRoute = routes?.edit?.replace(':id', collection[idField]); + const searchLink = getCollectionSearchLink(collection, routes, primaryColumn.key); return - + {searchLink ? ( - -

- {collection[primaryColumn.key]} -

- {collection.tag_name && } + + {collection[primaryColumn.key]} ) : ( - <> -

{collection[primaryColumn.key]}

- {collection.tag_name && } - + {collection[primaryColumn.key]} )} - {metadata.columns + {collection.tag_name && <> } + {mobileColumns .filter(col => col.type !== 'actions' && col.key !== primaryColumn.key && col.key !== 'tag_name') .map(col => { let value = collection[col.key]; @@ -164,17 +133,15 @@ function MobileCollectionRow({collection, metadata}) { value = formatFrequency(value); } return ( -

+

{col.label}: {value || '-'} -

+
); }) } - -

- {editRoute && Edit} -

+ + {editRoute && Edit} ; } @@ -183,12 +150,13 @@ function MobileCollectionRow({collection, metadata}) { * Reusable table component for displaying collections (Domains, Channels, etc). * * @param {Array} collections - Array of collection objects - * @param {Object} metadata - Backend-provided metadata containing columns, routes, etc. + * @param {Array} columns - Column configurations for the table + * @param {Object} routes - Routes configuration for navigation (edit, search, etc.) * @param {String} searchStr - Search filter string (managed by parent) * @param {Function} onRowClick - Optional callback when a row is clicked * @param {String} emptyMessage - Message to display when there are no collections */ -export function CollectionTable({collections, metadata, searchStr = '', onRowClick, emptyMessage = 'No items yet'}) { +export function CollectionTable({collections, columns, routes = {}, searchStr = '', onRowClick, emptyMessage = 'No items yet'}) { const {inverted} = useContext(ThemeContext); const {SingleTag} = useContext(TagsContext); @@ -216,10 +184,10 @@ export function CollectionTable({collections, metadata, searchStr = '', onRowCli ; } - // No metadata available (backward compatibility) - if (!metadata) { + // No columns configured + if (!columns || columns.length === 0) { return - No metadata available + No columns configured ; } @@ -238,19 +206,17 @@ export function CollectionTable({collections, metadata, searchStr = '', onRowCli }); } - // Calculate column widths dynamically - const columnWidths = calculateColumnWidths(metadata.columns); - - // Build table headers from metadata (desktop) - const headers = metadata.columns.map((col, index) => ({ + // Build table headers from columns (desktop) + const headers = columns.map((col) => ({ key: col.key, text: col.label, sortBy: col.sortable ? col.key : null, - width: columnWidths[index], + width: col.width, })); - // Build mobile headers - simplified columns - const primaryColumn = metadata.columns[0]; + // Build mobile columns (exclude hideOnMobile) and simplified 2-column headers + const mobileColumns = columns.filter(col => !col.hideOnMobile); + const primaryColumn = columns[0]; const mobileHeaders = [ { key: primaryColumn.key, @@ -263,8 +229,8 @@ export function CollectionTable({collections, metadata, searchStr = '', onRowCli } ]; - // Get default sort column from metadata (first column key) - const defaultSortColumn = metadata.columns[0]?.key || 'id'; + // Get default sort column (first column key) + const defaultSortColumn = columns[0]?.key || 'id'; return <> @@ -272,7 +238,7 @@ export function CollectionTable({collections, metadata, searchStr = '', onRowCli } + rowFunc={(collection) => } rowKey='id' tableHeaders={mobileHeaders} defaultSortColumn={defaultSortColumn} @@ -282,7 +248,7 @@ export function CollectionTable({collections, metadata, searchStr = '', onRowCli renderRow(collection, metadata, inverted, SingleTag, onRowClick, columnWidths)} + rowFunc={(collection) => renderRow(collection, columns, routes, inverted, SingleTag, onRowClick)} rowKey='id' tableHeaders={headers} defaultSortColumn={defaultSortColumn} diff --git a/app/src/components/collections/CollectionTable.test.js b/app/src/components/collections/CollectionTable.test.js index 0c249e97..65cf6b11 100644 --- a/app/src/components/collections/CollectionTable.test.js +++ b/app/src/components/collections/CollectionTable.test.js @@ -1,7 +1,7 @@ import React from 'react'; import {render, screen, renderInDarkMode, renderInLightMode, hasInvertedStyling} from '../../test-utils'; import {CollectionTable} from './CollectionTable'; -import {createMockMetadata, createMockDomains} from '../../test-utils'; +import {createMockDomains} from '../../test-utils'; // Mock the TagsContext jest.mock('../../Tags', () => ({ @@ -45,8 +45,23 @@ jest.mock('../SortableTable', () => ({ ), })); +// Test column and routes configurations +const DOMAIN_COLUMNS = [ + {key: 'domain', label: 'Domain', sortable: true, width: 7}, + {key: 'archive_count', label: 'Archives', sortable: true, align: 'right', width: 2}, + {key: 'size', label: 'Size', sortable: true, align: 'right', format: 'bytes', width: 2, hideOnMobile: true}, + {key: 'tag_name', label: 'Tag', sortable: true, width: 2}, + {key: 'actions', label: 'Manage', sortable: false, type: 'actions', width: 1} +]; + +const DOMAIN_ROUTES = { + list: '/archive/domains', + edit: '/archive/domain/:id/edit', + search: '/archive', + searchParam: 'domain' +}; + describe('CollectionTable', () => { - const mockMetadata = createMockMetadata(); const mockCollections = createMockDomains(3); describe('Loading and Error States', () => { @@ -54,7 +69,8 @@ describe('CollectionTable', () => { const {container} = render( ); @@ -65,7 +81,8 @@ describe('CollectionTable', () => { render( ); @@ -76,7 +93,8 @@ describe('CollectionTable', () => { render( ); @@ -87,7 +105,8 @@ describe('CollectionTable', () => { render( ); @@ -95,15 +114,28 @@ describe('CollectionTable', () => { expect(screen.getByText(/no domains found/i)).toBeInTheDocument(); }); - it('renders warning when metadata is null', () => { + it('renders warning when columns is null', () => { render( ); - expect(screen.getByText(/no metadata available/i)).toBeInTheDocument(); + expect(screen.getByText(/no columns configured/i)).toBeInTheDocument(); + }); + + it('renders warning when columns is empty', () => { + render( + + ); + + expect(screen.getByText(/no columns configured/i)).toBeInTheDocument(); }); }); @@ -112,7 +144,8 @@ describe('CollectionTable', () => { render( ); @@ -124,7 +157,8 @@ describe('CollectionTable', () => { render( ); @@ -138,7 +172,8 @@ describe('CollectionTable', () => { render( ); @@ -153,7 +188,8 @@ describe('CollectionTable', () => { render( ); @@ -168,7 +204,8 @@ describe('CollectionTable', () => { render( ); @@ -182,7 +219,8 @@ describe('CollectionTable', () => { render( ); @@ -193,77 +231,50 @@ describe('CollectionTable', () => { }); }); - describe('Column Width Calculation', () => { - it('calculates widths for 5-column metadata', () => { + describe('Column Widths', () => { + it('uses explicit widths from column config', () => { render( ); const desktopView = screen.getByTestId('desktop-view'); const headers = desktopView.querySelectorAll('th'); - // Mock metadata has 5 columns: domain, archive_count, size, tag_name, actions - // archive_count (right-aligned)=2, size (bytes)=2, tag_name=2, actions=1 - // Total fixed = 7, remaining = 16 - 7 = 9 for domain - expect(headers[0]).toHaveAttribute('data-width', '9'); - - // Actions column should be 1 - const actionsHeader = Array.from(headers).find(h => h.textContent === 'Manage'); - expect(actionsHeader).toHaveAttribute('data-width', '1'); - }); - - it('calculates widths for 4-column metadata', () => { - const fourColumnMetadata = { - ...mockMetadata, - columns: [ - {key: 'name', label: 'Name', sortable: true}, - {key: 'count', label: 'Count', sortable: true, align: 'right'}, - {key: 'size', label: 'Size', sortable: true, format: 'bytes'}, - {key: 'actions', label: 'Manage', type: 'actions'}, - ] - }; - - render( - - ); - - const desktopView = screen.getByTestId('desktop-view'); - const headers = desktopView.querySelectorAll('th'); - - // count (right-aligned)=2, size (bytes)=2, actions=1 - // Total fixed = 5, remaining = 16 - 5 = 11 for name - expect(headers[0]).toHaveAttribute('data-width', '11'); - }); - - it('distributes space among multiple grow columns', () => { - const twoGrowMetadata = { - ...mockMetadata, - columns: [ - {key: 'name', label: 'Name', sortable: true}, - {key: 'description', label: 'Description', sortable: false}, - {key: 'actions', label: 'Manage', type: 'actions'}, - ] - }; - - render( - - ); - - const desktopView = screen.getByTestId('desktop-view'); - const headers = desktopView.querySelectorAll('th'); - - // actions=1, remaining = 15 / 2 grow columns = 7 each + // DOMAIN_COLUMNS has explicit widths: domain=7, archive_count=2, size=2, tag_name=2, actions=1 expect(headers[0]).toHaveAttribute('data-width', '7'); - expect(headers[1]).toHaveAttribute('data-width', '7'); + expect(headers[1]).toHaveAttribute('data-width', '2'); + expect(headers[2]).toHaveAttribute('data-width', '2'); + expect(headers[3]).toHaveAttribute('data-width', '2'); + expect(headers[4]).toHaveAttribute('data-width', '1'); + }); + + it('uses widths from 4-column config', () => { + const fourColumns = [ + {key: 'name', label: 'Name', sortable: true, width: 9}, + {key: 'count', label: 'Count', sortable: true, align: 'right', width: 2}, + {key: 'size', label: 'Size', sortable: true, format: 'bytes', width: 2}, + {key: 'actions', label: 'Manage', type: 'actions', width: 1}, + ]; + + render( + + ); + + const desktopView = screen.getByTestId('desktop-view'); + const headers = desktopView.querySelectorAll('th'); + + expect(headers[0]).toHaveAttribute('data-width', '9'); + expect(headers[1]).toHaveAttribute('data-width', '2'); + expect(headers[2]).toHaveAttribute('data-width', '2'); + expect(headers[3]).toHaveAttribute('data-width', '1'); }); }); @@ -274,7 +285,8 @@ describe('CollectionTable', () => { render( ); @@ -292,7 +304,8 @@ describe('CollectionTable', () => { render( ); @@ -306,7 +319,8 @@ describe('CollectionTable', () => { render( ); @@ -328,7 +342,8 @@ describe('CollectionTable', () => { render( ); @@ -350,7 +365,8 @@ describe('CollectionTable', () => { render( ); @@ -358,20 +374,47 @@ describe('CollectionTable', () => { }); }); - describe('Links', () => { - it('renders search link on primary column when searchParam is configured', () => { - const metadataWithSearchParam = { - ...mockMetadata, - routes: { - ...mockMetadata.routes, - searchParam: 'domain' - } - }; + describe('Mobile View', () => { + it('hides columns with hideOnMobile in mobile view', () => { + const collectionsWithData = [{ + id: 1, + domain: 'test.com', + archive_count: 5, + size: 1048576, + tag_name: 'News', + }]; + render( + + ); + + const mobileView = screen.getByTestId('mobile-view'); + const mobileHeaders = mobileView.querySelectorAll('th'); + + // Mobile uses simplified 2-column layout: primary column + Manage + expect(mobileHeaders).toHaveLength(2); + expect(mobileHeaders[0]).toHaveTextContent('Domain'); + expect(mobileHeaders[1]).toHaveTextContent('Manage'); + + // Archives should be visible in row content (no hideOnMobile) + expect(mobileView).toHaveTextContent('Archives:'); + + // Size should be hidden in row content (hideOnMobile: true) + expect(mobileView).not.toHaveTextContent('Size:'); + }); + }); + + describe('Links', () => { + it('renders search link on primary column when searchParam is configured', () => { render( ); @@ -385,7 +428,8 @@ describe('CollectionTable', () => { render( ); diff --git a/app/src/hooks/customHooks.js b/app/src/hooks/customHooks.js index 26b815a9..b6dfc0f4 100644 --- a/app/src/hooks/customHooks.js +++ b/app/src/hooks/customHooks.js @@ -194,21 +194,17 @@ export const useAllQuery = (name) => { export const useDomains = () => { const [domains, setDomains] = useState(null); const [total, setTotal] = useState(null); - const [metadata, setMetadata] = useState(null); const _fetchDomains = async () => { setDomains(null); setTotal(0); - setMetadata(null); try { - let [domains, total, metadata] = await fetchDomains(); + let [domains, total] = await fetchDomains(); setDomains(domains); setTotal(total); - setMetadata(metadata); } catch (e) { setDomains(undefined); // Display error. setTotal(0); - setMetadata(undefined); } } @@ -216,39 +212,7 @@ export const useDomains = () => { _fetchDomains(); }, []); - return [domains, total, metadata]; -} - -export const useCollectionMetadata = (kind) => { - const [metadata, setMetadata] = useState(null); - const [loading, setLoading] = useState(false); - const [error, setError] = useState(null); - - const fetchMetadata = async () => { - setLoading(true); - setError(null); - try { - if (kind === 'domain') { - const [, , metadata] = await fetchDomains(); - setMetadata(metadata); - } else { - // Future: Support channels metadata - setMetadata(null); - } - } catch (e) { - console.error(e); - setError(e); - setMetadata(null); - } finally { - setLoading(false); - } - }; - - useEffect(() => { - fetchMetadata(); - }, [kind]); - - return {metadata, loading, error, fetchMetadata}; + return [domains, total]; } export const useDomain = (domainId) => { diff --git a/app/src/test-utils.js b/app/src/test-utils.js index 826dbe99..49d9dde5 100644 --- a/app/src/test-utils.js +++ b/app/src/test-utils.js @@ -58,37 +58,6 @@ export function renderWithProviders( return render(ui, {wrapper: Wrapper, ...renderOptions}); } -/** - * Creates a mock collection metadata object for testing - */ -export function createMockMetadata(kind = 'domain', overrides = {}) { - return { - kind, - columns: [ - {key: 'domain', label: 'Domain', sortable: true}, - {key: 'archive_count', label: 'Archives', sortable: true, align: 'right'}, - {key: 'size', label: 'Size', sortable: true, align: 'right', format: 'bytes'}, - {key: 'tag_name', label: 'Tag', sortable: true}, - {key: 'actions', label: 'Manage', sortable: false, type: 'actions'}, - ], - fields: [ - {key: 'directory', label: 'Directory', type: 'directory', placeholder: 'Optional directory path'}, - {key: 'tag_name', label: 'Tag', type: 'tag', placeholder: 'Select or create tag', depends_on: 'directory'}, - {key: 'description', label: 'Description', type: 'textarea', placeholder: 'Optional description'}, - ], - routes: { - list: '/archive/domains', - edit: '/archive/domain/:id/edit', - search: '/archive', - }, - messages: { - no_directory: 'Set a directory to enable tagging', - tag_will_move: 'Tagging will move files to a new directory' - }, - ...overrides - }; -} - /** * Creates a mock domain collection object for testing */ diff --git a/wrolpi/collections/api.py b/wrolpi/collections/api.py index 0b0dd313..3078899e 100644 --- a/wrolpi/collections/api.py +++ b/wrolpi/collections/api.py @@ -44,15 +44,11 @@ async def get_collections_endpoint(request: Request): collections = lib.get_collections(kind=kind) - # Include metadata if a specific kind is requested response_data = { 'collections': collections, 'totals': {'collections': len(collections)} } - if kind: - response_data['metadata'] = lib.get_collection_metadata(kind) - return json_response(response_data) diff --git a/wrolpi/collections/lib.py b/wrolpi/collections/lib.py index 165b71cb..526b7ae5 100644 --- a/wrolpi/collections/lib.py +++ b/wrolpi/collections/lib.py @@ -23,7 +23,6 @@ logger = logger.getChild(__name__) __all__ = [ 'get_collections', 'get_collection_with_stats', - 'get_collection_metadata', 'update_collection', 'refresh_collection', 'tag_collection', @@ -193,112 +192,6 @@ def get_collection_with_stats(collection_id: int, session: Session = None) -> di return data -def get_collection_metadata(kind: str) -> dict: - """ - Get metadata for a collection kind. - - Metadata includes: - - Column definitions for table display - - Field configurations for edit forms - - Route information for navigation - - User-facing messages - - Args: - kind: Collection kind ('domain', 'channel', etc.) - - Returns: - Metadata dict with columns, fields, routes, and messages - """ - if kind == 'domain': - return { - 'kind': 'domain', - 'columns': [ - {'key': 'domain', 'label': 'Domain', 'sortable': True}, - {'key': 'tag_name', 'label': 'Tag', 'sortable': True}, - {'key': 'archive_count', 'label': 'Archives', 'sortable': True, 'align': 'right'}, - {'key': 'min_download_frequency', 'label': 'Download Frequency', 'sortable': True, 'format': 'frequency'}, - {'key': 'size', 'label': 'Size', 'sortable': True, 'align': 'right', 'format': 'bytes'}, - {'key': 'actions', 'label': 'Manage', 'sortable': False, 'type': 'actions'} - ], - 'fields': [ - {'key': 'directory', 'label': 'Directory', 'type': 'directory', - 'placeholder': 'Optional directory path'}, - {'key': 'tag_name', 'label': 'Tag', 'type': 'tag', - 'placeholder': 'Select or create tag', 'depends_on': 'directory'}, - {'key': 'description', 'label': 'Description', 'type': 'textarea', - 'placeholder': 'Optional description'} - ], - 'routes': { - 'list': '/archive/domains', - 'edit': '/archive/domain/:id/edit', - 'search': '/archive', - 'searchParam': 'domain' - }, - 'messages': { - 'no_directory': 'Set a directory to enable tagging', - 'tag_will_move': 'Tagging will move files to a new directory' - } - } - elif kind == 'channel': - return { - 'kind': 'channel', - 'columns': [ - {'key': 'name', 'label': 'Name', 'sortable': True}, - {'key': 'tag_name', 'label': 'Tag', 'sortable': True}, - {'key': 'video_count', 'label': 'Videos', 'sortable': True, 'align': 'right'}, - {'key': 'min_download_frequency', 'label': 'Download Frequency', 'sortable': True, 'format': 'frequency'}, - {'key': 'total_size', 'label': 'Size', 'sortable': True, 'align': 'right', 'format': 'bytes'}, - {'key': 'actions', 'label': 'Manage', 'sortable': False, 'type': 'actions'} - ], - 'fields': [ - {'key': 'name', 'label': 'Name', 'type': 'text', 'required': True}, - {'key': 'url', 'label': 'URL', 'type': 'text', 'placeholder': 'Channel URL'}, - {'key': 'directory', 'label': 'Directory', 'type': 'directory', 'required': True}, - {'key': 'tag_name', 'label': 'Tag', 'type': 'tag', - 'placeholder': 'Select or create tag'}, - {'key': 'description', 'label': 'Description', 'type': 'textarea', - 'placeholder': 'Optional description'} - ], - 'routes': { - 'list': '/videos/channels', - 'edit': '/videos/channel/:id/edit', - 'search': '/videos/channel/:id/video', - 'id_field': 'channel_id' # Use channel_id instead of collection id for URLs - }, - 'messages': { - 'no_directory': 'Directory is required for channels', - 'tag_will_move': 'Tagging will move files to a new directory' - } - } - else: - # Generic metadata for unknown kinds - return { - 'kind': kind or 'collection', - 'columns': [ - {'key': 'name', 'label': 'Name', 'sortable': True}, - {'key': 'item_count', 'label': 'Items', 'sortable': True, 'align': 'right'}, - {'key': 'total_size', 'label': 'Size', 'sortable': True, 'align': 'right', 'format': 'bytes'}, - {'key': 'tag_name', 'label': 'Tag', 'sortable': True}, - {'key': 'actions', 'label': 'Manage', 'sortable': False, 'type': 'actions'} - ], - 'fields': [ - {'key': 'name', 'label': 'Name', 'type': 'text', 'required': True}, - {'key': 'directory', 'label': 'Directory', 'type': 'directory'}, - {'key': 'tag_name', 'label': 'Tag', 'type': 'tag'}, - {'key': 'description', 'label': 'Description', 'type': 'textarea'} - ], - 'routes': { - 'list': '/collections', - 'edit': '/collection/:id/edit', - 'search': '/search' - }, - 'messages': { - 'no_directory': 'Set a directory to enable tagging', - 'tag_will_move': 'Tagging will move files to a new directory' - } - } - - @optional_session def update_collection( collection_id: int, diff --git a/wrolpi/collections/test/test_api.py b/wrolpi/collections/test/test_api.py index 9c1012b2..52b741dc 100644 --- a/wrolpi/collections/test/test_api.py +++ b/wrolpi/collections/test/test_api.py @@ -10,10 +10,10 @@ class TestCollectionsAPI: """Test the GET /api/collections endpoint.""" @pytest.mark.asyncio - async def test_get_collections_returns_metadata_for_domains( + async def test_get_collections_for_domains( self, async_client, test_session ): - """Test that GET /api/collections?kind=domain returns metadata.""" + """Test that GET /api/collections?kind=domain returns domain collections.""" # Create a test domain collection collection = Collection(name='example.com', kind='domain') test_session.add(collection) @@ -29,7 +29,6 @@ class TestCollectionsAPI: # Check basic response structure assert 'collections' in data assert 'totals' in data - assert 'metadata' in data # Check collections assert len(data['collections']) == 1 @@ -39,48 +38,11 @@ class TestCollectionsAPI: # Check totals assert data['totals']['collections'] == 1 - # Check metadata structure - metadata = data['metadata'] - assert metadata['kind'] == 'domain' - assert 'columns' in metadata - assert 'fields' in metadata - assert 'routes' in metadata - assert 'messages' in metadata - - # Check metadata columns - columns = metadata['columns'] - assert len(columns) > 0 - column_keys = [col['key'] for col in columns] - assert 'domain' in column_keys - assert 'archive_count' in column_keys - assert 'size' in column_keys - assert 'tag_name' in column_keys - assert 'actions' in column_keys - - # Check metadata fields - fields = metadata['fields'] - assert len(fields) > 0 - field_keys = [field['key'] for field in fields] - assert 'directory' in field_keys - assert 'tag_name' in field_keys - assert 'description' in field_keys - - # Check metadata routes - routes = metadata['routes'] - assert routes['list'] == '/archive/domains' - assert routes['edit'] == '/archive/domain/:id/edit' - assert routes['search'] == '/archive' - - # Check metadata messages - messages = metadata['messages'] - assert 'no_directory' in messages - assert 'tag_will_move' in messages - @pytest.mark.asyncio - async def test_get_collections_returns_metadata_for_channels( + async def test_get_collections_for_channels( self, async_client, test_session ): - """Test that GET /api/collections?kind=channel returns channel metadata.""" + """Test that GET /api/collections?kind=channel returns channel collections.""" from pathlib import Path # Create a test channel collection @@ -99,27 +61,16 @@ class TestCollectionsAPI: assert response.status_code == HTTPStatus.OK data = response.json - # Check metadata structure - assert 'metadata' in data - metadata = data['metadata'] - assert metadata['kind'] == 'channel' - - # Check channel-specific metadata - column_keys = [col['key'] for col in metadata['columns']] - assert 'name' in column_keys - assert 'video_count' in column_keys - assert 'total_size' in column_keys - - # Check channel routes - routes = metadata['routes'] - assert routes['list'] == '/videos/channels' - assert routes['edit'] == '/videos/channel/:id/edit' + # Check collections + assert len(data['collections']) == 1 + assert data['collections'][0]['name'] == 'Test Channel' + assert data['collections'][0]['kind'] == 'channel' @pytest.mark.asyncio - async def test_get_collections_no_metadata_without_kind( + async def test_get_collections_without_kind( self, async_client, test_session ): - """Test that GET /api/collections without kind parameter does not include metadata.""" + """Test that GET /api/collections without kind parameter returns all collections.""" # Create test collections of different kinds domain = Collection(name='example.com', kind='domain') channel = Collection(name='Test Channel', kind='channel') @@ -137,14 +88,11 @@ class TestCollectionsAPI: assert len(data['collections']) == 2 assert data['totals']['collections'] == 2 - # Should NOT have metadata (since no specific kind was requested) - assert 'metadata' not in data - @pytest.mark.asyncio - async def test_get_empty_collections_with_metadata( + async def test_get_empty_collections( self, async_client, test_session ): - """Test that metadata is returned even when no collections exist.""" + """Test that GET /api/collections returns empty when no collections exist.""" # Don't create any collections # Make the request @@ -158,10 +106,6 @@ class TestCollectionsAPI: assert len(data['collections']) == 0 assert data['totals']['collections'] == 0 - # Should still have metadata - assert 'metadata' in data - assert data['metadata']['kind'] == 'domain' - @pytest.mark.asyncio async def test_get_collections_includes_video_count_for_channels( self, async_client, test_session, test_directory, channel_factory, video_factory diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py index e5f0105b..f559e33f 100644 --- a/wrolpi/test/test_downloader.py +++ b/wrolpi/test/test_downloader.py @@ -533,3 +533,38 @@ async def test_download_excluded_urls(test_session, test_download_manager, test_ assert len(downloads) == 3, 'Domain was not ignored' assert [i.url for i in downloads] == \ ['https://example.com/feed', 'https://example.com/a', 'https://example.com/c'], f'Domain was not ignored' + + +@pytest.mark.asyncio +async def test_download_with_collection_id(test_session, test_download_manager, test_downloader): + """Creating a Download with a collection_id should work correctly. + + This test verifies that downloads can be created and associated with collections. + Note: The trigger update_channel_minimum_frequency is not tested here because + pytest test DBs use Base.metadata.create_all() which doesn't run migrations. + The trigger fix is tested by running against a real DB with migrations applied. + """ + from wrolpi.collections import Collection + + # Create a Collection + collection = Collection( + name='Test Collection', + kind='channel', + directory='/tmp/test', + ) + test_session.add(collection) + test_session.flush([collection]) + + # Creating a recurring download with collection_id should work + download = test_download_manager.recurring_download( + 'https://example.com/test', + DownloadFrequency.weekly, + test_downloader.name, + collection_id=collection.id, + ) + test_session.commit() + + # Verify the download was created successfully + assert download.id is not None + assert download.collection_id == collection.id + assert download.frequency == DownloadFrequency.weekly