This is an automated email from the ASF dual-hosted git repository. michaelsmolina pushed a commit to branch 5.0 in repository https://gitbox.apache.org/repos/asf/superset.git
commit f27bf9ea6498e42026d51a4c77abea7d1901bdc2 Author: Vitor Avila <[email protected]> AuthorDate: Fri Mar 28 13:48:49 2025 -0300 fix(Jinja): Emit time grain to table charts even if they don't have a temporal column (#32871) (cherry picked from commit ab22bb18787b09f3b3e342ac496d105fb02fd04f) --- .../plugins/plugin-chart-table/src/buildQuery.ts | 5 - .../plugin-chart-table/test/buildQuery.test.ts | 9 -- tests/integration_tests/charts/api_tests.py | 58 ----------- tests/integration_tests/charts/data/api_tests.py | 112 +++++++++++++++------ 4 files changed, 80 insertions(+), 104 deletions(-) diff --git a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts index 7068ab1193..5b9cb684ed 100644 --- a/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-table/src/buildQuery.ts @@ -198,11 +198,6 @@ const buildQuery: BuildQuery<TableChartFormData> = ( (ownState.currentPage ?? 0) * (ownState.pageSize ?? 0); } - if (!temporalColumn) { - // This query is not using temporal column, so it doesn't need time grain - extras.time_grain_sqla = undefined; - } - let queryObject = { ...baseQueryObject, columns, diff --git a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts index f110b424c9..4badcc673a 100644 --- a/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts +++ b/superset-frontend/plugins/plugin-chart-table/test/buildQuery.test.ts @@ -148,14 +148,5 @@ describe('plugin-chart-table', () => { expect(queries[1].extras?.time_grain_sqla).toEqual(TimeGranularity.MONTH); expect(queries[1].extras?.where).toEqual("(status IN ('In Process'))"); }); - it('should not include time_grain_sqla in extras if temporal colum is not used and keep the rest', () => { - const { queries } = buildQuery(extraQueryFormData); - // Extras in regular query - expect(queries[0].extras?.time_grain_sqla).toBeUndefined(); - expect(queries[0].extras?.where).toEqual("(status IN ('In Process'))"); - // Extras in summary query - expect(queries[1].extras?.time_grain_sqla).toBeUndefined(); - expect(queries[1].extras?.where).toEqual("(status IN ('In Process'))"); - }); }); }); diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py index 5fd27bed0f..d4e8f01993 100644 --- a/tests/integration_tests/charts/api_tests.py +++ b/tests/integration_tests/charts/api_tests.py @@ -14,7 +14,6 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -"""Unit tests for Superset""" from io import BytesIO from unittest import mock @@ -24,7 +23,6 @@ from zipfile import is_zipfile, ZipFile import prison import pytest import yaml -from flask import g from flask_babel import lazy_gettext as _ from parameterized import parameterized from sqlalchemy import and_ @@ -63,7 +61,6 @@ from tests.integration_tests.fixtures.importexport import ( dataset_config, dataset_metadata_config, ) -from tests.integration_tests.fixtures.query_context import get_query_context from tests.integration_tests.fixtures.tags import ( create_custom_tags, # noqa: F401 get_filter_params, @@ -80,7 +77,6 @@ from tests.integration_tests.insert_chart_mixin import InsertChartMixin from tests.integration_tests.test_app import app from tests.integration_tests.utils.get_dashboards import get_dashboards_ids -CHART_DATA_URI = "api/v1/chart/data" CHARTS_FIXTURE_COUNT = 10 @@ -2330,57 +2326,3 @@ class TestChartApi(ApiOwnersTestCaseMixin, InsertChartMixin, SupersetTestCase): security_manager.add_permission_role(alpha_role, write_tags_perm) security_manager.add_permission_role(alpha_role, tag_charts_perm) - - @patch("superset.security.manager.SupersetSecurityManager.has_guest_access") - @patch("superset.security.manager.SupersetSecurityManager.is_guest_user") - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") - def test_get_chart_data_as_guest_user( - self, is_guest_user, has_guest_access - ): # get_guest_rls_filters - """ - Chart API: Test create simple chart - """ - self.login(ADMIN_USERNAME) - g.user.rls = [] - is_guest_user.return_value = True - has_guest_access.return_value = True - - with mock.patch.object(Slice, "get_query_context") as mock_get_query_context: - mock_get_query_context.return_value = get_query_context("birth_names") - rv = self.client.post( - "api/v1/chart/data", # noqa: F541 - json={ - "datasource": {"id": 2, "type": "table"}, - "queries": [ - { - "extras": {"where": "", "time_grain_sqla": "P1D"}, - "columns": ["name"], - "metrics": [{"label": "sum__num"}], - "orderby": [("sum__num", False)], - "row_limit": 100, - "granularity": "ds", - "time_range": "100 years ago : now", - "timeseries_limit": 0, - "timeseries_limit_metric": None, - "order_desc": True, - "filters": [ - {"col": "gender", "op": "==", "val": "boy"}, - {"col": "num", "op": "IS NOT NULL"}, - { - "col": "name", - "op": "NOT IN", - "val": ["<NULL>", '"abc"'], - }, - ], - "having": "", - "where": "", - } - ], - "result_format": "json", - "result_type": "full", - }, - ) - data = json.loads(rv.data.decode("utf-8")) - result = data["result"] - excluded_key = "query" - assert all([excluded_key not in query for query in result]) # noqa: C419 diff --git a/tests/integration_tests/charts/data/api_tests.py b/tests/integration_tests/charts/data/api_tests.py index dd9480c124..fa5e7b44ba 100644 --- a/tests/integration_tests/charts/data/api_tests.py +++ b/tests/integration_tests/charts/data/api_tests.py @@ -14,25 +14,45 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -# isort:skip_file -"""Unit tests for Superset""" -import unittest import copy +import time +import unittest from datetime import datetime from io import BytesIO -import time from typing import Any, Optional from unittest import mock from zipfile import ZipFile -from flask import Response +import pytest +from flask import g, Response from flask.ctx import AppContext -from tests.integration_tests.conftest import with_feature_flags + from superset.charts.data.api import ChartDataRestApi +from superset.commands.chart.data.get_data_command import ChartDataCommand +from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType +from superset.connectors.sqla.models import SqlaTable, TableColumn +from superset.errors import SupersetErrorType +from superset.extensions import async_query_manager_factory, db +from superset.models.annotations import AnnotationLayer +from superset.models.slice import Slice from superset.models.sql_lab import Query +from superset.superset_typing import AdhocColumn +from superset.utils import json +from superset.utils.core import ( + AdhocMetricExpressionType, + AnnotationType, + backend, + ExtraFiltersReasonType, + get_example_default_schema, +) +from superset.utils.database import get_example_database, get_main_database +from tests.common.query_context_generator import ANNOTATION_LAYERS +from tests.integration_tests.annotation_layers.fixtures import ( + create_annotation_layers, # noqa: F401 +) from tests.integration_tests.base_tests import SupersetTestCase, test_client -from tests.integration_tests.annotation_layers.fixtures import create_annotation_layers # noqa: F401 +from tests.integration_tests.conftest import with_feature_flags from tests.integration_tests.constants import ( ADMIN_USERNAME, GAMMA_NO_CSV_USERNAME, @@ -42,37 +62,13 @@ from tests.integration_tests.fixtures.birth_names_dashboard import ( load_birth_names_dashboard_with_slices, # noqa: F401 load_birth_names_data, # noqa: F401 ) -from tests.integration_tests.test_app import app from tests.integration_tests.fixtures.energy_dashboard import ( - load_energy_table_with_slice, # noqa: F401 load_energy_table_data, # noqa: F401 + load_energy_table_with_slice, # noqa: F401 ) -import pytest -from superset.models.slice import Slice - -from superset.commands.chart.data.get_data_command import ChartDataCommand -from superset.connectors.sqla.models import TableColumn, SqlaTable -from superset.errors import SupersetErrorType -from superset.extensions import async_query_manager_factory, db -from superset.models.annotations import AnnotationLayer -from superset.superset_typing import AdhocColumn -from superset.utils.core import ( - AnnotationType, - backend, - get_example_default_schema, - AdhocMetricExpressionType, - ExtraFiltersReasonType, -) -from superset.utils import json -from superset.utils.database import get_example_database, get_main_database -from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType - -from tests.common.query_context_generator import ANNOTATION_LAYERS from tests.integration_tests.fixtures.query_context import get_query_context - from tests.integration_tests.test_app import app # noqa: F811 - CHART_DATA_URI = "api/v1/chart/data" CHARTS_FIXTURE_COUNT = 10 ADHOC_COLUMN_FIXTURE: AdhocColumn = { @@ -1286,6 +1282,58 @@ class TestGetChartDataApi(BaseTestChartDataApi): } ] + @mock.patch("superset.security.manager.SupersetSecurityManager.has_guest_access") + @mock.patch("superset.security.manager.SupersetSecurityManager.is_guest_user") + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_chart_data_as_guest_user(self, is_guest_user, has_guest_access): + """ + Chart data API: Test response does not inlcude the SQL query for embedded + users. + """ + g.user.rls = [] + is_guest_user.return_value = True + has_guest_access.return_value = True + + rv = self.client.post(CHART_DATA_URI, json=self.query_context_payload) + data = json.loads(rv.data.decode("utf-8")) + result = data["result"] + excluded_key = "query" + assert all([excluded_key not in query for query in result]) # noqa: C419 + + def test_chart_data_table_chart_with_time_grain_filter(self): + """ + Chart data API: Test that a table chart that's not using a temporal column can + still receive a time grain filter (for Jinja purposes). + """ + metric_def = { + "aggregate": None, + "column": None, + "datasourceWarning": False, + "expressionType": "SQL", + "hasCustomLabel": True, + "label": "test", + "optionName": "metric_1eef4v0fryc_m7tm09g1hu", + "sqlExpression": "'{{ time_grain }}'", + } + self.query_context_payload["queries"][0]["columns"] = [] + self.query_context_payload["queries"][0]["metrics"] = [metric_def] + self.query_context_payload["queries"][0]["row_limit"] = 1 + self.query_context_payload["queries"][0]["extras"] = { + "where": "", + "having": "", + "time_grain_sqla": "PT5M", + } + self.query_context_payload["queries"][0]["orderby"] = [[metric_def, True]] + del self.query_context_payload["queries"][0]["granularity"] + del self.query_context_payload["queries"][0]["time_range"] + self.query_context_payload["queries"][0]["filters"] = [] + + rv = self.client.post(CHART_DATA_URI, json=self.query_context_payload) + data = json.loads(rv.data.decode("utf-8")) + result = data["result"][0] + assert "PT5M" in result["query"] + assert result["data"] == [{"test": "PT5M"}] + @pytest.fixture def physical_query_context(physical_dataset) -> dict[str, Any]:
