Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add NodeGroup.grouping_node #11613 #11614

Merged
merged 7 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion arches/app/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

IntegrityCheckDescriptions = {
1005: "Nodes with ontologies found in graphs without ontologies",
1012: "Node Groups without matching nodes",
1012: "Node Groups without contained nodes",
1013: "Node Groups without a grouping node",
1014: "Publication missing for language",
}

Expand All @@ -12,6 +13,7 @@
class IntegrityCheck(Enum):
NODE_HAS_ONTOLOGY_GRAPH_DOES_NOT = 1005
NODELESS_NODE_GROUP = 1012
NODEGROUP_WITHOUT_GROUPING_NODE = 1013
PUBLICATION_MISSING_FOR_LANGUAGE = 1014

def __str__(self):
Expand Down
30 changes: 21 additions & 9 deletions arches/app/models/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,9 @@ def flatten_tree(tree, node_id_list=[]):
if is_collector:
old_nodegroup_id = node.nodegroup_id
node.nodegroup = models.NodeGroup(
pk=node.pk, cardinality=node.nodegroup.cardinality
pk=node.pk,
cardinality=node.nodegroup.cardinality,
grouping_node=node,
)
if old_nodegroup_id not in nodegroup_map:
nodegroup_map[old_nodegroup_id] = node.nodegroup_id
Expand Down Expand Up @@ -1284,7 +1286,8 @@ def update_node(self, node):
new_node.fieldname = node["fieldname"]
self.populate_null_nodegroups()

# new_node will always have a nodegroup id even it if was set to None becuase populate_null_nodegroups
# new_node will always have a nodegroup id even if if was set to None
# because populate_null_nodegroups
# will populate the nodegroup id with the parent nodegroup
# add/remove a card if a nodegroup was added/removed
if new_node.nodegroup_id != old_node.nodegroup_id:
Expand Down Expand Up @@ -1317,7 +1320,7 @@ def update_node(self, node):

def delete_node(self, node=None):
"""
deletes a node and all if it's children from a graph
deletes a node and all of its children from a graph

Arguments:
node -- a node id or Node model to delete from the graph
Expand Down Expand Up @@ -1830,20 +1833,28 @@ def get_or_create_nodegroup(self, nodegroupid, nodegroups_list=[]):
"""
get a nodegroup from an id by first looking through the nodes and cards associated with this graph.
if not found then get the nodegroup instance from the database, otherwise return a new instance of a nodegroup
This is also the method responsible for setting the grouping_node_id
on the nodegroup when the first (collector) node is created.

Keyword Arguments

nodegroupid -- return a nodegroup with this id
nodegroups_list -- list of nodegroups from which to filter
"""

found = None
for nodegroup in nodegroups_list or self.get_nodegroups():
if str(nodegroup.nodegroupid) == str(nodegroupid):
return nodegroup
try:
return models.NodeGroup.objects.get(pk=nodegroupid)
except models.NodeGroup.DoesNotExist:
return models.NodeGroup(pk=nodegroupid)
found = nodegroup
break
else:
try:
found = models.NodeGroup.objects.get(pk=nodegroupid)
except models.NodeGroup.DoesNotExist:
found = models.NodeGroup(pk=nodegroupid, grouping_node_id=nodegroupid)

found.grouping_node_id = found.nodegroupid
return found

def get_root_nodegroup(self):
"""
Expand Down Expand Up @@ -2436,6 +2447,7 @@ def _update_source_nodegroup_hierarchy(nodegroup):

source_nodegroup.cardinality = nodegroup.cardinality
source_nodegroup.legacygroupid = nodegroup.legacygroupid
source_nodegroup.grouping_node_id = source_nodegroup.pk

if nodegroup.parentnodegroup_id:
nodegroup_parent_node = models.Node.objects.get(
Expand Down Expand Up @@ -2480,7 +2492,7 @@ def _update_source_nodegroup_hierarchy(nodegroup):
# graph ( the graph mapped to `self` ); we iterate over the item attributes and map
# them to source item. If the item does not have a `source_identifier` attribute, it
# has been newly created; we update the `graph_id` to match the source graph. We are
# not saving in this block so updates can accur in any order.
# not saving in this block so updates can occur in any order.
for future_widget in list(editable_future_graph.widgets.values()):
source_widget = future_widget.source_identifier

Expand Down
69 changes: 69 additions & 0 deletions arches/app/models/migrations/11613_nodegroup_grouping_node.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Generated by Django 5.1.3 on 2024-11-19 09:29

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("models", "11408_loadstaging_sortorder"),
]

def add_grouping_node(apps, schema_editor):
NodeGroup = apps.get_model("models", "NodeGroup")
nodegroups_with_nodes = NodeGroup.objects.filter(node__gt=0).distinct()
for nodegroup in nodegroups_with_nodes:
nodegroup.grouping_node_id = nodegroup.pk
NodeGroup.objects.bulk_update(nodegroups_with_nodes, ["grouping_node_id"])

PublishedGraph = apps.get_model("models", "PublishedGraph")
published_graphs = PublishedGraph.objects.all()
for published_graph in published_graphs:
for nodegroup_dict in published_graph.serialized_graph["nodegroups"]:
nodegroup_dict["grouping_node_id"] = nodegroup_dict["nodegroupid"]
PublishedGraph.objects.bulk_update(published_graphs, ["serialized_graph"])

def remove_grouping_node(apps, schema_editor):
PublishedGraph = apps.get_model("models", "PublishedGraph")
published_graphs = PublishedGraph.objects.all()
for published_graph in published_graphs:
for nodegroup_dict in published_graph.serialized_graph["nodegroups"]:
nodegroup_dict.pop("grouping_node_id", None)
PublishedGraph.objects.bulk_update(published_graphs, ["serialized_graph"])

operations = [
migrations.AddField(
model_name="nodegroup",
name="grouping_node",
field=models.OneToOneField(
blank=True,
db_column="groupingnodeid",
null=True,
on_delete=django.db.models.deletion.SET_NULL,
related_name="grouping_node_nodegroup",
to="models.node",
),
),
migrations.AlterField(
model_name="nodegroup",
name="cardinality",
field=models.CharField(
blank=True, choices=[("1", "1"), ("n", "n")], default="1", max_length=1
),
),
migrations.AlterField(
model_name="nodegroup",
name="parentnodegroup",
field=models.ForeignKey(
blank=True,
db_column="parentnodegroupid",
null=True,
on_delete=django.db.models.deletion.CASCADE,
related_name="children",
related_query_name="child",
to="models.nodegroup",
),
),
migrations.RunPython(add_grouping_node, remove_grouping_node),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Generated by Django 5.1.3 on 2024-11-19 09:33

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("models", "11613_nodegroup_grouping_node"),
]

operations = [
migrations.AddConstraint(
model_name="node",
constraint=models.CheckConstraint(
condition=models.Q(
("istopnode", True), ("nodegroup__isnull", False), _connector="OR"
),
name="has_nodegroup_or_istopnode",
),
),
migrations.AddConstraint(
model_name="nodegroup",
constraint=models.CheckConstraint(
condition=models.Q(
("grouping_node", models.F("pk")),
("grouping_node__isnull", True),
_connector="OR",
),
name="grouping_node_matches_pk_or_null",
),
),
]
2 changes: 1 addition & 1 deletion arches/app/models/migrations/7787_relational_data_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class Migration(migrations.Migration):

operations = [
migrations.RunSQL(
"""
r"""
chrabyrd marked this conversation as resolved.
Show resolved Hide resolved
create extension if not exists "unaccent";

create or replace function __arches_slugify(
Expand Down
33 changes: 30 additions & 3 deletions arches/app/models/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -709,14 +709,27 @@ class Meta:
class NodeGroup(models.Model):
nodegroupid = models.UUIDField(primary_key=True)
legacygroupid = models.TextField(blank=True, null=True)
cardinality = models.TextField(blank=True, default="1")
cardinality = models.CharField(
max_length=1, blank=True, default="1", choices={"1": "1", "n": "n"}
)
parentnodegroup = models.ForeignKey(
"self",
db_column="parentnodegroupid",
blank=True,
null=True,
on_delete=models.CASCADE,
related_name="children",
related_query_name="child",
) # Allows nodegroups within nodegroups
grouping_node = models.OneToOneField(
"Node",
db_column="groupingnodeid",
blank=True,
null=True,
# models.RESTRICT might be better, but revisit after future graph refactor.
on_delete=models.SET_NULL,
related_name="grouping_node_nodegroup",
)

def __init__(self, *args, **kwargs):
super(NodeGroup, self).__init__(*args, **kwargs)
Expand All @@ -726,6 +739,13 @@ def __init__(self, *args, **kwargs):
class Meta:
managed = True
db_table = "node_groups"
constraints = [
models.CheckConstraint(
condition=Q(grouping_node=models.F("pk"))
| Q(grouping_node__isnull=True),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry to potentially rehash but it's been a bit since I've looked at this. What's the condition for a NodeGroup to exist without a Node?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Purely for convenience when creating them -- we have to create one (NodeGroup) and then the other (Node), unless we want to require that everybody create them in a single transaction and then run this as a deferred constraint, but that's a breaking change I didn't want to impose.

There's a validation check in manage.py validate that checks for nodeless nodegroups. Apparently we're still creating some on setup_db:

Arches integrity report
Prepared by Arches 8.0.0a0 on Thu Jan  2 15:41:29 2025

        Error   Rows    Fixable?        Description

PASS    1005    0       N/A             Nodes with ontologies found in graphs without ontologies

FAIL    1012    1       Yes             Node Groups without contained nodes

PASS    1013    0       N/A             Node Groups without a grouping node

PASS    1014    0       N/A             Publication missing for language

name="grouping_node_matches_pk_or_null",
)
]

default_permissions = ()
permissions = (
Expand Down Expand Up @@ -874,15 +894,18 @@ def __init__(self, *args, **kwargs):
def clean(self):
if not self.alias:
Graph.objects.get(pk=self.graph_id).create_node_alias(self)
if self.pk == self.source_identifier_id:
chrabyrd marked this conversation as resolved.
Show resolved Hide resolved
self.source_identifier_id = None

def save(self, **kwargs):
if not self.alias:
self.clean()
add_to_update_fields(kwargs, "alias")
add_to_update_fields(kwargs, "hascustomalias")
if self.pk == self.source_identifier_id:
self.source_identifier_id = None
chrabyrd marked this conversation as resolved.
Show resolved Hide resolved
add_to_update_fields(kwargs, "source_identifier_id")

self.clean()

super(Node, self).save()

class Meta:
Expand All @@ -895,6 +918,10 @@ class Meta:
models.UniqueConstraint(
fields=["alias", "graph"], name="unique_alias_graph"
),
models.CheckConstraint(
condition=Q(istopnode=True) | Q(nodegroup__isnull=False),
name="has_nodegroup_or_istopnode",
),
]


Expand Down
5 changes: 5 additions & 0 deletions arches/management/commands/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ def handle(self, *args, **options):
),
fix_action=DELETE_QUERYSET,
)
self.check_integrity(
check=IntegrityCheck.NODEGROUP_WITHOUT_GROUPING_NODE, # 1013
queryset=models.NodeGroup.objects.filter(node__gt=0, grouping_node=None),
fix_action=None,
)
self.check_integrity(
check=IntegrityCheck.PUBLICATION_MISSING_FOR_LANGUAGE, # 1014
queryset=(
Expand Down
1 change: 1 addition & 0 deletions releases/8.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Arches 8.0.0 Release Notes
### Additional highlights
- Add session-based REST APIs for login, logout [#11261](https://github.com/archesproject/arches/issues/11261)
- Improve handling of longer model names [#11317](https://github.com/archesproject/arches/issues/11317)
- New column `NodeGroup.grouping_node`: one-to-one field to the grouping node [#11613](https://github.com/archesproject/arches/issues/11613)
- Support more expressive plugin URLs [#11320](https://github.com/archesproject/arches/issues/11320)
- Make node aliases not nullable [#10437](https://github.com/archesproject/arches/issues/10437)
- Concepts API no longer responds with empty body for error conditions [#11519](https://github.com/archesproject/arches/issues/11519)
Expand Down
Loading
Loading