Merge pull request #2702 from specify/issue-2668

Improve error response from backend

Author: melton-jason

specifyweb/businessrules/accessionagent_rules.py

@@ -5,4 +5,7 @@
 @orm_signal_handler('pre_save', 'Accessionagent')
 def agent_division_must_not_be_null(accessionagent):
     if accessionagent.agent_id is None:
-        raise BusinessRuleException(_("AccessionAgent -> Agent relationship is required."))
+        raise BusinessRuleException(
+            _("AccessionAgent -> Agent relationship is required."),
+            {"table" : "Accessionagent",
+             "fieldName" : "agent" })

specifyweb/businessrules/agent_rules.py

@@ -5,21 +5,37 @@
 @orm_signal_handler('pre_delete', 'Agent')
 def agent_delete_blocked_by_related_specifyuser(agent):
     try:
-        Specifyuser.objects.get(agents=agent)
+        user = Specifyuser.objects.get(agents=agent)
     except Specifyuser.DoesNotExist:
         return
-    raise BusinessRuleException("agent cannot be deleted while associated with a specifyuser")
+    raise BusinessRuleException(
+        "agent cannot be deleted while associated with a specifyuser", 
+        {"table" : "Agent",
+         "fieldName" : "specifyuser",
+         "agentid" : agent.id, 
+         "specifyuserid": user.id})
 
 # Disabling this rule because system agents must be created separate from divisions
 # @orm_signal_handler('pre_save', 'Agent')
 # def agent_division_must_not_be_null(agent):
 #     if agent.division is None:
-#         raise BusinessRuleException("agent.division cannot be null")
+#         raise BusinessRuleException(
+#             "agent.division cannot be null",
+#             {"table" : "Agent",
+#              "fieldName" : "division",
+#              "agentid" : agent.id})
 
 @orm_signal_handler('pre_save', 'Agent')
 def agent_types_other_and_group_do_not_have_addresses(agent):
     from specifyweb.specify.agent_types import agent_types
     if agent.agenttype is None:
-        raise BusinessRuleException("agenttype cannot be null")
-    if agent_types[agent.agenttype] in ('Other', 'Group'):
-        agent.addresses.all().delete()
+        raise BusinessRuleException(
+            "agenttype cannot be null", 
+            {"table" : "Agent",
+             "fieldName" : "agenttype",
+             "agentid" : agent.id})
+    
+    # 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()

specifyweb/businessrules/determination_rules.py

@@ -16,7 +16,7 @@ def determination_pre_save(det):
         while acceptedtaxon_id is not None:
             if acceptedtaxon_id == taxon_id: break
             limit -= 1
-            assert limit > 0 # in case of cycles or pathologically long synonym chains
+            if not limit > 0: raise AssertionError(f"Could not find accepted taxon for synonymized taxon (id ='{taxon_id}')", {"taxonId" : taxon_id, "localizationKey" : "limitReachedDeterminingAccepted"})
             taxon_id = acceptedtaxon_id
             acceptedtaxon_id = Taxon.objects.select_for_update().values_list('acceptedtaxon_id', flat=True).get(id=taxon_id)
 

specifyweb/businessrules/exceptions.py

@@ -1,5 +1,12 @@
+from ..middleware.general import SpecifyExceptionWrapper
+from typing import Dict
+import traceback
+
 class BusinessRuleException(Exception):
     pass
 
+class TreeBusinessRuleException(Exception):
+    pass
+    
 class AbortSave(Exception):
     pass

specifyweb/businessrules/groupperson_rules.py

@@ -6,7 +6,11 @@
 @orm_signal_handler('pre_save', 'Groupperson')
 def agent_cannot_be_in_self(groupperson):
     if groupperson.member_id == groupperson.group_id:
-        raise BusinessRuleException('a group cannot be made a member of itself')
+        raise BusinessRuleException(
+            'a group cannot be made a member of itself', 
+            {"table" : "GroupPerson",
+             "fieldName" : "member",
+             "groupid" : groupperson.group_id})
 
 @orm_signal_handler('pre_save', 'Groupperson')
 def grouppersion_pre_save(groupperson):

specifyweb/businessrules/interaction_rules.py

@@ -34,20 +34,39 @@ def loanprep_quantity_must_be_lte_availability(ipreparation):
         quantity = ipreparation.quantity or 0
         quantityresolved = ipreparation.quantityresolved or 0
         if available < (quantity - quantityresolved):
-            raise BusinessRuleException(f"loan preparation quantity exceeds availability ({ipreparation.id}: {quantity - quantityresolved} {available})")
+            raise BusinessRuleException(
+                f"loan preparation quantity exceeds availability ({ipreparation.id}: {quantity - quantityresolved} {available})", 
+                {"table" : "LoanPreparation",
+                 "fieldName" : "quantity",
+                 "preparationid" : ipreparation.id, 
+                 "quantity" : quantity, 
+                 "quantityresolved" : quantityresolved, 
+                 "available" : available})
 
 @orm_signal_handler('pre_save', 'Giftpreparation')
 def giftprep_quantity_must_be_lte_availability(ipreparation):
     if ipreparation.preparation is not None:
         available = get_availability(ipreparation.preparation, ipreparation.id, "giftpreparationid") or 0
         quantity = ipreparation.quantity or 0
         if available < quantity:
-            raise BusinessRuleException(f"gift preparation quantity exceeds availability ({ipreparation.id}: {quantity} {available})")
+            raise BusinessRuleException(
+                f"gift preparation quantity exceeds availability ({ipreparation.id}: {quantity} {available})", 
+                {"table" : "GiftPreparation",
+                 "fieldName" : "quantity",
+                 "preparationid" : ipreparation.id, 
+                 "quantity" : quantity, 
+                 "available" : available})
 
 @orm_signal_handler('pre_save', 'Exchangeoutprep')
 def exchangeoutprep_quantity_must_be_lte_availability(ipreparation):
     if ipreparation.preparation is not None:
         available = get_availability(ipreparation.preparation, ipreparation.id, "exchangeoutprepid") or 0
         quantity = ipreparation.quantity or 0
         if available < quantity:
-            raise BusinessRuleException("exchangeout preparation quantity exceeds availability ({ipreparation.id}: {quantity} {available})")
+            raise BusinessRuleException(
+                "exchangeout preparation quantity exceeds availability ({ipreparation.id}: {quantity} {available})", 
+                {"table" : "ExchangeOutPrep",
+                 "fieldName" : "quantity",
+                 "preparationid" : ipreparation.id, 
+                 "quantity" : quantity, 
+                 "available" : available})

specifyweb/businessrules/tests/agent.py

@@ -66,32 +66,3 @@ def test_agent_division_and_agenttype_cannot_be_null(self):
                 firstname="Test",
                 lastname="Agent",
                 division=self.division)
-
-
-    def test_other_and_group_do_not_have_addresses(self):
-        from specifyweb.specify.agent_types import agent_types
-        agent = models.Agent.objects.create(
-            agenttype=agent_types.index('Person'),
-            firstname="Test",
-            lastname="Agent",
-            division=self.division)
-
-        agent.addresses.create(address="somewhere")
-
-        models.Address.objects.get(agent=agent)
-
-        agent.agenttype = agent_types.index('Other')
-        agent.save()
-
-        with self.assertRaises(models.Address.DoesNotExist):
-            models.Address.objects.get(agent=agent)
-
-        agent.addresses.create(address="somewhere")
-
-        models.Address.objects.get(agent=agent)
-
-        agent.agenttype = agent_types.index('Group')
-        agent.save()
-
-        with self.assertRaises(models.Address.DoesNotExist):
-            models.Address.objects.get(agent=agent)

specifyweb/businessrules/tree_rules.py

@@ -1,13 +1,19 @@
 import logging
 
 from .orm_signal_handler import orm_signal_handler
-from .exceptions import BusinessRuleException
+from .exceptions import TreeBusinessRuleException
 
 logger = logging.getLogger(__name__)
 
 @orm_signal_handler('pre_delete')
 def cannot_delete_root_treedefitem(sender, obj):
     if hasattr(obj, 'treedef'): # is it a treedefitem?
         if sender.objects.get(id=obj.id).parent is None:
-            raise BusinessRuleException("cannot delete root level tree definition item")
+            raise TreeBusinessRuleException(
+                "cannot delete root level tree definition item", 
+                {"tree" : obj.__class__.__name__,
+                 "localizationKey" : 'deletingTreeRoot',
+                 "node" : {
+                    "id" : obj.id
+                 }})
 

specifyweb/businessrules/uniqueness_rules.py

@@ -5,6 +5,7 @@
 
 def make_uniqueness_rule(model_name, parent_field, unique_field):
     model = getattr(models, model_name)
+    table_name = models.datamodel.get_table(model_name).name
     if parent_field is None:
         # uniqueness is global
         @orm_signal_handler('pre_save', model_name)
@@ -16,7 +17,13 @@ def check_unique(instance):
             if instance.id is not None:
                 conflicts = conflicts.exclude(id=instance.id)
             if conflicts:
-                raise BusinessRuleException("{} must have unique {}".format(model.__name__, unique_field))
+                raise BusinessRuleException(
+                "{} must have unique {}".format(table_name, unique_field), 
+                {"table" : table_name,
+                 "localizationKey" : "fieldNotUnique",
+                 "fieldName" : unique_field,
+                 "fieldData" : (unique_field, value), 
+                 "conflicting" : list(conflicts.values_list('id', flat=True)[:100])})
     else:
         @orm_signal_handler('pre_save', model_name)
         def check_unique(instance):
@@ -34,7 +41,15 @@ def check_unique(instance):
             if instance.id is not None:
                 conflicts = conflicts.exclude(id=instance.id)
             if conflicts:
-                raise BusinessRuleException("{} must have unique {} in {}".format(model.__name__, unique_field, parent_field))
+                raise BusinessRuleException(
+                    "{} must have unique {} in {}".format(table_name, unique_field, parent_field),
+                    {"table" : table_name,
+                    "localizationKey" : "childFieldNotUnique",
+                     "fieldName" : unique_field,
+                     "fieldData" : (unique_field, value),
+                     "parentField" : parent_field,
+                     "parentData" : f"{parent_field}: id={parent}",
+                     "conflicting" : list(conflicts.values_list('id', flat=True)[:100])})
     return check_unique
 
 UNIQUENESS_RULES = {

specifyweb/businessrules/user_rules.py

@@ -43,7 +43,9 @@ def deleting_user(sender, instance, **kwargs):
     nonpersonal_appresources = user.spappresources.filter(spappresourcedir__ispersonal=False)
     if nonpersonal_appresources.exists():
         raise BusinessRuleException(
-            f"user {user.name} owns nonpersonal appresources {[r.name for r in nonpersonal_appresources]}"
+            f"user {user.name} owns nonpersonal appresources {[r.name for r in nonpersonal_appresources]}",
+            {"table" : user.__class__.__name__,
+             "userid" : user.id}
         )
 
     cursor = connection.cursor()

specifyweb/frontend/js_src/lib/components/Errors/FormatError.tsx

@@ -12,6 +12,7 @@ import { unsafeTriggerNotFound } from '../Router/Router';
 import { ErrorDialog } from './ErrorDialog';
 import { produceStackTrace } from './stackTrace';
 import { mainText } from '../../localization/main';
+import { formatJsonBackendResponse } from './JsonError';
 
 export function formatError(
   error: unknown,
@@ -111,8 +112,7 @@ export function formatError(
 /** Format error message as JSON, HTML or plain text */
 function formatErrorResponse(error: string): JSX.Element {
   try {
-    const json = JSON.parse(error);
-    return <pre>{jsonStringify(json, 2)}</pre>;
+    return formatJsonBackendResponse(error);
   } catch {
     // Failed parsing error message as JSON
   }