From 48763cdcfb247ab1347e55dfd3c55675e575dad1 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 12 Jan 2023 14:00:35 -0600 Subject: [PATCH 01/10] Refactor and fine tune deletion rules Fixes #1694, #2784, #1069 --- specifyweb/specify/build_models.py | 22 ++--------- specifyweb/specify/deletion_rules.py | 55 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 19 deletions(-) create mode 100644 specifyweb/specify/deletion_rules.py diff --git a/specifyweb/specify/build_models.py b/specifyweb/specify/build_models.py index 7645dd153d9..92faa668681 100644 --- a/specifyweb/specify/build_models.py +++ b/specifyweb/specify/build_models.py @@ -1,9 +1,9 @@ from django.db import models from specifyweb.businessrules.exceptions import AbortSave - from . import model_extras from .case_insensitive_bool import BooleanField, NullBooleanField +from .deletion_rules import SPECIAL_DELETION_RULES appname = __name__.split('.')[-2] @@ -73,23 +73,7 @@ def protect(collector, field, sub_objs, using): else: models.PROTECT(collector, field, sub_objs, using) -SPECIAL_DELETION_RULES = { - 'Agent.specifyuser': models.SET_NULL, - 'Recordsetitem.recordset': models.CASCADE, - - # Handle workbench deletion using raw sql in business rules. - 'Workbenchrow.workbench': models.DO_NOTHING, - 'Workbenchdataitem.workbenchrow': models.DO_NOTHING, - 'Workbenchrowimage.workbenchrow': models.DO_NOTHING, - 'Workbenchrowexportedrelationship.workbenchrow': models.DO_NOTHING, - - 'Spappresourcedir.specifyuser': models.CASCADE, - 'Spappresource.specifyuser': models.CASCADE, - 'Spappresource.spappresourcedir': models.CASCADE, - 'Spappresourcedata.spappresource': models.CASCADE, - 'Spappresourcedata.spviewsetobj': models.CASCADE, - 'Spreport.appresource': models.CASCADE, -} + def make_relationship(modelname, rel, datamodel): """Return a Django relationship field for the given relationship definition. @@ -112,7 +96,7 @@ def make_relationship(modelname, rel, datamodel): return None try: - on_delete = SPECIAL_DELETION_RULES["%s.%s" % (modelname.capitalize(), rel.name.lower())] + on_delete = SPECIAL_DELETION_RULES[f"{rel.name.capitalize()}"][f"{modelname.lower()}"] except KeyError: reverse = datamodel.reverse_relationship(rel) if reverse and reverse.dependent: diff --git a/specifyweb/specify/deletion_rules.py b/specifyweb/specify/deletion_rules.py new file mode 100644 index 00000000000..3da21d3eed7 --- /dev/null +++ b/specifyweb/specify/deletion_rules.py @@ -0,0 +1,55 @@ +from django.db import models + +""" Special Deletion Rules for table relationships + + Of the form: base_table : { relationship: action} + + Possible Django actions (w/ django 2.2) are: + CASCADE, PROTECT, SET_NULL, SET_DEFAULT, SET(...), DO_NOTHING + + More information can be found at django's docs here: + https://docs.djangoproject.com/en/2.2/ref/models/fields/#django.db.models.ForeignKey.on_delete +""" +SPECIAL_DELETION_RULES = { + 'Appresource' : {'spreport': models.CASCADE}, + + 'Picklist' : {'picklistitem': models.CASCADE}, + + 'Recordset' : {'Recordsetitem' : models.CASCADE}, + + 'Specifyuser' : { + 'agent' : models.SET_NULL, + 'spappresourcedir' : models.CASCADE, + 'spappresource': models.CASCADE, + 'spprincipal': models.CASCADE, + }, + + # Handle workbench deletion using raw sql in business rules. + 'Workbench' : {'workbenchrow': models.DO_NOTHING}, + 'Workbenchrow' : { + 'workbenchdataitem' : models.DO_NOTHING, + 'workbenchrowimage' : models.DO_NOTHING, + 'workbenchrowexportedrelationship' : models.DO_NOTHING, + }, + + # System Tables + + 'Spauditlog' : {'Spauditlogfield' : models.CASCADE}, + + 'Spappresource' : {'spappresourcedata': models.CASCADE}, + 'Spappresourcedir' : { + 'spappresource': models.CASCADE, + 'spviewsetobj' : models.CASCADE, + 'spappresourcedata': models.CASCADE, + }, + 'Spviewsetobj' : {'spappresourcedata' : models.CASCADE}, + + 'Splocalecontainer' : { + 'splocalecontaineritem' : models.CASCADE, + 'splocaleitemstr' : models.CASCADE + }, + 'Splocalecontaineritem' : { + 'splocaleitemstr' : models.CASCADE + }, + 'SpQuery' : {"spqueryfield" : models.CASCADE}, +} \ No newline at end of file From c25ab91a14b9f0d609e325759d6b503887bf84c3 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 12 Jan 2023 15:49:33 -0600 Subject: [PATCH 02/10] Add ability to extend deletion blockers This was originally in commit 56cb36f2, but is being moved here as this is more fitting for the functionality and easier to find in the future --- specifyweb/specify/build_models.py | 4 +++- specifyweb/specify/deletion_rules.py | 14 +++++++++++++- specifyweb/specify/views.py | 13 +++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/specifyweb/specify/build_models.py b/specifyweb/specify/build_models.py index 92faa668681..0e2f192e9a8 100644 --- a/specifyweb/specify/build_models.py +++ b/specifyweb/specify/build_models.py @@ -3,7 +3,7 @@ from specifyweb.businessrules.exceptions import AbortSave from . import model_extras from .case_insensitive_bool import BooleanField, NullBooleanField -from .deletion_rules import SPECIAL_DELETION_RULES +from .deletion_rules import SPECIAL_DELETION_RULES, ADDITIONAL_DELETE_BLOCKERS appname = __name__.split('.')[-2] @@ -101,6 +101,8 @@ def make_relationship(modelname, rel, datamodel): reverse = datamodel.reverse_relationship(rel) if reverse and reverse.dependent: on_delete = models.CASCADE + elif modelname in ADDITIONAL_DELETE_BLOCKERS.keys(): + on_delete = protect else: on_delete = protect diff --git a/specifyweb/specify/deletion_rules.py b/specifyweb/specify/deletion_rules.py index 3da21d3eed7..2d67f2a7ab5 100644 --- a/specifyweb/specify/deletion_rules.py +++ b/specifyweb/specify/deletion_rules.py @@ -9,6 +9,8 @@ More information can be found at django's docs here: https://docs.djangoproject.com/en/2.2/ref/models/fields/#django.db.models.ForeignKey.on_delete + + See .build_models.py for uses """ SPECIAL_DELETION_RULES = { 'Appresource' : {'spreport': models.CASCADE}, @@ -23,7 +25,7 @@ 'spappresource': models.CASCADE, 'spprincipal': models.CASCADE, }, - + # Handle workbench deletion using raw sql in business rules. 'Workbench' : {'workbenchrow': models.DO_NOTHING}, 'Workbenchrow' : { @@ -52,4 +54,14 @@ 'splocaleitemstr' : models.CASCADE }, 'SpQuery' : {"spqueryfield" : models.CASCADE}, +} + +""" Any additional desired delete blockers +Of the form 'base_table': ['field_1_name', 'field_2_name', ...] +Use the django attributes from the 'base_table' for field names + +See .build_models.py and .views.py for uses +""" +ADDITIONAL_DELETE_BLOCKERS = { + 'Agent' : ['specifyuser'], } \ No newline at end of file diff --git a/specifyweb/specify/views.py b/specifyweb/specify/views.py index d9b40dda107..b0269cb55c1 100644 --- a/specifyweb/specify/views.py +++ b/specifyweb/specify/views.py @@ -20,6 +20,7 @@ from specifyweb.workbench.upload.upload_result import FailedBusinessRule from . import api, models from .specify_jar import specify_jar +from .deletion_rules import ADDITIONAL_DELETE_BLOCKERS def login_maybe_required(view): @wraps(view) @@ -98,6 +99,18 @@ def delete_blockers(request, model, id): } for field, sub_objs in collector.delete_blockers ] + + # Check if the object has additional delete blockers. + # If so, add them to the response only if the object has data in the 'field' + if model.capitalize() in ADDITIONAL_DELETE_BLOCKERS: + rel_data = ADDITIONAL_DELETE_BLOCKERS[model.capitalize()] + result.extend([{ + 'table' : getattr(obj, data).specify_model.django_name, + "field" : data, + 'id' : getattr(obj, data).id + } + for data in rel_data if getattr(obj, data) is not None]) + return http.HttpResponse(api.toJson(result), content_type='application/json') @login_maybe_required From 789e692a831f429b6087e9f6a7b4d2aaafd14ab7 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Wed, 18 Jan 2023 16:58:30 -0600 Subject: [PATCH 03/10] Correct field names --- specifyweb/businessrules/agent_rules.py | 8 +- specifyweb/specify/build_models.py | 2 +- specifyweb/specify/deletion_rules.py | 131 ++++++++++++------------ 3 files changed, 71 insertions(+), 70 deletions(-) diff --git a/specifyweb/businessrules/agent_rules.py b/specifyweb/businessrules/agent_rules.py index 4e13f429a2c..44ef04e1032 100644 --- a/specifyweb/businessrules/agent_rules.py +++ b/specifyweb/businessrules/agent_rules.py @@ -37,5 +37,9 @@ def agent_types_other_and_group_do_not_have_addresses(agent): # This Business Rule (Agents of type Other/Group can not have Addresses) was removed # See https://github.com/specify/specify7/issues/2518 for more information - # if agent_types[agent.agenttype] in ('Other', 'Group'): - # agent.addresses.all().delete() + if agent_types[agent.agenttype] in ('Other', 'Group'): + from specifyweb.specify import models + rel = models.datamodel.get_table_strict('Splocalecontaineritem').get_relationship('splocaleitemstr') + raise KeyError(rel, f"Dep: {rel.dependent}", f"Reverse: {models.datamodel.reverse_relationship(rel)}") + raise KeyError(models.datamodel.get_table('Spappresource').get_relationship('spreports').dependent) + agent.addresses.all().delete() diff --git a/specifyweb/specify/build_models.py b/specifyweb/specify/build_models.py index c1d96de584c..15a648bce7b 100644 --- a/specifyweb/specify/build_models.py +++ b/specifyweb/specify/build_models.py @@ -100,7 +100,7 @@ def make_relationship(modelname, rel, datamodel): except KeyError: reverse = datamodel.reverse_relationship(rel) - if reverse and reverse.dependent: + if reverse is not None and reverse.dependent: on_delete = models.CASCADE elif modelname in ADDITIONAL_DELETE_BLOCKERS.keys(): on_delete = protect diff --git a/specifyweb/specify/deletion_rules.py b/specifyweb/specify/deletion_rules.py index 2d67f2a7ab5..2a3bfcb0287 100644 --- a/specifyweb/specify/deletion_rules.py +++ b/specifyweb/specify/deletion_rules.py @@ -1,67 +1,64 @@ -from django.db import models - -""" Special Deletion Rules for table relationships - - Of the form: base_table : { relationship: action} - - Possible Django actions (w/ django 2.2) are: - CASCADE, PROTECT, SET_NULL, SET_DEFAULT, SET(...), DO_NOTHING - - More information can be found at django's docs here: - https://docs.djangoproject.com/en/2.2/ref/models/fields/#django.db.models.ForeignKey.on_delete - - See .build_models.py for uses -""" -SPECIAL_DELETION_RULES = { - 'Appresource' : {'spreport': models.CASCADE}, - - 'Picklist' : {'picklistitem': models.CASCADE}, - - 'Recordset' : {'Recordsetitem' : models.CASCADE}, - - 'Specifyuser' : { - 'agent' : models.SET_NULL, - 'spappresourcedir' : models.CASCADE, - 'spappresource': models.CASCADE, - 'spprincipal': models.CASCADE, - }, - - # Handle workbench deletion using raw sql in business rules. - 'Workbench' : {'workbenchrow': models.DO_NOTHING}, - 'Workbenchrow' : { - 'workbenchdataitem' : models.DO_NOTHING, - 'workbenchrowimage' : models.DO_NOTHING, - 'workbenchrowexportedrelationship' : models.DO_NOTHING, - }, - - # System Tables - - 'Spauditlog' : {'Spauditlogfield' : models.CASCADE}, - - 'Spappresource' : {'spappresourcedata': models.CASCADE}, - 'Spappresourcedir' : { - 'spappresource': models.CASCADE, - 'spviewsetobj' : models.CASCADE, - 'spappresourcedata': models.CASCADE, - }, - 'Spviewsetobj' : {'spappresourcedata' : models.CASCADE}, - - 'Splocalecontainer' : { - 'splocalecontaineritem' : models.CASCADE, - 'splocaleitemstr' : models.CASCADE - }, - 'Splocalecontaineritem' : { - 'splocaleitemstr' : models.CASCADE - }, - 'SpQuery' : {"spqueryfield" : models.CASCADE}, -} - -""" Any additional desired delete blockers -Of the form 'base_table': ['field_1_name', 'field_2_name', ...] -Use the django attributes from the 'base_table' for field names - -See .build_models.py and .views.py for uses -""" -ADDITIONAL_DELETE_BLOCKERS = { - 'Agent' : ['specifyuser'], -} \ No newline at end of file +from django.db import models + +""" Special Deletion Rules for table relationships + + Of the form: base_table : { relationship: action} + + Possible Django actions (w/ django 2.2) are: + CASCADE, PROTECT, SET_NULL, SET_DEFAULT, SET(...), DO_NOTHING + + More information can be found at django's docs here: + https://docs.djangoproject.com/en/2.2/ref/models/fields/#django.db.models.ForeignKey.on_delete + + See .build_models.py for uses +""" +SPECIAL_DELETION_RULES = { + 'Spappresource' : {'spreports': models.CASCADE}, + + 'Recordset' : {'recordsetitems' : models.CASCADE}, + + 'Specifyuser' : { + 'agent' : models.SET_NULL, + 'spAppResourceDirs' : models.CASCADE, + 'spAppResources': models.CASCADE, + 'spPrincipals': models.CASCADE, + }, + + # Handle workbench deletion using raw sql in business rules. + 'Workbench' : {'workbenchrow': models.DO_NOTHING}, + 'Workbenchrow' : { + 'workbenchdataitem' : models.DO_NOTHING, + 'workbenchrowimage' : models.DO_NOTHING, + 'workbenchrowexportedrelationship' : models.DO_NOTHING, + }, + + # System Tables + + 'Spauditlog' : {'fields' : models.CASCADE}, + + 'Spappresource' : {'spAppResourceDatas': models.CASCADE}, + 'Spappresourcedir' : { + 'spPersistedAppResources': models.CASCADE, + 'spPersistedViewSets' : models.CASCADE, + }, + 'Spviewsetobj' : {'spAppResourceDatas' : models.CASCADE}, + + 'Splocalecontainer' : { + 'items' : models.CASCADE, + 'names' : models.CASCADE + }, + 'Splocalecontaineritem' : { + 'name' : models.CASCADE + }, + 'SpQuery' : {"fields" : models.CASCADE}, +} + +""" Any additional desired delete blockers +Of the form 'base_table': ['field_1_name', 'field_2_name', ...] +Use the django attributes from the 'base_table' for field names + +See .build_models.py and .views.py for uses +""" +ADDITIONAL_DELETE_BLOCKERS = { + 'Agent' : ['specifyuser'], +} From e054a5df86e52884b3f3195a65a02373bc0ed47e Mon Sep 17 00:00:00 2001 From: melton-jason Date: Thu, 19 Jan 2023 14:51:52 -0600 Subject: [PATCH 04/10] Delete 'names' rather than 'name' in splocalecontaineritem Additionally remove lines from agent_rules.py which were used for testing, and should not have gotten pushed to origin! --- specifyweb/businessrules/agent_rules.py | 8 ++------ specifyweb/specify/deletion_rules.py | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/specifyweb/businessrules/agent_rules.py b/specifyweb/businessrules/agent_rules.py index 44ef04e1032..2232124406a 100644 --- a/specifyweb/businessrules/agent_rules.py +++ b/specifyweb/businessrules/agent_rules.py @@ -37,9 +37,5 @@ def agent_types_other_and_group_do_not_have_addresses(agent): # This Business Rule (Agents of type Other/Group can not have Addresses) was removed # See https://github.com/specify/specify7/issues/2518 for more information - if agent_types[agent.agenttype] in ('Other', 'Group'): - from specifyweb.specify import models - rel = models.datamodel.get_table_strict('Splocalecontaineritem').get_relationship('splocaleitemstr') - raise KeyError(rel, f"Dep: {rel.dependent}", f"Reverse: {models.datamodel.reverse_relationship(rel)}") - raise KeyError(models.datamodel.get_table('Spappresource').get_relationship('spreports').dependent) - agent.addresses.all().delete() + # if agent_types[agent.agenttype] in ('Other', 'Group'): + # agent.addresses.all().delete() diff --git a/specifyweb/specify/deletion_rules.py b/specifyweb/specify/deletion_rules.py index 2a3bfcb0287..30b58cd28c4 100644 --- a/specifyweb/specify/deletion_rules.py +++ b/specifyweb/specify/deletion_rules.py @@ -48,7 +48,7 @@ 'names' : models.CASCADE }, 'Splocalecontaineritem' : { - 'name' : models.CASCADE + 'names' : models.CASCADE }, 'SpQuery' : {"fields" : models.CASCADE}, } From c4371b701e01658367c5f668f26397c5be8bfd8b Mon Sep 17 00:00:00 2001 From: melton-jason Date: Fri, 20 Jan 2023 16:49:44 -0600 Subject: [PATCH 05/10] Fix to-manys cascade deletion rules Turns out the models contain the attribute name as plural, to signify this is a to-many relationship. For deletion rules, the table name is needed and NOT the name of the attribute on the base_table model --- specifyweb/specify/deletion_rules.py | 30 ++++++++++++++-------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/specifyweb/specify/deletion_rules.py b/specifyweb/specify/deletion_rules.py index 30b58cd28c4..0a5080547b0 100644 --- a/specifyweb/specify/deletion_rules.py +++ b/specifyweb/specify/deletion_rules.py @@ -2,7 +2,7 @@ """ Special Deletion Rules for table relationships - Of the form: base_table : { relationship: action} + Of the form: base_table : { field_name: action} Possible Django actions (w/ django 2.2) are: CASCADE, PROTECT, SET_NULL, SET_DEFAULT, SET(...), DO_NOTHING @@ -13,15 +13,15 @@ See .build_models.py for uses """ SPECIAL_DELETION_RULES = { - 'Spappresource' : {'spreports': models.CASCADE}, + 'Spappresource' : {'spreport': models.CASCADE}, - 'Recordset' : {'recordsetitems' : models.CASCADE}, + 'Recordset' : {'recordsetitem' : models.CASCADE}, 'Specifyuser' : { 'agent' : models.SET_NULL, - 'spAppResourceDirs' : models.CASCADE, - 'spAppResources': models.CASCADE, - 'spPrincipals': models.CASCADE, + 'spAppResourceDir' : models.CASCADE, + 'spAppResource': models.CASCADE, + 'spPrincipal': models.CASCADE, }, # Handle workbench deletion using raw sql in business rules. @@ -34,23 +34,23 @@ # System Tables - 'Spauditlog' : {'fields' : models.CASCADE}, + 'Spauditlog' : {'field' : models.CASCADE}, - 'Spappresource' : {'spAppResourceDatas': models.CASCADE}, + 'Spappresource' : {'spAppResourceData': models.CASCADE}, 'Spappresourcedir' : { - 'spPersistedAppResources': models.CASCADE, - 'spPersistedViewSets' : models.CASCADE, + 'spPersistedAppResource': models.CASCADE, + 'spPersistedViewSet' : models.CASCADE, }, - 'Spviewsetobj' : {'spAppResourceDatas' : models.CASCADE}, + 'Spviewsetobj' : {'spAppResourceData' : models.CASCADE}, 'Splocalecontainer' : { - 'items' : models.CASCADE, - 'names' : models.CASCADE + 'item' : models.CASCADE, + 'name' : models.CASCADE }, 'Splocalecontaineritem' : { - 'names' : models.CASCADE + 'name' : models.CASCADE }, - 'SpQuery' : {"fields" : models.CASCADE}, + 'SpQuery' : {"field" : models.CASCADE}, } """ Any additional desired delete blockers From 212be10fdfcc89d4e7ebac50ba3014226c370d28 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Fri, 27 Jan 2023 12:44:43 -0600 Subject: [PATCH 06/10] Add typing to build_models.py --- specifyweb/specify/build_models.py | 30 ++++++++++++++++-------------- specifyweb/specify/models.py | 7 ++++++- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/specifyweb/specify/build_models.py b/specifyweb/specify/build_models.py index 15a648bce7b..0f3bfaeec33 100644 --- a/specifyweb/specify/build_models.py +++ b/specifyweb/specify/build_models.py @@ -1,8 +1,10 @@ from django.db import models +from typing import Dict from specifyweb.businessrules.exceptions import AbortSave from . import model_extras from .case_insensitive_bool import BooleanField, NullBooleanField +from .datamodel import Datamodel, Table, Relationship, Field from .deletion_rules import SPECIAL_DELETION_RULES, ADDITIONAL_DELETE_BLOCKERS appname = __name__.split('.')[-2] @@ -16,7 +18,7 @@ 'Collector': ('ordernumber',), } -def make_model(module, table, datamodel): +def make_model(module: str, table: Table, datamodel: Datamodel) -> models.base.ModelBase: """Returns a Django model class based on the definition of a Specify table. """ @@ -64,18 +66,16 @@ def save(self, *args, **kwargs): return model -def make_id_field(column): +def make_id_field(column: str) -> models.AutoField: return models.AutoField(primary_key=True, db_column=column.lower()) -def protect(collector, field, sub_objs, using): +def protect(collector: models.deletion.Collector, field: models.Field, sub_objs: models.query.QuerySet, using:str): if hasattr(collector, 'delete_blockers'): collector.delete_blockers.append((field, sub_objs)) else: models.PROTECT(collector, field, sub_objs, using) - - -def make_relationship(modelname, rel, datamodel): +def make_relationship(modelname : str, rel: Relationship, datamodel: Datamodel) -> models.ForeignKey or models.OneToOneField: """Return a Django relationship field for the given relationship definition. modelname - name of the model this field will be part of @@ -107,7 +107,7 @@ def make_relationship(modelname, rel, datamodel): else: on_delete = protect - def make_to_one(Field): + def make_to_one(Field: type) -> models.ForeignKey or models.OneToOneField: """Setup a field of the given 'Field' type which can be either ForeignKey (many-to-one) or OneToOneField. """ @@ -134,7 +134,7 @@ class make_field(object): mechanism to factor out common aspects of Field configuration. """ @classmethod - def get_field_class(cls, fld): + def get_field_class(cls: type, fld) -> models.Field: """Return the Django model field class to be used for the given field definition. Defaults to returning the 'field_class' attribute of the class, but can be overridden @@ -143,7 +143,7 @@ def get_field_class(cls, fld): return cls.field_class @classmethod - def make_args(cls, fld): + def make_args(cls, fld: Field)-> Dict[str, str or bool]: """Return a dict of arguments for the field constructor based on the XML definition. These are common arguements used by most field types. @@ -154,7 +154,7 @@ def make_args(cls, fld): unique = fld.unique, null = not fld.required) - def __new__(cls, fld, fldargs): + def __new__(cls, fld: Field, fldargs: Dict[str, bool]): """Override the instance constructor to return configured instances of the appropriant Django model field for given parameters. @@ -206,7 +206,7 @@ class make_decimal_field(make_field): field_class = models.DecimalField @classmethod - def make_args(cls, fld): + def make_args(cls, fld: Field): """Augment the standard field options with those specific to Decimal fields. """ @@ -224,7 +224,7 @@ def make_args(cls, fld): class make_boolean_field(make_field): """A specialization of make_field for Boolean type fields.""" @classmethod - def get_field_class(cls, fld): + def get_field_class(cls, fld: Field): """Django differentiates between boolean fields which can contain nulls and those that cannot with different types. @@ -232,7 +232,7 @@ def get_field_class(cls, fld): return BooleanField if fld.required else NullBooleanField @classmethod - def make_args(cls, fld): + def make_args(cls, fld: Field): """Make False the default as it was in Django 1.5""" args = super(make_boolean_field, cls).make_args(fld) if fld.required: @@ -257,7 +257,9 @@ def make_args(cls, fld): 'java.lang.Boolean': make_boolean_field, } -def build_models(module, datamodel): +# Build the table information as Django models +# See .models.py +def build_models(module : str, datamodel: Datamodel): return { model.specify_model.tableId: model for table in datamodel.tables for model in [ make_model(module, table, datamodel) ]} diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index bd7af4673fe..4344d967466 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -1,12 +1,17 @@ """ Sets up Django ORM with the Specify datamodel """ +from typing import Dict +from django.db.models.base import ModelBase from .build_models import build_models from .check_versions import check_versions from .datamodel import datamodel -models_by_tableid = build_models(__name__, datamodel) +# Returns a dictonary with the table's TableId as keys and the reated django models as values +# The values (class paths) are constructed with this Module's name followed by the table name +# Example: {7: } +models_by_tableid : Dict[int, ModelBase] = build_models(__name__, datamodel) # inject the model definitions into this module's namespace globals().update((model.__name__, model) From 5322cafb537163887c49b4b25093491d55cc7aa8 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Sat, 28 Jan 2023 21:34:04 -0600 Subject: [PATCH 07/10] Reformat special deletion rules --- specifyweb/specify/build_models.py | 14 ++++-- specifyweb/specify/deletion_rules.py | 69 ++++++++++++---------------- specifyweb/specify/load_datamodel.py | 4 ++ specifyweb/specify/models.py | 4 +- 4 files changed, 45 insertions(+), 46 deletions(-) diff --git a/specifyweb/specify/build_models.py b/specifyweb/specify/build_models.py index 0f3bfaeec33..34c6471e980 100644 --- a/specifyweb/specify/build_models.py +++ b/specifyweb/specify/build_models.py @@ -7,6 +7,7 @@ from .datamodel import Datamodel, Table, Relationship, Field from .deletion_rules import SPECIAL_DELETION_RULES, ADDITIONAL_DELETE_BLOCKERS +# this moduel's parent directory, 'specify' appname = __name__.split('.')[-2] orderings = { @@ -18,7 +19,7 @@ 'Collector': ('ordernumber',), } -def make_model(module: str, table: Table, datamodel: Datamodel) -> models.base.ModelBase: +def make_model(module: str, table: Table, datamodel: Datamodel) -> models.Model: """Returns a Django model class based on the definition of a Specify table. """ @@ -96,18 +97,23 @@ def make_relationship(modelname : str, rel: Relationship, datamodel: Datamodel) return None try: - on_delete = SPECIAL_DELETION_RULES[f"{rel.name.capitalize()}"][f"{modelname.lower()}"] + on_delete = SPECIAL_DELETION_RULES[f"{modelname}"][f"{rel.name.lower()}"] except KeyError: reverse = datamodel.reverse_relationship(rel) if reverse is not None and reverse.dependent: + + # Example: Current `rel` is Accessionagent.accession, + # Reverse (Accession.accessionAgents) is flagged as dependent + # (see dependent_fields in .load_datamodel.py), so Cascade Accession Agent + # when deleting Accession on_delete = models.CASCADE elif modelname in ADDITIONAL_DELETE_BLOCKERS.keys(): on_delete = protect else: on_delete = protect - def make_to_one(Field: type) -> models.ForeignKey or models.OneToOneField: + def make_to_one(Field: (models.ForeignKey or models.OneToOneField)) -> models.ForeignKey or models.OneToOneField: """Setup a field of the given 'Field' type which can be either ForeignKey (many-to-one) or OneToOneField. """ @@ -259,7 +265,7 @@ def make_args(cls, fld: Field): # Build the table information as Django models # See .models.py -def build_models(module : str, datamodel: Datamodel): +def build_models(module : str, datamodel: Datamodel) -> Dict[int, models.Model]: return { model.specify_model.tableId: model for table in datamodel.tables for model in [ make_model(module, table, datamodel) ]} diff --git a/specifyweb/specify/deletion_rules.py b/specifyweb/specify/deletion_rules.py index 0a5080547b0..c6f9a54f64f 100644 --- a/specifyweb/specify/deletion_rules.py +++ b/specifyweb/specify/deletion_rules.py @@ -2,7 +2,14 @@ """ Special Deletion Rules for table relationships - Of the form: base_table : { field_name: action} + Of the form: 'relatedTable.relName' : action, + where relName is the field name of the relationship in relatedTable + + For example, 'Accessionagent.accession' : models.CASCADE + would delete the associated Accession Agent when Accession is deleted + + The field and relationship names can be viewed at + https://files.specifysoftware.org/schema/version/2.10/ Possible Django actions (w/ django 2.2) are: CASCADE, PROTECT, SET_NULL, SET_DEFAULT, SET(...), DO_NOTHING @@ -13,49 +20,31 @@ See .build_models.py for uses """ SPECIAL_DELETION_RULES = { - 'Spappresource' : {'spreport': models.CASCADE}, - - 'Recordset' : {'recordsetitem' : models.CASCADE}, - - 'Specifyuser' : { - 'agent' : models.SET_NULL, - 'spAppResourceDir' : models.CASCADE, - 'spAppResource': models.CASCADE, - 'spPrincipal': models.CASCADE, - }, - - # Handle workbench deletion using raw sql in business rules. - 'Workbench' : {'workbenchrow': models.DO_NOTHING}, - 'Workbenchrow' : { - 'workbenchdataitem' : models.DO_NOTHING, - 'workbenchrowimage' : models.DO_NOTHING, - 'workbenchrowexportedrelationship' : models.DO_NOTHING, - }, - - # System Tables - - 'Spauditlog' : {'field' : models.CASCADE}, - - 'Spappresource' : {'spAppResourceData': models.CASCADE}, - 'Spappresourcedir' : { - 'spPersistedAppResource': models.CASCADE, - 'spPersistedViewSet' : models.CASCADE, - }, - 'Spviewsetobj' : {'spAppResourceData' : models.CASCADE}, - - 'Splocalecontainer' : { - 'item' : models.CASCADE, - 'name' : models.CASCADE - }, - 'Splocalecontaineritem' : { - 'name' : models.CASCADE - }, - 'SpQuery' : {"field" : models.CASCADE}, + 'Agent.specifyuser' : models.SET_NULL, + + # Handle workbench deletion using raw sql in business rules. + 'Workbenchrow.workbench': models.DO_NOTHING, + 'Workbenchdataitem.workbenchrow': models.DO_NOTHING, + 'Workbenchrowimage.workbenchrow': models.DO_NOTHING, + 'Workbenchrowexportedrelationship.workbenchrow': models.DO_NOTHING, + + # These fields are not marked as dependent because the relationship + # can be null + 'Spappresourcedir.specifyuser': models.CASCADE, + 'Spappresource.specifyuser': models.CASCADE, + 'Spappresourcedata.spappresource': models.CASCADE, + 'Spappresourcedata.spviewsetobj': models.CASCADE, + + # In addition to these rules, Specify Cascades relationships tagged as dependent + # For the complete list, see the global variable called + # dependent_fields in .load_datamodel.py + + # If the field/relationship is not dependent and not defined here, then it is + # protected } """ Any additional desired delete blockers Of the form 'base_table': ['field_1_name', 'field_2_name', ...] -Use the django attributes from the 'base_table' for field names See .build_models.py and .views.py for uses """ diff --git a/specifyweb/specify/load_datamodel.py b/specifyweb/specify/load_datamodel.py index 243a1bea5f3..ce852e43ffe 100644 --- a/specifyweb/specify/load_datamodel.py +++ b/specifyweb/specify/load_datamodel.py @@ -342,11 +342,15 @@ def flag_system_tables(datamodel: Datamodel) -> None: 'Preparation.preparationattrs', 'Preparation.preparationproperties', 'Preptype.attributedefs', + 'Recordset.recordsetitems', 'Referencework.authors', 'Repositoryagreement.addressofrecord', 'Repositoryagreement.repositoryagreementagents', 'Repositoryagreement.repositoryagreementauthorizations', + 'Spappresource.spAppResourceDir', + 'Spauditlog.fields', 'Spquery.fields', + 'Spreport.appResource', 'Taxon.commonnames', 'Taxon.taxoncitations', 'Taxon.taxonattribute', diff --git a/specifyweb/specify/models.py b/specifyweb/specify/models.py index 4344d967466..4da850d1084 100644 --- a/specifyweb/specify/models.py +++ b/specifyweb/specify/models.py @@ -2,7 +2,7 @@ Sets up Django ORM with the Specify datamodel """ from typing import Dict -from django.db.models.base import ModelBase +from django.db.models import Model from .build_models import build_models from .check_versions import check_versions @@ -11,7 +11,7 @@ # Returns a dictonary with the table's TableId as keys and the reated django models as values # The values (class paths) are constructed with this Module's name followed by the table name # Example: {7: } -models_by_tableid : Dict[int, ModelBase] = build_models(__name__, datamodel) +models_by_tableid : Dict[int, Model] = build_models(__name__, datamodel) # inject the model definitions into this module's namespace globals().update((model.__name__, model) From eb07a3be90434c2f1b256b4b7abc6d0ea921cb1c Mon Sep 17 00:00:00 2001 From: melton-jason Date: Sat, 28 Jan 2023 22:00:24 -0600 Subject: [PATCH 08/10] Add remaining deletion rules Fixes #2511, #1431 --- specifyweb/specify/build_models.py | 2 +- specifyweb/specify/deletion_rules.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/specifyweb/specify/build_models.py b/specifyweb/specify/build_models.py index 34c6471e980..2f2b4d81b46 100644 --- a/specifyweb/specify/build_models.py +++ b/specifyweb/specify/build_models.py @@ -97,7 +97,7 @@ def make_relationship(modelname : str, rel: Relationship, datamodel: Datamodel) return None try: - on_delete = SPECIAL_DELETION_RULES[f"{modelname}"][f"{rel.name.lower()}"] + on_delete = SPECIAL_DELETION_RULES[f'{modelname.capitalize()}.{rel.name.lower()}'] except KeyError: reverse = datamodel.reverse_relationship(rel) diff --git a/specifyweb/specify/deletion_rules.py b/specifyweb/specify/deletion_rules.py index c6f9a54f64f..f36c6bbc1f4 100644 --- a/specifyweb/specify/deletion_rules.py +++ b/specifyweb/specify/deletion_rules.py @@ -21,6 +21,8 @@ """ SPECIAL_DELETION_RULES = { 'Agent.specifyuser' : models.SET_NULL, + 'Borrow.addressOfRecord' : models.SET_NULL, + 'Loanpreparation.preparation' : models.SET_NULL, # Handle workbench deletion using raw sql in business rules. 'Workbenchrow.workbench': models.DO_NOTHING, From 99432c6e994e7b42e5efb77ddcc5ca90731c7ee8 Mon Sep 17 00:00:00 2001 From: melton-jason Date: Fri, 26 Jan 2024 17:17:26 +0000 Subject: [PATCH 09/10] Lint code with ESLint and Prettier Triggered by 7d3cccaa35b511b7c865e4e772f7aaa159114d26 on branch refs/heads/issue-1694 --- specifyweb/frontend/js_src/.prettierignore | 2 +- specifyweb/frontend/js_src/css/workbench.css | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/specifyweb/frontend/js_src/.prettierignore b/specifyweb/frontend/js_src/.prettierignore index 3c4880b7a5c..2a828b56f56 100644 --- a/specifyweb/frontend/js_src/.prettierignore +++ b/specifyweb/frontend/js_src/.prettierignore @@ -1,4 +1,4 @@ lib/tests/fixtures/ lib/tests/ajax/static lib/components/DataModel/*.js -*.snap \ No newline at end of file +*.snap diff --git a/specifyweb/frontend/js_src/css/workbench.css b/specifyweb/frontend/js_src/css/workbench.css index 5c190851c1e..9c0af09723d 100644 --- a/specifyweb/frontend/js_src/css/workbench.css +++ b/specifyweb/frontend/js_src/css/workbench.css @@ -50,8 +50,7 @@ /* Cell navigation */ .wbs-form - :is(.wb-no-match-cell, - .wb-modified-cell, .htCommentCell, .wb-search-match-cell), + :is(.wb-no-match-cell, .wb-modified-cell, .htCommentCell, .wb-search-match-cell), .wb-navigation-section { @apply !bg-[color:var(--accent-color)]; } From 94fe4e298f044f426b65e5ecba627b30894a49cf Mon Sep 17 00:00:00 2001 From: melton-jason Date: Sat, 27 Jan 2024 15:40:57 -0600 Subject: [PATCH 10/10] Return all instances of Additional Delete Blockers --- specifyweb/specify/api.py | 36 +++++++++++++++++++++++++++- specifyweb/specify/deletion_rules.py | 16 ++++++------- specifyweb/specify/views.py | 32 ++----------------------- 3 files changed, 45 insertions(+), 39 deletions(-) diff --git a/specifyweb/specify/api.py b/specifyweb/specify/api.py index b31becaf5a5..527eff73781 100644 --- a/specifyweb/specify/api.py +++ b/specifyweb/specify/api.py @@ -14,7 +14,8 @@ logger = logging.getLogger(__name__) from django import forms -from django.db import transaction +from django.db import transaction, router +from django.db.models.deletion import Collector from django.http import (HttpResponse, HttpResponseBadRequest, Http404, HttpResponseNotAllowed, QueryDict) from django.core.exceptions import ObjectDoesNotExist, FieldError, FieldDoesNotExist @@ -23,11 +24,13 @@ from specifyweb.permissions.permissions import enforce, check_table_permissions, check_field_permissions, table_permissions_checker from . import models +from .datamodel import datamodel from .autonumbering import autonumber_and_save from .uiformatters import AutonumberOverflowException from .filter_by_col import filter_by_collection from .auditlog import auditlog from .calculated_fields import calculate_extra_fields +from .deletion_rules import ADDITIONAL_DELETE_BLOCKERS ReadPermChecker = Callable[[Any], None] @@ -61,6 +64,37 @@ def default(self, obj): def toJson(obj: Any) -> str: return json.dumps(obj, cls=JsonEncoder) +def get_delete_blockers(obj): + using = router.db_for_write(obj.__class__, instance=obj) + model = obj.__class__.__name__ + collector = Collector(using=using) + collector.delete_blockers = [] + collector.collect([obj]) + result = [ + { + 'table': sub_objs[0].__class__.__name__, + 'field': field.name, + 'ids': [sub_obj.id for sub_obj in sub_objs] + } + for field, sub_objs in collector.delete_blockers + ] + # Check if the object has additional delete blockers. + # If so, add them to the response only if the object has data in the 'field' + if model in ADDITIONAL_DELETE_BLOCKERS: + + def get_blocker_information(field_name): + other_table = getattr(obj, field_name).specify_model + otherside_field = obj.specify_model.get_relationship(field_name).otherSideName + sub_objs = getattr(models, other_table.django_name).objects.filter(**{otherside_field: obj.id}) + + return {"table": other_table.django_name, "field": otherside_field, "ids": [sub_obj.id for sub_obj in sub_objs]} + + rel_data = ADDITIONAL_DELETE_BLOCKERS[model] + result.extend([get_blocker_information(field) + for field in rel_data if getattr(obj, field) is not None]) + + return result + class RecordSetException(Exception): """Raised for problems related to record sets.""" pass diff --git a/specifyweb/specify/deletion_rules.py b/specifyweb/specify/deletion_rules.py index f36c6bbc1f4..5a4170a8a72 100644 --- a/specifyweb/specify/deletion_rules.py +++ b/specifyweb/specify/deletion_rules.py @@ -20,11 +20,11 @@ See .build_models.py for uses """ SPECIAL_DELETION_RULES = { - 'Agent.specifyuser' : models.SET_NULL, - 'Borrow.addressOfRecord' : models.SET_NULL, - 'Loanpreparation.preparation' : models.SET_NULL, + 'Agent.specifyuser': models.SET_NULL, + 'Borrow.addressOfRecord': models.SET_NULL, + 'Loanpreparation.preparation': models.SET_NULL, - # Handle workbench deletion using raw sql in business rules. + # Handle workbench deletion using raw sql in business rules. 'Workbenchrow.workbench': models.DO_NOTHING, 'Workbenchdataitem.workbenchrow': models.DO_NOTHING, 'Workbenchrowimage.workbenchrow': models.DO_NOTHING, @@ -38,10 +38,10 @@ 'Spappresourcedata.spviewsetobj': models.CASCADE, # In addition to these rules, Specify Cascades relationships tagged as dependent - # For the complete list, see the global variable called - # dependent_fields in .load_datamodel.py + # For the complete list, see the global variable called + # dependent_fields in .load_datamodel.py - # If the field/relationship is not dependent and not defined here, then it is + # If the field/relationship is not dependent and not defined here, then it is # protected } @@ -51,5 +51,5 @@ See .build_models.py and .views.py for uses """ ADDITIONAL_DELETE_BLOCKERS = { - 'Agent' : ['specifyuser'], + 'Agent': ['specifyuser'], } diff --git a/specifyweb/specify/views.py b/specifyweb/specify/views.py index 0c3f6c6684d..0837b1fceee 100644 --- a/specifyweb/specify/views.py +++ b/specifyweb/specify/views.py @@ -24,7 +24,6 @@ from . import api, models as spmodels from .build_models import orderings from .specify_jar import specify_jar -from .deletion_rules import ADDITIONAL_DELETE_BLOCKERS from celery.utils.log import get_task_logger # type: ignore logger = get_task_logger(__name__) @@ -101,35 +100,8 @@ def delete_blockers(request, model, id): deleted. """ obj = api.get_object_or_404(model, id=int(id)) - using = router.db_for_write(obj.__class__, instance=obj) - collector = Collector(using=using) - collector.delete_blockers = [] - collector.collect([obj]) - result = [ - { - 'table': sub_objs[0].__class__.__name__, - 'field': field.name, - 'id': sub_objs[0].id - } - for field, sub_objs in collector.delete_blockers - ] - - # Check if the object has additional delete blockers. - # If so, add them to the response only if the object has data in the 'field' - if model.capitalize() in ADDITIONAL_DELETE_BLOCKERS: - rel_data = ADDITIONAL_DELETE_BLOCKERS[model.capitalize()] - result.extend([{ - 'table': getattr(obj, data).specify_model.django_name, - "field": data, - 'id': getattr(obj, data).id - } - for data in rel_data if getattr(obj, data) is not None]) - - return http.HttpResponse(api.toJson(flatten(result)), content_type='application/json') - - -def flatten(l): - return [item for sublist in l for item in sublist] + result = api.get_delete_blockers(obj) + return http.HttpResponse(api.toJson(result), content_type='application/json') @login_maybe_required