Merge pull request #6308 from specify/issue-6266#8094
Open
CarolineDenis wants to merge 2 commits into
Open
Conversation
run_key_migration_functions django command
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis pull request enables running key data migrations when new collections are added to Specify 6 and synced to 7. It introduces a Django management command pipeline, extracts migration logic into reusable utility modules, refactors permissions initialization to detect and migrate SP6 users, and updates all existing schema/permission migrations to use centralized helpers instead of inline code. ChangesMigration orchestration and infrastructure
Business rules and uniqueness rule management
Patch data migrations (SQL to Python)
Permissions initialization and migration
Core migration utilities extraction
Migration files refactored to use utilities
Supporting changes and test updates
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
Author
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
Contributor
Author
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
specifyweb/backend/businessrules/uniqueness_rules.py (1)
266-309:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSubset matching can suppress required rules and delete broader ones.
Both helpers treat "requested fields/scopes are present" as an exact rule match. A discipline rule with extra fields or scopes will satisfy this check, so
create_uniqueness_rule(...)can skip creating a required default rule, andremove_uniqueness_rule(...)can later delete the broader custom rule by mistake.Suggested fix
for rule in candidate_rules: all_fields = rule.uniquenessrulefield_set.all() matching_fields = all_fields.filter(fieldPath__in=fields, isScope=False) matching_scopes = all_fields.filter(fieldPath__in=scopes, isScope=True) - if len(matching_fields) == len(fields) and len(matching_scopes) == len(scopes): + if ( + all_fields.filter(isScope=False).count() == len(fields) + and all_fields.filter(isScope=True).count() == len(scopes) + and matching_fields.count() == len(fields) + and matching_scopes.count() == len(scopes) + ): return @@ for rule in candidate_rules: all_fields = rule.uniquenessrulefield_set.all() matching_fields = all_fields.filter(fieldPath__in=fields, isScope=False) matching_scopes = all_fields.filter(fieldPath__in=scopes, isScope=True) - if len(matching_fields) == len(fields) and len(matching_scopes) == len(scopes): + if ( + all_fields.filter(isScope=False).count() == len(fields) + and all_fields.filter(isScope=True).count() == len(scopes) + and matching_fields.count() == len(fields) + and matching_scopes.count() == len(scopes) + ): rule_ids.append(rule.id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/backend/businessrules/uniqueness_rules.py` around lines 266 - 309, The current subset checks in create_uniqueness_rule and remove_uniqueness_rule allow rules with extra fields/scopes to match; change the logic to require exact set equality: for each candidate UniquenessRule (use rule.uniquenessrulefield_set.all()), build the sets of fieldPath for isScope=False and isScope=True and compare them to the provided fields and scopes as sets (or check counts plus element equality) so a rule only matches when both the field set and scope set are exactly equal; apply this exact-match test where matching_fields/matching_scopes are currently used before skipping creation or appending rule.id for deletion.specifyweb/backend/businessrules/migrations/0004_catnum_uniquerule.py (1)
3-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
RunPython(...)still binds to the old helper implementations.The local
catnum_rule_editable/catnum_rule_uneditabledefinitions shadow the imported helpers, so this migration never uses the exact-match logic frommigration_utils.py. The current forward query will still flip any rule that merely containscatalogNumberandcollection, including broader composite rules.Suggested fix
from django.db import migrations from specifyweb.backend.businessrules.migration_utils import catnum_rule_editable, catnum_rule_uneditable -from specifyweb.backend.businessrules.uniqueness_rules import create_uniqueness_rule - - -def catnum_rule_editable(apps, schema_editor): - ... - -def catnum_rule_uneditable(apps, schema_editor): - ... class Migration(migrations.Migration):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/backend/businessrules/migrations/0004_catnum_uniquerule.py` around lines 3 - 40, The migration defines local functions catnum_rule_editable and catnum_rule_uneditable that shadow the imported helpers from migration_utils, so RunPython is calling the wrong implementations; fix by removing or renaming the local function definitions and make migrations.RunPython refer to the imported helpers catnum_rule_editable and catnum_rule_uneditable from migration_utils (or explicitly call migration_utils.catnum_rule_editable / migration_utils.catnum_rule_uneditable) so the migration uses the exact-match logic in migration_utils.create_uniqueness_rule.
🟡 Minor comments (1)
specifyweb/specify/migration_utils/sp7_schemaconfig.py-150-151 (1)
150-151:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the
RelativeAgeCitationprimary-key field here.
RelativeAgeCitationis still pointing atabsoluteAgeCitationId, so the 0023 hidden-field update will miss the actualRelativeAgeCitationid row.Suggested fix
- 'RelativeAgeCitation': ['absoluteAgeCitationId', 'collectionMember'], + 'RelativeAgeCitation': ['relativeAgeCitationId', 'collectionMember'],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/specify/migration_utils/sp7_schemaconfig.py` around lines 150 - 151, The mapping for 'RelativeAgeCitation' in sp7_schemaconfig.py is incorrectly using 'absoluteAgeCitationId' instead of the RelativeAgeCitation primary-key field; update the value for the 'RelativeAgeCitation' key to use its actual PK field (e.g., 'relativeAgeCitationId' or whatever the schema defines) so the 0023 hidden-field update targets the correct id row, leaving the 'AbsoluteAgeCitation' entry unchanged.
🧹 Nitpick comments (3)
specifyweb/specify/management/commands/run_key_migration_functions.py (2)
84-86: ⚡ Quick winAvoid loading all uniqueness-rule discipline IDs into memory.
Line 85 builds a Python
set(...)from the full queryset before filtering. Use a DB subquery to keep this scalable.Proposed refactor
def apply_default_uniqueness_rules_to_disciplines(apps): Discipline = apps.get_model('specify', 'Discipline') UniquenessRule = apps.get_model('businessrules', 'UniquenessRule') - for discipline in Discipline.objects.exclude( - id__in=set(UniquenessRule.objects.values_list('discipline_id', flat=True).distinct())): + discipline_ids_with_rules = UniquenessRule.objects.values_list('discipline_id', flat=True).distinct() + for discipline in Discipline.objects.exclude(id__in=discipline_ids_with_rules): apply_default_uniqueness_rules(discipline, registry=apps)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/specify/management/commands/run_key_migration_functions.py` around lines 84 - 86, The code currently materializes all uniqueness-rule discipline IDs into a Python set before excluding them, which can blow memory; change the exclude to use a DB subquery instead (e.g. remove set(...) and pass the UniquenessRule queryset or wrap it in django.db.models.Subquery) so the filter uses a database-side subquery; update the loop that iterates Discipline.objects.exclude(...) and keep the call to apply_default_uniqueness_rules(discipline, registry=apps) unchanged.
175-175: ⚡ Quick winLog the traceback on migration failures.
Line 175 logs only the error message. Use
logger.exception(...)so failures include stack traces for diagnosis.Proposed fix
- except Exception as e: - logger.error(f"An error occurred: {e}") + except Exception: + logger.exception("An error occurred while running key migration functions") raise🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/specify/management/commands/run_key_migration_functions.py` at line 175, Replace the plain error log call that currently uses logger.error(f"An error occurred: {e}") with a logger.exception(...) call inside the except block so the full stack trace is recorded; locate the try/except in the run_key_migration_functions command (the Command.handle or run_key_migration_functions function where the logger variable is used) and change the log invocation to logger.exception("An error occurred while running key migrations") to capture traceback and context.specifyweb/specify/migrations/0021_update_hidden_geo_tables.py (1)
10-54: ⚡ Quick winRemove the commented-out legacy migration implementation.
Please delete the old commented function bodies and commented
RunPythoncall now that this migration delegates tousc.*; this file is harder to maintain with both paths present.Also applies to: 64-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@specifyweb/specify/migrations/0021_update_hidden_geo_tables.py` around lines 10 - 54, Delete the commented-out legacy migration implementation: remove the entire commented bodies of fix_hidden_geo_prop and reverse_fix_hidden_geo_prop and any commented RunPython invocation so the file only contains the new delegation to usc.*; specifically remove the commented blocks referencing Splocalecontainer, Splocalecontaineritem, Discipline, PALEO_DISCIPLINES | GEOLOGY_DISCIPLINES, SCHEMA_CONFIG_MOD_TABLE_FIELDS and the duplicated items.update(ishidden=True) code as well as the commented RunPython call that referenced these functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@specifyweb/backend/businessrules/migration_utils.py`:
- Around line 46-71: The loop over Discipline/UniquenessRule currently detects
an existing catalog-number rule (using Discipline, UniquenessRule and
uniquenessrulefield_set) but only sets has_catalognumber_rule = True and skips
create_uniqueness_rule; instead, when a matching rule is found and its
isDatabaseConstraint is False you should flip that existing rule to
isDatabaseConstraint = True (update the UniquenessRule instance or queryset and
save) so reverse migration restores the database-constraint flag; keep the same
matching logic (fields.get().fieldPath and scopes.get().fieldPath comparisons)
and update the identified rule before continuing.
In `@specifyweb/backend/businessrules/uniqueness_rules.py`:
- Around line 330-377: The current cleanup in fix_global_default_rules deletes
discipline-specific UniquenessRuleField rows by matching individual field tuples
(modelName, isDatabaseConstraint, fieldPath, isScope), which can break composite
rules; instead, only remove a discipline-specific UniquenessRule if its entire
set of fields exactly matches a global rule's entire field set. Implement this
by loading global rules grouped by (modelName, isDatabaseConstraint) into a map
of field-sets (set of (fieldPath,isScope)) keyed by a unique global rule id,
then iterate discipline-specific UniquenessRule objects (for each discipline_id)
and for each discipline rule compute its set of (fieldPath,isScope); if that set
equals any global field-set for the same (modelName,isDatabaseConstraint) then
delete that whole UniquenessRule (and its UniquenessRuleField children) inside
the transaction; otherwise leave partial-overlap rules untouched. Ensure you
update references to global_rule_fields, matching_fields_qs and empty_rules_qs
to use full-rule comparisons on UniquenessRule and UniquenessRuleField sets
rather than per-field Exists filters.
In `@specifyweb/backend/patches/migration_utils.py`:
- Around line 8-36: The migration helpers (apply_migrations, update_is_accepted,
update_coordinates) ignore the provided schema_editor and thus always use the
default DB; obtain the migration db alias (e.g. db_alias =
schema_editor.connection.alias if schema_editor is not None else "default") and
route ORM calls through each model's _base_manager.using(db_alias) so updates
target the migration connection; update calls that currently use
tree_model.objects.filter(...).update(...) to
tree_model._base_manager.using(db_alias).filter(...).update(...) (referencing
SPECIFY_TREES and tree_model) and similarly change
Locality.objects.filter(...).update(...) to
Locality._base_manager.using(db_alias).filter(...).update(...) in
update_coordinates.
In `@specifyweb/backend/permissions/initialize.py`:
- Around line 106-111: The NOT IN subquery using spuserrole/sprole/collection
(SELECT DISTINCT r.collection_id ...) excludes entire collections once any
spuserrole exists and breaks partial backfills; replace it with a correlated NOT
EXISTS that checks for a role record for the specific user and collection (e.g.,
NOT EXISTS (SELECT 1 FROM spuserrole ur JOIN sprole r ON r.id = ur.role_id WHERE
r.collection_id = c.UserGroupScopeId AND ur.user_id = <user id column>)), using
the existing aliases (spuserrole ur, sprole r, collection c with
c.UserGroupScopeId) so only users already migrated for that collection are
skipped instead of the whole collection.
- Around line 56-57: The current early return in create_admins() uses
is_sp6_user_permissions_migrated(user, apps) which erroneously skips users who
merely have any legacy UserRole/UserPolicy; change this to only skip when the
user already has the wildcard/global admin permission: replace the generic
migration check with a specific query that checks for a UserPolicy (and/or
UserRole pointing to such a policy) where collection == '%' and namespace == '%'
(or whatever the wildcard identifiers are) for the given user, and only continue
(skip creating admin) if that wildcard/global admin policy/role exists; update
create_admins() to perform that targeted existence check instead of calling
is_sp6_user_permissions_migrated().
In `@specifyweb/permissions/migration_utils/edit_permissions.py`:
- Around line 4-59: The migration callables add_permission and
add_stats_edit_permission are referenced by RunPython in migrations but live in
this module, which makes migrations non-deterministic; to fix, copy the full
bodies of add_permission and add_stats_edit_permission into the migration files
that call them (or move these exact implementations into a new versioned
migration-only module and import that module from the migrations), and update
the migrations to call the frozen/locally-versioned callables so future edits to
this file won’t change already-shipped migrations.
In `@specifyweb/specify/api/utils.py`:
- Around line 24-28: The current helper in specifyweb/specify/api/utils.py
inlines runtime values by using query.statement.compile(...,
compile_kwargs={"literal_binds": True}) and unconditionally logs raw_sql via
logger.debug; remove literal_binds=True so the SQL remains parameterized,
compile the statement without literal binds and capture the parameter list
separately (e.g., compiled.params or query.parameters), and change the three
call sites that rely on this helper (execution paths at execution.py and
geology_time.py) to log the parameterized SQL plus parameters only when Django
settings.DEBUG is True; also apply the same fix to the duplicate helper in
specifyweb/backend/stored_queries/utils.py and ensure logger.debug calls are
wrapped in a settings.DEBUG check.
In `@specifyweb/specify/management/commands/run_key_migration_functions.py`:
- Line 141: The help text for the add_argument in the management command
contains an unnecessary f-string prefix (help=f"...") which causes F541; remove
the leading "f" so the help parameter uses a plain string (help="Optional:
specify one or more functions to run") in the Command/add_argument call in
run_key_migration_functions.py.
In `@specifyweb/specify/migration_utils/default_cots.py`:
- Around line 140-156: The current loops reuse .first()/.last() and don't pair
rows one-to-one; replace them with a single pairing pass that iterates the two
QuerySets together (e.g., zip or itertools.zip_longest semantics) and assigns
each Tectonicunittreedef to a single Discipline and vice versa, updating the
discipline.tectonicunittreedef and treedef.discipline fields for each pair
(referencing Tectonicunittreedef, empty_disciplines,
empty_tectonic_unit_treedefs, and the discipline/tectonicunittreedef fields),
and handle mismatched counts by creating new Tectonicunittreedef objects for
leftover disciplines or assigning leftover treedefs to leftover disciplines
rather than reusing .first()/.last() or writing None.
In `@specifyweb/specify/migration_utils/tectonic_ranks.py`:
- Around line 101-120: The Root TectonicUnitTreeDefItem is being created without
the root metadata (rankid=0 and parent=None), so update the
TectonicUnitTreeDefItem.objects.get_or_create call that constructs the "Root"
item (variable tectonic_tree_def_item) to include rankid=0 and parent=None so
the schema matches create_default_tectonic_ranks; keep other fields (name,
title, treedef, isenforced) the same.
- Around line 9-16: The query is excluding disciplines that need repair; remove
the unnecessary exclude so we iterate all disciplines with
Discipline.tectonicunittreedef__isnull=True (i.e. change to
Discipline.objects.filter(tectonicunittreedef__isnull=True)), then keep the
existing loop that uses TectonicTreeDef.objects.get_or_create(name="Tectonic
Unit", discipline=discipline) so an existing TectonicTreeDef that already points
to the discipline will be found and a missing one will be created and
associated; reference symbols: Discipline, TectonicTreeDef, tectonicunittreedef,
get_or_create.
- Around line 137-141: The reverse helper is deleting all TectonicUnit objects
named "Root" and entire tree-def items broadly; restrict deletions to only the
records created for the current tree definition by changing the TectonicUnit
delete to filter by the same treedef and ensure it's the actual root (e.g.,
TectonicUnit.objects.filter(name="Root", treedef=tectonic_tree_def,
parent__isnull=True).delete()), and keep
TectonicUnitTreeDefItem.objects.filter(treedef=tectonic_tree_def).delete() as-is
so both deletions are scoped to the specific `tectonic_tree_def` used in this
migration.
In `@specifyweb/specify/migration_utils/update_schema_config.py`:
- Around line 1351-1368: The revert_0034_schema_config_field_desc function
currently skips undoing the mutations performed by update_accession_date_fields;
update it to restore each Splocalecontaineritem's ishidden, text (label) and
description using the values from the MIGRATION_0034_UPDATE_FIELDS tuple for
that field: treat each tuple as (field_name, original_label,
original_description), query Splocalecontaineritem for the container/name, set
item.ishidden back to False (or the original visibility if you have that saved),
set item.text = original_label and item.description = original_description, then
call item.save() so downgrades fully revert; reference the function name
revert_0034_schema_config_field_desc, the models Splocalecontainer and
Splocalecontaineritem, and the MIGRATION_0034_UPDATE_FIELDS symbol to locate the
code to change.
- Around line 470-478: The filter used to find duplicate Splocalecontaineritem
rows is using container__name="CollectionObject" but containers are created with
name=table.name.lower(), so change the query in update_schema_config to match
lowercase (or use a case-insensitive lookup). Update the container_items
QuerySet expression (Splocalecontaineritem.objects.filter(...,
container__name="CollectionObject", ...)) to use
container__name__iexact="collectionobject" or container__name="collectionobject"
so the intended duplicates are actually matched and then removed along with the
related Splocaleitemstr rows.
- Around line 883-886: The query in
update_table_field_schema_config_with_defaults filters Splocalecontaineritem by
name using field_name.lower(), which misses original-case names (e.g.,
collectionObjectType); change the filter to use the field_name as-is (or a
consistent case match strategy) so
Splocalecontaineritem.objects.filter(container=container, name=field_name)
matches stored names exactly; ensure any other comparisons in
update_table_field_schema_config_with_defaults that relied on lowercasing are
updated to preserve original casing or use a case-insensitive lookup (e.g.,
__iexact) if intended.
- Around line 124-125: Remove the early "return" when "is_new" is False and
instead limit that check to only skip creating the container row; always run the
subsequent backfill logic that creates/updates Splocalecontaineritem rows so
reruns can fill partially populated tables. Concretely, change the control flow
around the "if not is_new" branch so you only bypass the container-creation code
path when is_new is False but continue executing the backfill that writes
Splocalecontaineritem; make the backfill idempotent by checking for existing
items before inserting/updating.
In `@specifyweb/specify/migrations/0002_geo.py`:
- Around line 13-19: There is a shadowed import: create_default_collection_types
is imported both from specifyweb.specify.migration_utils.default_cots and from
specifyweb.specify.api.utils, so the RunPython migration step is calling the
runtime helper instead of the migration-safe helper; fix by removing (or
renaming/aliasing) the import from specifyweb.specify.api.utils so that
create_default_collection_types refers to the migration helper imported from
specifyweb.specify.migration_utils.default_cots, and confirm the RunPython
invocation uses that migration-safe create_default_collection_types symbol (or
explicitly reference the migration helper by a unique alias).
In `@specifyweb/specify/migrations/0008_ageCitations_fix.py`:
- Around line 17-18: The revert_migration currently calls
usc.update_relative_age_fields(apps) which re-applies the forward change instead
of undoing it; replace that call with a real reverse helper such as
usc.revert_update_relative_age_fields(apps, schema_editor) (or implement
usc.revert_update_relative_age_fields if missing) that explicitly removes the
schema-config entries for absoluteagecitation and relativeagecitation before
Django drops those fields so rollback cleans up those entries properly.
In `@specifyweb/specify/migrations/0009_tectonic_ranks.py`:
- Around line 20-22: The rollback calls in
revert_cosolidated_python_django_migration_operations are ordered wrong:
revert_create_root_tectonic_node must run before revert_default_tectonic_ranks
because the root node depends on the ranks/tree data; update
revert_cosolidated_python_django_migration_operations to call
revert_create_root_tectonic_node(apps, schema_editor) first and then
revert_default_tectonic_ranks(apps, schema_editor) so the protected TectonicUnit
FKs can be removed before deleting ranks.
In `@specifyweb/specify/migrations/0027_CO_children.py`:
- Around line 5-17: The migration imports mutable helpers
update_co_children_fields and revert_co_children_fields from
specifyweb.specify.migration_utils, which makes this historical migration
non‑self‑contained; copy the current implementation of update_co_children_fields
and revert_co_children_fields directly into the Migration class as the bodies of
apply_migration and revert_migration (or embed a private, versioned helper
within this file) and remove the import so Migration.apply_migration and
Migration.revert_migration contain all schema-change logic locally; keep the
function names apply_migration and revert_migration and any model/apps usage
exactly as in the original helpers when pasting to ensure behavior is preserved.
---
Outside diff comments:
In `@specifyweb/backend/businessrules/migrations/0004_catnum_uniquerule.py`:
- Around line 3-40: The migration defines local functions catnum_rule_editable
and catnum_rule_uneditable that shadow the imported helpers from
migration_utils, so RunPython is calling the wrong implementations; fix by
removing or renaming the local function definitions and make
migrations.RunPython refer to the imported helpers catnum_rule_editable and
catnum_rule_uneditable from migration_utils (or explicitly call
migration_utils.catnum_rule_editable / migration_utils.catnum_rule_uneditable)
so the migration uses the exact-match logic in
migration_utils.create_uniqueness_rule.
In `@specifyweb/backend/businessrules/uniqueness_rules.py`:
- Around line 266-309: The current subset checks in create_uniqueness_rule and
remove_uniqueness_rule allow rules with extra fields/scopes to match; change the
logic to require exact set equality: for each candidate UniquenessRule (use
rule.uniquenessrulefield_set.all()), build the sets of fieldPath for
isScope=False and isScope=True and compare them to the provided fields and
scopes as sets (or check counts plus element equality) so a rule only matches
when both the field set and scope set are exactly equal; apply this exact-match
test where matching_fields/matching_scopes are currently used before skipping
creation or appending rule.id for deletion.
---
Minor comments:
In `@specifyweb/specify/migration_utils/sp7_schemaconfig.py`:
- Around line 150-151: The mapping for 'RelativeAgeCitation' in
sp7_schemaconfig.py is incorrectly using 'absoluteAgeCitationId' instead of the
RelativeAgeCitation primary-key field; update the value for the
'RelativeAgeCitation' key to use its actual PK field (e.g.,
'relativeAgeCitationId' or whatever the schema defines) so the 0023 hidden-field
update targets the correct id row, leaving the 'AbsoluteAgeCitation' entry
unchanged.
---
Nitpick comments:
In `@specifyweb/specify/management/commands/run_key_migration_functions.py`:
- Around line 84-86: The code currently materializes all uniqueness-rule
discipline IDs into a Python set before excluding them, which can blow memory;
change the exclude to use a DB subquery instead (e.g. remove set(...) and pass
the UniquenessRule queryset or wrap it in django.db.models.Subquery) so the
filter uses a database-side subquery; update the loop that iterates
Discipline.objects.exclude(...) and keep the call to
apply_default_uniqueness_rules(discipline, registry=apps) unchanged.
- Line 175: Replace the plain error log call that currently uses
logger.error(f"An error occurred: {e}") with a logger.exception(...) call inside
the except block so the full stack trace is recorded; locate the try/except in
the run_key_migration_functions command (the Command.handle or
run_key_migration_functions function where the logger variable is used) and
change the log invocation to logger.exception("An error occurred while running
key migrations") to capture traceback and context.
In `@specifyweb/specify/migrations/0021_update_hidden_geo_tables.py`:
- Around line 10-54: Delete the commented-out legacy migration implementation:
remove the entire commented bodies of fix_hidden_geo_prop and
reverse_fix_hidden_geo_prop and any commented RunPython invocation so the file
only contains the new delegation to usc.*; specifically remove the commented
blocks referencing Splocalecontainer, Splocalecontaineritem, Discipline,
PALEO_DISCIPLINES | GEOLOGY_DISCIPLINES, SCHEMA_CONFIG_MOD_TABLE_FIELDS and the
duplicated items.update(ishidden=True) code as well as the commented RunPython
call that referenced these functions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d7c847d-2052-48f5-abee-688567845119
📒 Files selected for processing (53)
.envdocker-entrypoint.shspecifyweb/backend/businessrules/migration_utils.pyspecifyweb/backend/businessrules/migrations/0004_catnum_uniquerule.pyspecifyweb/backend/businessrules/migrations/0005_cojo.pyspecifyweb/backend/businessrules/migrations/0008_fix_global_default_rules.pyspecifyweb/backend/businessrules/uniqueness_rules.jsonspecifyweb/backend/businessrules/uniqueness_rules.pyspecifyweb/backend/patches/migration_utils.pyspecifyweb/backend/patches/migrations/0001_restore_separators.pyspecifyweb/backend/patches/migrations/0002_fix_accepted_taxon.pyspecifyweb/backend/patches/migrations/0003_coordinate_fields_fix.pyspecifyweb/backend/permissions/initialize.pyspecifyweb/backend/permissions/migrations/0006_add_dataset_create_recordset_permission.pyspecifyweb/backend/permissions/migrations/0007_add_stats_edit_permission.pyspecifyweb/backend/permissions/migrations/0008_attachment_import_role.pyspecifyweb/backend/workbench/upload/auditlog.pyspecifyweb/frontend/js_src/lib/components/Attachments/__tests__/UploadAttachment.test.tsxspecifyweb/frontend/js_src/lib/components/WorkBench/WbAttachmentsPreview.tsxspecifyweb/permissions/migration_utils/__init__.pyspecifyweb/permissions/migration_utils/edit_permissions.pyspecifyweb/specify/api/utils.pyspecifyweb/specify/management/commands/run_key_migration_functions.pyspecifyweb/specify/migration_utils/default_cots.pyspecifyweb/specify/migration_utils/misc_migrations.pyspecifyweb/specify/migration_utils/sp7_schemaconfig.pyspecifyweb/specify/migration_utils/tectonic_ranks.pyspecifyweb/specify/migration_utils/update_schema_config.pyspecifyweb/specify/migrations/0002_geo.pyspecifyweb/specify/migrations/0003_cotype_picklist.pyspecifyweb/specify/migrations/0004_stratigraphy_age.pyspecifyweb/specify/migrations/0007_schema_config_update.pyspecifyweb/specify/migrations/0008_ageCitations_fix.pyspecifyweb/specify/migrations/0009_tectonic_ranks.pyspecifyweb/specify/migrations/0012_add_cojo_to_schema_config.pyspecifyweb/specify/migrations/0013_collectionobjectgroup_parentcog.pyspecifyweb/specify/migrations/0015_add_version_to_ages.pyspecifyweb/specify/migrations/0017_schemaconfig_fixes.pyspecifyweb/specify/migrations/0018_cot_catnum_schema.pyspecifyweb/specify/migrations/0020_add_tectonicunit_to_pc_in_schema_config.pyspecifyweb/specify/migrations/0021_update_hidden_geo_tables.pyspecifyweb/specify/migrations/0022_ensure_default_cots.pyspecifyweb/specify/migrations/0023_update_schema_config_text.pyspecifyweb/specify/migrations/0024_add_uniqueIdentifier_storage.pyspecifyweb/specify/migrations/0027_CO_children.pyspecifyweb/specify/migrations/0029_remove_collectionobject_parentco.pyspecifyweb/specify/migrations/0031_add_default_for_selectseries.pyspecifyweb/specify/migrations/0032_add_quantities_gift.pyspecifyweb/specify/migrations/0033_update_paleo_desc.pyspecifyweb/specify/migrations/0034_accession_date_fields.pyspecifyweb/specify/migrations/0035_version_required.pyspecifyweb/specify/tests/test_utils/test_create_default_collection_types.pyspecifyweb/specify/utils/field_change_info.py
github-project-automation
Bot
moved this from 📋Back Log
to Dev Attention Needed
in General Tester Board
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
run_key_migration_functions django command
Fixes #6266
Checklist
self-explanatory (or properly documented)
specify7/specifyweb/specify/management/commands/run_key_migration_functions.py
Line 50 in ea04665
Testing instructions
Summary by CodeRabbit
New Features
Bug Fixes