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

Make node aliases not nullable #10437 #11600

Merged
merged 6 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 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 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),
),
]
11 changes: 9 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,14 @@ 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")
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 @@ -59,6 +60,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
26 changes: 26 additions & 0 deletions tests/models/node_tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
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")
new_node.hascustomalias = True
new_node.alias = ""
new_node.clean()
self.assertEqual(new_node.alias, "test_node")
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