Cloning published resource model breaks the original and makes instances inaccessible

tris-ots commented 5 months ago

So far I've only been able to reproduce this issue in Arches 7.5.1 and with a specific ontology, and only with published RMs. I was unable to reproduce the error using 7.5.0, or with the more standard CIDOC CRM 6.2 ontology package.

Steps to reproduce:

Clone a published RM in the Arches Designer.
Delete the cloned RM.
Attempt to view the original RM in the Arches Designer.

Expected behavior:

The original RM and its resource instances, if any, are unaffected.

Observed behavior:

Clicking on the original RM in the Arches Designer returns a Django error page.
Resource Instances of the original RM are no longer present or searchable on the Search page.

The error:

  DoesNotExist at /graph_designer/e63cea5e-01a1-11ef-ab4f-5254004626ce

  NodeGroup matching query does not exist.

  Request Method:   GET
  Request URL:  http://localhost:8080/graph_designer/e63cea5e-01a1-11ef-ab4f-5254004626ce
  Django Version:   4.2.10
  Exception Type:   DoesNotExist
  Exception Value:  

  NodeGroup matching query does not exist.

  Exception Location:   /opt/arches/.venv/lib/python3.11/site-packages/django/db/models/query.py, line 637, in get
  Raised during:    arches.app.views.graph.GraphDesignerView
  Python Executable:    /opt/arches/.venv/bin/uwsgi
  Python Version:   3.11.2
  Python Path:  

  ['/opt/arches/arches_project',
  '.',
  '',
  '/usr/lib/python311.zip',
  '/usr/lib/python3.11',
  '/usr/lib/python3.11/lib-dynload',
  '/opt/arches/.venv/lib/python3.11/site-packages',
  '/opt/arches/arches_project/arches_project']

  Server time:  Tue, 23 Apr 2024 17:11:34 -0500

  Traceback (most recent call last):
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 218, in __get__
      rel_obj = self.field.get_cached_value(instance)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/db/models/fields/mixins.py", line 15, in get_cached_value
      return instance._state.fields_cache[cache_name]
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

  During handling of the above exception ('nodegroup'), another exception occurred:
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
      response = get_response(request)
                ^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
      response = wrapped_callback(request, *callback_args, **callback_kwargs)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/views/generic/base.py", line 104, in view
      return self.dispatch(request, *args, **kwargs)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/utils/decorators.py", line 46, in _wrapper
      return bound_method(*args, **kwargs)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/contrib/auth/decorators.py", line 23, in _wrapper_view
      return view_func(request, *args, **kwargs)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/views/generic/base.py", line 143, in dispatch
      return handler(request, *args, **kwargs)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/arches_source/arches/app/views/graph.py", line 196, in get
      serialized_graph = self.graph.serialize(force_recalculation=True)  # calling `serialize` directly returns a dict
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/arches_source/arches/app/models/graph.py", line 1480, in serialize
      self.get_cards(check_if_editable=check_if_editable, use_raw_i18n_json=use_raw_i18n_json)
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/arches_source/arches/app/models/graph.py", line 1408, in get_cards
      is_editable = card.is_editable()
                    ^^^^^^^^^^^^^^^^^^
    File "/opt/arches/arches_source/arches/app/models/models.py", line 98, in is_editable
      return not TileModel.objects.filter(nodegroup=self.nodegroup).exists()
                                                    ^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 236, in __get__
      rel_obj = self.get_object(instance)
                ^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/db/models/fields/related_descriptors.py", line 199, in get_object
      return qs.get(self.field.get_reverse_related_filter(instance))
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/opt/arches/.venv/lib/python3.11/site-packages/django/db/models/query.py", line 637, in get
      raise self.model.DoesNotExist(
      ^

  Exception Type: DoesNotExist at /graph_designer/e63cea5e-01a1-11ef-ab4f-5254004626ce
  Exception Value: NodeGroup matching query does not exist.

kfogel commented 5 months ago

Thanks, @tris-ots. Question for @chiatt or anyone else interested:

Would it be further help for us to try to reproduce this with Arches 7.5.2, or even latest dev sources? (If you pretty much know what the problem is from looking at the report already, then further reproductions on close versions probably wouldn't add much information.)

whatisgalen commented 5 months ago

Here's another way to reproduce (on dev/7.6.x):

Clone a published model from the graph designer view of that resource model
Edit the name of the (cloned) resource model now in graph designer view
Attempt to Unpublish

https://github.com/archesproject/arches/assets/8366985/ae149934-1af4-42ca-be5c-c0cf8feffe96

I haven't looked into the root of the problem, but I think it's safe to say published graphs should not be clonable (for now).

@chrabyrd FYI

whatisgalen commented 5 months ago

@tris-ots can you rename this issue to something like "Cloning published resource model breaks the original and makes instances inaccessible" ?

tris-ots commented 5 months ago

@whatisgalen done, thanks for your experiments and for labeling this issue.

whatisgalen commented 5 months ago

ty!

kfogel commented 5 months ago

I haven't looked into the root of the problem, but I think it's safe to say published graphs should not be clonable (for now).

@whatisgalen, could you say more about that? It's not obvious to me why a graph shouldn't be cloneable just because it's published. Or does your "(for now)" parenthetical aside mean you just think they shouldn't be cloneable until this bug is fixed? (I.e., Not that there's some reason in principle why published graphs shouldn't be cloneable, just that we should prevent it until it no longer leads to a known bug.)

One thought: maybe cloning shouldn't include the published state -- you clone it, but the clone starts out as unpublished until you publish it.

/CC @chrabyrd

whatisgalen commented 5 months ago

@kfogel apologies for the ambiguity in my comment! What I mean is that when a graph is in "published" state, if it's cloned, the clone should not also be in published state. It appears that cloning is altogether broken when done against a published state, therefore until the issue is fully resolved, a hotfix might be: clicking "clone" on a published graph should give an error and tell you to unpublish first.

whatisgalen commented 5 months ago

If this bug is reproducible on stable/7.5.2 then we should fast-track a fix for stable/7.5.3

kfogel commented 5 months ago

@kfogel apologies for the ambiguity in my comment! What I mean is that when a graph is in "published" state, if it's cloned, the clone should not also be in published state. It appears that cloning is altogether broken when done against a published state, therefore until the issue is fully resolved, a hotfix might be: clicking "clone" on a published graph should give an error and tell you to unpublish first.

Ah, okay @whatisgalen, totally understand then, and agree (unless it turns out to be just as easy to fix the underlying issue as to do the quick fix -- but I haven't looked at that part of the code so I don't know).

whatisgalen commented 5 months ago

@kfogel I haven't had time to look into the root cause but I'd love to be surprised with an easy fix :)

tris-ots commented 5 months ago

@whatisgalen @kfogel Galen, thanks again for your work on this. I've experimented with this in a 7.5.2 install and I can't seem to replicate the error where deleting the clone corrupts the original. However, I am seeing this very generic message when I attempt to delete a cloned but unpublished RM. Sorry! The request failed. Please try again. Contact your system administrator if the problem persists. The install has Debug turned on, but there's nothing about this in the arches.log.

chrabyrd commented 5 months ago

@tris-ots @kfogel @whatisgalen

This issues has been identified and fixed in https://github.com/archesproject/arches/pull/10952

Once it's merged in it will be cherry-picked down into pre-7.6 patch releases 👍

Thank you for identifying the issue and clearly outlining reproduction steps!

kfogel commented 5 months ago

@chrabyrd Thank you so much! (And +1: Hat tips to @tris-ots and @whatisgalen for their clear descriptions.)

tris-ots commented 5 months ago

Thanks so much @chrabyrd !

archesproject / arches

Cloning published resource model breaks the original and makes instances inaccessible #10839