specify / specify7

Specify 7
https://www.specifysoftware.org/products/specify-7/
GNU General Public License v2.0
66 stars 36 forks source link

Merging two agents failing #4017

Open grantfitzsimmons opened 1 year ago

grantfitzsimmons commented 1 year ago

Happens on both v.7.9-load-manual-ref and v7.9-dev

https://fwri6123-v79-load-manual-ref.test.specifysystems.org/specify/overlay/merge/Agent/?records=1%2C2

https://github.com/specify/specify7/assets/37256050/467fceb2-d23e-4feb-ac3f-efd8bfcc8f3b

Merging 9835f5fb-7de7-4dfd-b1bb-03974517d8ab Crash Report - 2023-09-17T18 10 04.309Z.txt

{"message":"Traceback (most recent call last):\n  File \"/opt/specify7/specifyweb/specify/views.py\", line 439, in resolve_record_merge_response\n    response = start_function()\n  File \"/opt/specify7/specifyweb/specify/views.py\", line 688, in <lambda>\n    lambda: record_merge_fx(model_name, old_model_ids, int(new_model_id), progress, new_record_info))\n  File \"/usr/lib/python3.8/contextlib.py\", line 75, in inner\n    return func(*args, **kwds)\n  File \"/opt/specify7/specifyweb/specify/views.py\", line 553, in record_merge_fx\n    obj.delete()\n  File \"/opt/specify7/ve/lib/python3.8/site-packages/django/db/models/base.py\", line 967, in delete\n    return collector.delete()\n  File \"/opt/specify7/ve/lib/python3.8/site-packages/django/db/models/deletion.py\", line 435, in delete\n    signals.post_delete.send(\n  File \"/opt/specify7/ve/lib/python3.8/site-packages/django/dispatch/dispatcher.py\", line 180, in send\n    return [\n  File \"/opt/specify7/ve/lib/python3.8/site-packages/django/dispatch/dispatcher.py\", line 181, in <listcomp>\n    (receiver, receiver(signal=self, sender=sender, **named))\n  File \"/opt/specify7/specifyweb/businessrules/orm_signal_handler.py\", line 19, in handler\n    rule(sender, kwargs['instance'])\n  File \"/opt/specify7/specifyweb/businessrules/attachment_rules.py\", line 33, in attachment_jointable_deletion\n    obj.attachment.delete()\n  File \"/opt/specify7/ve/lib/python3.8/site-packages/django/db/models/base.py\", line 967, in delete\n    return collector.delete()\n  File \"/opt/specify7/ve/lib/python3.8/site-packages/django/db/models/deletion.py\", line 435, in delete\n    signals.post_delete.send(\n  File \"/opt/specify7/ve/lib/python3.8/site-packages/django/dispatch/dispatcher.py\", line 180, in send\n    return [\n  File \"/opt/specify7/ve/lib/python3.8/site-packages/django/dispatch/dispatcher.py\", line 181, in <listcomp>\n    (receiver, receiver(signal=self, sender=sender, **named))\n  File \"/opt/specify7/specifyweb/businessrules/orm_signal_handler.py\", line 15, in handler\n    rule(kwargs['instance'])\n  File \"/opt/specify7/specifyweb/businessrules/attachment_rules.py\", line 45, in attachment_deletion\n    delete_attachment_file(attachment.attachmentlocation)\n  File \"/opt/specify7/specifyweb/attachment_gw/views.py\", line 174, in delete_attachment_file\n    'token': generate_token(get_timestamp(), attch_loc)\n  File \"/opt/specify7/specifyweb/attachment_gw/views.py\", line 193, in get_timestamp\n    return int(time.time()) + server_time_delta\nTypeError: unsupported operand type(s) for +: 'int' and 'NoneType'\n","userInformation":
grantfitzsimmons commented 1 year ago

The main issue here (for me) is that the crash report is incredibly cryptic and not useful, instead of retrieving the URL of where the crash occurred. Where do I look to see the issue? Does it give me any kind of useful information that I can give to the user? We need to make these crashes somewhat intuitive to resolve on the user's end, or at least intelligible for me.

realVinayak commented 1 year ago

Piggy banking on this.

Need to properly demonstrate what exactly caused the merge failing. If you are merging A into B, but B was modified by A, then if the modifedbyagent field is preserved from B, then the merge will fail - tested this also. Simply clearing out the modifiedbyagent field in new record fixes this. In cases like these, user should be notified by dependencies.

TLDR: a simple click can convert a failed merge into successful one. we should hint user towards (or force) the successful one.

ianengelbrecht commented 1 year ago

Possibly the same error when trying to merge agents, I get the following file download: Merging 29700913-bf1e-4bb6-973b-5b208db3b72f Crash Report - 2023-10-31T07_20_37.969Z.txt

realVinayak commented 1 year ago

@ianengelbrecht that error happened because you were trying to merge two (or more) agents that have a specify user associated with them. Currently, that is not supported. So, if you are merging agent A and agent B, and there is a specify user associated with both of them, the merging cannot occur.

emenslin commented 4 months ago

Cannot recreate the specific issue from https://github.com/specify/specify7/issues/4017#issuecomment-1722563343 but when an agent merge does fail (e.g. by merging 2 agents with a specify user attached) you get a similar not very useful error in edge (7.9.6)