From d327426558d5ebdbd50188574c0250a8d16551eb Mon Sep 17 00:00:00 2001 From: alec_dev Date: Fri, 17 Apr 2026 11:24:04 -0500 Subject: [PATCH 1/8] Fix key migration Issues with duplicates & new permissions --- specifyweb/backend/permissions/initialize.py | 7 +- .../setup_tool/app_resource_defaults.py | 77 +++++++++++++------ .../commands/run_key_migration_functions.py | 7 +- 3 files changed, 62 insertions(+), 29 deletions(-) diff --git a/specifyweb/backend/permissions/initialize.py b/specifyweb/backend/permissions/initialize.py index dc047388065..d254fe215b7 100644 --- a/specifyweb/backend/permissions/initialize.py +++ b/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') diff --git a/specifyweb/backend/setup_tool/app_resource_defaults.py b/specifyweb/backend/setup_tool/app_resource_defaults.py index 2d9b6a9be45..61ada0f997c 100644 --- a/specifyweb/backend/setup_tool/app_resource_defaults.py +++ b/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, } diff --git a/specifyweb/specify/management/commands/run_key_migration_functions.py b/specifyweb/specify/management/commands/run_key_migration_functions.py index 249e122a253..4dd52deb3e3 100644 --- a/specifyweb/specify/management/commands/run_key_migration_functions.py +++ b/specifyweb/specify/management/commands/run_key_migration_functions.py @@ -19,7 +19,7 @@ set_discipline_for_taxon_treedefs, fix_tectonic_unit_treedef_discipline_links ) -from specifyweb.backend.permissions.initialize import initialize +from specifyweb.backend.permissions.initialize import initialize_defaults from specifyweb.specify.migration_utils import update_schema_config as usc from specifyweb.specify.migration_utils.misc_migrations import make_selectseries_false from specifyweb.specify.migration_utils.tectonic_ranks import create_default_tectonic_ranks, create_root_tectonic_node @@ -104,7 +104,8 @@ def fix_app_resource_dirs(stdout: WriteToStdOut | None = None): "Ensured discipline app resource directories: " f"total={results['total_disciplines']}, " f"created={results['created']}, " - f"updated={results['updated']}" + f"updated={results['updated']}, " + f"deduplicated={results['deduplicated']}" ) def apply_default_uniqueness_rules_to_disciplines(apps): @@ -129,7 +130,7 @@ def fix_business_rules(stdout: WriteToStdOut | None = None): log_and_run(funcs, stdout) def initialize_permissions(apps): - initialize(False, apps) + initialize_defaults(apps) def fix_permissions(stdout: WriteToStdOut | None = None): funcs = [ From d9b2895787807d14651218e404e70419f7c3a1a0 Mon Sep 17 00:00:00 2001 From: alec_dev Date: Fri, 17 Apr 2026 11:24:31 -0500 Subject: [PATCH 2/8] Make run_key_migrations optional at startup --- docker-entrypoint.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index e52a68dc66e..65fe58ba9a8 100755 --- a/docker-entrypoint.sh +++ b/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_FUNCTIONS_ON_STARTUP:-0}" = "1" ]; then + echo "Running key migration functions." + ve/bin/python manage.py run_key_migration_functions + fi set -e fi exec "$@" From dad07ab7d445d7f3a8098e661648bde8e68baf00 Mon Sep 17 00:00:00 2001 From: alec_dev Date: Fri, 17 Apr 2026 11:24:52 -0500 Subject: [PATCH 3/8] Add deduplicate_discipline_resource_dirs migration --- ...02_deduplicate_discipline_resource_dirs.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 specifyweb/backend/setup_tool/migrations/0002_deduplicate_discipline_resource_dirs.py diff --git a/specifyweb/backend/setup_tool/migrations/0002_deduplicate_discipline_resource_dirs.py b/specifyweb/backend/setup_tool/migrations/0002_deduplicate_discipline_resource_dirs.py new file mode 100644 index 00000000000..0f87b12b644 --- /dev/null +++ b/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, + ), + ] From 7b209717d0de52494f772d8b7ad6f62600fb2211 Mon Sep 17 00:00:00 2001 From: alec_dev Date: Fri, 17 Apr 2026 11:52:23 -0500 Subject: [PATCH 4/8] RUN_KEY_MIGRATION_ON_STARTUP --- .env | 3 +++ docker-entrypoint.sh | 2 +- .../js_src/lib/components/AppResources/filtersHelpers.ts | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.env b/.env index 71fe3b0c430..d49bbef3cb0 100644 --- a/.env +++ b/.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=0 + # 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 diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 65fe58ba9a8..571640c69b0 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -13,7 +13,7 @@ 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 - if [ "${RUN_KEY_MIGRATION_FUNCTIONS_ON_STARTUP:-0}" = "1" ]; then + if [ "${RUN_KEY_MIGRATION_ON_STARTUP:-0}" = "1" ]; then echo "Running key migration functions." ve/bin/python manage.py run_key_migration_functions fi diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/filtersHelpers.ts b/specifyweb/frontend/js_src/lib/components/AppResources/filtersHelpers.ts index 42210ecc84e..e5a51a0617a 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/filtersHelpers.ts +++ b/specifyweb/frontend/js_src/lib/components/AppResources/filtersHelpers.ts @@ -58,7 +58,7 @@ export const filterAppResources = ( ); const finalAppResources = baseAppResources.filter((resource) => { - if (resource.name !== 'CollectionPreferences') { + if (resource.name === 'CollectionPreferences') { return hasEditPermission; } return true; From 509109dbf5bc86f91eff2ec386e28120c9ae7b72 Mon Sep 17 00:00:00 2001 From: alec_dev Date: Fri, 17 Apr 2026 12:54:27 -0500 Subject: [PATCH 5/8] avoid null task warning --- specifyweb/backend/setup_tool/schema_defaults.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/specifyweb/backend/setup_tool/schema_defaults.py b/specifyweb/backend/setup_tool/schema_defaults.py index 791ac736ac5..d3bd7a4a493 100644 --- a/specifyweb/backend/setup_tool/schema_defaults.py +++ b/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) From 38beb3733e2af924c108cce4b4951bf87c1e3336 Mon Sep 17 00:00:00 2001 From: alec_dev Date: Fri, 17 Apr 2026 16:10:04 -0500 Subject: [PATCH 6/8] jest snapshot fix --- .../__tests__/AppResourcesAside.test.tsx | 19 +- .../AppResourcesAside.test.tsx.snap | 575 ------------------ 2 files changed, 9 insertions(+), 585 deletions(-) delete mode 100644 specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesAside.test.tsx.snap diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/AppResourcesAside.test.tsx b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/AppResourcesAside.test.tsx index 1b51e0b0bfa..29ac26cbb7c 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/AppResourcesAside.test.tsx +++ b/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().textContent).toBe( + '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} /> - ); + ); 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(); }); }); diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesAside.test.tsx.snap b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesAside.test.tsx.snap deleted file mode 100644 index 3d16068e4d4..00000000000 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/__snapshots__/AppResourcesAside.test.tsx.snap +++ /dev/null @@ -1,575 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`AppResourcesAside (expanded case) expanded case 1`] = ` - - - -`; - -exports[`AppResourcesAside (expanded case) expanded case 2`] = ` - - - -`; - -exports[`AppResourcesAside (simple no conformation case) simple no conformation case 1`] = ` - - - -`; From cc02180391058ff55341cc156abf7cdf09d3c88e Mon Sep 17 00:00:00 2001 From: alec_dev Date: Fri, 17 Apr 2026 21:14:12 +0000 Subject: [PATCH 7/8] Lint code with ESLint and Prettier Triggered by 38beb3733e2af924c108cce4b4951bf87c1e3336 on branch refs/heads/key-migration-fix --- .../AppResources/__tests__/AppResourcesAside.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/AppResourcesAside.test.tsx b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/AppResourcesAside.test.tsx index 29ac26cbb7c..8e5fbbc943b 100644 --- a/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/AppResourcesAside.test.tsx +++ b/specifyweb/frontend/js_src/lib/components/AppResources/__tests__/AppResourcesAside.test.tsx @@ -30,7 +30,7 @@ describe('AppResourcesAside (simple no conformation case)', () => { /> ); - expect(asFragment().textContent).toBe( + expect(asFragment()).toHaveTextContent( 'Global Resources (2)Discipline Resources (3)Expand AllCollapse All' ); unmount(); From 5724ae3e3934f938a4b339a4c3ef7c07e611b401 Mon Sep 17 00:00:00 2001 From: alec_dev Date: Fri, 17 Apr 2026 16:29:53 -0500 Subject: [PATCH 8/8] change default state of run key migration on startup --- .env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.env b/.env index d49bbef3cb0..5afb85c6f9b 100644 --- a/.env +++ b/.env @@ -4,7 +4,7 @@ MYSQL_ROOT_PASSWORD=password DATABASE_NAME=specify # Decide whether to run key migration functions on startup. -RUN_KEY_MIGRATION_ON_STARTUP=0 +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.