From bb42c3d8c103f9e2aef5272a1d4cdf7b5b031bcd Mon Sep 17 00:00:00 2001 From: Johnathan Clementi Date: Wed, 13 Nov 2024 11:17:46 -0500 Subject: [PATCH] Improve concept node migration (#50) * Accept graph slugs for concept node migration #37 * Migrate default values in node migration #37 * Test accepting graph slugs and check for default val #37 * clean up & pr feedback #37 * Handle failed default value lookups #37 --- .../management/commands/controlled_lists.py | 141 ++++++++++-------- tests/cli_tests.py | 7 +- .../concept_node_migration_test_data.json | 2 +- 3 files changed, 88 insertions(+), 62 deletions(-) diff --git a/arches_references/management/commands/controlled_lists.py b/arches_references/management/commands/controlled_lists.py index 41c53ca..af0d819 100644 --- a/arches_references/management/commands/controlled_lists.py +++ b/arches_references/management/commands/controlled_lists.py @@ -1,20 +1,22 @@ +from django.core.exceptions import ValidationError +from django.core.management.base import BaseCommand, CommandError +from django.db import models, transaction +from django.db.models.expressions import CombinedExpression +from django.db.models.fields.json import KT +from django.db.models.functions import Cast +from uuid import UUID + +from arches.app.datatypes.datatypes import DataTypeFactory from arches.app.models.fields.i18n import I18n_JSONField +from arches.app.models.graph import Graph from arches.app.models.models import ( - CardXNodeXWidget, GraphModel, Language, Node, Value, Widget, ) -from arches.app.models.graph import Graph from arches_references.models import List -from django.core.exceptions import ValidationError -from django.core.management.base import BaseCommand, CommandError -from django.db import models, transaction -from django.db.models.expressions import CombinedExpression -from django.db.models.fields.json import KT -from django.db.models.functions import Cast class Command(BaseCommand): @@ -78,7 +80,7 @@ def add_arguments(self, parser): "--graph", action="store", dest="graph", - help="The graphid which associated concept nodes will be migrated to use the reference datatype", + help="The graphid or slug which associated concept nodes will be migrated to use the reference datatype", ) def handle(self, *args, **options): @@ -110,7 +112,10 @@ def handle(self, *args, **options): preferred_sort_language=psl, ) elif options["operation"] == "migrate_concept_nodes_to_reference_datatype": - self.migrate_concept_nodes_to_reference_datatype(options["graph"]) + graph = options["graph"] + if not graph or graph is None: + raise CommandError("Please provide a graph id or slug") + self.migrate_concept_nodes_to_reference_datatype(graph) def migrate_collections_to_controlled_lists( self, @@ -169,15 +174,23 @@ def migrate_collections_to_controlled_lists( result = cursor.fetchone() self.stdout.write(result[0]) - def migrate_concept_nodes_to_reference_datatype(self, graph_id): + def migrate_concept_nodes_to_reference_datatype(self, graph): + try: + UUID(graph) + query = models.Q(graphid=graph) + except ValueError: + query = models.Q(slug=graph, source_identifier__isnull=True) + try: - source_graph = GraphModel.objects.get(pk=graph_id) - except (GraphModel.DoesNotExist, ValidationError) as e: + source_graph = GraphModel.objects.get(query) + except GraphModel.DoesNotExist as e: raise CommandError(e) + graph_id = source_graph.graphid + nodes = ( Node.objects.filter( - graph_id=source_graph.graphid, + graph_id=graph_id, datatype__in=["concept", "concept-list"], is_immutable=False, ) @@ -198,12 +211,29 @@ def migrate_concept_nodes_to_reference_datatype(self, graph_id): ) REFERENCE_SELECT_WIDGET = Widget.objects.get(name="reference-select-widget") + REFERENCE_FACTORY = DataTypeFactory().get_instance("reference") controlled_list_ids = List.objects.all().values_list("id", flat=True) errors = [] - with transaction.atomic(): - for node in nodes: - if node.collection_id in controlled_list_ids: + # Check that collections have been migrated to controlled lists + for node in nodes: + if node.collection_id not in controlled_list_ids: + errors.append( + {"node_alias": node.alias, "collection_id": node.collection_id} + ) + if errors: + self.stderr.write( + "The following collections for the associated nodes have not been migrated to controlled lists:" + ) + for error in errors: + self.stderr.write( + "Node alias: {0}, Collection ID: {1}".format( + error["node_alias"], error["collection_id"] + ) + ) + else: + with transaction.atomic(): + for node in nodes: if node.datatype == "concept": node.config = { "multiValue": False, @@ -218,32 +248,12 @@ def migrate_concept_nodes_to_reference_datatype(self, graph_id): node.full_clean() node.save() - cross_records = ( - node.cardxnodexwidget_set.annotate( - config_without_i18n=Cast( - models.F("config"), - output_field=models.JSONField(), - ) - ) - .annotate( - without_default=CombinedExpression( - models.F("config_without_i18n"), - "-", - models.Value( - "defaultValue", output_field=models.CharField() - ), - output_field=models.JSONField(), - ) - ) - .annotate( - without_default_and_options=CombinedExpression( - models.F("without_default"), - "-", - models.Value( - "options", output_field=models.CharField() - ), - output_field=I18n_JSONField(), - ) + cross_records = node.cardxnodexwidget_set.annotate( + config_without_options=CombinedExpression( + models.F("config"), + "-", + models.Value("options", output_field=models.CharField()), + output_field=I18n_JSONField(), ) ) for cross_record in cross_records: @@ -251,27 +261,38 @@ def migrate_concept_nodes_to_reference_datatype(self, graph_id): cross_record.config = {} cross_record.save() - cross_record.config = cross_record.without_default_and_options + # Crosswalk concept version of default values to reference versions + original_default_value = ( + cross_record.config_without_options.get( + "defaultValue", None + ) + ) + if original_default_value: + new_default_value = [] + if isinstance(original_default_value, str): + original_default_value = [original_default_value] + for value in original_default_value: + value_rec = Value.objects.get(pk=value) + config = {"controlledList": node.collection_id} + new_value = REFERENCE_FACTORY.transform_value_for_tile( + value=value_rec.value, + **config, + ) + if isinstance(new_value, list): + new_default_value.append(new_value[0]) + else: + raise CommandError( + f"Failed to convert original default value: {value_rec.value} in list: {node.collection_id} for node: {node.name} into a reference datatype instance" + ) + cross_record.config_without_options["defaultValue"] = ( + new_default_value + ) + + cross_record.config = cross_record.config_without_options cross_record.widget = REFERENCE_SELECT_WIDGET cross_record.full_clean() cross_record.save() - elif node.collection_id not in controlled_list_ids: - errors.append( - {"node_alias": node.alias, "collection_id": node.collection_id} - ) - - if errors: - self.stderr.write( - "The following collections for the associated nodes have not been migrated to controlled lists:" - ) - for error in errors: - self.stderr.write( - "Node alias: {0}, Collection ID: {1}".format( - error["node_alias"], error["collection_id"] - ) - ) - else: source_graph = Graph.objects.get(pk=graph_id) # Refresh the nodes to ensure the changes are reflected in the serialized graph diff --git a/tests/cli_tests.py b/tests/cli_tests.py index b8c9d18..313ae90 100644 --- a/tests/cli_tests.py +++ b/tests/cli_tests.py @@ -191,7 +191,12 @@ def test_migrate_concept_nodes_to_reference_datatype(self): self.assertEqual(len(reference_nodes), 4) expected_node_config_keys = ["multiValue", "controlledList"] - expected_widget_config_keys = ["label", "placeholder", "i18n_properties"] + expected_widget_config_keys = [ + "label", + "placeholder", + "defaultValue", + "i18n_properties", + ] for node in reference_nodes: self.assertEqual(expected_node_config_keys, list(node.config.keys())) for widget in node.cardxnodexwidget_set.all(): diff --git a/tests/fixtures/data/concept_node_migration_test_data.json b/tests/fixtures/data/concept_node_migration_test_data.json index 9b2c4e5..bfb0f14 100644 --- a/tests/fixtures/data/concept_node_migration_test_data.json +++ b/tests/fixtures/data/concept_node_migration_test_data.json @@ -3799,7 +3799,7 @@ "jsonldcontext": null, "template": "50000000-0000-0000-0000-000000000001", "config": {}, - "slug": null, + "slug": "concept-node-migration-test", "publication": "ab3f6684-7b6d-11ef-a937-0aa766c61b64", "source_identifier": null, "has_unpublished_changes": false,