diff --git a/pyproject.toml b/pyproject.toml index 792de0fa9db..8de6216d331 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,7 +45,7 @@ dependencies = [ "cryptography>=42.0.4, <45.0.0", "deprecation>=2.1.0, <2.2.0", "flask>=2.2.5, <3.0.0", - "flask-appbuilder>=4.7.0, <5.0.0", + "flask-appbuilder>=4.8.0, <5.0.0", "flask-caching>=2.1.0, <3", "flask-compress>=1.13, <2.0", "flask-talisman>=1.0.0, <2.0", diff --git a/requirements/base.txt b/requirements/base.txt index 0fcaca9592e..fd2d44b0404 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -112,7 +112,7 @@ flask==2.3.3 # flask-session # flask-sqlalchemy # flask-wtf -flask-appbuilder==4.7.0 +flask-appbuilder==4.8.0 # via apache-superset (pyproject.toml) flask-babel==2.0.0 # via flask-appbuilder @@ -154,6 +154,7 @@ greenlet==3.1.1 # via # apache-superset (pyproject.toml) # shillelagh + # sqlalchemy gunicorn==23.0.0 # via apache-superset (pyproject.toml) h11==0.16.0 diff --git a/requirements/development.txt b/requirements/development.txt index e09b972e68c..76da8f463d3 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -151,7 +151,7 @@ flask==2.3.3 # flask-sqlalchemy # flask-testing # flask-wtf -flask-appbuilder==4.7.0 +flask-appbuilder==4.8.0 # via # apache-superset (pyproject.toml) # apache-superset @@ -257,6 +257,7 @@ greenlet==3.1.1 # apache-superset # gevent # shillelagh + # sqlalchemy grpcio==1.71.0 # via # apache-superset diff --git a/requirements/translations.txt b/requirements/translations.txt index a767b0baf95..88513034d04 100644 --- a/requirements/translations.txt +++ b/requirements/translations.txt @@ -109,7 +109,7 @@ flask==2.3.3 # flask-session # flask-sqlalchemy # flask-wtf -flask-appbuilder==4.7.0 +flask-appbuilder==4.8.0 # via apache-superset (pyproject.toml) flask-babel==2.0.0 # via flask-appbuilder @@ -149,6 +149,7 @@ greenlet==3.1.1 # via # apache-superset (pyproject.toml) # shillelagh + # sqlalchemy gunicorn==23.0.0 # via apache-superset (pyproject.toml) h11==0.16.0 diff --git a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx index cbe8f925402..61e4d6c7729 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/PropertiesModal.test.tsx @@ -17,7 +17,6 @@ * under the License. */ -import { VizType } from '@superset-ui/core'; import { render, screen, @@ -25,8 +24,14 @@ import { waitFor, } from 'spec/helpers/testing-library'; import fetchMock from 'fetch-mock'; +import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; import PropertiesModal, { PropertiesModalProps } from '.'; +jest.mock('@superset-ui/core', () => ({ + ...jest.requireActual('@superset-ui/core'), + isFeatureEnabled: jest.fn(), +})); + const createProps = () => ({ slice: { @@ -44,59 +49,51 @@ const createProps = () => addSuccessToast: jest.fn(), }) as PropertiesModalProps; -fetchMock.get('glob:*/api/v1/chart/318', { +fetchMock.get('glob:*/api/v1/chart/318*', { body: { description_columns: {}, id: 318, label_columns: { - cache_timeout: 'Cache Timeout', - 'dashboards.dashboard_title': 'Dashboards Dashboard Title', - 'dashboards.id': 'Dashboards Id', - description: 'Description', 'owners.first_name': 'Owners First Name', 'owners.id': 'Owners Id', 'owners.last_name': 'Owners Last Name', - 'owners.username': 'Owners Username', - params: 'Params', - slice_name: 'Slice Name', - viz_type: 'Viz Type', + 'tags.id': 'Tags Id', + 'tags.name': 'Tags Name', + 'tags.type': 'Tags Type', }, result: { - cache_timeout: null, - certified_by: 'John Doe', - certification_details: 'Sample certification', - dashboards: [ - { - dashboard_title: 'FCC New Coder Survey 2018', - id: 23, - }, - ], - description: null, owners: [ { first_name: 'Superset', id: 1, last_name: 'Admin', - username: 'admin', }, ], - params: - '{"adhoc_filters": [], "all_columns_x": ["age"], "color_scheme": "supersetColors", "datasource": "42__table", "granularity_sqla": "time_start", "groupby": null, "label_colors": {}, "link_length": "25", "queryFields": {"groupby": "groupby"}, "row_limit": 10000, "slice_id": 1380, "time_range": "No filter", "url_params": {}, "viz_type": "histogram", "x_axis_label": "age", "y_axis_label": "count"}', - slice_name: 'Age distribution of respondents', - viz_type: VizType.Histogram, + tags: [ + { + id: 1, + name: 'type:chart', + type: 2, + }, + { + id: 2, + name: 'owner:1', + type: 3, + }, + { + id: 3, + name: 'my test tag', + type: 1, + }, + ], }, show_columns: [ - 'cache_timeout', - 'dashboards.dashboard_title', - 'dashboards.id', - 'description', - 'owners.first_name', 'owners.id', + 'owners.first_name', 'owners.last_name', - 'owners.username', - 'params', - 'slice_name', - 'viz_type', + 'tags.id', + 'tags.name', + 'tags.type', ], show_title: 'Show Slice', }, @@ -422,3 +419,32 @@ test('"Certification details" should not be empty when saved', async () => { ); }); }); + +test('Should display only custom tags when tagging system is enabled', async () => { + const mockIsFeatureEnabled = isFeatureEnabled as jest.MockedFunction< + typeof isFeatureEnabled + >; + mockIsFeatureEnabled.mockImplementation( + flag => flag === FeatureFlag.TaggingSystem, + ); + + const props = createProps(); + renderModal(props); + + await waitFor(async () => { + expect( + await screen.findByRole('heading', { name: 'Tags' }), + ).toBeInTheDocument(); + expect( + await screen.findByRole('combobox', { name: 'Tags' }), + ).toBeInTheDocument(); + }); + + await waitFor(async () => { + expect(await screen.findByText('my test tag')).toBeInTheDocument(); + expect(screen.queryByText('type:chart')).not.toBeInTheDocument(); + expect(screen.queryByText('owner:1')).not.toBeInTheDocument(); + }); + + mockIsFeatureEnabled.mockRestore(); +}); diff --git a/superset-frontend/src/explore/components/PropertiesModal/index.tsx b/superset-frontend/src/explore/components/PropertiesModal/index.tsx index e66bd770254..63d7824c909 100644 --- a/superset-frontend/src/explore/components/PropertiesModal/index.tsx +++ b/superset-frontend/src/explore/components/PropertiesModal/index.tsx @@ -47,7 +47,6 @@ import Chart, { Slice } from 'src/types/Chart'; import withToasts from 'src/components/MessageToasts/withToasts'; import { type TagType } from 'src/components'; import { loadTags } from 'src/components/Tag/utils'; -import { fetchTags, OBJECT_TYPES } from 'src/features/tags/tags'; export type PropertiesModalProps = { slice: Slice; @@ -101,11 +100,21 @@ function PropertiesModal({ }); } - const fetchChartOwners = useCallback( - async function fetchChartOwners() { + const fetchChartProperties = useCallback( + async function fetchChartProperties() { + const queryParams = rison.encode({ + select_columns: [ + 'owners.id', + 'owners.first_name', + 'owners.last_name', + 'tags.id', + 'tags.name', + 'tags.type', + ], + }); try { const response = await SupersetClient.get({ - endpoint: `/api/v1/chart/${slice.slice_id}`, + endpoint: `/api/v1/chart/${slice.slice_id}?q=${queryParams}`, }); const chart = response.json.result; setSelectedOwners( @@ -114,6 +123,12 @@ function PropertiesModal({ label: `${owner.first_name} ${owner.last_name}`, })), ); + if (isFeatureEnabled(FeatureFlag.TaggingSystem)) { + const customTags = chart.tags?.filter( + (tag: TagType) => tag.type === 1, + ); + setTags(customTags); + } } catch (response) { const clientError = await getClientErrorObject(response); showError(clientError); @@ -206,33 +221,14 @@ function PropertiesModal({ // get the owners of this slice useEffect(() => { - fetchChartOwners(); - }, [fetchChartOwners]); + fetchChartProperties(); + }, [slice.slice_id]); // update name after it's changed in another modal useEffect(() => { setName(slice.slice_name || ''); }, [slice.slice_name]); - useEffect(() => { - if (!isFeatureEnabled(FeatureFlag.TaggingSystem)) return; - try { - fetchTags( - { - objectType: OBJECT_TYPES.CHART, - objectId: slice.slice_id, - includeTypes: false, - }, - (tags: TagType[]) => setTags(tags), - error => { - showError(error); - }, - ); - } catch (error) { - showError(error); - } - }, [slice.slice_id]); - const handleChangeTags = (tags: { label: string; value: number }[]) => { const parsedTags: TagType[] = ensureIsArray(tags).map(r => ({ id: r.value,