From dea5af248ab8f6a904b66e52c52c00c3c55b8a77 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Wed, 29 Jul 2026 10:32:24 +0200 Subject: [PATCH 1/6] Fix: Pull existing locality values from WB dataset --- .../lib/components/WbToolkit/GeoLocate.tsx | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/WbToolkit/GeoLocate.tsx b/specifyweb/frontend/js_src/lib/components/WbToolkit/GeoLocate.tsx index dd4a7a09823..27c9fb34944 100644 --- a/specifyweb/frontend/js_src/lib/components/WbToolkit/GeoLocate.tsx +++ b/specifyweb/frontend/js_src/lib/components/WbToolkit/GeoLocate.tsx @@ -10,8 +10,8 @@ import type { IR, RA } from '../../utils/types'; import { filterArray } from '../../utils/types'; import { sortFunction } from '../../utils/utils'; import { Button } from '../Atoms/Button'; +import { formatCoordinate, getLocalityField } from '../Leaflet/helpers'; import { - getLocalityCoordinate, getSelectedLocalityColumns, } from '../Leaflet/wbLocalityDataExtractor'; import type { GeoLocatePayload } from '../Molecules/GeoLocate'; @@ -255,24 +255,32 @@ function getGeoLocateData( readonly visualRow: number; } ): IR { - const visualHeaders = getVisualHeaders(hot, columns); + return buildGeoLocateData( + hot.getDataAtRow(visualRow), + getVisualHeaders(hot, columns), + localityColumns + ); +} + +export function buildGeoLocateData( + row: RA, + headers: RA, + localityColumns: IR +): IR { + const getValue = (fieldName: string): string => + getLocalityField(row, headers, localityColumns, fieldName); - const localityData = - getLocalityCoordinate( - hot.getDataAtRow(visualRow), - visualHeaders, - localityColumns - ) || {}; + const latitude = getValue('locality.latitude1'); + const longitude = getValue('locality.longitude1'); const rawData = { - country: localityData['locality.geography.$country.name']?.value, - state: localityData['locality.geography.$state.name']?.value, - county: localityData['locality.geography.$county.name']?.value, - locality: localityData['locality.localityname']?.value, + country: getValue('locality.geography.$country.name') || undefined, + state: getValue('locality.geography.$state.name') || undefined, + county: getValue('locality.geography.$county.name') || undefined, + locality: getValue('locality.localityname') || undefined, points: - typeof localityData['locality.latitude1'] === 'object' && - typeof localityData['locality.longitude1'] === 'object' - ? `${localityData['locality.latitude1'].value}|${localityData['locality.longitude1'].value}` + latitude !== '' && longitude !== '' + ? `${formatCoordinate(latitude)}|${formatCoordinate(longitude)}` : undefined, }; From 5a439c4c9b4206e5dbd12b6012fb09251819962c Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Wed, 29 Jul 2026 14:21:30 +0200 Subject: [PATCH 2/6] Fix: Add a new custom delete for parent field in taxontreedefitem --- ..._taxontreedefitem_parent_context_delete.py | 25 +++++++++++++++++++ specifyweb/specify/models.py | 17 ++++++++++++- 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 specifyweb/specify/migrations/0047_taxontreedefitem_parent_context_delete.py diff --git a/specifyweb/specify/migrations/0047_taxontreedefitem_parent_context_delete.py b/specifyweb/specify/migrations/0047_taxontreedefitem_parent_context_delete.py new file mode 100644 index 00000000000..7e895dc2db0 --- /dev/null +++ b/specifyweb/specify/migrations/0047_taxontreedefitem_parent_context_delete.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.24 on 2026-07-29 + +from django.db import migrations, models +import specifyweb.specify.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('specify', '0046_alter_specifyuser_spprincipals'), + ] + + operations = [ + migrations.AlterField( + model_name='taxontreedefitem', + name='parent', + field=models.ForeignKey( + db_column='ParentItemID', + null=True, + on_delete=specifyweb.specify.models.delete_taxon_rank_parent_with_context, + related_name='children', + to='specify.taxontreedefitem', + ), + ), + ] diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index 3560232bb95..4d3af4aabbf 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -16,6 +16,21 @@ def protect_with_blockers(collector, field, sub_objs, using): else: return models.PROTECT(collector, field, sub_objs, using) + +def delete_taxon_rank_parent_with_context(collector, field, sub_objs, using): + """ + Use CASCADE while deleting an entire TaxonTreeDef, but preserve children + during single-rank deletions so rank reparenting logic can handle them. + """ + deleting_models = getattr(collector, 'data', {}) + is_tree_delete = any( + getattr(model, '__name__', '').lower() == 'taxontreedef' + for model in deleting_models.keys() + ) + if is_tree_delete: + return models.CASCADE(collector, field, sub_objs, using) + return models.DO_NOTHING(collector, field, sub_objs, using) + def custom_save(self, *args, **kwargs): try: # Custom save logic here, if necessary @@ -7337,7 +7352,7 @@ class Taxontreedefitem(model_extras.Taxontreedefitem): # Relationships: Many-to-One createdbyagent = models.ForeignKey('Agent', db_column='CreatedByAgentID', related_name='+', null=True, on_delete=protect_with_blockers) modifiedbyagent = models.ForeignKey('Agent', db_column='ModifiedByAgentID', related_name='+', null=True, on_delete=protect_with_blockers) - parent = models.ForeignKey('TaxonTreeDefItem', db_column='ParentItemID', related_name='children', null=True, on_delete=models.CASCADE) + parent = models.ForeignKey('TaxonTreeDefItem', db_column='ParentItemID', related_name='children', null=True, on_delete=delete_taxon_rank_parent_with_context) treedef = models.ForeignKey('TaxonTreeDef', db_column='TaxonTreeDefID', related_name='treedefitems', null=False, on_delete=models.CASCADE) class Meta: From 09cbf2441c88200e9f270421eab225f21ddeb2ff Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Wed, 29 Jul 2026 14:56:40 +0200 Subject: [PATCH 3/6] Revert unwanted changes --- .../lib/components/WbToolkit/GeoLocate.tsx | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/specifyweb/frontend/js_src/lib/components/WbToolkit/GeoLocate.tsx b/specifyweb/frontend/js_src/lib/components/WbToolkit/GeoLocate.tsx index 27c9fb34944..b862f1610be 100644 --- a/specifyweb/frontend/js_src/lib/components/WbToolkit/GeoLocate.tsx +++ b/specifyweb/frontend/js_src/lib/components/WbToolkit/GeoLocate.tsx @@ -10,8 +10,8 @@ import type { IR, RA } from '../../utils/types'; import { filterArray } from '../../utils/types'; import { sortFunction } from '../../utils/utils'; import { Button } from '../Atoms/Button'; -import { formatCoordinate, getLocalityField } from '../Leaflet/helpers'; import { + getLocalityCoordinate, getSelectedLocalityColumns, } from '../Leaflet/wbLocalityDataExtractor'; import type { GeoLocatePayload } from '../Molecules/GeoLocate'; @@ -255,32 +255,24 @@ function getGeoLocateData( readonly visualRow: number; } ): IR { - return buildGeoLocateData( - hot.getDataAtRow(visualRow), - getVisualHeaders(hot, columns), - localityColumns - ); -} + const visualHeaders = getVisualHeaders(hot, columns); -export function buildGeoLocateData( - row: RA, - headers: RA, - localityColumns: IR -): IR { - const getValue = (fieldName: string): string => - getLocalityField(row, headers, localityColumns, fieldName); - - const latitude = getValue('locality.latitude1'); - const longitude = getValue('locality.longitude1'); + const localityData = + getLocalityCoordinate( + hot.getDataAtRow(visualRow), + visualHeaders, + localityColumns + ) || {}; const rawData = { - country: getValue('locality.geography.$country.name') || undefined, - state: getValue('locality.geography.$state.name') || undefined, - county: getValue('locality.geography.$county.name') || undefined, - locality: getValue('locality.localityname') || undefined, + country: localityData['locality.geography.$country.name']?.value, + state: localityData['locality.geography.$state.name']?.value, + county: localityData['locality.geography.$county.name']?.value, + locality: localityData['locality.localityname']?.value, points: - latitude !== '' && longitude !== '' - ? `${formatCoordinate(latitude)}|${formatCoordinate(longitude)}` + typeof localityData['locality.latitude1'] === 'object' && + typeof localityData['locality.longitude1'] === 'object' + ? `${localityData['locality.latitude1'].value}|${localityData['locality.longitude1'].value}` : undefined, }; @@ -291,4 +283,4 @@ export function buildGeoLocateData( ) ) ); -} +} \ No newline at end of file From 8a06f94f55e55591c4bca642f41c93745fc3a556 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Wed, 29 Jul 2026 16:03:32 +0200 Subject: [PATCH 4/6] Fix: Reparent when deleting a rank --- .../tests/test_taxontreedefitem.py | 34 ++++++++++++++-- specifyweb/specify/models.py | 40 +++++++++++++++++-- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/specifyweb/backend/businessrules/tests/test_taxontreedefitem.py b/specifyweb/backend/businessrules/tests/test_taxontreedefitem.py index 123897020c4..60e1c7552bc 100644 --- a/specifyweb/backend/businessrules/tests/test_taxontreedefitem.py +++ b/specifyweb/backend/businessrules/tests/test_taxontreedefitem.py @@ -1,6 +1,4 @@ -from django.db.models import ProtectedError from specifyweb.specify.tests.test_api import ApiTests -from ..exceptions import TreeBusinessRuleException from specifyweb.specify import models class TaxonTreeDefItemTests(ApiTests): @@ -30,5 +28,35 @@ def test_delete_blocked_by_taxon(self): name="Animals", definition=kingdom.treedef, rankid=kingdom.rankid) - animals.delete() + + def test_delete_unused_rank_reparents_children(self): + kingdom = self.roottaxontreedefitem.children.create( + name="Kingdom", + treedef=self.taxontreedef, + rankid=self.roottaxontreedefitem.rankid+100) + phylum = kingdom.children.create( + name="Phylum", + treedef=self.taxontreedef, + rankid=kingdom.rankid+100) + + models.Taxontreedefitem.objects.filter(id=kingdom.id).delete() + + phylum.refresh_from_db() + self.assertEqual(phylum.parent_id, self.roottaxontreedefitem.id) + self.assertFalse(models.Taxontreedefitem.objects.filter(id=kingdom.id).exists()) + + def test_full_tree_delete_still_cascades(self): + kingdom = self.roottaxontreedefitem.children.create( + name="Kingdom", + treedef=self.taxontreedef, + rankid=self.roottaxontreedefitem.rankid+100) + kingdom.treeentries.create( + parent=self.roottaxon, + name="Animals", + definition=kingdom.treedef, + rankid=kingdom.rankid) + + self.taxontreedef.delete() + + self.assertFalse(models.Taxontreedef.objects.filter(id=self.taxontreedef.id).exists()) diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index 4d3af4aabbf..d6db3142860 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -17,10 +17,32 @@ def protect_with_blockers(collector, field, sub_objs, using): return models.PROTECT(collector, field, sub_objs, using) +def _get_collector_model_instances(collector, model_name): + deleting_models = getattr(collector, 'data', {}) + for model, objs in deleting_models.items(): + if getattr(model, '__name__', '').lower() == model_name.lower(): + return list(objs) + return [] + + +def _reparent_taxon_rank_children(ranks): + rank_list = list(ranks) + if not rank_list: + return + + deleting_rank_ids = {rank.id for rank in rank_list} + for rank in rank_list: + Taxontreedefitem.objects.filter(parent_id=rank.id)\ + .exclude(id__in=deleting_rank_ids)\ + .update(parent_id=rank.parent_id) + + def delete_taxon_rank_parent_with_context(collector, field, sub_objs, using): """ - Use CASCADE while deleting an entire TaxonTreeDef, but preserve children - during single-rank deletions so rank reparenting logic can handle them. + Use CASCADE while deleting an entire TaxonTreeDef. + + For single-rank deletion, reparent child ranks before deleting so + ParentItemID remains valid. """ deleting_models = getattr(collector, 'data', {}) is_tree_delete = any( @@ -29,7 +51,19 @@ def delete_taxon_rank_parent_with_context(collector, field, sub_objs, using): ) if is_tree_delete: return models.CASCADE(collector, field, sub_objs, using) - return models.DO_NOTHING(collector, field, sub_objs, using) + + processed_ids = getattr(collector, '_taxon_rank_delete_prepared_ids', set()) + ranks_to_delete = [ + rank + for rank in _get_collector_model_instances(collector, 'taxontreedefitem') + if rank.id not in processed_ids + ] + _reparent_taxon_rank_children(ranks_to_delete) + + if ranks_to_delete: + collector._taxon_rank_delete_prepared_ids = processed_ids.union( + {rank.id for rank in ranks_to_delete} + ) def custom_save(self, *args, **kwargs): try: From fb39fa641ef200f8cb4887434437b76c9d9c6c69 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Thu, 30 Jul 2026 14:15:36 +0200 Subject: [PATCH 5/6] Potential fix for pull request finding 'CodeQL / Explicit returns mixed with implicit (fall through) returns' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- specifyweb/specify/models.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index d6db3142860..75f754cbe93 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -65,6 +65,8 @@ def delete_taxon_rank_parent_with_context(collector, field, sub_objs, using): {rank.id for rank in ranks_to_delete} ) + return None + def custom_save(self, *args, **kwargs): try: # Custom save logic here, if necessary From c621cb1aa4ab54a24f709e430f5fa5e0401f5d21 Mon Sep 17 00:00:00 2001 From: Caroline Denis Date: Thu, 30 Jul 2026 14:18:17 +0200 Subject: [PATCH 6/6] Fix: Typing --- .../businessrules/migrations/0002_default_unique_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/specifyweb/backend/businessrules/migrations/0002_default_unique_rules.py b/specifyweb/backend/businessrules/migrations/0002_default_unique_rules.py index ce308832494..e926cba165a 100644 --- a/specifyweb/backend/businessrules/migrations/0002_default_unique_rules.py +++ b/specifyweb/backend/businessrules/migrations/0002_default_unique_rules.py @@ -56,5 +56,5 @@ class Migration(migrations.Migration): operations = [ migrations.RunPython(apply_default_rules, - remove_default_rules, atomic=True), + remove_default_rules, atomic=True), ]