From 0e75bd79e0b9121915d08d15454a687fa6416968 Mon Sep 17 00:00:00 2001 From: Tsering Paljor Date: Thu, 26 Mar 2026 15:51:38 +0700 Subject: [PATCH 1/2] fix: Ensure broken list rows data source formula doesn't cause a crash (#5041) * Ensure broken list rows data source formula doesn't cause a crash. * List rows service type should return all fields when rest is empty * Empty rest handling should apply to get and list * Refactor existing test * Add new tests * Refactor service_type.extract_properties() to always expect a service arg. * Fix tests --- .../data_providers/data_provider_types.py | 27 +++++++-- .../integrations/core/service_types.py | 4 +- .../local_baserow/service_types.py | 24 ++++++-- .../src/baserow/core/services/registries.py | 4 +- .../test_data_provider_types.py | 59 ++++++++++++++++--- .../test_core_http_request_service_type.py | 8 ++- .../test_get_row_service_type.py | 23 +++++++- .../test_list_rows_service_type.py | 23 +++++++- .../test_upsert_row_service_type.py | 5 +- .../baserow/core/service/test_service_type.py | 4 +- ...ken_list_rows_data_source_could_cause.json | 9 +++ 11 files changed, 158 insertions(+), 32 deletions(-) create mode 100644 changelog/entries/unreleased/bug/fixed_a_bug_where_a_broken_list_rows_data_source_could_cause.json 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 59ca6515e4..7a40b0a607 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 @@ -246,10 +246,15 @@ def extract_properties(self, path: List[str], **kwargs) -> Dict[str, List[str]]: service_type = data_source.service.specific.get_type() if service_type.returns_list: - # We remove the row id from the path - _, *rest = rest + if rest: + # We remove the row id from the path + _, *rest = rest - return {data_source.service_id: service_type.extract_properties(rest, **kwargs)} + return { + data_source.service_id: service_type.extract_properties( + data_source.service.specific, rest, **kwargs + ) + } class DataSourceContextDataProviderType(BuilderDataProviderType): @@ -328,7 +333,11 @@ def extract_properties(self, path: List[str], **kwargs) -> Dict[str, List[str]]: service_type = data_source.service.specific.get_type() - return {data_source.service_id: service_type.extract_properties(rest, **kwargs)} + return { + data_source.service_id: service_type.extract_properties( + data_source.service.specific, rest, **kwargs + ) + } class CurrentRecordDataProviderType(BuilderDataProviderType): @@ -445,7 +454,11 @@ def extract_properties( else: path = [schema_property, *path] - return {data_source.service_id: service_type.extract_properties(path, **kwargs)} + return { + data_source.service_id: service_type.extract_properties( + data_source.service.specific, path, **kwargs + ) + } class PreviousActionProviderType(BuilderDataProviderType): @@ -584,7 +597,9 @@ def extract_properties( service_type = previous_action.service.specific.get_type() return { - previous_action.service.id: service_type.extract_properties(rest, **kwargs) + previous_action.service.id: service_type.extract_properties( + previous_action.service.specific, rest, **kwargs + ) } diff --git a/backend/src/baserow/contrib/integrations/core/service_types.py b/backend/src/baserow/contrib/integrations/core/service_types.py index 566f9251fd..c296a3e4fc 100644 --- a/backend/src/baserow/contrib/integrations/core/service_types.py +++ b/backend/src/baserow/contrib/integrations/core/service_types.py @@ -282,7 +282,9 @@ def formula_generator( query_param.value = new_formula yield query_param - def extract_properties(self, path: List[str], **kwargs) -> List[str]: + def extract_properties( + self, service: Service, path: List[str], **kwargs + ) -> List[str]: """Returns the first path element if any""" if path: diff --git a/backend/src/baserow/contrib/integrations/local_baserow/service_types.py b/backend/src/baserow/contrib/integrations/local_baserow/service_types.py index d3d1aef37b..116e00a33e 100644 --- a/backend/src/baserow/contrib/integrations/local_baserow/service_types.py +++ b/backend/src/baserow/contrib/integrations/local_baserow/service_types.py @@ -374,7 +374,9 @@ def import_property_name( return f"field_{new_field_id}" if new_field_id else None return property_name - def extract_properties(self, path: List[str], **kwargs) -> List[str]: + def extract_properties( + self, service: Service, path: List[str], **kwargs + ) -> List[str]: """ Given a list of formula path parts, call the ServiceType's extract_properties() method and return a set of unique field IDs. @@ -387,11 +389,11 @@ def extract_properties(self, path: List[str], **kwargs) -> List[str]: The path can contain one or more parts, depending on the field type and the formula. Some examples of `path` are: - An element that specifies a specific a field: - ['field_5439'] + An element that specifies a specific field: + ['field_5439'] - An element that uses a Link Row Field formula - ['field_5569', '0', 'value'] + An element that uses a Link Row Field formula: + ['field_5569', '0', 'value'] """ # If the path length is greater or equal to 1, then we have @@ -400,6 +402,14 @@ def extract_properties(self, path: List[str], **kwargs) -> List[str]: if len(path) >= 1: field_dbname, *rest = path else: + # When path is empty, e.g. `get('data_source.606')`, we should + # return all fields since we don't know which specific fields + # are needed. + if field_objects := self.get_table_field_objects(service): + return ["id"] + [ + field_object["field"].db_column for field_object in field_objects + ] + # In any other scenario, we have a formula that is not a format we # can currently parse properly, so we return an empty list. return [] @@ -1536,7 +1546,9 @@ def dispatch_transform( return DispatchResult(data={"result": result}) - def extract_properties(self, path: List[str], **kwargs) -> List[str]: + def extract_properties( + self, service: Service, path: List[str], **kwargs + ) -> List[str]: """ Returns the usual properties for this service type. """ diff --git a/backend/src/baserow/core/services/registries.py b/backend/src/baserow/core/services/registries.py index 419e8e4c61..765de4c367 100644 --- a/backend/src/baserow/core/services/registries.py +++ b/backend/src/baserow/core/services/registries.py @@ -508,7 +508,9 @@ def import_serialized( return created_instance - def extract_properties(self, path: List[str], **kwargs) -> List[str]: + def extract_properties( + self, service: Service, path: List[str], **kwargs + ) -> List[str]: return [] def import_property_name( 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 71c4b69977..4a103f808c 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 @@ -1517,7 +1517,9 @@ def test_data_source_data_extract_properties_calls_correct_service_type( assert result == {mocked_data_source.service_id: expected} mocked_get_data_source.assert_called_once_with(int(data_source_id), with_cache=True) - mocked_service_type.extract_properties.assert_called_once_with([expected]) + mocked_service_type.extract_properties.assert_called_once_with( + mocked_data_source.service.specific, [expected] + ) mocked_service_type.returns_list = True mocked_service_type.extract_properties.reset_mock() @@ -1525,10 +1527,43 @@ def test_data_source_data_extract_properties_calls_correct_service_type( result = DataSourceDataProviderType().extract_properties( [data_source_id, "1", expected] ) - mocked_service_type.extract_properties.assert_called_once_with([expected]) + mocked_service_type.extract_properties.assert_called_once_with( + mocked_data_source.service.specific, [expected] + ) assert result == {mocked_data_source.service_id: expected} +@patch.object(DataSourceHandler, "get_data_source") +@pytest.mark.django_db +def test_data_source_data_extract_properties_returns_all_fields_when_no_row_id_or_field_name( + mocked_get_data_source, +): + """ + Test the DataSourceDataProviderType::extract_properties() method. + + Ensure that when there is no row ID or field name, all fields are returned. + """ + + mocked_service_type = MagicMock() + mocked_service_type.extract_properties.return_value = ["id", "field_123"] + mocked_data_source = MagicMock() + mocked_data_source.service.specific.get_type = MagicMock( + return_value=mocked_service_type + ) + mocked_get_data_source.return_value = mocked_data_source + + data_source_id = "1" + # mock a path without any row_id or field name + path = [data_source_id] + result = DataSourceDataProviderType().extract_properties(path) + + assert result == {mocked_data_source.service_id: ["id", "field_123"]} + mocked_get_data_source.assert_called_once_with(int(data_source_id), with_cache=True) + mocked_service_type.extract_properties.assert_called_once_with( + mocked_data_source.service.specific, [] + ) + + @pytest.mark.django_db def test_data_source_data_extract_properties_returns_expected_results(data_fixture): """ @@ -1623,7 +1658,9 @@ def test_data_source_context_extract_properties_calls_correct_service_type( assert result == {mocked_data_source.service_id: expected} mocked_get_data_source.assert_called_once_with(int(data_source_id), with_cache=True) - mocked_service_type.extract_properties.assert_called_once_with([expected]) + mocked_service_type.extract_properties.assert_called_once_with( + mocked_data_source.service.specific, [expected] + ) mocked_service_type.returns_list = True mocked_service_type.extract_properties.reset_mock() @@ -1633,7 +1670,9 @@ def test_data_source_context_extract_properties_calls_correct_service_type( [data_source_id, expected] ) - mocked_service_type.extract_properties.assert_called_once_with([expected]) + mocked_service_type.extract_properties.assert_called_once_with( + mocked_data_source.service.specific, [expected] + ) assert result == {mocked_data_source.service_id: expected} @@ -1822,7 +1861,9 @@ def test_current_record_extract_properties_calls_correct_service_type( assert result == {mocked_data_source.service_id: expected_field} mock_get_data_source.assert_called_once_with(fake_element_id, with_cache=True) - mocked_service_type.extract_properties.assert_called_once_with([expected_field]) + mocked_service_type.extract_properties.assert_called_once_with( + mocked_data_source.service.specific, [expected_field] + ) @pytest.mark.django_db @@ -1883,15 +1924,17 @@ def test_current_record_extract_properties_called_with_correct_path( if returns_list: if schema_property: mock_service_type.extract_properties.assert_called_once_with( - [schema_property, *path] + mock_data_source.service.specific, [schema_property, *path] ) else: - mock_service_type.extract_properties.assert_called_once_with(path) + mock_service_type.extract_properties.assert_called_once_with( + mock_data_source.service.specific, path + ) assert result == {service_id: ["field_999"]} else: if schema_property: mock_service_type.extract_properties.assert_called_once_with( - [schema_property, *path] + mock_data_source.service.specific, [schema_property, *path] ) assert result == {service_id: ["field_999"]} else: diff --git a/backend/tests/baserow/contrib/integrations/core/test_core_http_request_service_type.py b/backend/tests/baserow/contrib/integrations/core/test_core_http_request_service_type.py index a5f8ea1b58..6c98581e79 100644 --- a/backend/tests/baserow/contrib/integrations/core/test_core_http_request_service_type.py +++ b/backend/tests/baserow/contrib/integrations/core/test_core_http_request_service_type.py @@ -1,6 +1,6 @@ import json from contextlib import contextmanager -from unittest.mock import Mock, patch +from unittest.mock import MagicMock, Mock, patch import pytest from requests import exceptions as request_exceptions @@ -365,11 +365,13 @@ def test_core_http_request_formula_generator(): @pytest.mark.django_db def test_core_http_request_extract_properties(data_fixture): + mock_service = MagicMock() + assert CoreHTTPRequestServiceType().extract_properties( - ["headers", "content_type"] + mock_service, ["headers", "content_type"] ) == ["headers"] - assert CoreHTTPRequestServiceType().extract_properties([]) == [] + assert CoreHTTPRequestServiceType().extract_properties(mock_service, []) == [] @pytest.mark.django_db diff --git a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_get_row_service_type.py b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_get_row_service_type.py index e3e9745734..bdd985bcab 100644 --- a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_get_row_service_type.py +++ b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_get_row_service_type.py @@ -803,7 +803,7 @@ def test_dispatch_transform_passes_field_ids(mock_get_serializer, field_names): [ ( [], - [], + ["id"], ), ( ["foo"], @@ -836,7 +836,8 @@ def test_extract_properties(path, expected): service_type = LocalBaserowGetRowUserServiceType() - result = service_type.extract_properties(path) + mock_service = MagicMock() + result = service_type.extract_properties(mock_service, path) assert result == expected @@ -918,3 +919,21 @@ def test_can_dispatch_interesting_table(data_fixture): dispatch_context = FakeDispatchContext(public_allowed_properties=field_names) assert len(result.data.keys()) == 1 + 1 + + +@pytest.mark.django_db +def test_extract_properties_with_empty_path_returns_all_fields(data_fixture): + """ + When path is empty, extract_properties should return all field names. + """ + + user = data_fixture.create_user() + table = data_fixture.create_database_table(user=user) + field_1 = data_fixture.create_text_field(table=table, name="Fruit") + field_2 = data_fixture.create_number_field(table=table, name="Count") + service = data_fixture.create_local_baserow_get_row_service(table=table) + service_type = LocalBaserowGetRowUserServiceType() + + result = service_type.extract_properties(service, []) + + assert result == ["id", field_1.db_column, field_2.db_column] diff --git a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_type.py b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_type.py index f0d22afcfe..6dd8c14be3 100644 --- a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_type.py +++ b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_list_rows_service_type.py @@ -1156,7 +1156,7 @@ def test_dispatch_transform_passes_field_ids(mock_get_serializer, field_names): [ ( [], - [], + ["id"], ), ( ["foo"], @@ -1187,7 +1187,8 @@ def test_extract_properties(path, expected): service_type = LocalBaserowListRowsUserServiceType() - result = service_type.extract_properties(path) + mock_service = MagicMock() + result = service_type.extract_properties(mock_service, path) assert result == expected @@ -1250,3 +1251,21 @@ def test_search_on_multiple_select_with_list(data_fixture): ) assert len(dispatch_data["results"]) == 1 assert dispatch_data["results"][0]._primary_field_id == options[0].field_id + + +@pytest.mark.django_db +def test_extract_properties_with_empty_path_returns_all_fields(data_fixture): + """ + When path is empty, extract_properties should return all field names. + """ + + user = data_fixture.create_user() + table = data_fixture.create_database_table(user=user) + field_1 = data_fixture.create_text_field(table=table, name="Fruit") + field_2 = data_fixture.create_number_field(table=table, name="Count") + service = data_fixture.create_local_baserow_list_rows_service(table=table) + service_type = LocalBaserowListRowsUserServiceType() + + result = service_type.extract_properties(service, []) + + assert result == ["id", field_1.db_column, field_2.db_column] diff --git a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_upsert_row_service_type.py b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_upsert_row_service_type.py index 4d2fc33aab..7c4e593e15 100644 --- a/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_upsert_row_service_type.py +++ b/backend/tests/baserow/contrib/integrations/local_baserow/service_types/test_upsert_row_service_type.py @@ -809,7 +809,7 @@ def test_dispatch_transform_passes_field_ids( @pytest.mark.parametrize( "path,expected", [ - ([], []), + ([], ["id"]), ([None], []), ([""], []), (["foo"], []), @@ -830,7 +830,8 @@ def test_extract_properties_returns_expected_list(path, expected): service_type = LocalBaserowUpsertRowServiceType() - result = service_type.extract_properties(path) + mock_service = MagicMock() + result = service_type.extract_properties(mock_service, path) assert result == expected diff --git a/backend/tests/baserow/core/service/test_service_type.py b/backend/tests/baserow/core/service/test_service_type.py index 078fcf6268..8c9df7896b 100644 --- a/backend/tests/baserow/core/service/test_service_type.py +++ b/backend/tests/baserow/core/service/test_service_type.py @@ -193,7 +193,9 @@ def test_extract_properties(): service_type_cls.model_class = MagicMock() service_type = service_type_cls() - result = service_type.extract_properties(["foo"]) + mock_service = MagicMock() + result = service_type.extract_properties(mock_service, ["foo"]) + assert result == [] diff --git a/changelog/entries/unreleased/bug/fixed_a_bug_where_a_broken_list_rows_data_source_could_cause.json b/changelog/entries/unreleased/bug/fixed_a_bug_where_a_broken_list_rows_data_source_could_cause.json new file mode 100644 index 0000000000..7aa6959312 --- /dev/null +++ b/changelog/entries/unreleased/bug/fixed_a_bug_where_a_broken_list_rows_data_source_could_cause.json @@ -0,0 +1,9 @@ +{ + "type": "bug", + "message": "Fixed a bug where a broken List Rows data source could cause a crash.", + "issue_origin": "github", + "issue_number": null, + "domain": "builder", + "bullet_points": [], + "created_at": "2026-03-25" +} \ No newline at end of file From 16ceee386221eb82f2df321452b95a22bc35e20e Mon Sep 17 00:00:00 2001 From: Tsering Paljor Date: Thu, 26 Mar 2026 15:52:07 +0700 Subject: [PATCH 2/2] Fix template placeholders (#5050) --- ..._bot_forms_translations_to_use_the_correct_pl.json | 9 +++++++++ web-frontend/modules/integrations/locales/en.json | 11 ++++++----- 2 files changed, 15 insertions(+), 5 deletions(-) create mode 100644 changelog/entries/unreleased/bug/fixed_the_slack_bot_forms_translations_to_use_the_correct_pl.json diff --git a/changelog/entries/unreleased/bug/fixed_the_slack_bot_forms_translations_to_use_the_correct_pl.json b/changelog/entries/unreleased/bug/fixed_the_slack_bot_forms_translations_to_use_the_correct_pl.json new file mode 100644 index 0000000000..490d37d10b --- /dev/null +++ b/changelog/entries/unreleased/bug/fixed_the_slack_bot_forms_translations_to_use_the_correct_pl.json @@ -0,0 +1,9 @@ +{ + "type": "bug", + "message": "Fixed the Slack Bot Form's translations to use the correct placeholders.", + "issue_origin": "github", + "issue_number": null, + "domain": "integration", + "bullet_points": [], + "created_at": "2026-03-26" +} \ No newline at end of file diff --git a/web-frontend/modules/integrations/locales/en.json b/web-frontend/modules/integrations/locales/en.json index f860ec009a..b803d38b62 100644 --- a/web-frontend/modules/integrations/locales/en.json +++ b/web-frontend/modules/integrations/locales/en.json @@ -206,14 +206,15 @@ "supportDescription": "If you need any assistance with pairing with your Slack app, please refer to the steps below.", "supportSetupHeading": "1. Setting up the app", "supportSetupDescription": "Depending on your Slack workspace settings, you may be able to create a new Slack app. Otherwise, an admin will need to do this for you. If you are re-using an existing app which can write messages, skip to the section called 'Pairing with your Slack app'.", - "supportSetupStep1": "Navigate to your workspace's <a href=\"https://api.slack.com/apps\" target=\"_blank\">apps page</a>.", + "supportSetupStep1": "Navigate to your workspace's {link}.", + "supportSetupStep1Link": "apps page", "supportSetupStep2": "Create a new app, choose 'From scratch' and enter a name. Select the workspace your app should operate in, and click 'Create'.", - "supportSetupStep3": "In the left sidebar, navigate to 'OAuth & Permissions', scroll down to Scopes and under 'Bot Token Scopes', select 'Add an OAuth Scope'.", - "supportSetupStep4": "To allow your app to post messages, add the <pre>chat:write</pre> scope.", + "supportSetupStep3": "In the left sidebar, navigate to 'OAuth > Permissions', scroll down to Scopes and under 'Bot Token Scopes', select 'Add an OAuth Scope'.", + "supportSetupStep4": "To allow your app to post messages, add the {scope} scope.", "supportPairingHeading": "2. Pairing with your Slack app", - "supportPairingStep1": "If your app is new: navigate to the 'Settings' > 'Install App'. Click the green button to install the app to your workspace.", + "supportPairingStep1": "If your app is new: navigate to 'Settings' > 'Install App'. Click the green button to install the app to your workspace.", "supportPairingStep2": "Copy your 'Bot User OAuth Token' and store it in the 'Bot User Token' field in this form.", - "supportPairingStep3": "Finally, if your app is new: in Slack, invite your app to your chosen channel with <pre>/invite {'@'}yourAppName yourChannel</pre>" + "supportPairingStep3": "Finally, if your app is new: in Slack, invite your app to your chosen channel with {command}" }, "slackWriteMessageServiceForm": { "alertMessage": "This action must be paired with a Slack app. Please follow the guide in the integrations popup to get started.",