From 7298732cf3f8aa6f92bc275199bc3ad66047b664 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Tue, 5 Nov 2024 13:51:27 -0500 Subject: [PATCH] Make node aliases not nullable #10437 (#11600) --- arches/app/models/graph.py | 3 +- .../migrations/10437_node_alias_not_null.py | 31 +++++++++++++++++++ arches/app/models/models.py | 12 +++++-- releases/8.0.0.md | 4 +++ tests/models/node_tests.py | 27 ++++++++++++++++ tests/models/tile_model_tests.py | 3 ++ 6 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 arches/app/models/migrations/10437_node_alias_not_null.py create mode 100644 tests/models/node_tests.py diff --git a/arches/app/models/graph.py b/arches/app/models/graph.py index 7b0f7479f86..6bc3e78aa07 100644 --- a/arches/app/models/graph.py +++ b/arches/app/models/graph.py @@ -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: @@ -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): diff --git a/arches/app/models/migrations/10437_node_alias_not_null.py b/arches/app/models/migrations/10437_node_alias_not_null.py new file mode 100644 index 00000000000..750076b2000 --- /dev/null +++ b/arches/app/models/migrations/10437_node_alias_not_null.py @@ -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), + ), + ] diff --git a/arches/app/models/models.py b/arches/app/models/models.py index 3479c2e9f23..9421c5b833d 100644 --- a/arches/app/models/models.py +++ b/arches/app/models/models.py @@ -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", @@ -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") diff --git a/releases/8.0.0.md b/releases/8.0.0.md index 4298de52fb8..c438346ee2d 100644 --- a/releases/8.0.0.md +++ b/releases/8.0.0.md @@ -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) @@ -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 []() diff --git a/tests/models/node_tests.py b/tests/models/node_tests.py new file mode 100644 index 00000000000..b5d07900ec8 --- /dev/null +++ b/tests/models/node_tests.py @@ -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) diff --git a/tests/models/tile_model_tests.py b/tests/models/tile_model_tests.py index 43c1a9b3923..e56ad1ba94a 100644 --- a/tests/models/tile_model_tests.py +++ b/tests/models/tile_model_tests.py @@ -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 ( @@ -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,