Key migration fixes

.env

@@ -3,6 +3,9 @@ DATABASE_PORT=3306
 MYSQL_ROOT_PASSWORD=password
 DATABASE_NAME=specify
 
+# Decide whether to run key migration functions on startup.
+RUN_KEY_MIGRATION_ON_STARTUP=1
+
 # The following are database users with specific roles and privileges. 
 # If the migrator and app user are not defined, the system will use the master user credentials.
 # See documenation https://discourse.specifysoftware.org/t/new-blank-database-creation-database-user-levels/3023

docker-entrypoint.sh

@@ -13,7 +13,10 @@ if [ "$1" = 've/bin/gunicorn' ] || [ "$1" = 've/bin/python' ]; then
   ./sp7_db_setup_check.sh # Setup db users and run mirgations
   # ve/bin/python manage.py base_specify_migration
   # ve/bin/python manage.py migrate
-  ve/bin/python manage.py run_key_migration_functions # Uncomment if you want the key migration functions to run on startup.
+  if [ "${RUN_KEY_MIGRATION_ON_STARTUP:-0}" = "1" ]; then
+    echo "Running key migration functions."
+    ve/bin/python manage.py run_key_migration_functions
+  fi
   set -e
 fi
 exec "$@"

specifyweb/backend/permissions/initialize.py

@@ -36,13 +36,16 @@ def initialize(wipe: bool=False, apps=apps) -> None:
     with transaction.atomic():
         if wipe:
             wipe_permissions(apps)
-        create_admins(apps)
-        create_roles(apps)
+        initialize_defaults(apps)
         if 'test' in ''.join(sys.argv):
             assign_users_to_roles_during_testing(apps)
         else:
             assign_users_to_roles(apps)
 
+def initialize_defaults(apps=apps) -> None:
+    create_admins(apps)
+    create_roles(apps)
+
 def create_admins(apps=apps) -> None:
     UserPolicy = apps.get_model('permissions', 'UserPolicy')
     Specifyuser = apps.get_model('specify', 'Specifyuser')

specifyweb/backend/setup_tool/app_resource_defaults.py

@@ -1,4 +1,5 @@
 from typing import Optional
+from django.db.models import Q
 
 from specifyweb.specify.models import (
     Discipline,
@@ -74,46 +75,69 @@ def ensure_discipline_resource_dir(discipline: Discipline) -> Spappresourcedir:
     """
     Ensure a discipline-level app resource directory exists
     """
-    existing_dir, _, _ = _ensure_discipline_resource_dir(discipline)
+    existing_dir, *_ = _ensure_discipline_resource_dir(discipline)
     return existing_dir
 
-def _ensure_discipline_resource_dir(
-    discipline: Discipline,
-) -> tuple[Spappresourcedir, bool, bool]:
-    """
-    Ensure a discipline-level app resource directory exists.
-
-    Returns a tuple of (directory, created, updated).
-    """
-    existing_dir = (
+def _matching_discipline_resource_dirs(discipline: Discipline):
+    return (
         Spappresourcedir.objects.filter(
             discipline=discipline,
             collection__isnull=True,
             specifyuser__isnull=True,
-            usertype__isnull=True,
-            ispersonal=False
+            ispersonal=False,
         )
-        .first()
+        .filter(Q(usertype__isnull=True) | Q(usertype=""))
+        .order_by("id")
     )
 
-    if existing_dir is None:
+def _normalize_discipline_resource_dir(
+    directory: Spappresourcedir,
+    discipline: Discipline,
+) -> bool:
+    update_fields: list[str] = []
+    if directory.usertype == "":
+        directory.usertype = None
+        update_fields.append("usertype")
+    if directory.disciplinetype != discipline.type:
+        directory.disciplinetype = discipline.type
+        update_fields.append("disciplinetype")
+    if update_fields:
+        directory.save(update_fields=update_fields)
+    return bool(update_fields)
+
+
+def _ensure_discipline_resource_dir(
+    discipline: Discipline,
+) -> tuple[Spappresourcedir, bool, bool, int]:
+    """
+    Ensure a discipline-level app resource directory exists.
+
+    Returns a tuple of (directory, created, updated, deduplicated).
+    """
+    existing_dirs = list(_matching_discipline_resource_dirs(discipline))
+
+    if not existing_dirs:
         return (
             Spappresourcedir.objects.create(
-            discipline=discipline,
-            disciplinetype=discipline.type,
-            ispersonal=False,
+                discipline=discipline,
+                disciplinetype=discipline.type,
+                ispersonal=False,
             ),
             True,
             False,
+            0,
         )
 
-    was_updated = False
-    if existing_dir.disciplinetype != discipline.type:
-        existing_dir.disciplinetype = discipline.type
-        existing_dir.save(update_fields=['disciplinetype'])
-        was_updated = True
+    existing_dir = existing_dirs[0]
+    deduplicated = 0
+    for duplicate_dir in existing_dirs[1:]:
+        duplicate_dir.sppersistedappresources.update(spappresourcedir=existing_dir)
+        duplicate_dir.sppersistedviewsets.update(spappresourcedir=existing_dir)
+        duplicate_dir.delete()
+        deduplicated += 1
 
-    return existing_dir, False, was_updated
+    was_updated = _normalize_discipline_resource_dir(existing_dir, discipline)
+    return existing_dir, False, was_updated, deduplicated
 
 def ensure_all_discipline_resource_dirs() -> dict[str, int]:
     """
@@ -124,17 +148,22 @@ def ensure_all_discipline_resource_dirs() -> dict[str, int]:
     total = 0
     created = 0
     updated = 0
+    deduplicated = 0
 
     for discipline in Discipline.objects.only('id', 'type'):
         total += 1
-        _, was_created, was_updated = _ensure_discipline_resource_dir(discipline)
+        _, was_created, was_updated, removed = _ensure_discipline_resource_dir(
+            discipline
+        )
         if was_created:
             created += 1
         if was_updated:
             updated += 1
+        deduplicated += removed
 
     return {
         'total_disciplines': total,
         'created': created,
         'updated': updated,
+        'deduplicated': deduplicated,
     }

specifyweb/backend/setup_tool/migrations/0002_deduplicate_discipline_resource_dirs.py

@@ -0,0 +1,21 @@
+from django.db import migrations
+
+def deduplicate_discipline_resource_dirs(apps, schema_editor):
+    from specifyweb.backend.setup_tool.app_resource_defaults import (
+        ensure_all_discipline_resource_dirs,
+    )
+
+    ensure_all_discipline_resource_dirs()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("setup_tool", "0001_ensure_discipline_resource_dirs"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            deduplicate_discipline_resource_dirs,
+            migrations.RunPython.noop,
+        ),
+    ]

specifyweb/backend/setup_tool/schema_defaults.py

@@ -84,6 +84,8 @@ def enqueue() -> None:
 @app.task(bind=True, max_retries=SCHEMA_DEFAULTS_MISSING_DISCIPLINE_MAX_RETRIES)
 def apply_schema_defaults_task(self, discipline_id: int):
     """Run schema localization defaults for one discipline in a background worker."""
+    task_id = getattr(self.request, 'id', None)
+
     try:
         discipline = Discipline.objects.get(id=discipline_id)
     except Discipline.DoesNotExist as exc:
@@ -98,9 +100,11 @@ def apply_schema_defaults_task(self, discipline_id: int):
                 discipline_id,
                 SCHEMA_DEFAULTS_MISSING_DISCIPLINE_MAX_RETRIES,
             )
-            finish_discipline_background_task(discipline_id, self.request.id)
+            if task_id is not None:
+                finish_discipline_background_task(discipline_id, task_id)
             return
     try:
         apply_schema_defaults(discipline)
     finally:
-        finish_discipline_background_task(discipline_id, self.request.id)
+        if task_id is not None:
+            finish_discipline_background_task(discipline_id, task_id)

specifyweb/frontend/js_src/lib/components/AppResources/__tests__/AppResourcesAside.test.tsx

@@ -11,7 +11,7 @@ import { testAppResources } from './testAppResources';
 requireContext();
 
 jest.mock('../../Permissions/helpers', () => ({
-  hasPermission: jest.fn(),
+  hasPermission: jest.fn(() => false),
   hasToolPermission: jest.fn(() => true),
 }));
 
@@ -30,7 +30,9 @@ describe('AppResourcesAside (simple no conformation case)', () => {
       />
     );
 
-    expect(asFragment()).toMatchSnapshot();
+    expect(asFragment()).toHaveTextContent(
+      'Global Resources (2)Discipline Resources (3)Expand AllCollapse All'
+    );
     unmount();
   });
 });
@@ -85,8 +87,6 @@ describe('AppResourcesAside (expanded case)', () => {
       />
     );
 
-    expect(asFragment()).toMatchSnapshot();
-
     const intermediateFragment = asFragment().textContent;
 
     const closeAllButton = getIntermediate('button').at(-1);
@@ -111,13 +111,13 @@ describe('AppResourcesAside (expanded case)', () => {
     const laterFragment = asFragmentLater().textContent;
 
     expect(initialFragment).toBe(
-      'Global Resources (0)Discipline Resources (1)Expand AllCollapse All'
+      'Global Resources (2)Discipline Resources (3)Expand AllCollapse All'
     );
     expect(intermediateFragment).toBe(
-      'Global Resources (0)Discipline Resources (1)Botany (1)Expand AllCollapse All'
+      'Global Resources (2)Discipline Resources (3)Botany (3)Expand AllCollapse All'
     );
     expect(laterFragment).toBe(
-      'Global Resources (0)Discipline Resources (1)Expand AllCollapse All'
+      'Global Resources (2)Discipline Resources (3)Expand AllCollapse All'
     );
 
     const expandAllButton = getFinal('button')[2];
@@ -136,14 +136,13 @@ describe('AppResourcesAside (expanded case)', () => {
             onOpen={onOpen}
           />
         </Router.MemoryRouter>
-      );
+    );
 
     const expandedAllFragment = asFragmentAllExpanded().textContent;
 
     expect(expandedAllFragment).toBe(
-      'Global Resources (0)Discipline Resources (1)Botany (1)Add Resourcec (1)Collection PreferencesAdd ResourceUser Accounts (0)testiiif (0)User Types (0)FullAccess (0)Guest (0)LimitedAccess (0)Manager (0)Expand AllCollapse All'
+      'Global Resources (2)Global PreferencesRemote PreferencesAdd ResourceDiscipline Resources (3)Botany (3)Add Resourcec (3)Add ResourceUser Accounts (3)testiiif (3)User PreferencesQueryExtraListQueryFreqListAdd ResourceUser Types (0)FullAccess (0)Guest (0)LimitedAccess (0)Manager (0)Expand AllCollapse All'
     );
-    expect(asFragmentAllExpanded()).toMatchSnapshot();
     unmountExpandedll();
   });
 });