From 55d4ed8f6912514da3467dfaa244efbe340d8499 Mon Sep 17 00:00:00 2001 From: Peter Evans <peter@baserow.io> Date: Fri, 24 May 2024 10:35:59 +0000 Subject: [PATCH] Resolve "Repeat element: add current_record data provider support" --- .../builder/api/workflow_actions/views.py | 4 +- .../data_providers/data_provider_types.py | 35 ++++- .../data_sources/builder_dispatch_context.py | 46 +++++-- .../contrib/builder/elements/handler.py | 81 +++++++++++- .../baserow/core/services/dispatch_context.py | 24 ++++ backend/src/baserow/core/services/types.py | 7 +- .../test_utils/fixtures/data_source.py | 6 +- .../baserow/test_utils/fixtures/element.py | 9 ++ .../test_data_provider_types.py | 67 ++++++++++ .../data_sources/test_dispatch_context.py | 26 ++++ .../builder/elements/test_element_handler.py | 72 ++++++++++- .../ApplicationBuilderFormulaInputGroup.vue | 1 + .../components/elements/ElementPreview.vue | 17 ++- .../elements/components/ColumnElement.vue | 9 +- .../components/FormContainerElement.vue | 2 +- .../elements/components/RepeatElement.vue | 10 +- .../builder/components/page/PageElement.vue | 12 +- .../page/header/DataSourceContext.vue | 1 + .../page/sidePanels/GeneralSidePanel.vue | 9 ++ .../modules/builder/dataProviderTypes.js | 44 +++++-- web-frontend/modules/builder/elementTypes.js | 59 ++++++++- .../modules/builder/mixins/element.js | 24 ++-- .../modules/builder/pages/pageEditor.vue | 1 + .../modules/builder/pages/publicPage.vue | 1 + .../modules/builder/store/dataSource.js | 14 ++ web-frontend/modules/builder/store/element.js | 43 +++---- .../test/unit/builder/elementTypes.spec.js | 121 +++++++++++++++++- 27 files changed, 664 insertions(+), 81 deletions(-) diff --git a/backend/src/baserow/contrib/builder/api/workflow_actions/views.py b/backend/src/baserow/contrib/builder/api/workflow_actions/views.py index 210e0fd0a..bba58a91c 100644 --- a/backend/src/baserow/contrib/builder/api/workflow_actions/views.py +++ b/backend/src/baserow/contrib/builder/api/workflow_actions/views.py @@ -391,7 +391,9 @@ class DispatchBuilderWorkflowActionView(APIView): workflow_action_id ) - dispatch_context = BuilderDispatchContext(request, workflow_action.page) + dispatch_context = BuilderDispatchContext( + request, workflow_action.page, workflow_action=workflow_action + ) response = BuilderWorkflowActionService().dispatch_action( request.user, workflow_action, dispatch_context # type: ignore diff --git a/backend/src/baserow/contrib/builder/data_providers/data_provider_types.py b/backend/src/baserow/contrib/builder/data_providers/data_provider_types.py index 92cb63c2d..1c43257e7 100644 --- a/backend/src/baserow/contrib/builder/data_providers/data_provider_types.py +++ b/backend/src/baserow/contrib/builder/data_providers/data_provider_types.py @@ -13,7 +13,10 @@ from baserow.contrib.builder.data_sources.exceptions import ( ) from baserow.contrib.builder.data_sources.handler import DataSourceHandler from baserow.contrib.builder.elements.handler import ElementHandler -from baserow.contrib.builder.elements.mixins import FormElementTypeMixin +from baserow.contrib.builder.elements.mixins import ( + CollectionElementTypeMixin, + FormElementTypeMixin, +) from baserow.contrib.builder.elements.models import FormElement from baserow.contrib.builder.workflow_actions.handler import ( BuilderWorkflowActionHandler, @@ -185,9 +188,35 @@ class CurrentRecordDataProviderType(DataProviderType): type = "current_record" def get_data_chunk(self, dispatch_context: BuilderDispatchContext, path: List[str]): - """Doesn't make sense in the backend yet""" + """ + Get the current record data from the request data. - return None + :param dispatch_context: The dispatch context. + :param path: The path to the data. + :return: The data at the path. + """ + + try: + current_record = dispatch_context.request.data["current_record"] + except KeyError: + return None + + first_collection_element_ancestor = ElementHandler().get_first_ancestor_of_type( + dispatch_context.workflow_action.element_id, + CollectionElementTypeMixin, + ) + data_source_id = first_collection_element_ancestor.specific.data_source_id + + # Narrow down our range to just our record index. + dispatch_context = BuilderDispatchContext.from_context( + dispatch_context, + offset=current_record, + count=1, + ) + + return DataSourceDataProviderType().get_data_chunk( + dispatch_context, [data_source_id, "0", *path] + ) def import_path(self, path, id_mapping, data_source_id=None, **kwargs): """ diff --git a/backend/src/baserow/contrib/builder/data_sources/builder_dispatch_context.py b/backend/src/baserow/contrib/builder/data_sources/builder_dispatch_context.py index fb687c63d..1aa3144a5 100644 --- a/backend/src/baserow/contrib/builder/data_sources/builder_dispatch_context.py +++ b/backend/src/baserow/contrib/builder/data_sources/builder_dispatch_context.py @@ -1,3 +1,5 @@ +from typing import TYPE_CHECKING, Optional + from rest_framework.request import Request from baserow.contrib.builder.data_providers.registries import ( @@ -6,11 +8,28 @@ from baserow.contrib.builder.data_providers.registries import ( from baserow.contrib.builder.pages.models import Page from baserow.core.services.dispatch_context import DispatchContext +if TYPE_CHECKING: + from baserow.core.workflow_actions.models import WorkflowAction + class BuilderDispatchContext(DispatchContext): - def __init__(self, request: Request, page: Page): + own_properties = ["request", "page", "workflow_action", "offset", "count"] + + def __init__( + self, + request: Request, + page: Page, + workflow_action: Optional["WorkflowAction"] = None, + offset: Optional[int] = None, + count: Optional[int] = None, + ): self.request = request self.page = page + self.workflow_action = workflow_action + + # Overrides the `request` GET offset/count values. + self.offset = offset + self.count = count super().__init__() @@ -19,17 +38,24 @@ class BuilderDispatchContext(DispatchContext): return builder_data_provider_type_registry def range(self, service): - """Return page range from the GET parameters.""" + """ + Return page range from the `offset`, `count` kwargs, + or the GET parameters. + """ - try: - offset = int(self.request.GET.get("offset", 0)) - except ValueError: - offset = 0 + if self.offset is not None and self.count is not None: + offset = self.offset + count = self.count + else: + try: + offset = int(self.request.GET.get("offset", 0)) + except ValueError: + offset = 0 - try: - count = int(self.request.GET.get("count", 20)) - except ValueError: - count = 20 + try: + count = int(self.request.GET.get("count", 20)) + except ValueError: + count = 20 # max prevent negative values return [ diff --git a/backend/src/baserow/contrib/builder/elements/handler.py b/backend/src/baserow/contrib/builder/elements/handler.py index 06185a0c4..363e200fc 100644 --- a/backend/src/baserow/contrib/builder/elements/handler.py +++ b/backend/src/baserow/contrib/builder/elements/handler.py @@ -1,5 +1,5 @@ from collections import defaultdict -from typing import Any, Dict, Iterable, List, Optional, Union, cast +from typing import Any, Dict, Iterable, List, Optional, Type, Union, cast from zipfile import ZipFile from django.core.files.storage import Storage @@ -12,17 +12,22 @@ from baserow.contrib.builder.elements.exceptions import ( from baserow.contrib.builder.elements.models import ContainerElement, Element from baserow.contrib.builder.elements.registries import ( ElementType, + ElementTypeSubClass, element_type_registry, ) +from baserow.contrib.builder.elements.types import ( + ElementForUpdate, + ElementsAndWorkflowActions, +) from baserow.contrib.builder.pages.models import Page +from baserow.contrib.builder.workflow_actions.models import BuilderWorkflowAction +from baserow.contrib.builder.workflow_actions.registries import ( + builder_workflow_action_type_registry, +) from baserow.core.db import specific_iterator from baserow.core.exceptions import IdDoesNotExist from baserow.core.utils import MirrorDict, extract_allowed -from ..workflow_actions.models import BuilderWorkflowAction -from ..workflow_actions.registries import builder_workflow_action_type_registry -from .types import ElementForUpdate, ElementsAndWorkflowActions - class ElementHandler: allowed_fields_create = [ @@ -94,6 +99,47 @@ class ElementHandler: return element + def get_ancestors(self, element_id: int, page: Page) -> List[Element]: + """ + Returns a list of all the ancestors of the given element. The ancestry + results are cached in the dispatch context to avoid multiple queries in + the same HTTP request. + + :param element_id: The ID of the element. + :param page: The page that holds the elements. + :return: A list of the ancestors of the given element. + """ + + elements = self.get_elements(page) + grouped_elements = {element.id: element for element in elements} + element = grouped_elements[element_id] + + ancestry = [] + while element.parent_element_id is not None: + element = grouped_elements[element.parent_element_id] + ancestry.append(element) + + return ancestry + + def get_first_ancestor_of_type( + self, + element_id: int, + target_type: Type[ElementTypeSubClass], + ) -> Optional[Element]: + """ + Returns the first ancestor of the given type belonging to this element. + + :param element_id: The ID of the element. + :param target_type: The type of the element to find. + :return: The first ancestor of the given type or None if not found. + """ + + element = ElementHandler().get_element(element_id) + for ancestor in self.get_ancestors(element_id, element.page): + ancestor_type = element_type_registry.get_by_model(ancestor.specific_class) + if isinstance(ancestor_type, target_type): + return ancestor + def get_element_for_update( self, element_id: int, base_queryset: Optional[QuerySet] = None ) -> ElementForUpdate: @@ -120,6 +166,7 @@ class ElementHandler: page: Page, base_queryset: Optional[QuerySet] = None, specific: bool = True, + use_cache: bool = True, ) -> Union[QuerySet[Element], Iterable[Element]]: """ Gets all the specific elements of a given page. @@ -128,18 +175,38 @@ class ElementHandler: :param base_queryset: The base queryset to use to build the query. :param specific: Whether to return the generic elements or the specific instances. + :param use_cache: Whether to use the cached elements on the page or not. :return: The elements of that page. """ + # Our cache key varies if we're using specific or generic elements. + cache_key = "_page_elements" if not specific else "_page_elements_specific" + + # If a `base_queryset` is given, we can't use caching, clear + # both cache keys in case the specific argument has changed. + if base_queryset is not None: + use_cache = False + setattr(page, "_page_elements", None) + setattr(page, "_page_elements_specific", None) + + elements_cache = getattr(page, cache_key, None) + if use_cache and elements_cache is not None: + return elements_cache + queryset = base_queryset if base_queryset is not None else Element.objects.all() queryset = queryset.filter(page=page) if specific: queryset = queryset.select_related("content_type") - return specific_iterator(queryset) + elements = specific_iterator(queryset) else: - return queryset + elements = queryset + + if use_cache: + setattr(page, cache_key, list(elements)) + + return elements def create_element( self, diff --git a/backend/src/baserow/core/services/dispatch_context.py b/backend/src/baserow/core/services/dispatch_context.py index 48a642f11..ab778dc49 100644 --- a/backend/src/baserow/core/services/dispatch_context.py +++ b/backend/src/baserow/core/services/dispatch_context.py @@ -2,9 +2,12 @@ from abc import ABC, abstractmethod from baserow.core.formula.runtime_formula_context import RuntimeFormulaContext from baserow.core.services.models import Service +from baserow.core.services.types import RuntimeFormulaContextSubClass class DispatchContext(RuntimeFormulaContext, ABC): + own_properties = [] + def __init__(self): self.cache = {} # can be used by data providers to save queries super().__init__() @@ -16,3 +19,24 @@ class DispatchContext(RuntimeFormulaContext, ABC): :params service: The service we want the pagination for. """ + + @classmethod + def from_context( + cls, context: RuntimeFormulaContextSubClass, **kwargs + ) -> RuntimeFormulaContextSubClass: + """ + Return a new DispatchContext instance from the given context, without + losing the original cached data. + + :params context: The context to create a new DispatchContext instance from. + """ + + new_values = {} + for prop in cls.own_properties: + new_values[prop] = getattr(context, prop) + new_values.update(kwargs) + + new_context = cls(**new_values) + new_context.cache = {**context.cache} + + return new_context diff --git a/backend/src/baserow/core/services/types.py b/backend/src/baserow/core/services/types.py index a4fcf7059..ac74e0b9d 100644 --- a/backend/src/baserow/core/services/types.py +++ b/backend/src/baserow/core/services/types.py @@ -1,6 +1,7 @@ from typing import NewType, Optional, TypedDict, TypeVar -from .models import Service +from baserow.core.formula.runtime_formula_context import RuntimeFormulaContext +from baserow.core.services.models import Service class ServiceDict(TypedDict): @@ -34,3 +35,7 @@ ServiceSortDictSubClass = TypeVar("ServiceSortDictSubClass", bound="ServiceSortD ServiceSubClass = TypeVar("ServiceSubClass", bound="Service") ServiceForUpdate = NewType("ServiceForUpdate", Service) + +RuntimeFormulaContextSubClass = TypeVar( + "RuntimeFormulaContextSubClass", bound=RuntimeFormulaContext +) diff --git a/backend/src/baserow/test_utils/fixtures/data_source.py b/backend/src/baserow/test_utils/fixtures/data_source.py index 3c758619b..a4fce0e95 100644 --- a/backend/src/baserow/test_utils/fixtures/data_source.py +++ b/backend/src/baserow/test_utils/fixtures/data_source.py @@ -35,10 +35,10 @@ class DataSourceFixtures: service = kwargs.pop("service", None) if service is None and service_model_class: + integrations_args = kwargs.pop("integration_args", {}) + integrations_args["application"] = page.builder service = self.create_service( - service_model_class, - integration_args={"application": page.builder}, - **kwargs, + service_model_class, integration_args=integrations_args, **kwargs ) if order is None: diff --git a/backend/src/baserow/test_utils/fixtures/element.py b/backend/src/baserow/test_utils/fixtures/element.py index ae43bcf46..fa9625713 100644 --- a/backend/src/baserow/test_utils/fixtures/element.py +++ b/backend/src/baserow/test_utils/fixtures/element.py @@ -10,6 +10,7 @@ from baserow.contrib.builder.elements.models import ( ImageElement, InputTextElement, LinkElement, + RepeatElement, TableElement, TextElement, ) @@ -96,6 +97,14 @@ class ElementFixtures: element = self.create_builder_element(DropdownElement, user, page, **kwargs) return element + def create_builder_repeat_element(self, user=None, page=None, **kwargs): + if "data_source" not in kwargs: + kwargs[ + "data_source" + ] = self.create_builder_local_baserow_list_rows_data_source(page=page) + element = self.create_builder_element(RepeatElement, user, page, **kwargs) + return element + def create_builder_element(self, model_class, user=None, page=None, **kwargs): if user is None: user = self.create_user() diff --git a/backend/tests/baserow/contrib/builder/data_providers/test_data_provider_types.py b/backend/tests/baserow/contrib/builder/data_providers/test_data_provider_types.py index d862b47a6..917437164 100644 --- a/backend/tests/baserow/contrib/builder/data_providers/test_data_provider_types.py +++ b/backend/tests/baserow/contrib/builder/data_providers/test_data_provider_types.py @@ -22,6 +22,7 @@ from baserow.contrib.builder.data_sources.builder_dispatch_context import ( ) from baserow.contrib.builder.elements.handler import ElementHandler from baserow.contrib.builder.formula_importer import import_formula +from baserow.contrib.builder.workflow_actions.models import EventTypes from baserow.core.services.dispatch_context import DispatchContext from baserow.core.services.exceptions import ServiceImproperlyConfigured from baserow.core.user_sources.user_source_user import UserSourceUser @@ -759,6 +760,72 @@ def test_user_data_provider_get_data_chunk(data_fixture): assert user_data_provider_type.get_data_chunk(dispatch_context, ["id"]) == 42 +@pytest.mark.django_db +def test_current_record_provider_get_data_chunk_without_record_index(data_fixture): + current_record_provider = CurrentRecordDataProviderType() + + user, token = data_fixture.create_user_and_token() + + fake_request = MagicMock() + fake_request.data = {} + + builder = data_fixture.create_builder_application(user=user) + page = data_fixture.create_builder_page(user=user, builder=builder) + workflow_action = data_fixture.create_local_baserow_create_row_workflow_action( + page=page, event=EventTypes.CLICK, user=user + ) + + dispatch_context = BuilderDispatchContext(fake_request, page, workflow_action) + assert current_record_provider.get_data_chunk(dispatch_context, ["path"]) is None + + +@pytest.mark.django_db +def test_current_record_provider_get_data_chunk(data_fixture): + current_record_provider = CurrentRecordDataProviderType() + + user, token = data_fixture.create_user_and_token() + + fake_request = MagicMock() + fake_request.user = user + fake_request.data = {"current_record": 0} + + table, fields, rows = data_fixture.build_table( + user=user, + columns=[ + ("Animal", "text"), + ], + rows=[ + ["Badger"], + ["Horse"], + ["Bison"], + ], + ) + field = table.field_set.get(name="Animal") + builder = data_fixture.create_builder_application(user=user) + page = data_fixture.create_builder_page(user=user, builder=builder) + + data_source = data_fixture.create_builder_local_baserow_list_rows_data_source( + page=page, table=table, integration_args={"authorized_user": user} + ) + repeat_element = data_fixture.create_builder_repeat_element( + page=page, data_source=data_source + ) + button_element = data_fixture.create_builder_button_element( + page=page, parent_element=repeat_element + ) + + workflow_action = data_fixture.create_local_baserow_create_row_workflow_action( + page=page, element=button_element, event=EventTypes.CLICK, user=user + ) + + dispatch_context = BuilderDispatchContext(fake_request, page, workflow_action) + + assert ( + current_record_provider.get_data_chunk(dispatch_context, [field.db_column]) + == "Badger" + ) + + @pytest.mark.django_db def test_current_record_provider_type_import_path(data_fixture): # When a `current_record` provider is imported, and the path only contains the diff --git a/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py b/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py index eaae8a3da..356fc6144 100644 --- a/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py +++ b/backend/tests/baserow/contrib/builder/data_sources/test_dispatch_context.py @@ -1,5 +1,10 @@ from unittest.mock import MagicMock +from django.http import HttpRequest + +import pytest +from rest_framework.request import Request + from baserow.contrib.builder.data_sources.builder_dispatch_context import ( BuilderDispatchContext, ) @@ -24,3 +29,24 @@ def test_dispatch_context_page_range(): dispatch_context = BuilderDispatchContext(request, None) assert dispatch_context.range(None) == [0, 1] + + +@pytest.mark.django_db +def test_dispatch_context_page_from_context(data_fixture): + user = data_fixture.create_user() + page = data_fixture.create_builder_page(user=user) + request = Request(HttpRequest()) + request.user = user + + dispatch_context = BuilderDispatchContext(request, page, offset=0, count=5) + dispatch_context.annotated_data = "foobar" + dispatch_context.cache = {"key": "value"} + new_dispatch_context = BuilderDispatchContext.from_context( + dispatch_context, offset=5, count=1 + ) + assert getattr(new_dispatch_context, "annotated_data", None) is None + assert new_dispatch_context.cache == {"key": "value"} + assert new_dispatch_context.request == request + assert new_dispatch_context.page == page + assert new_dispatch_context.offset == 5 + assert new_dispatch_context.count == 1 diff --git a/backend/tests/baserow/contrib/builder/elements/test_element_handler.py b/backend/tests/baserow/contrib/builder/elements/test_element_handler.py index 62000c955..91e678738 100644 --- a/backend/tests/baserow/contrib/builder/elements/test_element_handler.py +++ b/backend/tests/baserow/contrib/builder/elements/test_element_handler.py @@ -54,13 +54,17 @@ def test_get_element_does_not_exist(data_fixture): @pytest.mark.django_db -def test_get_elements(data_fixture): +def test_get_elements(data_fixture, django_assert_num_queries): page = data_fixture.create_builder_page() element1 = data_fixture.create_builder_heading_element(page=page) element2 = data_fixture.create_builder_heading_element(page=page) element3 = data_fixture.create_builder_text_element(page=page) - elements = ElementHandler().get_elements(page) + with django_assert_num_queries(3): + elements = ElementHandler().get_elements(page) + + # Cache of specific elements is set. + assert getattr(page, "_page_elements_specific") == elements assert [e.id for e in elements] == [ element1.id, @@ -69,8 +73,31 @@ def test_get_elements(data_fixture): ] assert isinstance(elements[0], HeadingElement) + assert isinstance(elements[1], HeadingElement) assert isinstance(elements[2], TextElement) + # Cache of specific elements is re-used. + with django_assert_num_queries(0): + elements = ElementHandler().get_elements(page) + assert getattr(page, "_page_elements_specific") == elements + + # We request non-specific records, the cache changes. + with django_assert_num_queries(1): + elements = list(ElementHandler().get_elements(page, specific=False)) + assert getattr(page, "_page_elements") == elements + + # We request non-specific records, the cache is reused. + with django_assert_num_queries(0): + elements = list(ElementHandler().get_elements(page, specific=False)) + assert getattr(page, "_page_elements") == elements + + # We pass in a base queryset, no caching strategy is available. + base_queryset = Element.objects.filter(page=page, visibility="all") + with django_assert_num_queries(3): + ElementHandler().get_elements(page, base_queryset) + assert getattr(page, "_page_elements") is None + assert getattr(page, "_page_elements_specific") is None + @pytest.mark.django_db def test_delete_element(data_fixture): @@ -505,3 +532,44 @@ def test_duplicate_element_with_workflow_action_in_container(data_fixture): ] assert duplicated_workflow_action1.page_id == workflow_action1.page_id assert duplicated_workflow_action2.page_id == workflow_action2.page_id + + +@pytest.mark.django_db +def test_get_ancestors(data_fixture, django_assert_num_queries): + page = data_fixture.create_builder_page() + grandparent = data_fixture.create_builder_column_element(column_amount=1, page=page) + parent = data_fixture.create_builder_column_element( + column_amount=3, parent_element=grandparent, page=page + ) + child = data_fixture.create_builder_heading_element( + page=page, parent_element=parent + ) + + # Query and cache the page's elements for the same context. + # Query 1: fetch the elements on the page. + # 2: fetch the specific column types. + # 3: fetch the specific heading type. + with django_assert_num_queries(3): + ancestors = ElementHandler().get_ancestors(child.id, page) + + assert len(ancestors) == 2 + assert ancestors == [parent, grandparent] + + +@pytest.mark.django_db +def test_get_first_ancestor_of_type(data_fixture, django_assert_num_queries): + page = data_fixture.create_builder_page() + grandparent = data_fixture.create_builder_column_element(column_amount=1, page=page) + parent = data_fixture.create_builder_form_container_element( + parent_element=grandparent, page=page + ) + child = data_fixture.create_builder_dropdown_element( + page=page, parent_element=parent + ) + + with django_assert_num_queries(7): + nearest_column_ancestor = ElementHandler().get_first_ancestor_of_type( + child.id, ColumnElementType + ) + + assert nearest_column_ancestor.specific == grandparent diff --git a/web-frontend/modules/builder/components/ApplicationBuilderFormulaInputGroup.vue b/web-frontend/modules/builder/components/ApplicationBuilderFormulaInputGroup.vue index a84866b09..e891145b5 100644 --- a/web-frontend/modules/builder/components/ApplicationBuilderFormulaInputGroup.vue +++ b/web-frontend/modules/builder/components/ApplicationBuilderFormulaInputGroup.vue @@ -22,6 +22,7 @@ <script> import FormulaInputGroup from '@baserow/modules/core/components/formula/FormulaInputGroup' import { DataSourceDataProviderType } from '@baserow/modules/builder/dataProviderTypes' + export default { name: 'ApplicationBuilderFormulaInputGroup', components: { FormulaInputGroup }, diff --git a/web-frontend/modules/builder/components/elements/ElementPreview.vue b/web-frontend/modules/builder/components/elements/ElementPreview.vue index abd3116cd..e06b61760 100644 --- a/web-frontend/modules/builder/components/elements/ElementPreview.vue +++ b/web-frontend/modules/builder/components/elements/ElementPreview.vue @@ -34,7 +34,12 @@ @select-parent="selectParentElement()" /> - <PageElement :element="element" :mode="mode" class="element--read-only" /> + <PageElement + :element="element" + :mode="mode" + class="element--read-only" + :application-context-additions="applicationContextAdditions" + /> <InsertElementButton v-show="isSelected" @@ -95,6 +100,11 @@ export default { required: false, default: false, }, + applicationContextAdditions: { + type: Object, + required: false, + default: null, + }, }, data() { return { @@ -144,7 +154,10 @@ export default { return elementType.getPlacementsDisabled(this.page, this.element) }, elementTypesAllowed() { - return this.parentElementType?.childElementTypes || null + return ( + this.parentElementType?.childElementTypes(this.page, this.element) || + null + ) }, canCreate() { return this.$hasPermission( diff --git a/web-frontend/modules/builder/components/elements/components/ColumnElement.vue b/web-frontend/modules/builder/components/elements/components/ColumnElement.vue index 8f7eb96ff..624e4fc34 100644 --- a/web-frontend/modules/builder/components/elements/components/ColumnElement.vue +++ b/web-frontend/modules/builder/components/elements/components/ColumnElement.vue @@ -21,12 +21,14 @@ <ElementPreview v-if="mode === 'editing'" :element="childCurrent" + :application-context-additions="applicationContextAdditions" @move="move(childCurrent, $event)" ></ElementPreview> <PageElement v-else :element="childCurrent" :mode="mode" + :application-context-additions="applicationContextAdditions" ></PageElement> </div> </template> @@ -41,7 +43,7 @@ <AddElementModal ref="addElementModal" :page="page" - :element-types-allowed="elementType.childElementTypes" + :element-types-allowed="elementType.childElementTypes(page, element)" /> </div> </template> @@ -79,6 +81,11 @@ export default { type: Object, required: true, }, + applicationContextAdditions: { + type: Object, + required: false, + default: null, + }, }, computed: { flexAlignment() { diff --git a/web-frontend/modules/builder/components/elements/components/FormContainerElement.vue b/web-frontend/modules/builder/components/elements/components/FormContainerElement.vue index 00b1d1b35..1c50c3bfb 100644 --- a/web-frontend/modules/builder/components/elements/components/FormContainerElement.vue +++ b/web-frontend/modules/builder/components/elements/components/FormContainerElement.vue @@ -15,7 +15,7 @@ <AddElementModal ref="addElementModal" :page="page" - :element-types-allowed="elementType.childElementTypes" + :element-types-allowed="elementType.childElementTypes(page, element)" ></AddElementModal> </div> <div v-else> diff --git a/web-frontend/modules/builder/components/elements/components/RepeatElement.vue b/web-frontend/modules/builder/components/elements/components/RepeatElement.vue index 5e2b56389..5e1e7cbce 100644 --- a/web-frontend/modules/builder/components/elements/components/RepeatElement.vue +++ b/web-frontend/modules/builder/components/elements/components/RepeatElement.vue @@ -13,6 +13,9 @@ v-if="index === 0 && isEditMode" :key="child.id" :element="child" + :application-context-additions="{ + recordIndex: index, + }" @move="moveElement(child, $event)" /> <!-- Other iterations are not editable --> @@ -22,6 +25,9 @@ :key="child.id" :element="child" :force-mode="'preview'" + :application-context-additions="{ + recordIndex: index, + }" :class="{ 'repeat-element-preview': index > 0 && isEditMode, }" @@ -36,7 +42,7 @@ <AddElementModal ref="addElementModal" :page="page" - :element-types-allowed="elementType.childElementTypes" + :element-types-allowed="elementType.childElementTypes(page, element)" ></AddElementModal> </template> </template> @@ -48,7 +54,7 @@ <AddElementModal ref="addElementModal" :page="page" - :element-types-allowed="elementType.childElementTypes" + :element-types-allowed="elementType.childElementTypes(page, element)" ></AddElementModal> </template> <!-- We have no contents, but we do have children in edit mode --> diff --git a/web-frontend/modules/builder/components/page/PageElement.vue b/web-frontend/modules/builder/components/page/PageElement.vue index c54a6100a..0e378b5bd 100644 --- a/web-frontend/modules/builder/components/page/PageElement.vue +++ b/web-frontend/modules/builder/components/page/PageElement.vue @@ -32,10 +32,15 @@ import { BACKGROUND_TYPES, WIDTH_TYPES } from '@baserow/modules/builder/enums' export default { name: 'PageElement', - inject: ['builder', 'page', 'mode'], + inject: ['builder', 'page', 'mode', 'applicationContext'], provide() { return { mode: this.elementMode, + applicationContext: { + ...this.applicationContext, + element: this.element, + ...this.applicationContextAdditions, + }, } }, props: { @@ -48,6 +53,11 @@ export default { required: false, default: null, }, + applicationContextAdditions: { + type: Object, + required: false, + default: null, + }, }, computed: { BACKGROUND_TYPES: () => BACKGROUND_TYPES, diff --git a/web-frontend/modules/builder/components/page/header/DataSourceContext.vue b/web-frontend/modules/builder/components/page/header/DataSourceContext.vue index af98344fb..92aab0614 100644 --- a/web-frontend/modules/builder/components/page/header/DataSourceContext.vue +++ b/web-frontend/modules/builder/components/page/header/DataSourceContext.vue @@ -103,6 +103,7 @@ export default { actionUpdateDataSource: 'dataSource/debouncedUpdate', actionDeleteDataSource: 'dataSource/delete', actionFetchDataSources: 'dataSource/fetch', + clearElementContent: 'elementContent/clearElementContent', }), async shown() { try { diff --git a/web-frontend/modules/builder/components/page/sidePanels/GeneralSidePanel.vue b/web-frontend/modules/builder/components/page/sidePanels/GeneralSidePanel.vue index 7d955ad84..c006745b4 100644 --- a/web-frontend/modules/builder/components/page/sidePanels/GeneralSidePanel.vue +++ b/web-frontend/modules/builder/components/page/sidePanels/GeneralSidePanel.vue @@ -15,5 +15,14 @@ import elementSidePanel from '@baserow/modules/builder/mixins/elementSidePanel' export default { name: 'GeneralSidePanel', mixins: [elementSidePanel], + inject: ['applicationContext'], + provide() { + return { + applicationContext: { + ...this.applicationContext, + element: this.element, + }, + } + }, } </script> diff --git a/web-frontend/modules/builder/dataProviderTypes.js b/web-frontend/modules/builder/dataProviderTypes.js index e7f0b423a..8ce922fd5 100644 --- a/web-frontend/modules/builder/dataProviderTypes.js +++ b/web-frontend/modules/builder/dataProviderTypes.js @@ -213,6 +213,26 @@ export class CurrentRecordDataProviderType extends DataProviderType { return true } + getFirstCollectionAncestor(page, element) { + if (!element) { + return null + } + const elementType = this.app.$registry.get('element', element.type) + if (elementType.isCollectionElement) { + return element + } + const ancestors = this.app.store.getters['element/getAncestors']( + page, + element + ) + for (const ancestor of ancestors) { + const ancestorType = this.app.$registry.get('element', ancestor.type) + if (ancestorType.isCollectionElement) { + return ancestor + } + } + } + // Loads all element contents async init(applicationContext) { const { page } = applicationContext @@ -252,20 +272,26 @@ export class CurrentRecordDataProviderType extends DataProviderType { ) } + getActionDispatchContext(applicationContext) { + return applicationContext.recordIndex + } + getDataChunk(applicationContext, path) { const content = this.getDataContent(applicationContext) return getValueAtPath(content, path.join('.')) } getDataContent(applicationContext) { - const { element, recordIndex = 0 } = applicationContext - - if (!element) { + const { page, element, recordIndex = 0 } = applicationContext + const collectionElement = this.getFirstCollectionAncestor(page, element) + if (!collectionElement) { return [] } const rows = - this.app.store.getters['elementContent/getElementContent'](element) + this.app.store.getters['elementContent/getElementContent']( + collectionElement + ) const row = { [this.indexKey]: recordIndex, ...rows[recordIndex] } @@ -284,8 +310,9 @@ export class CurrentRecordDataProviderType extends DataProviderType { } getDataSchema(applicationContext) { - const { page, element: { data_source_id: dataSourceId } = {} } = - applicationContext + const { page, element } = applicationContext + const collectionElement = this.getFirstCollectionAncestor(page, element) + const dataSourceId = collectionElement?.data_source_id if (!dataSourceId) { return null @@ -312,8 +339,9 @@ export class CurrentRecordDataProviderType extends DataProviderType { getPathTitle(applicationContext, pathParts) { if (pathParts.length === 1) { - const { page, element: { data_source_id: dataSourceId } = {} } = - applicationContext + const { page, element } = applicationContext + const collectionElement = this.getFirstCollectionAncestor(page, element) + const dataSourceId = collectionElement?.data_source_id const dataSource = this.app.store.getters[ 'dataSource/getPageDataSourceById' diff --git a/web-frontend/modules/builder/elementTypes.js b/web-frontend/modules/builder/elementTypes.js index 61a686f0a..72aa1a8dd 100644 --- a/web-frontend/modules/builder/elementTypes.js +++ b/web-frontend/modules/builder/elementTypes.js @@ -372,22 +372,44 @@ export class ElementType extends Registerable { const ContainerElementTypeMixin = (Base) => class extends Base { - isContainerElementType = true + isContainerElement = true get elementTypesAll() { return Object.values(this.app.$registry.getAll('element')) } /** - * Returns an array of element types that are not allowed as children of this element. - * - * @returns {Array} + * Returns an array of element types that are not allowed as children of this element type. + * @returns {Array} An array of forbidden child element types. */ get childElementTypesForbidden() { return [] } - get childElementTypes() { + /** + * Returns an array of element types that are allowed as children of this element. + * If the parent element we're trying to add a child to has a parent, we'll check + * each parent until the root element if they have any forbidden element types to + * include as well. + * @param page + * @param element + * @returns {Array} An array of permitted child element types. + */ + childElementTypes(page, element) { + if (element.parent_element_id) { + const parentElement = this.app.store.getters['element/getElementById']( + page, + element.parent_element_id + ) + const parentElementType = this.app.$registry.get( + 'element', + parentElement.type + ) + return _.difference( + parentElementType.childElementTypes(page, parentElement), + this.childElementTypesForbidden + ) + } return _.difference(this.elementTypesAll, this.childElementTypesForbidden) } @@ -475,6 +497,10 @@ export class FormContainerElementType extends ContainerElementTypeMixin( return FormContainerElementForm } + /** + * Exclude element types which are not a form element. + * @returns {Array} An array of non-form element types. + */ get childElementTypesForbidden() { return this.elementTypesAll.filter((type) => !type.isFormElement) } @@ -530,9 +556,13 @@ export class ColumnElementType extends ContainerElementTypeMixin(ElementType) { return ColumnElementForm } + /** + * Exclude element types which are containers. + * @returns {Array} An array of container element types. + */ get childElementTypesForbidden() { return this.elementTypesAll.filter( - (elementType) => elementType.isContainerElementType + (elementType) => elementType.isContainerElement ) } @@ -630,6 +660,7 @@ export class ColumnElementType extends ContainerElementTypeMixin(ElementType) { const CollectionElementTypeMixin = (Base) => class extends Base { + isCollectionElement = true getDisplayName(element, { page }) { let suffix = '' @@ -715,7 +746,7 @@ export class TableElementType extends CollectionElementTypeMixin(ElementType) { export class RepeatElementType extends ContainerElementTypeMixin( CollectionElementTypeMixin(ElementType) ) { - getType() { + static getType() { return 'repeat' } @@ -739,6 +770,20 @@ export class RepeatElementType extends ContainerElementTypeMixin( return RepeatElementForm } + /** + * The repeat elements will disallow itself, all form elements, and the + * form container, from being added as children. + * @returns {Array} An array of disallowed child element types. + */ + get childElementTypesForbidden() { + const repeatElement = this.app.$registry.get('element', 'repeat') + const formContainer = this.app.$registry.get('element', 'form_container') + return this.elementTypesAll.filter( + (type) => + type.isFormElement || type === formContainer || type === repeatElement + ) + } + /** * Return an array of placements that are disallowed for the elements to move * in their container. diff --git a/web-frontend/modules/builder/mixins/element.js b/web-frontend/modules/builder/mixins/element.js index 60fa60077..3c217bb86 100644 --- a/web-frontend/modules/builder/mixins/element.js +++ b/web-frontend/modules/builder/mixins/element.js @@ -9,12 +9,26 @@ import { resolveColor } from '@baserow/modules/core/utils/colors' import { themeToColorVariables } from '@baserow/modules/builder/utils/theme' export default { - inject: ['workspace', 'builder', 'page', 'mode'], + inject: ['workspace', 'builder', 'page', 'mode', 'applicationContext'], + provide() { + return { + applicationContext: { + ...this.applicationContext, + element: this.element, + ...this.applicationContextAdditions, + }, + } + }, props: { element: { type: Object, required: true, }, + applicationContextAdditions: { + type: Object, + required: false, + default: null, + }, }, computed: { workflowActionsInProgress() { @@ -31,14 +45,6 @@ export default { isEditMode() { return this.mode === 'editing' }, - applicationContext() { - return { - builder: this.builder, - page: this.page, - mode: this.mode, - element: this.element, - } - }, runtimeFormulaContext() { /** * This proxy allow the RuntimeFormulaContextClass to act like a regular object. diff --git a/web-frontend/modules/builder/pages/pageEditor.vue b/web-frontend/modules/builder/pages/pageEditor.vue index 6af7c484f..936f08a14 100644 --- a/web-frontend/modules/builder/pages/pageEditor.vue +++ b/web-frontend/modules/builder/pages/pageEditor.vue @@ -37,6 +37,7 @@ export default { page: this.page, mode, formulaComponent: ApplicationBuilderFormulaInputGroup, + applicationContext: this.applicationContext, } }, /** diff --git a/web-frontend/modules/builder/pages/publicPage.vue b/web-frontend/modules/builder/pages/publicPage.vue index c2c6919f4..347ed5d68 100644 --- a/web-frontend/modules/builder/pages/publicPage.vue +++ b/web-frontend/modules/builder/pages/publicPage.vue @@ -34,6 +34,7 @@ export default { page: this.page, mode: this.mode, formulaComponent: ApplicationBuilderFormulaInputGroup, + applicationContext: this.applicationContext, } }, async asyncData({ store, params, error, $registry, app, req, route }) { diff --git a/web-frontend/modules/builder/store/dataSource.js b/web-frontend/modules/builder/store/dataSource.js index 6e7f5aa14..1594bae77 100644 --- a/web-frontend/modules/builder/store/dataSource.js +++ b/web-frontend/modules/builder/store/dataSource.js @@ -254,6 +254,20 @@ const actions = { }) throw error } + // After deleting the data source, find all collection elements + // which use this data source, and clear their element content. + const dataSourceCollectionElements = page.elements.filter((element) => { + return element.data_source_id === dataSourceToDelete.id + }) + dataSourceCollectionElements.map(async (collectionElement) => { + await dispatch( + 'elementContent/clearElementContent', + { + element: collectionElement, + }, + { root: true } + ) + }) commit('SET_LOADING', { page, value: false }) }, async fetch({ dispatch, commit }, { page }) { diff --git a/web-frontend/modules/builder/store/element.js b/web-frontend/modules/builder/store/element.js index 546b160a0..00a517d4b 100644 --- a/web-frontend/modules/builder/store/element.js +++ b/web-frontend/modules/builder/store/element.js @@ -60,7 +60,7 @@ const orderElements = (elements, parentElementId = null) => { const updateCachedValues = (page) => { page.orderedElements = orderElements(page.elements) page.elementMap = Object.fromEntries( - page.orderedElements.map((element) => [`${element.id}`, element]) + page.elements.map((element) => [`${element.id}`, element]) ) } @@ -120,11 +120,7 @@ const actions = { elementType.afterUpdate(element, page) }, forceDelete({ commit, getters }, { page, elementId }) { - const elementsOfPage = getters.getElementsOrdered(page) - const elementIndex = elementsOfPage.findIndex( - (element) => element.id === elementId - ) - const elementToDelete = elementsOfPage[elementIndex] + const elementToDelete = getters.getElementById(page, elementId) if (getters.getSelected?.id === elementId) { commit('SELECT_ITEM', { element: null }) @@ -276,12 +272,7 @@ const actions = { }) }, async delete({ dispatch, getters }, { page, elementId }) { - const elementsOfPage = getters.getElementsOrdered(page) - const elementIndex = elementsOfPage.findIndex( - (element) => element.id === elementId - ) - const elementToDelete = elementsOfPage[elementIndex] - + const elementToDelete = getters.getElementById(page, elementId) const descendants = getters.getDescendants(page, elementToDelete) // First delete all children @@ -456,17 +447,25 @@ const getters = { getParent: (state, getters) => (page, element) => { return getters.getElementById(page, element?.parent_element_id) }, - getAncestors: (state, getters) => (page, element) => { - const getElementAncestors = (element) => { - const parentElement = getters.getParent(page, element) - if (parentElement) { - return [...getElementAncestors(parentElement), parentElement] - } else { - return [] + /** + * Given an element, return all its ancestors until we reach the root element. + * If `parentFirst` is `true` then we reverse the array of elements so that + * the element's immediate parent is first, otherwise the root element will be first. + */ + getAncestors: + (state, getters) => + (page, element, parentFirst = true) => { + const getElementAncestors = (element) => { + const parentElement = getters.getParent(page, element) + if (parentElement) { + return [...getElementAncestors(parentElement), parentElement] + } else { + return [] + } } - } - return getElementAncestors(element) - }, + const ancestors = getElementAncestors(element) + return parentFirst ? ancestors.reverse() : ancestors + }, getSiblings: (state, getters) => (page, element) => { return getters .getElementsOrdered(page) diff --git a/web-frontend/test/unit/builder/elementTypes.spec.js b/web-frontend/test/unit/builder/elementTypes.spec.js index 6ae9f8cdb..4dc6b5702 100644 --- a/web-frontend/test/unit/builder/elementTypes.spec.js +++ b/web-frontend/test/unit/builder/elementTypes.spec.js @@ -1,10 +1,11 @@ import { - ElementType, CheckboxElementType, DropdownElementType, + ElementType, InputTextElementType, } from '@baserow/modules/builder/elementTypes' import { TestApp } from '@baserow/test/helpers/testApp' +import _ from 'lodash' describe('elementTypes tests', () => { const testApp = new TestApp() @@ -346,4 +347,122 @@ describe('elementTypes tests', () => { expect(elementType.isValid(element, 'uk')).toBe(true) }) }) + + describe('elementType childElementTypesForbidden tests', () => { + test('FormContainerElementType forbids non-form elements as children.', () => { + const formContainerElementType = testApp + .getRegistry() + .get('element', 'form_container') + const nonFormElementTypes = Object.values( + testApp.getRegistry().getAll('element') + ) + .filter((elementType) => !elementType.isFormElement) + .map((elementType) => elementType.getType()) + + const forbiddenChildTypes = + formContainerElementType.childElementTypesForbidden.map((el) => + el.getType() + ) + expect(forbiddenChildTypes).toEqual(nonFormElementTypes) + }) + test('ColumnElementType forbids container elements as children.', () => { + const columnElementType = testApp.getRegistry().get('element', 'column') + const containerElementTypes = Object.values( + testApp.getRegistry().getAll('element') + ) + .filter((elementType) => elementType.isContainerElement) + .map((elementType) => elementType.getType()) + + const forbiddenChildTypes = + columnElementType.childElementTypesForbidden.map((el) => el.getType()) + expect(forbiddenChildTypes).toEqual(containerElementTypes) + }) + test('RepeatElementType forbids itself, form elements and the form container as children.', () => { + const repeatElementType = testApp.getRegistry().get('element', 'repeat') + + const formContainerElementType = testApp + .getRegistry() + .get('element', 'form_container') + + let expectedForbiddenChildTypes = [ + repeatElementType.type, + formContainerElementType.type, + ] + const formElementTypes = Object.values( + testApp.getRegistry().getAll('element') + ) + .filter((elementType) => elementType.isFormElement) + .map((elementType) => elementType.getType()) + + expectedForbiddenChildTypes = + expectedForbiddenChildTypes.concat(formElementTypes) + + const forbiddenChildTypes = + repeatElementType.childElementTypesForbidden.map((el) => el.getType()) + expect(forbiddenChildTypes.sort()).toEqual( + expectedForbiddenChildTypes.sort() + ) + }) + }) + + describe('elementType childElementTypes tests', () => { + test('childElementTypes called with element with no parent only restricts child types to its own requirements.', () => { + const page = { id: 1, name: 'Contact Us' } + const element = { id: 2, parent_element_id: null } + const formContainerElementType = testApp + .getRegistry() + .get('element', 'form_container') + const formElementTypes = Object.values( + testApp.getRegistry().getAll('element') + ) + .filter((elementType) => elementType.isFormElement) + .map((elementType) => elementType.getType()) + + const childElementTypes = formContainerElementType + .childElementTypes(page, element) + .map((el) => el.getType()) + expect(childElementTypes).toEqual(formElementTypes) + }) + test('childElementTypes called with element with parent restricts child types using all ancestor childElementTypes requirements.', () => { + const page = { id: 1, name: 'Contact Us' } + const parentElement = { + id: 1, + page_id: page.id, + parent_element_id: null, + type: 'repeat', + } + const element = { + id: 2, + page_id: page.id, + parent_element_id: parentElement.id, + type: 'column', + } + page.elementMap = { 1: parentElement, 2: element } + + const allElementTypes = Object.values( + testApp.getRegistry().getAll('element') + ).map((el) => el.getType()) + + const columnElementType = testApp.getRegistry().get('element', 'column') + const forbiddenColumnChildTypes = + columnElementType.childElementTypesForbidden.map((el) => el.getType()) + + const repeatElementType = testApp.getRegistry().get('element', 'repeat') + const forbiddenRepeatChildTypes = + repeatElementType.childElementTypesForbidden.map((el) => el.getType()) + + const allExpectedForbiddenChildTypes = forbiddenColumnChildTypes.concat( + forbiddenRepeatChildTypes + ) + const expectedAllowedChildTypes = _.difference( + allElementTypes, + allExpectedForbiddenChildTypes + ) + + const childElementTypes = columnElementType + .childElementTypes(page, element) + .map((el) => el.getType()) + expect(childElementTypes).toEqual(expectedAllowedChildTypes) + }) + }) })