Skip to content

Commit

Permalink
fix: work better with (single) related models
Browse files Browse the repository at this point in the history
  • Loading branch information
philtweir committed Jun 5, 2024
1 parent f019248 commit a01d67d
Show file tree
Hide file tree
Showing 6 changed files with 122 additions and 8 deletions.
9 changes: 8 additions & 1 deletion arches_orm/arches_django/datatypes/resource_instances.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,13 @@ def resource_instance(
)

value = value or tile.data.get(str(node.nodeid))
if isinstance(value, list):
if len(value) > 1:
raise RuntimeError("Resource instance should be a list if it contains multiple entries")
elif len(value) == 1:
value = value[0]
else:
value = None
if isinstance(value, dict):
value = value.get("resourceId")
resource_instance_id = None
Expand Down Expand Up @@ -124,4 +131,4 @@ def resource_instance(

@resource_instance.as_tile_data
def ri_as_tile_data(ri):
return {}, [ri]
return [], [ri]
6 changes: 4 additions & 2 deletions arches_orm/arches_django/pseudo_nodes.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from arches.app.models.tile import Tile as TileProxyModel
from collections import UserList

from arches_orm.view_models import ViewModel, NodeListViewModel, UnavailableViewModel
from arches_orm.view_models import ViewModel, NodeListViewModel, UnavailableViewModel, ResourceInstanceViewModel

from .datatypes import get_view_model_for_datatype

Expand Down Expand Up @@ -91,6 +91,7 @@ class PseudoNodeValue:
_value = None
_datatype = None
_multiple = False
_as_tile_data = None

def __init__(self, node, tile=None, value=None, parent=None, child_nodes=None, parent_cls=None):
self.node = node
Expand Down Expand Up @@ -142,6 +143,7 @@ def get_tile(self):
str(self.node.nodeid)
] = tile_value # TODO: ensure this works for any value
tile = self.tile if self.node.is_collector else None

return tile, relationships

def clear(self):
Expand Down Expand Up @@ -193,7 +195,7 @@ def value(self):

@value.setter
def value(self, value):
if not isinstance(value, ViewModel):
if not isinstance(value, ViewModel) or isinstance(value, ResourceInstanceViewModel):
self.get_tile()
value, self._as_tile_data, self._datatype, self._multiple = get_view_model_for_datatype(
self.tile,
Expand Down
9 changes: 8 additions & 1 deletion arches_orm/arches_django/wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
from arches_orm.wrapper import ResourceWrapper
from arches_orm.utils import snake
from arches_orm.errors import WKRIPermissionDenied, WKRMPermissionDenied, DescriptorsNotYetSet
from arches_orm.view_models.resources import RelatedResourceInstanceViewModelMixin


from .bulk_create import BulkImportWKRM
from .pseudo_nodes import PseudoNodeList, PseudoNodeValue, PseudoNodeUnavailable
Expand Down Expand Up @@ -196,6 +198,11 @@ def _update_tiles(
if not isinstance(root, PseudoNodeList):
parent = root
for pseudo_node in root.get_children():
if isinstance(pseudo_node.value, RelatedResourceInstanceViewModelMixin):
# Do not cross between resources. The relationship should already
# be captured. The canonical example of this is a semantic node that
# gives us a related resource instance.
continue
if isinstance(pseudo_node, PseudoNodeList) or pseudo_node.accessed:
if len(pseudo_node):
subrelationships = self._update_tiles(
Expand All @@ -209,7 +216,7 @@ def _update_tiles(
if pseudo_node._original_tile and hasattr(pseudo_node._original_tile, "_original_data"):
if t.data == pseudo_node._original_tile._original_data:
continue
raise RuntimeError(f"Attempt to modify data that this user does not have permissions to: {t.nodegroup_id}")
raise RuntimeError(f"Attempt to modify data that this user does not have permissions to: {t.nodegroup_id} in {self}")
else:
combined_tiles.append((t, r))
# This avoids loading a tile as a set of view models, simply to re-save it.
Expand Down
11 changes: 7 additions & 4 deletions arches_orm/wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,10 @@ def set_orm_attribute(self, key, value):
raise AttributeError("Field not available in remapped model")
else:
setattr(self.get_root().value, key, value)
elif (root := self.get_root()):
setattr(root.value, key, value)
else:
setattr(self.get_root().value, key, value)
raise RuntimeError(f"Tried to set {key} on {self}, which has no root")

def _get_remap(self, real_key: str):
if real_key is None:
Expand Down Expand Up @@ -129,14 +131,15 @@ def get_orm_attribute(self, key):
"""Retrieve Python values for nodes attributes."""

if self._remap and self._model_remapping is not None:
print(self, type(self), self._remap, self._model_remapping, key)
if key in self._model_remapping:
real_key = self._model_remapping[key]
return self._get_remap(real_key)
elif self._remap_total:
raise AttributeError("Field not available in remapped model")
print(self, type(self), key)
val = getattr(self.get_root().value, key)
if (root := self.get_root()):
val = getattr(root.value, key)
else:
raise RuntimeError(f"Tried to get {key} on {self}, which has no root")
return val

def __init__(
Expand Down
81 changes: 81 additions & 0 deletions tests/arches_django/_django/Person.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,40 @@
"sortorder": 5,
"visible": true
},
{
"active": true,
"cardid": "9e6bd847-46da-4bb7-9ea6-43afbbd11c66",
"component_id": "f05e4d3a-53c1-11e8-b0ea-784f435179ea",
"config": null,
"constraints": [
{
"card_id": "9e6bd847-46da-4bb7-9ea6-43afbbd11c66",
"constraintid": "b6cce5a2-df2f-4b3f-a1e2-b78e00f9ae1b",
"nodes": [],
"uniquetoallinstances": false
}
],
"cssclass": null,
"description": "",
"graph_id": "22477f01-1a44-11e9-b0a9-000d3ab1e588",
"helpenabled": false,
"helptext": {
"en": null
},
"helptitle": {
"en": null
},
"instructions": {
"en": null
},
"is_editable": true,
"name": {
"en": "Favourite Activity"
},
"nodegroup_id": "312aa645-bf2f-4783-bf25-1bb6423fb91c",
"sortorder": 9,
"visible": true
},
{
"active": true,
"cardid": "25b3afc9-186c-11eb-bbd2-f875a44e0e11",
Expand Down Expand Up @@ -200,6 +234,15 @@
"en": "Used to describe individuals associated with a heritage resource. \nA person can either be current (eg. owners, developers etc) or a person of historic interest (eg. an author who formerly lived in a heritage resource)."
},
"edges": [
{
"description": null,
"domainnode_id": "22477f00-1a44-11e9-a60e-000d3ab1e588",
"edgeid": "3cf39a08-bd21-4bdf-9d57-fa0d9a802d9b",
"graph_id": "22477f01-1a44-11e9-b0a9-000d3ab1e588",
"name": null,
"ontologyproperty": "http://www.cidoc-crm.org/cidoc-crm/P11i_participated_in",
"rangenode_id": "312aa645-bf2f-4783-bf25-1bb6423fb91c"
},
{
"description": null,
"domainnode_id": "22477f00-1a44-11e9-a60e-000d3ab1e588",
Expand Down Expand Up @@ -1771,6 +1814,12 @@
"en": "Person"
},
"nodegroups": [
{
"cardinality": "1",
"legacygroupid": null,
"nodegroupid": "312aa645-bf2f-4783-bf25-1bb6423fb91c",
"parentnodegroup_id": null
},
{
"cardinality": "1",
"legacygroupid": null,
Expand Down Expand Up @@ -5812,6 +5861,38 @@
"parentproperty": "http://www.cidoc-crm.org/cidoc-crm/P7_took_place_at",
"sortorder": 0
},
{
"alias": "favourite_activity",
"config": {
"graphid": [
"b9e0701e-5463-11e9-b5f5-000d3ab1e588"
],
"graphs": [
{
"graphid": "b9e0701e-5463-11e9-b5f5-000d3ab1e588",
"name": "Activity"
}
],
"searchDsl": "",
"searchString": ""
},
"datatype": "resource-instance",
"description": "",
"exportable": true,
"fieldname": "Fav_Act",
"graph_id": "22477f01-1a44-11e9-b0a9-000d3ab1e588",
"hascustomalias": false,
"is_collector": true,
"isrequired": false,
"issearchable": true,
"istopnode": false,
"name": "Favourite Activity",
"nodegroup_id": "312aa645-bf2f-4783-bf25-1bb6423fb91c",
"nodeid": "312aa645-bf2f-4783-bf25-1bb6423fb91c",
"ontologyclass": "http://www.cidoc-crm.org/cidoc-crm/E7_Activity",
"parentproperty": "http://www.cidoc-crm.org/cidoc-crm/P11i_participated_in",
"sortorder": 0
},
{
"alias": "associated_activities",
"config": {
Expand Down
14 changes: 14 additions & 0 deletions tests/arches_django/test_arches_django.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,20 @@ def test_can_save_a_surname(arches_orm, person_ashs, lazy):
reloaded_person = arches_orm.models.Person.find(person_ashs.id, lazy=lazy)
assert reloaded_person.name[1].surnames.surname == "Ashb"

@pytest.mark.django_db
@context_free
@pytest.mark.parametrize("lazy", [False, True])
def test_can_save_two_related_resources_singly(arches_orm, person_ashs, lazy):
act_1 = arches_orm.models.Activity()
person_ashs.favourite_activity = act_1
person_ashs.save()
assert person_ashs.favourite_activity.id == act_1.id

reloaded_person = arches_orm.models.Person.find(person_ashs.id, lazy=lazy)
# FIXME: Arches itself treats single resource instances as lists, so will require
# work either here or upstream to mitigate this on load.
assert reloaded_person.favourite_activity[0].id == act_1.id

@pytest.mark.django_db
@context_free
@pytest.mark.parametrize("lazy", [False, True])
Expand Down

0 comments on commit a01d67d

Please sign in to comment.