From 8cbab4e78ae94723adec0a5bcd515f1334d17a05 Mon Sep 17 00:00:00 2001 From: Peter Evans <peter@baserow.io> Date: Wed, 26 Mar 2025 15:58:51 +0000 Subject: [PATCH] Resolve "Table element collection field type uid don't regenerate after duplication." --- .../contrib/builder/elements/registries.py | 19 ++++++- .../builder/workflow_actions/models.py | 36 +++++++++++++- .../builder/workflow_actions/registries.py | 37 +++++++++++++- .../elements/test_table_element_type.py | 37 ++++++++++++++ .../builder/test_builder_application_type.py | 3 +- .../test_workflow_action_models.py | 49 +++++++++++++++++++ .../test_workflow_action_types.py | 3 ++ ..._caused_table_element_actions_to_disa.json | 8 +++ 8 files changed, 187 insertions(+), 5 deletions(-) create mode 100644 backend/tests/baserow/contrib/builder/workflow_actions/test_workflow_action_models.py create mode 100644 changelog/entries/unreleased/bug/3521_resolved_an_issue_which_caused_table_element_actions_to_disa.json diff --git a/backend/src/baserow/contrib/builder/elements/registries.py b/backend/src/baserow/contrib/builder/elements/registries.py index 2b09989ee..394faba8d 100644 --- a/backend/src/baserow/contrib/builder/elements/registries.py +++ b/backend/src/baserow/contrib/builder/elements/registries.py @@ -1,3 +1,4 @@ +import uuid from abc import ABC, abstractmethod from typing import ( Any, @@ -22,7 +23,6 @@ from rest_framework.exceptions import ValidationError from baserow.contrib.builder.formula_importer import import_formula from baserow.contrib.builder.mixins import BuilderInstanceWithFormulaMixin from baserow.contrib.builder.pages.models import Page -from baserow.contrib.database.db.functions import RandomUUID from baserow.core.registry import ( CustomFieldsInstanceMixin, CustomFieldsRegistryMixin, @@ -500,8 +500,23 @@ class CollectionFieldType( **kwargs, ) + # Generate a new `uid` for this collection field. + deserialized_uid = str(uuid.uuid4()) + + if "uid" in serialized_values: + # Ensure we have a mapping for the collection field uids. + if "builder_collection_fields_uids" not in id_mapping: + id_mapping["builder_collection_fields_uids"] = {} + + # Map the old uid to the new uid. This ensures that any workflow + # actions with an `event` pointing to the old uid will have the + # pointer to the new uid. + id_mapping["builder_collection_fields_uids"][ + serialized_values["uid"] + ] = deserialized_uid + deserialized_values = { - "uid": serialized_values.get("uid", RandomUUID()), + "uid": deserialized_uid, "config": deserialized_config, "type": serialized_values["type"], "styles": serialized_values.get("styles", {}), diff --git a/backend/src/baserow/contrib/builder/workflow_actions/models.py b/backend/src/baserow/contrib/builder/workflow_actions/models.py index c5da0f79a..4fcb7d2fb 100644 --- a/backend/src/baserow/contrib/builder/workflow_actions/models.py +++ b/backend/src/baserow/contrib/builder/workflow_actions/models.py @@ -1,7 +1,13 @@ +from typing import Union + from django.contrib.contenttypes.models import ContentType from django.db import models -from baserow.contrib.builder.elements.models import Element, NavigationElementMixin +from baserow.contrib.builder.elements.models import ( + CollectionField, + Element, + NavigationElementMixin, +) from baserow.contrib.builder.pages.models import Page from baserow.core.formula.field import FormulaField from baserow.core.mixins import OrderableMixin @@ -36,6 +42,34 @@ class BuilderWorkflowAction( Element, on_delete=models.CASCADE, null=True, default=None ) + @classmethod + def is_collection_field_action(cls, event: str) -> bool: + """ + Returns whether this workflow action is associated with a collection field. + + :return: Whether this workflow action is associated with a collection field. + """ + + default_event_types = [e.value for e in EventTypes] + return event and event not in default_event_types + + @property + def target(self) -> Union[Element, CollectionField]: + """ + If this workflow action's `event` is in the format `{uid}_{event_type}`, then + it's associated with a collection element with fields. If that's the case, the + target is the field with the matching `uid`. Otherwise, the target is the + element itself. + + :return: The target of the workflow action. + """ + + if BuilderWorkflowAction.is_collection_field_action(self.event): + uid, event = self.event.split("_", 1) + return self.element.fields.get(uid=uid) + + return self.element + @staticmethod def get_type_registry() -> ModelRegistryMixin: from baserow.contrib.builder.workflow_actions.registries import ( diff --git a/backend/src/baserow/contrib/builder/workflow_actions/registries.py b/backend/src/baserow/contrib/builder/workflow_actions/registries.py index ec39db948..321685ca9 100644 --- a/backend/src/baserow/contrib/builder/workflow_actions/registries.py +++ b/backend/src/baserow/contrib/builder/workflow_actions/registries.py @@ -1,6 +1,8 @@ -from typing import Any, Dict +from typing import Any, Dict, Optional, Type +from zipfile import ZipFile from django.contrib.auth.models import AbstractUser +from django.core.files.storage import Storage from baserow.contrib.builder.formula_importer import import_formula from baserow.contrib.builder.mixins import BuilderInstanceWithFormulaMixin @@ -35,6 +37,39 @@ class BuilderWorkflowActionType( return super().prepare_values(values, user, instance) + def create_instance_from_serialized( + self, + serialized_values: Dict[str, Any], + id_mapping, + files_zip: Optional[ZipFile] = None, + storage: Optional[Storage] = None, + cache: Optional[Dict[str, any]] = None, + **kwargs, + ) -> Type[BuilderWorkflowAction]: + """ + Responsible for ensuring that when a new workflow action is created from a + serialized value, if the event points to a collection field (as opposed to an + element), the event's `uid` is migrated from the serialized value to the + corresponding `uid` in the `id_mapping` dict. + + :param serialized_values: The serialized values for the new workflow action. + :param id_mapping: The mapping of old to new ids. + :param files_zip: An optional zip file for the related files. + :param storage: The storage instance. + :param cache: An optional cache dict. + :param kwargs: Additional keyword arguments. + :return: The new workflow action instance. + """ + + if BuilderWorkflowAction.is_collection_field_action(serialized_values["event"]): + exported_uid, exported_event = serialized_values["event"].split("_", 1) + imported_uid = id_mapping["builder_collection_fields_uids"][exported_uid] + serialized_values["event"] = f"{imported_uid}_{exported_event}" + + return super().create_instance_from_serialized( + serialized_values, id_mapping, files_zip, storage, cache + ) + def deserialize_property( self, prop_name: str, diff --git a/backend/tests/baserow/contrib/builder/elements/test_table_element_type.py b/backend/tests/baserow/contrib/builder/elements/test_table_element_type.py index 895c1f588..38c705f40 100644 --- a/backend/tests/baserow/contrib/builder/elements/test_table_element_type.py +++ b/backend/tests/baserow/contrib/builder/elements/test_table_element_type.py @@ -513,3 +513,40 @@ def test_import_context_addition_returns_data_source_id(data_fixture): context = table_element_type.import_context_addition(table_element) assert context["data_source_id"] == data_source.id + + +@pytest.mark.django_db +def test_table_element_duplication_regenerates_collection_field_uids(data_fixture): + data_source = data_fixture.create_builder_local_baserow_list_rows_data_source() + source_table_element = data_fixture.create_builder_table_element( + data_source=data_source, + fields=[ + { + "name": "FieldA", + "type": "button", + "config": {"value": f"'Click me')"}, + }, + ], + ) + source_collection_field = source_table_element.fields.get(name="FieldA") + source_workflow_action = data_fixture.create_workflow_action( + NotificationWorkflowAction, + page=source_table_element.page, + element=source_table_element, + event=f"{source_collection_field.uid}_click", + ) + + elements_and_workflow_actions_duplicated = ElementHandler().duplicate_element( + source_table_element + ) + duplicated_table_element = elements_and_workflow_actions_duplicated["elements"][0] + duplicated_collection_field = duplicated_table_element.fields.get(name="FieldA") + duplicated_workflow_action = elements_and_workflow_actions_duplicated[ + "workflow_actions" + ][0] + + assert source_collection_field.uid != duplicated_collection_field.uid + assert source_workflow_action.event != duplicated_workflow_action.event + assert ( + duplicated_workflow_action.event == f"{duplicated_collection_field.uid}_click" + ) diff --git a/backend/tests/baserow/contrib/builder/test_builder_application_type.py b/backend/tests/baserow/contrib/builder/test_builder_application_type.py index ce0f98a24..a44af0cd4 100644 --- a/backend/tests/baserow/contrib/builder/test_builder_application_type.py +++ b/backend/tests/baserow/contrib/builder/test_builder_application_type.py @@ -817,6 +817,7 @@ IMPORT_REFERENCE = { "order": 1, "page_id": 999, "element_id": 998, + "event": "click", "type": "notification", "description": "'hello'", "title": "'there'", @@ -1276,7 +1277,7 @@ def test_builder_application_imports_page_with_default_visibility( ): """ Ensure that the importer sets default values for Page Visibility when the - Page Visiblity related values are missing in the exported data. + Page Visibility related values are missing in the exported data. """ user = data_fixture.create_user(email="test@baserow.io") diff --git a/backend/tests/baserow/contrib/builder/workflow_actions/test_workflow_action_models.py b/backend/tests/baserow/contrib/builder/workflow_actions/test_workflow_action_models.py new file mode 100644 index 000000000..816ffda05 --- /dev/null +++ b/backend/tests/baserow/contrib/builder/workflow_actions/test_workflow_action_models.py @@ -0,0 +1,49 @@ +import pytest + +from baserow.contrib.builder.workflow_actions.models import ( + BuilderWorkflowAction, + EventTypes, + NotificationWorkflowAction, +) + + +def test_builder_workflow_action_is_collection_field_action(): + assert not BuilderWorkflowAction.is_collection_field_action("") + assert not BuilderWorkflowAction.is_collection_field_action("click") + assert BuilderWorkflowAction.is_collection_field_action( + "f1594a0a-3ff0-4c8c-a175-992039b11411_click" + ) + + +@pytest.mark.django_db +def test_builder_workflow_action_target(data_fixture): + page = data_fixture.create_builder_page() + table_element = data_fixture.create_builder_table_element( + page=page, + fields=[ + { + "name": "FieldA", + "type": "button", + "config": {"value": f"'Click me')"}, + }, + ], + ) + collection_field = table_element.fields.get() + collection_field_workflow_action = data_fixture.create_workflow_action( + NotificationWorkflowAction, + page=page, + element=table_element, + event=f"{collection_field.uid}_click", + ) + + assert collection_field_workflow_action.target == collection_field + + button_element = data_fixture.create_builder_button_element(page=page) + button_element_workflow_action = data_fixture.create_workflow_action( + NotificationWorkflowAction, + page=page, + element=button_element, + event=EventTypes.CLICK, + ) + + assert button_element_workflow_action.target == button_element diff --git a/backend/tests/baserow/contrib/builder/workflow_actions/test_workflow_action_types.py b/backend/tests/baserow/contrib/builder/workflow_actions/test_workflow_action_types.py index 0631232a5..ae2b5131e 100644 --- a/backend/tests/baserow/contrib/builder/workflow_actions/test_workflow_action_types.py +++ b/backend/tests/baserow/contrib/builder/workflow_actions/test_workflow_action_types.py @@ -65,6 +65,7 @@ def test_import_workflow_action(data_fixture, workflow_action_type: WorkflowActi "page_id": 41, "element_id": 42, "order": 0, + "event": EventTypes.CLICK, } serialized.update(workflow_action_type.get_pytest_params_serialized(pytest_params)) @@ -299,6 +300,7 @@ def test_import_notification_workflow_action(data_fixture): exported_workflow_action = data_fixture.create_notification_workflow_action( page=page, element=button_1, + event=EventTypes.CLICK, title=f"get('data_source.{data_source_1.id}.field_1')", description=f"get('data_source.{data_source_1.id}.field_1')", ) @@ -330,6 +332,7 @@ def test_import_open_page_workflow_action(data_fixture): exported_workflow_action = data_fixture.create_open_page_workflow_action( page=page, + event=EventTypes.CLICK, element=button_1, navigate_to_url=f"get('data_source.{data_source_1.id}.field_1')", page_parameters=[ diff --git a/changelog/entries/unreleased/bug/3521_resolved_an_issue_which_caused_table_element_actions_to_disa.json b/changelog/entries/unreleased/bug/3521_resolved_an_issue_which_caused_table_element_actions_to_disa.json new file mode 100644 index 000000000..88421d44e --- /dev/null +++ b/changelog/entries/unreleased/bug/3521_resolved_an_issue_which_caused_table_element_actions_to_disa.json @@ -0,0 +1,8 @@ +{ + "type": "bug", + "message": "Resolved an issue which caused table element actions to disappear when it was duplicated.", + "domain": "builder", + "issue_number": 3521, + "bullet_points": [], + "created_at": "2025-03-20" +} \ No newline at end of file