diff --git a/api/main/rest/_attributes.py b/api/main/rest/_attributes.py index a855293a8..e430f866f 100644 --- a/api/main/rest/_attributes.py +++ b/api/main/rest/_attributes.py @@ -3,6 +3,7 @@ import logging from django.db.models.expressions import Func +from django.db.models import Case, When, F from django.contrib.gis.measure import D as GisDistance from django.contrib.gis.geos import Point from django.shortcuts import get_object_or_404 @@ -50,7 +51,7 @@ class ReplaceKey(Func): # pylint: disable=abstract-method """ function = "jsonb_set" - template = "%(function)s(%(expressions)s #- '{\"%(old_key)s\"}', '{\"%(new_key)s\"}', %(expressions)s #> '{\"%(old_key)s\"}', %(create_missing)s)" # pylint: disable=line-too-long + template = "%(function)s(%(expressions)s #- '{\"%(old_key)s\"}', '{\"%(new_key)s\"}', COALESCE(%(expressions)s #> '{\"%(old_key)s\"}','null'::jsonb), %(create_missing)s)" # pylint: disable=line-too-long arity = 1 def __init__( @@ -287,12 +288,17 @@ def bulk_rename_attributes(new_attrs, q_s): Updates attribute keys. """ for old_key, new_key in new_attrs.items(): + old = old_key.replace("%", "%%") + new = new_key.replace("%", "%%") q_s.update( - attributes=ReplaceKey( - "attributes", - old_key=old_key.replace("%", "%%"), - new_key=new_key.replace("%", "%%"), - create_missing=True, + attributes=Case( + When(attributes__has_key=old, then=ReplaceKey( + "attributes", + old_key=old, + new_key=new, + create_missing=True, + )), + default=F("attributes"), ) ) diff --git a/api/main/tests.py b/api/main/tests.py index 22dc10508..3d6ce1d06 100644 --- a/api/main/tests.py +++ b/api/main/tests.py @@ -55,6 +55,7 @@ def _fixture_teardown(self): def wait_for_indices(entity_type): + entity_type.refresh_from_db() built_ins = BUILT_IN_INDICES.get(type(entity_type), []) types_to_scan = [*entity_type.attribute_types, *built_ins] # Wait for btree indices too @@ -468,6 +469,128 @@ def latlon_distance(lat0, lon0, lat1, lon1): ] +class AttributeRenameMixin: + def test_attribute_rename(self): + """Test that renaming an attribute works.""" + # Fetching existing values + type_name = self.detail_uri + "Type" + resp = self.client.get( + f"/rest/{self.list_uri}/{self.project.pk}?type={self.entity_type.pk}" + ) + values = [x["attributes"].get("Float Test") for x in resp.data] + + # Rename float test to something else the back again + resp = self.client.patch( + f"/rest/AttributeType/{self.entity_type.pk}", + { + "entity_type": type_name, + "current_name": "Float Test", + "attribute_type_update": {"name": "Float Test Renamed"}, + }, + format="json", + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp = self.client.get( + f"/rest/{self.list_uri}/{self.project.pk}?type={self.entity_type.pk}" + ) + new_values = [x["attributes"].get("Float Test Renamed") for x in resp.data] + for idx, val in enumerate(values): + self.assertEqual(val, new_values[idx]) + + # TODO: This could result in 400 if someone slams the UI, but it won't break + # for test reliability, wait for all indices to complete prior to renaming back + wait_for_indices(self.entity_type) + # Rename back + resp = self.client.patch( + f"/rest/AttributeType/{self.entity_type.pk}", + { + "entity_type": type_name, + "current_name": "Float Test Renamed", + "attribute_type_update": {"name": "Float Test"}, + }, + format="json", + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + resp = self.client.get( + f"/rest/{self.list_uri}/{self.project.pk}?type={self.entity_type.pk}" + ) + new_values = [x["attributes"].get("Float Test") for x in resp.data] + for idx, val in enumerate(values): + self.assertEqual(val, new_values[idx]) + + # A corner case of renaming when the attribute is not present in the data + # to emulate this corner case we will add a new attribute to the data w/o a default + # and then rename it + # We then refetch the entities and ensure that the existing attributes are + # unadulterated. + resp = self.client.post( + f"/rest/AttributeType/{self.entity_type.pk}", + { + "entity_type": type_name, + "addition": {"name": "Attribute Not Present", "dtype": "float"}, + }, + format="json", + ) + + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + wait_for_indices(self.entity_type) + resp = self.client.get( + f"/rest/{self.list_uri}/{self.project.pk}?type={self.entity_type.pk}" + ) + + # Check 2; attribute has no default set but it is registered + new_values = [x["attributes"].get("Float Test") for x in resp.data] + for idx, val in enumerate(values): + self.assertEqual(val, new_values[idx]) + self.assertTrue("Attribute Not Present" not in resp.data[idx]["attributes"]) + + resp = self.client.patch( + f"/rest/AttributeType/{self.entity_type.pk}", + { + "entity_type": type_name, + "current_name": "Attribute Not Present", + "attribute_type_update": {"name": "BingoBango"}, + }, + format="json", + ) + + self.assertEqual(resp.status_code, status.HTTP_200_OK) + wait_for_indices(self.entity_type) + + resp = self.client.get( + f"/rest/{self.list_uri}/{self.project.pk}?type={self.entity_type.pk}" + ) + + # Check 3; attributes are not effected by renaming a non-existant attribute + new_values = [x["attributes"].get("Float Test") for x in resp.data] + for idx, val in enumerate(values): + self.assertEqual(val, new_values[idx]) + self.assertTrue("Attribute Not Present" not in resp.data[idx]["attributes"]) + + # Delete attribute and verify no records have changed + resp = self.client.delete( + f"/rest/AttributeType/{self.entity_type.pk}", + { + "entity_type": type_name, + "name": "BingoBango", + }, + format="json", + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + wait_for_indices(self.entity_type) + + resp = self.client.get( + f"/rest/{self.list_uri}/{self.project.pk}?type={self.entity_type.pk}" + ) + + # Check 3; attributes are not effected by renaming a non-existant attribute + new_values = [x["attributes"].get("Float Test") for x in resp.data] + for idx, val in enumerate(values): + self.assertEqual(val, new_values[idx]) + self.assertTrue("Attribute Not Present" not in resp.data[idx]["attributes"]) + + class ElementalIDChangeMixin: def test_elemental_id_create(self): endpoint = f"/rest/{self.list_uri}/{self.project.pk}" @@ -2486,6 +2609,7 @@ class LocalizationBoxTestCase( PermissionDetailTestMixin, EntityAuthorChangeMixin, ElementalIDChangeMixin, + AttributeRenameMixin, ): def setUp(self): print(f"\n{self.__class__.__name__}=", end="", flush=True) @@ -2567,6 +2691,7 @@ class LocalizationLineTestCase( PermissionDetailTestMixin, EntityAuthorChangeMixin, ElementalIDChangeMixin, + AttributeRenameMixin, ): def setUp(self): print(f"\n{self.__class__.__name__}=", end="", flush=True) @@ -2648,6 +2773,7 @@ class LocalizationDotTestCase( PermissionDetailTestMixin, EntityAuthorChangeMixin, ElementalIDChangeMixin, + AttributeRenameMixin, ): def setUp(self): print(f"\n{self.__class__.__name__}=", end="", flush=True) @@ -2727,6 +2853,7 @@ class LocalizationPolyTestCase( PermissionDetailTestMixin, EntityAuthorChangeMixin, ElementalIDChangeMixin, + AttributeRenameMixin, ): def setUp(self): print(f"\n{self.__class__.__name__}=", end="", flush=True) @@ -2805,6 +2932,7 @@ class StateTestCase( PermissionDetailTestMixin, EntityAuthorChangeMixin, ElementalIDChangeMixin, + AttributeRenameMixin, ): def setUp(self): print(f"\n{self.__class__.__name__}=", end="", flush=True) diff --git a/scripts/packages/tator-js b/scripts/packages/tator-js index 8c0929a42..30103c67d 160000 --- a/scripts/packages/tator-js +++ b/scripts/packages/tator-js @@ -1 +1 @@ -Subproject commit 8c0929a42794606b97453343a07524c2c9e65816 +Subproject commit 30103c67d6931bb7858f139c078f9310e473f893