Skip to content

Commit

Permalink
Make node aliases not nullable #10437 (#11600)
Browse files Browse the repository at this point in the history
  • Loading branch information
jacobtylerwalls authored Nov 5, 2024
1 parent ce9ca42 commit 7298732
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 3 deletions.
3 changes: 2 additions & 1 deletion arches/app/models/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -2107,7 +2107,7 @@ def create_node_alias(self, node):
Assigns a unique, slugified version of a node's name as that node's alias.
"""
with connection.cursor() as cursor:
if node.hascustomalias:
if node.hascustomalias and node.alias:
cursor.callproc("__arches_slugify", [node.alias])
node.alias = cursor.fetchone()[0]
else:
Expand All @@ -2117,6 +2117,7 @@ def create_node_alias(self, node):
n.alias for n in self.nodes.values() if node.alias != n.alias
]
node.alias = self.make_name_unique(row[0], aliases, "_n")
node.hascustomalias = False
return node.alias

def validate(self):
Expand Down
31 changes: 31 additions & 0 deletions arches/app/models/migrations/10437_node_alias_not_null.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django 5.1.2 on 2024-11-01 15:08

from django.db import migrations, models

# Once GraphModel.create_node_alias is a static method, we can remove this import.
# Until then, this is safe to use in a migration, because it doesn't use Graph
# fields (and thus is a lot like a static method).
from arches.app.models.graph import Graph


class Migration(migrations.Migration):

dependencies = [
("models", "11042_update__arches_staging_to_tile"),
]

def create_node_aliases(apps, schema_editor):
Node = apps.get_model("models", "Node")
nodes_needing_alias = Node.objects.filter(alias__isnull=True)
for node in nodes_needing_alias:
Graph.objects.get(pk=node.graph_id).create_node_alias(node)
Node.objects.bulk_update(nodes_needing_alias, ["alias"])

operations = [
migrations.RunPython(create_node_aliases, migrations.RunPython.noop),
migrations.AlterField(
model_name="node",
name="alias",
field=models.TextField(blank=True),
),
]
12 changes: 10 additions & 2 deletions arches/app/models/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,7 @@ def __init__(self, *args, **kwargs):
sortorder = models.IntegerField(blank=True, null=True, default=0)
fieldname = models.TextField(blank=True, null=True)
exportable = models.BooleanField(default=False, null=True)
alias = models.TextField(blank=True, null=True)
alias = models.TextField(blank=True)
hascustomalias = models.BooleanField(default=False)
source_identifier = models.ForeignKey(
"self",
Expand Down Expand Up @@ -904,7 +904,15 @@ def __init__(self, *args, **kwargs):
if not self.nodeid:
self.nodeid = uuid.uuid4()

def save(self, *args, **kwargs):
def clean(self):
if not self.alias:
Graph.objects.get(pk=self.graph_id).create_node_alias(self)

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
add_to_update_fields(kwargs, "source_identifier_id")
Expand Down
4 changes: 4 additions & 0 deletions releases/8.0.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Arches 8.0.0 Release Notes
- Add system check advising next action when enabling additional languages without updating graphs [#10079](https://github.com/archesproject/arches/issues/10079)
- Improve handling of longer model names [#11317](https://github.com/archesproject/arches/issues/11317)
- 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)
- Removes sample index from new projects, updates test coverage behavior [#11591](https://github.com/archesproject/arches/issues/11519)

Expand Down Expand Up @@ -61,6 +62,9 @@ JavaScript:

- `ensure_userprofile_exists()` was removed from the `Tile` model.

- The following fields are no longer nullable. If you have custom SQL (or Python code that uses direct ORM operations to bypass model methods, etc.), you will need to set these fields directly on creation:
- `Node.alias`

### Upgrading Arches

1. You must be upgraded to at least version before proceeding. If you are on an earlier version, please refer to the upgrade process in the []()
Expand Down
27 changes: 27 additions & 0 deletions tests/models/node_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from arches.app.models.graph import Graph
from arches.app.models.models import Node
from tests.base_test import ArchesTestCase

# these tests can be run from the command line via
# python manage.py test tests.models.node_tests --settings="tests.test_settings"


class NodeTests(ArchesTestCase):
@classmethod
def setUpTestData(cls):
cls.graph = Graph.new(name="Node Tests Graph")

def test_missing_alias_supplied(self):
new_node = Node(graph_id=self.graph.pk)
new_node.clean()
self.assertIsNotNone(new_node.alias)

def test_empty_custom_alias_regenerated(self):
"""One dubiously empty alias per graph is currently allowed at the
database level. Ensure it is regenerated via the application."""
new_node = Node(
graph_id=self.graph.pk, name="Test node", alias="", hascustomalias=True
)
new_node.clean()
self.assertEqual(new_node.alias, "test_node")
self.assertIs(new_node.hascustomalias, False)
3 changes: 3 additions & 0 deletions tests/models/tile_model_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from django.contrib.auth.models import User
from django.db.utils import ProgrammingError
from django.http import HttpRequest
from arches.app.models.graph import Graph
from arches.app.models.tile import Tile, TileValidationError
from arches.app.models.resource import Resource
from arches.app.models.models import (
Expand Down Expand Up @@ -726,10 +727,12 @@ def test_related_resources_managed(self):

def test_check_for_missing_nodes(self):
# Required file list node.
graph = Graph.new(name="Test Graph")
node_group = NodeGroup.objects.get(
pk=UUID("41111111-0000-0000-0000-000000000000")
)
required_file_list_node = Node(
graph=graph,
name="Required file list",
datatype="file-list",
nodegroup=node_group,
Expand Down

0 comments on commit 7298732

Please sign in to comment.