Review fixes for the Affiliation Catalogs plugin

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')

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 <Loader active inline="centered" />;
   }
+  if (error) {
+    return (
+      <Message error content={Translate.string('Could not load the affiliations for this list')} />
+    );
+  }
   if (!resolvedAffiliations.length) {
     return <Message content={Translate.string('This list is currently empty')} info />;
   }

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' && (
           <FinalModalForm
-            id={`catalog-list-${value.id ?? index}`}
+            id={`catalog-list-${value.id ?? value._frontendId}`}
             onClose={closeModal}
             onSubmit={({members}) => {
               onChange({...value, ...members});
@@ -227,7 +231,8 @@ function CatalogListField({
   onBlur: () => void;
   targetLocator: Record<string, number>;
 }) {
-  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({
             <tbody>
               {normalizedValues.map((value, idx) => (
                 <CatalogListRow
-                  key={value.id ?? `new-${idx}`}
+                  key={value.id ?? value._frontendId ?? `new-${idx}`}
                   index={idx}
                   value={value}
                   targetLocator={targetLocator}
@@ -300,7 +305,7 @@ function CatalogListField({
         type="button"
         icon="add"
         content={Translate.string('Add list')}
-        onClick={() => 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

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))
       ) && (
         <Message
           visible

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 ''

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,

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',
+)

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')

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()