From 2217f7f13e3f0ac3ceb26107a6b8490ce5bd5545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Molina?= Date: Tue, 9 Jun 2026 14:41:16 +0200 Subject: [PATCH 1/8] test: mock plugin manifest so manage pages render in tests --- affiliation_extras/conftest.py | 22 +++++++++++++ .../client/dashboard/types.ts | 32 ------------------- 2 files changed, 22 insertions(+), 32 deletions(-) create mode 100644 affiliation_extras/conftest.py delete mode 100644 affiliation_extras/indico_affiliation_extras/client/dashboard/types.ts diff --git a/affiliation_extras/conftest.py b/affiliation_extras/conftest.py new file mode 100644 index 0000000..9121d33 --- /dev/null +++ b/affiliation_extras/conftest.py @@ -0,0 +1,22 @@ +# This file is part of the third-party Indico plugins. +# Copyright (C) 2026 CERN +# +# The third-party Indico plugins are free software; you can +# redistribute them and/or modify them under the terms of the; +# MIT License see the LICENSE file for more details. + +import pytest + +from indico.core.plugins import IndicoPlugin + + +@pytest.fixture(autouse=True) +def _plugin_manifest(mocker): + """Mock the plugin's webpack manifest. + + Plugin assets are not built when running tests, so ``IndicoPlugin.manifest`` + returns ``None`` and any page that injects a plugin bundle raises a + ``RuntimeError``. Indico core mocks its own manifest the same way in + ``make_test_client``; we do the same for plugin bundles here. + """ + mocker.patch.object(IndicoPlugin, 'manifest') diff --git a/affiliation_extras/indico_affiliation_extras/client/dashboard/types.ts b/affiliation_extras/indico_affiliation_extras/client/dashboard/types.ts deleted file mode 100644 index 6aa31c7..0000000 --- a/affiliation_extras/indico_affiliation_extras/client/dashboard/types.ts +++ /dev/null @@ -1,32 +0,0 @@ -// This file is part of the third-party Indico plugins. -// Copyright (C) 2026 CERN -// -// The third-party Indico plugins are free software; you can -// redistribute them and/or modify them under the terms of the; -// MIT License see the LICENSE file for more details. - -import {SemanticCOLORS} from 'semantic-ui-react'; - -import {Affiliation} from 'indico/modules/users/affiliations/types'; - -import {ContactList} from './components/ContactListField'; - -export interface GroupInfo { - id: number; - name: string; - code: string; -} - -export interface TagInfo { - id: number; - name: string; - code: string; - color: SemanticCOLORS; -} - -export interface ExtendedAffiliation extends Affiliation { - contact_lists: ContactList[]; - groups: GroupInfo[]; - tags: TagInfo[]; - group_tags: TagInfo[]; -} From 55d671349b69e2de0d4991178ace0f2a427e885a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Molina?= Date: Tue, 9 Jun 2026 14:41:18 +0200 Subject: [PATCH 2/8] fix: keep unchanged representation value valid after catalog changes --- .../indico_affiliation_extras/fields.py | 18 ++++++++++---- .../indico_affiliation_extras/fields_test.py | 24 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/affiliation_extras/indico_affiliation_extras/fields.py b/affiliation_extras/indico_affiliation_extras/fields.py index b6a870b..01c5e82 100644 --- a/affiliation_extras/indico_affiliation_extras/fields.py +++ b/affiliation_extras/indico_affiliation_extras/fields.py @@ -69,21 +69,29 @@ def _validate_representation(value): affiliation_text = affiliation['text'] affiliation_id = affiliation['id'] if self.form_item.is_required and representation_id is None: - raise ValidationError('Please select a representation type') + raise ValidationError(_('Please select a representation type')) if representation_id is None and affiliation_id is not None: - raise ValidationError('Please select a representation type') + raise ValidationError(_('Please select a representation type')) if affiliation_id is None: if affiliation_text or representation_id is not None or self.form_item.is_required: - raise ValidationError('Please select an affiliation from the list') + raise ValidationError(_('Please select an affiliation from the list')) return _validate_representation def process_form_data(self, registration, value, old_data=None, billable_items_locked=False): + # Skip re-validation when the stored value is unchanged: the catalog/list config + # may have changed since submission (list disabled/removed, default catalog + # switched), and unrelated edits to the registration must still succeed. + if old_data is not None and value == old_data.data: + return RegistrationFormFieldBase.process_form_data( + self, registration, value, old_data, billable_items_locked + ) + event = self.form_item.registration_form.event representation_id = value['representation_id'] affiliation_list = get_representation_affiliation_list(event, representation_id) if representation_id is not None and affiliation_list is None: - raise ValidationError('Invalid representation type') + raise ValidationError(_('Invalid representation type')) affiliation = value['affiliation'] if affiliation['id'] is not None: @@ -103,7 +111,7 @@ def process_form_data(self, registration, value, old_data=None, billable_items_l if filters: query = query.filter(*filters) if not (matched_affiliation := query.one_or_none()): - raise ValidationError('Invalid affiliation') + raise ValidationError(_('Invalid affiliation')) affiliation['text'] = matched_affiliation.name value['representation_name'] = affiliation_list.name if affiliation_list else '' diff --git a/affiliation_extras/indico_affiliation_extras/fields_test.py b/affiliation_extras/indico_affiliation_extras/fields_test.py index 403493d..e8dbbaa 100644 --- a/affiliation_extras/indico_affiliation_extras/fields_test.py +++ b/affiliation_extras/indico_affiliation_extras/fields_test.py @@ -136,6 +136,30 @@ def test_representation_field_canonicalizes_and_snapshots_value(db, representati } +def test_representation_field_keeps_unchanged_value_when_list_disabled(db, representation_field, dummy_reg): + affiliation = Affiliation(name='CERN') + db.session.add(affiliation) + db.session.flush() + affiliation_list = _create_affiliation_list( + db, representation_field.registration_form.event, name='Delegates', affiliations={affiliation} + ) + stored = { + 'representation_id': affiliation_list.id, + 'representation_name': 'Delegates', + 'affiliation': {'id': affiliation.id, 'text': 'CERN'}, + } + old_data = RegistrationData(registration=dummy_reg, field_data=representation_field.current_data, data=stored) + db.session.flush() + # The list is disabled after submission; re-saving the registration (e.g. editing an + # unrelated field) must not re-validate the unchanged value against the live config. + affiliation_list.is_enabled = False + db.session.flush() + + rv = representation_field.field_impl.process_form_data(dummy_reg, dict(stored), old_data=old_data) + + assert rv == {} + + def test_representation_field_renders_summary_and_reglist_data(representation_field): registration_data = RegistrationData( field_data=representation_field.current_data, From afb1c7370e0f151443462a220484badcd1d8e78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Molina?= Date: Tue, 9 Jun 2026 14:41:19 +0200 Subject: [PATCH 3/8] perf: resolve inherited default catalogs without per-ancestor queries --- .../indico_affiliation_extras/util.py | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/affiliation_extras/indico_affiliation_extras/util.py b/affiliation_extras/indico_affiliation_extras/util.py index c7dfd50..9956559 100644 --- a/affiliation_extras/indico_affiliation_extras/util.py +++ b/affiliation_extras/indico_affiliation_extras/util.py @@ -24,6 +24,7 @@ from indico.modules.events.models.events import Event from indico.modules.files.models.files import File from indico.modules.users.models.affiliations import Affiliation +from indico.util.i18n import _ from indico.util.signing import secure_serializer from indico_affiliation_extras.models.catalogs import AffiliationCatalog @@ -186,9 +187,9 @@ def populate_contacts(affiliation: Affiliation, contact_lists: list[dict]) -> tu else: contact_id = contact.id if contact_id in used_ids: - raise UserValueError('Contact list IDs must be unique') + raise UserValueError(_('Contact list IDs must be unique')) if contact_id not in existing_by_id: - raise UserValueError('Contact list does not belong to this affiliation') + raise UserValueError(_('Contact list does not belong to this affiliation')) touched_ids.add(contact_id) used_ids.add(contact_id) contact.name = contact_data['name'] @@ -275,7 +276,7 @@ def _apply_catalog_lists(catalog: AffiliationCatalog, catalog_lists: list[dict]) db.session.add(list_obj) else: if list_obj.id not in existing_by_id: - raise UserValueError('List does not belong to this catalog') + raise UserValueError(_('List does not belong to this catalog')) touched_ids.add(list_obj.id) list_obj.name = list_data['name'].strip() list_obj.is_enabled = list_data['is_enabled'] @@ -376,14 +377,24 @@ def _get_catalog_setting(target: Category | Event): def _get_default_catalog_on_category(category: Category, *, only_inherited: bool = False): - if not only_inherited: - catalog = _get_catalog_setting(category) - if catalog: - return catalog - parent_chain = category.parent_chain_query.all() - for parent in reversed(parent_chain): - catalog = _get_catalog_setting(parent) - if catalog: + # Fetch every catalog usable anywhere in this category's chain in a single query, then + # resolve the default per category without re-querying get_all_catalogs for each ancestor. + chain_catalogs = get_all_catalogs(category) + + def _resolve(target: Category): + catalog_id = category_settings.get(target, 'default_catalog_id') + if not catalog_id: + return None + target_chain_ids = {categ['id'] for categ in target.chain} + return next( + (c for c in chain_catalogs if c.id == catalog_id and c.category_id in target_chain_ids), + None, + ) + + if not only_inherited and (catalog := _resolve(category)): + return catalog + for parent in reversed(category.parent_chain_query.all()): + if catalog := _resolve(parent): return catalog return None From 407a89450c2b1295bc96e372c20e325260a4c1d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Molina?= Date: Tue, 9 Jun 2026 14:41:20 +0200 Subject: [PATCH 4/8] fix: reject duplicate affiliation list names in a catalog --- .../models/catalogs_test.py | 39 +++++++++++++++++++ .../indico_affiliation_extras/schemas.py | 9 +++++ 2 files changed, 48 insertions(+) diff --git a/affiliation_extras/indico_affiliation_extras/models/catalogs_test.py b/affiliation_extras/indico_affiliation_extras/models/catalogs_test.py index 35002dc..329ad18 100644 --- a/affiliation_extras/indico_affiliation_extras/models/catalogs_test.py +++ b/affiliation_extras/indico_affiliation_extras/models/catalogs_test.py @@ -148,6 +148,45 @@ def test_event_catalog_api_crud_clone_and_toggle_default(test_client, db, dummy_ assert {entry.meta.get('affiliation_catalog_id') for entry in event_log_entries} == {created_id, clone_id} +@pytest.mark.usefixtures('no_csrf_check') +def test_event_catalog_api_rejects_duplicate_list_names(test_client, db, dummy_user, create_category, create_event): + category = create_category(title='Category') + category.update_principal(dummy_user, full_access=True) + event = create_event(category=category) + event.update_principal(dummy_user, full_access=True) + affiliation = _create_affiliation(db) + _login(test_client, dummy_user) + + payload = { + 'name': 'Catalog', + 'lists': [ + { + 'id': None, + 'name': 'Members', + 'position': 1, + 'is_enabled': True, + 'groups': [], + 'tags': [], + 'affiliations': [affiliation.id], + }, + { + 'id': None, + 'name': 'members', + 'position': 2, + 'is_enabled': True, + 'groups': [], + 'tags': [], + 'affiliations': [affiliation.id], + }, + ], + } + resp = test_client.post( + f'/event/{event.id}/manage/affiliations/api/affiliations/catalogs', + json=payload, + ) + assert resp.status_code == 422 + + @pytest.mark.usefixtures('no_csrf_check') def test_event_catalog_api_forbids_cross_scope_catalog(test_client, db, dummy_user, create_category, create_event): category = create_category(title='Category') diff --git a/affiliation_extras/indico_affiliation_extras/schemas.py b/affiliation_extras/indico_affiliation_extras/schemas.py index 4e5738a..ac37605 100644 --- a/affiliation_extras/indico_affiliation_extras/schemas.py +++ b/affiliation_extras/indico_affiliation_extras/schemas.py @@ -153,6 +153,9 @@ class Meta: class AffiliationCatalogListArgs(mm.Schema): + class Meta: + unknown = EXCLUDE + list_link = ModelField(AffiliationList, data_key='id', load_default=None, allow_none=True, load_only=True) name = fields.String(required=True, validate=not_empty) position = fields.Integer(required=True) @@ -174,6 +177,12 @@ class Meta: name = fields.String(required=True, validate=not_empty) lists = fields.List(fields.Nested(AffiliationCatalogListArgs), required=True, validate=not_empty) + @validates('lists') + def _validate_unique_list_names(self, lists, **kwargs): + names = [lst['name'].strip().lower() for lst in lists] + if len(names) != len(set(names)): + raise ValidationError(_('List names must be unique.')) + class OwnerDataSchema(mm.Schema): id = fields.Int() From ce9db03e31ccf4cb17e1babd0c62621abd5be1da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Molina?= Date: Tue, 9 Jun 2026 14:41:21 +0200 Subject: [PATCH 5/8] refactor: register all models from the package init --- .../models/__init__.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/affiliation_extras/indico_affiliation_extras/models/__init__.py b/affiliation_extras/indico_affiliation_extras/models/__init__.py index e69de29..1e155db 100644 --- a/affiliation_extras/indico_affiliation_extras/models/__init__.py +++ b/affiliation_extras/indico_affiliation_extras/models/__init__.py @@ -0,0 +1,24 @@ +# This file is part of the third-party Indico plugins. +# Copyright (C) 2026 CERN +# +# The third-party Indico plugins are free software; you can +# redistribute them and/or modify them under the terms of the; +# MIT License see the LICENSE file for more details. + +# Import every model so that importing the package registers all SQLAlchemy mappers, +# regardless of which module triggered the import (relationships reference each other +# by name, so a partial import leaves mappers unresolvable). +from indico_affiliation_extras.models.catalogs import AffiliationCatalog +from indico_affiliation_extras.models.contacts import AffiliationContactList +from indico_affiliation_extras.models.groups import AffiliationGroup +from indico_affiliation_extras.models.lists import AffiliationList +from indico_affiliation_extras.models.tags import AffiliationTag + + +__all__ = ( + 'AffiliationCatalog', + 'AffiliationContactList', + 'AffiliationGroup', + 'AffiliationList', + 'AffiliationTag', +) From 8580c9f2a5a20fc5cf56e438792483e75eb3c5db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Molina?= Date: Tue, 9 Jun 2026 14:41:31 +0200 Subject: [PATCH 6/8] fix: handle failed affiliation resolve without crashing the list --- .../client/components/AffiliationList.tsx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/affiliation_extras/indico_affiliation_extras/client/components/AffiliationList.tsx b/affiliation_extras/indico_affiliation_extras/client/components/AffiliationList.tsx index f6b048f..db9dd00 100644 --- a/affiliation_extras/indico_affiliation_extras/client/components/AffiliationList.tsx +++ b/affiliation_extras/indico_affiliation_extras/client/components/AffiliationList.tsx @@ -47,7 +47,7 @@ export function AffiliationList({ tagIds: number[]; affiliationIds: number[]; }) { - const {data, loading} = useIndicoAxios({ + const {data, loading, error} = useIndicoAxios({ url: resolveAffiliationsURL, method: 'POST', data: {groups, tags, affiliations}, @@ -59,13 +59,18 @@ export function AffiliationList({ groups: a.groups.filter(g => groups.includes(g.id)), tags: a.tags.filter(t => tags.includes(t.id)), group_tags: a.group_tags.filter(t => tags.includes(t.id)), - })), + })) ?? [], [data] ); if (loading) { return ; } + if (error) { + return ( + + ); + } if (!resolvedAffiliations.length) { return ; } From 3ddc4ca176875f59661df6c041667eefda3fd98c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Molina?= Date: Tue, 9 Jun 2026 14:41:32 +0200 Subject: [PATCH 7/8] fix: use stable keys for unsaved catalog list rows --- .../client/components/CatalogListField.tsx | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/affiliation_extras/indico_affiliation_extras/client/components/CatalogListField.tsx b/affiliation_extras/indico_affiliation_extras/client/components/CatalogListField.tsx index a6341a0..8d7dd16 100644 --- a/affiliation_extras/indico_affiliation_extras/client/components/CatalogListField.tsx +++ b/affiliation_extras/indico_affiliation_extras/client/components/CatalogListField.tsx @@ -8,7 +8,7 @@ import resolveAffiliationsURL from 'indico-url:plugin_affiliation_extras.api_resolve_affiliations'; import _ from 'lodash'; -import React, {useState} from 'react'; +import React, {useMemo, useState} from 'react'; import {Button, Confirm, Icon, Input, Loader, Modal, Popup} from 'semantic-ui-react'; import {FinalField} from 'indico/react/forms'; @@ -26,19 +26,12 @@ import {AffiliationList} from './AffiliationList'; import './CatalogListField.module.scss'; -const DEFAULT_LIST_VALUE = { - id: null, - name: '', - position: null, - is_enabled: true, - groups: [], - tags: [], - affiliations: [], -}; const DRAG_TYPE = 'affiliations-catalog-list'; export interface CatalogItem { id?: number | null; + // client-only stable id for unsaved rows (used as React key / drag id; dropped server-side) + _frontendId?: string; name: string; position?: number | null; is_enabled?: boolean; @@ -47,6 +40,17 @@ export interface CatalogItem { affiliations: Affiliation[]; } +const makeDefaultList = (): CatalogItem => ({ + id: null, + _frontendId: _.uniqueId('list-'), + name: '', + position: null, + is_enabled: true, + groups: [], + tags: [], + affiliations: [], +}); + interface CatalogListRowProps { value: CatalogItem; index: number; @@ -71,7 +75,7 @@ function CatalogListRow({ const closeModal = () => setModalOpen(null); const [handleRef, itemRef, style] = useSortableItem({ type: DRAG_TYPE, - id: value.id ?? `new-${index}`, + id: value.id ?? value._frontendId ?? `new-${index}`, index, active: true, separateHandle: true, @@ -164,7 +168,7 @@ function CatalogListRow({ /> {modalOpen === 'edit' && ( { onChange({...value, ...members}); @@ -227,7 +231,8 @@ function CatalogListField({ onBlur: () => void; targetLocator: Record; }) { - const values = _value?.length ? _value : [DEFAULT_LIST_VALUE]; + const emptyDefault = useMemo(makeDefaultList, []); + const values = _value?.length ? _value : [emptyDefault]; const normalizePositions = (items: CatalogItem[]) => items.map((item, idx) => ({ ...item, @@ -276,7 +281,7 @@ function CatalogListField({ {normalizedValues.map((value, idx) => ( handleChange([...normalizedValues, DEFAULT_LIST_VALUE], false)} + onClick={() => handleChange([...normalizedValues, makeDefaultList()], false)} disabled={normalizedValues.some(v => !v.name.trim())} style={{marginTop: '0.5em'}} compact @@ -314,6 +319,10 @@ const validateCatalogLists = (value: CatalogItem[]) => { if (value.some(({name}) => !name.trim())) { return Translate.string('List names must not be empty.'); } + const names = value.map(({name}) => name.trim().toLowerCase()); + if (new Set(names).size !== names.length) { + return Translate.string('List names must be unique.'); + } if ( value.some( ({groups, tags, affiliations}) => !groups.length && !tags.length && !affiliations.length From 122edd0a7373a3e22256b994544c8095ed2a640d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Molina?= Date: Tue, 9 Jun 2026 14:41:33 +0200 Subject: [PATCH 8/8] fix: correct invalid contact emails warning condition --- .../client/dashboard/EmailAffiliations.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/affiliation_extras/indico_affiliation_extras/client/dashboard/EmailAffiliations.tsx b/affiliation_extras/indico_affiliation_extras/client/dashboard/EmailAffiliations.tsx index efd86a2..cd00cf9 100644 --- a/affiliation_extras/indico_affiliation_extras/client/dashboard/EmailAffiliations.tsx +++ b/affiliation_extras/indico_affiliation_extras/client/dashboard/EmailAffiliations.tsx @@ -137,7 +137,7 @@ function RecipientsWarning({ /> )} {affiliations.some(a => - getAffiliationEmails(a, contactLists, includeUnnamedLists).filter(e => invalidEmails.has(e)) + getAffiliationEmails(a, contactLists, includeUnnamedLists).some(e => invalidEmails.has(e)) ) && (