diff --git a/scripts/cypress_run.py b/scripts/cypress_run.py index 1154a3661dda3..e2bb5bb6982e7 100644 --- a/scripts/cypress_run.py +++ b/scripts/cypress_run.py @@ -54,6 +54,9 @@ def get_cypress_cmd( build_id = generate_build_id() browser = os.getenv("CYPRESS_BROWSER", "chrome") + # Add --disable-dev-shm-usage for Chrome browser + chrome_flags = "--disable-dev-shm-usage" + if use_dashboard: # Run using cypress.io service spec: str = "cypress/e2e/*/**/*" @@ -61,7 +64,8 @@ def get_cypress_cmd( f"{XVFB_PRE_CMD} " f'{cypress_cmd} --spec "{spec}" --browser {browser} ' f"--record --group {group} --tag {REPO},{GITHUB_EVENT_NAME} " - f"--parallel --ci-build-id {build_id}" + f"--parallel --ci-build-id {build_id} " + f"-- {chrome_flags}" ) else: # Run local, but split the execution @@ -73,6 +77,7 @@ def get_cypress_cmd( f"{XVFB_PRE_CMD} " f"{cypress_cmd} --browser {browser} " f'--spec "{spec_list_str}" ' + f"-- {chrome_flags}" ) return cmd diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/horizontalFilterBar.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/horizontalFilterBar.test.ts new file mode 100644 index 0000000000000..d9ed21258f20a --- /dev/null +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/horizontalFilterBar.test.ts @@ -0,0 +1,248 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { nativeFilters } from 'cypress/support/directories'; + +import { + addCountryNameFilter, + applyNativeFilterValueWithIndex, + enterNativeFilterEditModal, + inputNativeFilterDefaultValue, + saveNativeFilterSettings, + validateFilterNameOnDashboard, + testItems, + interceptFilterState, +} from './utils'; +import { + prepareDashboardFilters, + SAMPLE_CHART, + visitDashboard, +} from './shared_dashboard_functions'; + +function openMoreFilters(waitFilterState = true) { + interceptFilterState(); + cy.getBySel('dropdown-container-btn').click(); + + if (waitFilterState) { + cy.wait('@postFilterState'); + } +} + +function openVerticalFilterBar() { + cy.getBySel('dashboard-filters-panel').should('exist'); + cy.getBySel('filter-bar__expand-button').click(); +} + +function setFilterBarOrientation(orientation: 'vertical' | 'horizontal') { + cy.getBySel('filterbar-orientation-icon').click(); + cy.wait(250); + cy.getBySel('dropdown-selectable-icon-submenu') + .contains('Orientation of filter bar') + .should('exist') + .trigger('mouseover'); + + if (orientation === 'vertical') { + cy.get('.ant-dropdown-menu-item-selected') + .contains('Horizontal (Top)') + .should('exist'); + cy.get('.ant-dropdown-menu-item').contains('Vertical (Left)').click(); + cy.getBySel('dashboard-filters-panel').should('exist'); + } else { + cy.get('.ant-dropdown-menu-item-selected') + .contains('Vertical (Left)') + .should('exist'); + cy.get('.ant-dropdown-menu-item').contains('Horizontal (Top)').click(); + cy.getBySel('loading-indicator').should('exist'); + cy.getBySel('filter-bar').should('exist'); + cy.getBySel('dashboard-filters-panel').should('not.exist'); + } +} + +describe('Horizontal FilterBar', () => { + it('should go from vertical to horizontal and the opposite', () => { + visitDashboard(); + openVerticalFilterBar(); + setFilterBarOrientation('horizontal'); + setFilterBarOrientation('vertical'); + }); + + it('should show all default actions in horizontal mode', () => { + visitDashboard(); + openVerticalFilterBar(); + setFilterBarOrientation('horizontal'); + cy.getBySel('horizontal-filterbar-empty') + .contains('No filters are currently added to this dashboard.') + .should('exist'); + cy.getBySel('filter-bar__create-filter').should('exist'); + cy.getBySel('filterbar-action-buttons').should('exist'); + }); + + it('should stay in horizontal mode when reloading', () => { + visitDashboard(); + openVerticalFilterBar(); + setFilterBarOrientation('horizontal'); + cy.reload(); + cy.getBySel('dashboard-filters-panel').should('not.exist'); + }); + + it('should show all filters in available space on load', () => { + prepareDashboardFilters([ + { name: 'test_1', column: 'country_name', datasetId: 2 }, + { name: 'test_2', column: 'country_code', datasetId: 2 }, + { name: 'test_3', column: 'region', datasetId: 2 }, + ]); + setFilterBarOrientation('horizontal'); + cy.get('.filter-item-wrapper').should('have.length', 3); + }); + + it('should show "more filters" on window resizing up and down', () => { + prepareDashboardFilters([ + { name: 'test_1', column: 'country_name', datasetId: 2 }, + { name: 'test_2', column: 'country_code', datasetId: 2 }, + { name: 'test_3', column: 'region', datasetId: 2 }, + ]); + setFilterBarOrientation('horizontal'); + + cy.getBySel('form-item-value').should('have.length', 3); + cy.viewport(768, 1024); + cy.getBySel('form-item-value').should('have.length', 0); + openMoreFilters(false); + cy.getBySel('form-item-value').should('have.length', 3); + + cy.getBySel('filter-bar').click(); + cy.viewport(1000, 1024); + openMoreFilters(false); + cy.getBySel('form-item-value').should('have.length', 3); + + cy.getBySel('filter-bar').click(); + cy.viewport(1300, 1024); + cy.getBySel('form-item-value').should('have.length', 3); + cy.getBySel('dropdown-container-btn').should('not.exist'); + }); + + it('should show "more filters" and scroll', () => { + prepareDashboardFilters([ + { name: 'test_1', column: 'country_name', datasetId: 2 }, + { name: 'test_2', column: 'country_code', datasetId: 2 }, + { name: 'test_3', column: 'region', datasetId: 2 }, + { name: 'test_4', column: 'year', datasetId: 2 }, + { name: 'test_5', column: 'country_name', datasetId: 2 }, + { name: 'test_6', column: 'country_code', datasetId: 2 }, + { name: 'test_7', column: 'region', datasetId: 2 }, + { name: 'test_8', column: 'year', datasetId: 2 }, + { name: 'test_9', column: 'country_name', datasetId: 2 }, + { name: 'test_10', column: 'country_code', datasetId: 2 }, + { name: 'test_11', column: 'region', datasetId: 2 }, + { name: 'test_12', column: 'year', datasetId: 2 }, + ]); + setFilterBarOrientation('horizontal'); + cy.get('.filter-item-wrapper').should('have.length', 3); + openMoreFilters(); + cy.getBySel('form-item-value').should('have.length', 12); + cy.getBySel('filter-control-name').contains('test_10').should('be.visible'); + cy.getBySel('filter-control-name') + .contains('test_12') + .should('not.be.visible'); + cy.get('.ant-popover-inner-content').scrollTo('bottom'); + cy.getBySel('filter-control-name').contains('test_12').should('be.visible'); + }); + + it('should display newly added filter', () => { + visitDashboard(); + openVerticalFilterBar(); + setFilterBarOrientation('horizontal'); + + enterNativeFilterEditModal(false); + addCountryNameFilter(); + saveNativeFilterSettings([]); + validateFilterNameOnDashboard(testItems.topTenChart.filterColumn); + }); + + it('should spot changes in "more filters" and apply their values', () => { + cy.intercept(`/api/v1/chart/data?form_data=**`).as('chart'); + prepareDashboardFilters([ + { name: 'test_1', column: 'country_name', datasetId: 2 }, + { name: 'test_2', column: 'country_code', datasetId: 2 }, + { name: 'test_3', column: 'region', datasetId: 2 }, + { name: 'test_4', column: 'year', datasetId: 2 }, + { name: 'test_5', column: 'country_name', datasetId: 2 }, + { name: 'test_6', column: 'country_code', datasetId: 2 }, + { name: 'test_7', column: 'region', datasetId: 2 }, + { name: 'test_8', column: 'year', datasetId: 2 }, + { name: 'test_9', column: 'country_name', datasetId: 2 }, + { name: 'test_10', column: 'country_code', datasetId: 2 }, + { name: 'test_11', column: 'region', datasetId: 2 }, + { name: 'test_12', column: 'year', datasetId: 2 }, + ]); + setFilterBarOrientation('horizontal'); + openMoreFilters(); + applyNativeFilterValueWithIndex(8, testItems.filterDefaultValue); + cy.get(nativeFilters.applyFilter).click({ force: true }); + cy.wait('@chart'); + cy.get('.antd5-scroll-number.antd5-badge-count').should( + 'have.attr', + 'title', + '1', + ); + }); + + it('should focus filter and open "more filters" programmatically', () => { + prepareDashboardFilters([ + { name: 'test_1', column: 'country_name', datasetId: 2 }, + { name: 'test_2', column: 'country_code', datasetId: 2 }, + { name: 'test_3', column: 'region', datasetId: 2 }, + { name: 'test_4', column: 'year', datasetId: 2 }, + { name: 'test_5', column: 'country_name', datasetId: 2 }, + { name: 'test_6', column: 'country_code', datasetId: 2 }, + { name: 'test_7', column: 'region', datasetId: 2 }, + { name: 'test_8', column: 'year', datasetId: 2 }, + { name: 'test_9', column: 'country_name', datasetId: 2 }, + { name: 'test_10', column: 'country_code', datasetId: 2 }, + { name: 'test_11', column: 'region', datasetId: 2 }, + { name: 'test_12', column: 'year', datasetId: 2 }, + ]); + setFilterBarOrientation('horizontal'); + openMoreFilters(); + applyNativeFilterValueWithIndex(8, testItems.filterDefaultValue); + cy.get(nativeFilters.applyFilter).click({ force: true }); + cy.getBySel('slice-header').within(() => { + cy.get('.filter-counts').trigger('mouseover'); + }); + cy.get('.filterStatusPopover').contains('test_9').click(); + cy.getBySel('dropdown-content').should('be.visible'); + cy.get('.ant-select-focused').should('be.visible'); + }); + + it('should show tag count and one plain tag on focus and only count on blur in select ', () => { + prepareDashboardFilters([ + { name: 'test_1', column: 'country_name', datasetId: 2 }, + ]); + setFilterBarOrientation('horizontal'); + enterNativeFilterEditModal(); + inputNativeFilterDefaultValue('Albania'); + cy.get('.ant-select-selection-search-input').clear({ force: true }); + inputNativeFilterDefaultValue('Algeria', true); + saveNativeFilterSettings([SAMPLE_CHART]); + cy.getBySel('filter-bar').within(() => { + cy.get(nativeFilters.filterItem).contains('Albania').should('be.visible'); + cy.get(nativeFilters.filterItem).contains('+ 1 ...').should('be.visible'); + cy.get('.ant-select-selection-search-input').click(); + cy.get(nativeFilters.filterItem).contains('+ 2 ...').should('be.visible'); + }); + }); +}); diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts index 44b26be9ea67b..8a67841de12ae 100644 --- a/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/nativeFilters.test.ts @@ -23,7 +23,6 @@ import { exploreView, dataTestChartName, } from 'cypress/support/directories'; -import { SAMPLE_DASHBOARD_1 } from 'cypress/utils/urls'; import { addCountryNameFilter, @@ -48,138 +47,12 @@ import { validateFilterNameOnDashboard, testItems, WORLD_HEALTH_CHARTS, - interceptGet, - interceptCharts, - interceptDatasets, - interceptFilterState, } from './utils'; - -const SAMPLE_CHART = { name: 'Most Populated Countries', viz: 'table' }; - -function visitDashboard(createSample = true) { - interceptCharts(); - interceptGet(); - interceptDatasets(); - - if (createSample) { - cy.createSampleDashboards([0]); - } - - cy.visit(SAMPLE_DASHBOARD_1); - cy.wait('@get'); - cy.wait('@getCharts'); - cy.wait('@getDatasets'); - cy.url().should('contain', 'native_filters_key'); -} - -function prepareDashboardFilters( - filters: { name: string; column: string; datasetId: number }[], -) { - cy.createSampleDashboards([0]); - cy.request({ - method: 'GET', - url: `api/v1/dashboard/1-sample-dashboard`, - }).then(res => { - const { body } = res; - const dashboardId = body.result.id; - const allFilters: Record[] = []; - filters.forEach((f, i) => { - allFilters.push({ - id: `NATIVE_FILTER-fLH0pxFQ${i}`, - controlValues: { - enableEmptyFilter: false, - defaultToFirstItem: false, - multiSelect: true, - searchAllOptions: false, - inverseSelection: false, - }, - name: f.name, - filterType: 'filter_select', - targets: [ - { - datasetId: f.datasetId, - column: { name: f.column }, - }, - ], - defaultDataMask: { - extraFormData: {}, - filterState: {}, - ownState: {}, - }, - cascadeParentIds: [], - scope: { - rootPath: ['ROOT_ID'], - excluded: [], - }, - type: 'NATIVE_FILTER', - description: '', - chartsInScope: [5], - tabsInScope: [], - }); - }); - if (dashboardId) { - const jsonMetadata = { - native_filter_configuration: allFilters, - timed_refresh_immune_slices: [], - expanded_slices: {}, - refresh_frequency: 0, - color_scheme: '', - label_colors: {}, - shared_label_colors: {}, - color_scheme_domain: [], - cross_filters_enabled: false, - positions: { - DASHBOARD_VERSION_KEY: 'v2', - ROOT_ID: { type: 'ROOT', id: 'ROOT_ID', children: ['GRID_ID'] }, - GRID_ID: { - type: 'GRID', - id: 'GRID_ID', - children: ['ROW-0rHnUz4nMA'], - parents: ['ROOT_ID'], - }, - HEADER_ID: { - id: 'HEADER_ID', - type: 'HEADER', - meta: { text: '1 - Sample dashboard' }, - }, - 'CHART-DF6EfI55F-': { - type: 'CHART', - id: 'CHART-DF6EfI55F-', - children: [], - parents: ['ROOT_ID', 'GRID_ID', 'ROW-0rHnUz4nMA'], - meta: { - width: 4, - height: 50, - chartId: 5, - sliceName: 'Most Populated Countries', - }, - }, - 'ROW-0rHnUz4nMA': { - type: 'ROW', - id: 'ROW-0rHnUz4nMA', - children: ['CHART-DF6EfI55F-'], - parents: ['ROOT_ID', 'GRID_ID'], - meta: { background: 'BACKGROUND_TRANSPARENT' }, - }, - }, - default_filters: '{}', - filter_scopes: {}, - chart_configuration: {}, - }; - - return cy - .request({ - method: 'PUT', - url: `api/v1/dashboard/${dashboardId}`, - body: { - json_metadata: JSON.stringify(jsonMetadata), - }, - }) - .then(() => visitDashboard(false)); - } - return cy; - }); -} +import { + prepareDashboardFilters, + SAMPLE_CHART, + visitDashboard, +} from './shared_dashboard_functions'; function selectFilter(index: number) { cy.get("[data-test='filter-title-container'] [draggable='true']") @@ -195,219 +68,6 @@ function closeFilterModal() { }); } -function openVerticalFilterBar() { - cy.getBySel('dashboard-filters-panel').should('exist'); - cy.getBySel('filter-bar__expand-button').click(); -} - -function setFilterBarOrientation(orientation: 'vertical' | 'horizontal') { - cy.getBySel('filterbar-orientation-icon').click(); - cy.wait(250); - cy.getBySel('dropdown-selectable-icon-submenu') - .contains('Orientation of filter bar') - .should('exist') - .trigger('mouseover'); - - if (orientation === 'vertical') { - cy.get('.ant-dropdown-menu-item-selected') - .contains('Horizontal (Top)') - .should('exist'); - cy.get('.ant-dropdown-menu-item').contains('Vertical (Left)').click(); - cy.getBySel('dashboard-filters-panel').should('exist'); - } else { - cy.get('.ant-dropdown-menu-item-selected') - .contains('Vertical (Left)') - .should('exist'); - cy.get('.ant-dropdown-menu-item').contains('Horizontal (Top)').click(); - cy.getBySel('loading-indicator').should('exist'); - cy.getBySel('filter-bar').should('exist'); - cy.getBySel('dashboard-filters-panel').should('not.exist'); - } -} - -function openMoreFilters(waitFilterState = true) { - interceptFilterState(); - cy.getBySel('dropdown-container-btn').click(); - - if (waitFilterState) { - cy.wait('@postFilterState'); - } -} - -describe('Horizontal FilterBar', () => { - it('should go from vertical to horizontal and the opposite', () => { - visitDashboard(); - openVerticalFilterBar(); - setFilterBarOrientation('horizontal'); - setFilterBarOrientation('vertical'); - }); - - it('should show all default actions in horizontal mode', () => { - visitDashboard(); - openVerticalFilterBar(); - setFilterBarOrientation('horizontal'); - cy.getBySel('horizontal-filterbar-empty') - .contains('No filters are currently added to this dashboard.') - .should('exist'); - cy.getBySel('filter-bar__create-filter').should('exist'); - cy.getBySel('filterbar-action-buttons').should('exist'); - }); - - it('should stay in horizontal mode when reloading', () => { - visitDashboard(); - openVerticalFilterBar(); - setFilterBarOrientation('horizontal'); - cy.reload(); - cy.getBySel('dashboard-filters-panel').should('not.exist'); - }); - - it('should show all filters in available space on load', () => { - prepareDashboardFilters([ - { name: 'test_1', column: 'country_name', datasetId: 2 }, - { name: 'test_2', column: 'country_code', datasetId: 2 }, - { name: 'test_3', column: 'region', datasetId: 2 }, - ]); - setFilterBarOrientation('horizontal'); - cy.get('.filter-item-wrapper').should('have.length', 3); - }); - - it('should show "more filters" on window resizing up and down', () => { - prepareDashboardFilters([ - { name: 'test_1', column: 'country_name', datasetId: 2 }, - { name: 'test_2', column: 'country_code', datasetId: 2 }, - { name: 'test_3', column: 'region', datasetId: 2 }, - ]); - setFilterBarOrientation('horizontal'); - - cy.getBySel('form-item-value').should('have.length', 3); - cy.viewport(768, 1024); - cy.getBySel('form-item-value').should('have.length', 0); - openMoreFilters(false); - cy.getBySel('form-item-value').should('have.length', 3); - - cy.getBySel('filter-bar').click(); - cy.viewport(1000, 1024); - openMoreFilters(false); - cy.getBySel('form-item-value').should('have.length', 3); - - cy.getBySel('filter-bar').click(); - cy.viewport(1300, 1024); - cy.getBySel('form-item-value').should('have.length', 3); - cy.getBySel('dropdown-container-btn').should('not.exist'); - }); - - it('should show "more filters" and scroll', () => { - prepareDashboardFilters([ - { name: 'test_1', column: 'country_name', datasetId: 2 }, - { name: 'test_2', column: 'country_code', datasetId: 2 }, - { name: 'test_3', column: 'region', datasetId: 2 }, - { name: 'test_4', column: 'year', datasetId: 2 }, - { name: 'test_5', column: 'country_name', datasetId: 2 }, - { name: 'test_6', column: 'country_code', datasetId: 2 }, - { name: 'test_7', column: 'region', datasetId: 2 }, - { name: 'test_8', column: 'year', datasetId: 2 }, - { name: 'test_9', column: 'country_name', datasetId: 2 }, - { name: 'test_10', column: 'country_code', datasetId: 2 }, - { name: 'test_11', column: 'region', datasetId: 2 }, - { name: 'test_12', column: 'year', datasetId: 2 }, - ]); - setFilterBarOrientation('horizontal'); - cy.get('.filter-item-wrapper').should('have.length', 3); - openMoreFilters(); - cy.getBySel('form-item-value').should('have.length', 12); - cy.getBySel('filter-control-name').contains('test_10').should('be.visible'); - cy.getBySel('filter-control-name') - .contains('test_12') - .should('not.be.visible'); - cy.get('.ant-popover-inner-content').scrollTo('bottom'); - cy.getBySel('filter-control-name').contains('test_12').should('be.visible'); - }); - - it('should display newly added filter', () => { - visitDashboard(); - openVerticalFilterBar(); - setFilterBarOrientation('horizontal'); - - enterNativeFilterEditModal(false); - addCountryNameFilter(); - saveNativeFilterSettings([]); - validateFilterNameOnDashboard(testItems.topTenChart.filterColumn); - }); - - it('should spot changes in "more filters" and apply their values', () => { - cy.intercept(`/api/v1/chart/data?form_data=**`).as('chart'); - prepareDashboardFilters([ - { name: 'test_1', column: 'country_name', datasetId: 2 }, - { name: 'test_2', column: 'country_code', datasetId: 2 }, - { name: 'test_3', column: 'region', datasetId: 2 }, - { name: 'test_4', column: 'year', datasetId: 2 }, - { name: 'test_5', column: 'country_name', datasetId: 2 }, - { name: 'test_6', column: 'country_code', datasetId: 2 }, - { name: 'test_7', column: 'region', datasetId: 2 }, - { name: 'test_8', column: 'year', datasetId: 2 }, - { name: 'test_9', column: 'country_name', datasetId: 2 }, - { name: 'test_10', column: 'country_code', datasetId: 2 }, - { name: 'test_11', column: 'region', datasetId: 2 }, - { name: 'test_12', column: 'year', datasetId: 2 }, - ]); - setFilterBarOrientation('horizontal'); - openMoreFilters(); - applyNativeFilterValueWithIndex(8, testItems.filterDefaultValue); - cy.get(nativeFilters.applyFilter).click({ force: true }); - cy.wait('@chart'); - cy.get('.antd5-scroll-number.antd5-badge-count').should( - 'have.attr', - 'title', - '1', - ); - }); - - it('should focus filter and open "more filters" programmatically', () => { - prepareDashboardFilters([ - { name: 'test_1', column: 'country_name', datasetId: 2 }, - { name: 'test_2', column: 'country_code', datasetId: 2 }, - { name: 'test_3', column: 'region', datasetId: 2 }, - { name: 'test_4', column: 'year', datasetId: 2 }, - { name: 'test_5', column: 'country_name', datasetId: 2 }, - { name: 'test_6', column: 'country_code', datasetId: 2 }, - { name: 'test_7', column: 'region', datasetId: 2 }, - { name: 'test_8', column: 'year', datasetId: 2 }, - { name: 'test_9', column: 'country_name', datasetId: 2 }, - { name: 'test_10', column: 'country_code', datasetId: 2 }, - { name: 'test_11', column: 'region', datasetId: 2 }, - { name: 'test_12', column: 'year', datasetId: 2 }, - ]); - setFilterBarOrientation('horizontal'); - openMoreFilters(); - applyNativeFilterValueWithIndex(8, testItems.filterDefaultValue); - cy.get(nativeFilters.applyFilter).click({ force: true }); - cy.getBySel('slice-header').within(() => { - cy.get('.filter-counts').trigger('mouseover'); - }); - cy.get('.filterStatusPopover').contains('test_9').click(); - cy.getBySel('dropdown-content').should('be.visible'); - cy.get('.ant-select-focused').should('be.visible'); - }); - - it('should show tag count and one plain tag on focus and only count on blur in select ', () => { - prepareDashboardFilters([ - { name: 'test_1', column: 'country_name', datasetId: 2 }, - ]); - setFilterBarOrientation('horizontal'); - enterNativeFilterEditModal(); - inputNativeFilterDefaultValue('Albania'); - cy.get('.ant-select-selection-search-input').clear({ force: true }); - inputNativeFilterDefaultValue('Algeria', true); - saveNativeFilterSettings([SAMPLE_CHART]); - cy.getBySel('filter-bar').within(() => { - cy.get(nativeFilters.filterItem).contains('Albania').should('be.visible'); - cy.get(nativeFilters.filterItem).contains('+ 1 ...').should('be.visible'); - cy.get('.ant-select-selection-search-input').click(); - cy.get(nativeFilters.filterItem).contains('+ 2 ...').should('be.visible'); - }); - }); -}); - describe('Native filters', () => { describe('Nativefilters tests initial state required', () => { beforeEach(() => { diff --git a/superset-frontend/cypress-base/cypress/e2e/dashboard/shared_dashboard_functions.ts b/superset-frontend/cypress-base/cypress/e2e/dashboard/shared_dashboard_functions.ts new file mode 100644 index 0000000000000..ad47116d34546 --- /dev/null +++ b/superset-frontend/cypress-base/cypress/e2e/dashboard/shared_dashboard_functions.ts @@ -0,0 +1,148 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { SAMPLE_DASHBOARD_1 } from 'cypress/utils/urls'; +import { interceptCharts, interceptDatasets, interceptGet } from './utils'; + +export const SAMPLE_CHART = { name: 'Most Populated Countries', viz: 'table' }; + +export function visitDashboard(createSample = true) { + interceptCharts(); + interceptGet(); + interceptDatasets(); + + if (createSample) { + cy.createSampleDashboards([0]); + } + + cy.visit(SAMPLE_DASHBOARD_1); + cy.wait('@get'); + cy.wait('@getCharts'); + cy.wait('@getDatasets'); + cy.url().should('contain', 'native_filters_key'); +} + +export function prepareDashboardFilters( + filters: { name: string; column: string; datasetId: number }[], +) { + cy.createSampleDashboards([0]); + cy.request({ + method: 'GET', + url: `api/v1/dashboard/1-sample-dashboard`, + }).then(res => { + const { body } = res; + const dashboardId = body.result.id; + const allFilters: Record[] = []; + filters.forEach((f, i) => { + allFilters.push({ + id: `NATIVE_FILTER-fLH0pxFQ${i}`, + controlValues: { + enableEmptyFilter: false, + defaultToFirstItem: false, + multiSelect: true, + searchAllOptions: false, + inverseSelection: false, + }, + name: f.name, + filterType: 'filter_select', + targets: [ + { + datasetId: f.datasetId, + column: { name: f.column }, + }, + ], + defaultDataMask: { + extraFormData: {}, + filterState: {}, + ownState: {}, + }, + cascadeParentIds: [], + scope: { + rootPath: ['ROOT_ID'], + excluded: [], + }, + type: 'NATIVE_FILTER', + description: '', + chartsInScope: [5], + tabsInScope: [], + }); + }); + if (dashboardId) { + const jsonMetadata = { + native_filter_configuration: allFilters, + timed_refresh_immune_slices: [], + expanded_slices: {}, + refresh_frequency: 0, + color_scheme: '', + label_colors: {}, + shared_label_colors: {}, + color_scheme_domain: [], + cross_filters_enabled: false, + positions: { + DASHBOARD_VERSION_KEY: 'v2', + ROOT_ID: { type: 'ROOT', id: 'ROOT_ID', children: ['GRID_ID'] }, + GRID_ID: { + type: 'GRID', + id: 'GRID_ID', + children: ['ROW-0rHnUz4nMA'], + parents: ['ROOT_ID'], + }, + HEADER_ID: { + id: 'HEADER_ID', + type: 'HEADER', + meta: { text: '1 - Sample dashboard' }, + }, + 'CHART-DF6EfI55F-': { + type: 'CHART', + id: 'CHART-DF6EfI55F-', + children: [], + parents: ['ROOT_ID', 'GRID_ID', 'ROW-0rHnUz4nMA'], + meta: { + width: 4, + height: 50, + chartId: 5, + sliceName: 'Most Populated Countries', + }, + }, + 'ROW-0rHnUz4nMA': { + type: 'ROW', + id: 'ROW-0rHnUz4nMA', + children: ['CHART-DF6EfI55F-'], + parents: ['ROOT_ID', 'GRID_ID'], + meta: { background: 'BACKGROUND_TRANSPARENT' }, + }, + }, + default_filters: '{}', + filter_scopes: {}, + chart_configuration: {}, + }; + + return cy + .request({ + method: 'PUT', + url: `api/v1/dashboard/${dashboardId}`, + body: { + json_metadata: JSON.stringify(jsonMetadata), + }, + }) + .then(() => visitDashboard(false)); + } + return cy; + }); +} diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index edaeca313d870..85335fffb0c24 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -274,7 +274,7 @@ "react-resizable": "^3.0.5", "react-test-renderer": "^16.14.0", "redux-mock-store": "^1.5.4", - "sinon": "^18.0.0", + "sinon": "^18.0.1", "source-map": "^0.7.4", "source-map-support": "^0.5.21", "speed-measure-webpack-plugin": "^1.5.0", @@ -48267,13 +48267,13 @@ } }, "node_modules/sinon": { - "version": "18.0.0", - "resolved": "https://registry.npmjs.org/sinon/-/sinon-18.0.0.tgz", - "integrity": "sha512-+dXDXzD1sBO6HlmZDd7mXZCR/y5ECiEiGCBSGuFD/kZ0bDTofPYc6JaeGmPSF+1j1MejGUWkORbYOLDyvqCWpA==", + "version": "18.0.1", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-18.0.1.tgz", + "integrity": "sha512-a2N2TDY1uGviajJ6r4D1CyRAkzE9NNVlYOV1wX5xQDuAk0ONgzgRl0EjCQuRCPxOwp13ghsMwt9Gdldujs39qw==", "dev": true, "dependencies": { "@sinonjs/commons": "^3.0.1", - "@sinonjs/fake-timers": "^11.2.2", + "@sinonjs/fake-timers": "11.2.2", "@sinonjs/samsam": "^8.0.0", "diff": "^5.2.0", "nise": "^6.0.0", @@ -90743,13 +90743,13 @@ } }, "sinon": { - "version": "18.0.0", - "resolved": "https://registry.npmjs.org/sinon/-/sinon-18.0.0.tgz", - "integrity": "sha512-+dXDXzD1sBO6HlmZDd7mXZCR/y5ECiEiGCBSGuFD/kZ0bDTofPYc6JaeGmPSF+1j1MejGUWkORbYOLDyvqCWpA==", + "version": "18.0.1", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-18.0.1.tgz", + "integrity": "sha512-a2N2TDY1uGviajJ6r4D1CyRAkzE9NNVlYOV1wX5xQDuAk0ONgzgRl0EjCQuRCPxOwp13ghsMwt9Gdldujs39qw==", "dev": true, "requires": { "@sinonjs/commons": "^3.0.1", - "@sinonjs/fake-timers": "^11.2.2", + "@sinonjs/fake-timers": "11.2.2", "@sinonjs/samsam": "^8.0.0", "diff": "^5.2.0", "nise": "^6.0.0", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index a892dd4615012..f5917be788531 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -339,7 +339,7 @@ "react-resizable": "^3.0.5", "react-test-renderer": "^16.14.0", "redux-mock-store": "^1.5.4", - "sinon": "^18.0.0", + "sinon": "^18.0.1", "source-map": "^0.7.4", "source-map-support": "^0.5.21", "speed-measure-webpack-plugin": "^1.5.0", diff --git a/superset-frontend/src/components/Chart/chartAction.js b/superset-frontend/src/components/Chart/chartAction.js index 8a9124818424f..f7008c9de37ba 100644 --- a/superset-frontend/src/components/Chart/chartAction.js +++ b/superset-frontend/src/components/Chart/chartAction.js @@ -249,7 +249,7 @@ export async function getChartDataRequest({ export function runAnnotationQuery({ annotation, timeout, - formData = null, + formData, key, isDashboardRequest = false, force = false, diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.tsx similarity index 72% rename from superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx rename to superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.tsx index 760717a3d9a11..7b77319c6d099 100644 --- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/index.tsx @@ -16,16 +16,26 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent } from 'react'; -import PropTypes from 'prop-types'; import { List } from 'src/components'; import { connect } from 'react-redux'; -import { t, withTheme } from '@superset-ui/core'; +import { PureComponent } from 'react'; +import { + HandlerFunction, + JsonObject, + Payload, + QueryFormData, + SupersetTheme, + t, + withTheme, +} from '@superset-ui/core'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; import AsyncEsmComponent from 'src/components/AsyncEsmComponent'; import { getChartKey } from 'src/explore/exploreUtils'; import { runAnnotationQuery } from 'src/components/Chart/chartAction'; import CustomListItem from 'src/explore/components/controls/CustomListItem'; +import { ChartState, ExplorePageState } from 'src/explore/types'; +import { AnyAction } from 'redux'; +import { ThunkDispatch } from 'redux-thunk'; import ControlPopover, { getSectionContainerElement, } from '../ControlPopover/ControlPopover'; @@ -36,19 +46,37 @@ const AnnotationLayer = AsyncEsmComponent( () =>
, ); -const propTypes = { - colorScheme: PropTypes.string.isRequired, - annotationError: PropTypes.object, - annotationQuery: PropTypes.object, - vizType: PropTypes.string, +export interface Annotation { + name: string; + show?: boolean; + annotation: string; + timeout: Date; + key: string; + formData: QueryFormData | null; + isDashboardRequest?: boolean; + force?: boolean; +} - validationErrors: PropTypes.array, - name: PropTypes.string.isRequired, - actions: PropTypes.object, - value: PropTypes.arrayOf(PropTypes.object), - onChange: PropTypes.func, - refreshAnnotationData: PropTypes.func, -}; +export interface Props { + colorScheme: string; + annotationError: Record; + annotationQuery: Record; + vizType: string; + validationErrors: JsonObject[]; + name: string; + actions: { + setControlValue: HandlerFunction; + }; + value: Annotation[]; + onChange: (annotations: Annotation[]) => void; + refreshAnnotationData: (payload: Payload) => void; + theme: SupersetTheme; +} + +export interface PopoverState { + popoverVisible: Record; + addedAnnotationIndex: number | null; +} const defaultProps = { vizType: '', @@ -57,9 +85,10 @@ const defaultProps = { annotationQuery: {}, onChange: () => {}, }; +class AnnotationLayerControl extends PureComponent { + static defaultProps = defaultProps; -class AnnotationLayerControl extends PureComponent { - constructor(props) { + constructor(props: Props) { super(props); this.state = { popoverVisible: {}, @@ -75,7 +104,7 @@ class AnnotationLayerControl extends PureComponent { AnnotationLayer.preload(); } - UNSAFE_componentWillReceiveProps(nextProps) { + UNSAFE_componentWillReceiveProps(nextProps: Props) { const { name, annotationError, validationErrors, value } = nextProps; if (Object.keys(annotationError).length && !validationErrors.length) { this.props.actions.setControlValue( @@ -89,9 +118,12 @@ class AnnotationLayerControl extends PureComponent { } } - addAnnotationLayer(originalAnnotation, newAnnotation) { + addAnnotationLayer = ( + originalAnnotation: Annotation | null, + newAnnotation: Annotation, + ) => { let annotations = this.props.value; - if (annotations.includes(originalAnnotation)) { + if (originalAnnotation && annotations.includes(originalAnnotation)) { annotations = annotations.map(anno => anno === originalAnnotation ? newAnnotation : anno, ); @@ -106,15 +138,15 @@ class AnnotationLayerControl extends PureComponent { }); this.props.onChange(annotations); - } + }; - handleVisibleChange(visible, popoverKey) { + handleVisibleChange = (visible: boolean, popoverKey: number | string) => { this.setState(prevState => ({ popoverVisible: { ...prevState.popoverVisible, [popoverKey]: visible }, })); - } + }; - removeAnnotationLayer(annotation) { + removeAnnotationLayer(annotation: Annotation | null) { const annotations = this.props.value.filter(anno => anno !== annotation); // So scrollbar doesnt get stuck on hidden const element = getSectionContainerElement(); @@ -124,17 +156,21 @@ class AnnotationLayerControl extends PureComponent { this.props.onChange(annotations); } - renderPopover(popoverKey, annotation, error) { + renderPopover = ( + popoverKey: number | string, + annotation: Annotation | null, + error: string, + ) => { const id = annotation?.name || '_new'; return (
+ addAnnotationLayer={(newAnnotation: Annotation) => this.addAnnotationLayer(annotation, newAnnotation) } removeAnnotationLayer={() => this.removeAnnotationLayer(annotation)} @@ -145,9 +181,9 @@ class AnnotationLayerControl extends PureComponent { />
); - } + }; - renderInfo(anno) { + renderInfo(anno: Annotation) { const { annotationError, annotationQuery, theme } = this.props; if (annotationQuery[anno.name]) { return ( @@ -175,8 +211,10 @@ class AnnotationLayerControl extends PureComponent { render() { const { addedAnnotationIndex } = this.state; - const addedAnnotation = this.props.value[addedAnnotationIndex]; - + const addedAnnotation = + addedAnnotationIndex !== null + ? this.props.value[addedAnnotationIndex] + : null; const annotations = this.props.value.map((anno, i) => ( )); - const addLayerPopoverKey = 'add'; + return (
({ borderRadius: theme.gridUnit })}> {annotations} ) { const chartKey = getChartKey(explore); - const chart = charts[chartKey] || charts[0] || {}; + + const defaultChartState: Partial = { + annotationError: {}, + annotationQuery: {}, + }; + + const chart = + chartKey && charts[chartKey] ? charts[chartKey] : defaultChartState; return { // eslint-disable-next-line camelcase colorScheme: explore.controls?.color_scheme?.value, - annotationError: chart.annotationError, - annotationQuery: chart.annotationQuery, - vizType: explore.controls.viz_type.value, + annotationError: chart.annotationError ?? {}, + annotationQuery: chart.annotationQuery ?? {}, + vizType: explore.controls?.viz_type.value, }; } -function mapDispatchToProps(dispatch) { +function mapDispatchToProps( + dispatch: ThunkDispatch, +) { return { - refreshAnnotationData: annotationObj => + refreshAnnotationData: (annotationObj: Annotation) => dispatch(runAnnotationQuery(annotationObj)), }; } diff --git a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts index f0eb399267b11..df1afd69a0a0c 100644 --- a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts +++ b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.test.ts @@ -286,3 +286,175 @@ test('SQL ad-hoc filter values', () => { sqlExpression: 'select * from sample_column_1;', }); }); + +test('no controlState value but valid column in datasource', () => { + const controlState = { + ...sharedControls.columns, + options: [], // no options in the control state + }; + + expect( + getValues({ + ...controlState, + value: 'sample_column_1', // column only available in datasource + }), + ).toEqual('sample_column_1'); + + expect( + getValues({ + ...controlState, + value: 'non_existing_column', + }), + ).toEqual(controlState.default); +}); + +test('no controlState value but valid saved metric in datasource', () => { + const controlState = { + ...sharedControls.metrics, + savedMetrics: [], // no saved metrics in the control state + }; + + expect( + getValues({ + ...controlState, + value: 'saved_metric_2', // metric only available in datasource + }), + ).toEqual('saved_metric_2'); + + expect( + getValues({ + ...controlState, + value: 'non_existing_metric', + }), + ).toEqual(controlState.default); +}); + +test('no controlState value but valid adhoc metric in datasource', () => { + const controlState = { + ...sharedControls.metrics, + columns: [], // no columns in control state + }; + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SIMPLE', + column: { column_name: 'sample_column_1' }, // only in datasource + }, + }), + ).toEqual({ + expressionType: 'SIMPLE', + column: { column_name: 'sample_column_1' }, + }); + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SIMPLE', + column: { column_name: 'non_existing_column' }, + }, + }), + ).toEqual(controlState.default); +}); + +test('no controlState value but valid adhoc filter in datasource', () => { + const controlState = { + ...sharedControls.adhoc_filters, + columns: [], // no columns in control state + }; + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SIMPLE', + subject: 'sample_column_1', // column available in datasource + }, + }), + ).toEqual({ + expressionType: 'SIMPLE', + subject: 'sample_column_1', + }); + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SIMPLE', + subject: 'non_existing_column', + }, + }), + ).toEqual(controlState.default); +}); + +test('SQL ad-hoc metric values without controlState columns', () => { + const controlState = { + ...sharedControls.metrics, + columns: [], // No columns in controlState + }; + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SQL', + sqlExpression: 'SELECT COUNT(*) FROM sample_table;', + }, + }), + ).toEqual({ + datasourceWarning: true, + expressionType: 'SQL', + sqlExpression: 'SELECT COUNT(*) FROM sample_table;', + }); + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SQL', + sqlExpression: 'SELECT column FROM non_existing_table;', + }, + }), + ).toEqual({ + datasourceWarning: true, + expressionType: 'SQL', + sqlExpression: 'SELECT column FROM non_existing_table;', + }); +}); + +test('SQL ad-hoc filter values without controlState columns', () => { + const controlState = { + ...sharedControls.adhoc_filters, + columns: [], // No columns in controlState + }; + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SQL', + sqlExpression: 'SELECT * FROM sample_table WHERE column = 1;', + }, + }), + ).toEqual({ + datasourceWarning: true, + expressionType: 'SQL', + sqlExpression: 'SELECT * FROM sample_table WHERE column = 1;', + }); + + expect( + getValues({ + ...controlState, + value: { + expressionType: 'SQL', + sqlExpression: 'SELECT * FROM non_existing_table;', + }, + }), + ).toEqual({ + datasourceWarning: true, + expressionType: 'SQL', + sqlExpression: 'SELECT * FROM non_existing_table;', + }); +}); diff --git a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts index 7013b59764cb0..bfc53dc6d8257 100644 --- a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts +++ b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts @@ -27,6 +27,7 @@ import { JsonValue, SimpleAdhocFilter, } from '@superset-ui/core'; +import { isEmpty } from 'lodash'; import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric'; const isControlValueCompatibleWithDatasource = ( @@ -34,24 +35,32 @@ const isControlValueCompatibleWithDatasource = ( controlState: ControlState, value: any, ) => { + // A datasource might have been deleted, in which case we can't validate + // only using the control state since it might have been hydrated with + // the wrong options or columns (empty arrays). if (controlState.options && typeof value === 'string') { if ( - controlState.options.some( - (option: [string | number, string] | { column_name: string }) => - Array.isArray(option) - ? option[0] === value - : option.column_name === value, - ) + (!isEmpty(controlState.options) && + controlState.options.some( + (option: [string | number, string] | { column_name: string }) => + Array.isArray(option) + ? option[0] === value + : option.column_name === value, + )) || + !isEmpty(datasource?.columns) ) { - return datasource.columns.some(column => column.column_name === value); + return datasource.columns.some( + (column: Column) => column.column_name === value, + ); } } if ( controlState.savedMetrics && isSavedMetric(value) && - controlState.savedMetrics.some( + (controlState.savedMetrics.some( (savedMetric: Metric) => savedMetric.metric_name === value, - ) + ) || + !isEmpty(datasource?.metrics)) ) { return datasource.metrics.some( (metric: Metric) => metric.metric_name === value, @@ -60,11 +69,13 @@ const isControlValueCompatibleWithDatasource = ( if ( controlState.columns && (isAdhocMetricSimple(value) || isSimpleAdhocFilter(value)) && - controlState.columns.some( - (column: Column) => - column.column_name === (value as AdhocMetric).column?.column_name || - column.column_name === (value as SimpleAdhocFilter).subject, - ) + ((!isEmpty(controlState.columns) && + controlState.columns.some( + (column: Column) => + column.column_name === (value as AdhocMetric).column?.column_name || + column.column_name === (value as SimpleAdhocFilter).subject, + )) || + !isEmpty(datasource?.columns)) ) { return datasource.columns.some( (column: Column) => diff --git a/superset-frontend/src/pages/Chart/index.tsx b/superset-frontend/src/pages/Chart/index.tsx index e8e9ae127fe5a..af9386279f027 100644 --- a/superset-frontend/src/pages/Chart/index.tsx +++ b/superset-frontend/src/pages/Chart/index.tsx @@ -43,7 +43,10 @@ import { getItem, LocalStorageKeys } from 'src/utils/localStorageHelpers'; import { getFormDataWithDashboardContext } from 'src/explore/controlUtils/getFormDataWithDashboardContext'; const isValidResult = (rv: JsonObject): boolean => - rv?.result?.form_data && isDefined(rv?.result?.dataset?.id); + rv?.result?.form_data && rv?.result?.dataset; + +const hasDatasetId = (rv: JsonObject): boolean => + isDefined(rv?.result?.dataset?.id); const fetchExploreData = async (exploreUrlParams: URLSearchParams) => { try { @@ -52,7 +55,19 @@ const fetchExploreData = async (exploreUrlParams: URLSearchParams) => { endpoint: 'api/v1/explore/', })(exploreUrlParams); if (isValidResult(rv)) { - return rv; + if (hasDatasetId(rv)) { + return rv; + } + // Since there's no dataset id but the API responded with a valid payload, + // we assume the dataset was deleted, so we preserve some values from previous + // state so if the user decide to swap the datasource, the chart config remains + fallbackExploreInitialData.form_data = { + ...rv.result.form_data, + ...fallbackExploreInitialData.form_data, + }; + if (rv.result?.slice) { + fallbackExploreInitialData.slice = rv.result.slice; + } } let message = t('Failed to load chart data'); const responseError = rv?.result?.message; diff --git a/superset/charts/post_processing.py b/superset/charts/post_processing.py index ebcae32f8f486..4c5abd8db19f1 100644 --- a/superset/charts/post_processing.py +++ b/superset/charts/post_processing.py @@ -29,6 +29,7 @@ from io import StringIO from typing import Any, Optional, TYPE_CHECKING, Union +import numpy as np import pandas as pd from flask_babel import gettext as __ @@ -83,10 +84,11 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s else: axis = {"columns": 1, "rows": 0} + # pivoting with null values will create an empty df + df = df.fillna("SUPERSET_PANDAS_NAN") + # pivot data; we'll compute totals and subtotals later if rows or columns: - # pivoting with null values will create an empty df - df = df.fillna("NULL") df = df.pivot_table( index=rows, columns=columns, @@ -151,6 +153,18 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s # add subtotal for each group and overall total; we start from the # overall group, and iterate deeper into subgroups groups = df.columns + if not apply_metrics_on_rows: + for col in df.columns: + # we need to replace the temporary placeholder with either a string + # or np.nan, depending on the column type so that they can sum correctly + if pd.api.types.is_numeric_dtype(df[col]): + df[col].replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True) + else: + df[col].replace("SUPERSET_PANDAS_NAN", "nan", inplace=True) + else: + # when we applied metrics on rows, we switched the columns and rows + # so checking column type doesn't apply. Replace everything with np.nan + df.replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True) for level in range(df.columns.nlevels): subgroups = {group[:level] for group in groups} for subgroup in subgroups: @@ -171,7 +185,7 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s for subgroup in subgroups: slice_ = df.index.get_loc(subgroup) subtotal = pivot_v2_aggfunc_map[aggfunc]( - df.iloc[slice_, :].apply(pd.to_numeric), axis=0 + df.iloc[slice_, :].apply(pd.to_numeric, errors="coerce"), axis=0 ) depth = df.index.nlevels - len(subgroup) - 1 total = metric_name if level == 0 else __("Subtotal") @@ -186,6 +200,14 @@ def pivot_df( # pylint: disable=too-many-locals, too-many-arguments, too-many-s if apply_metrics_on_rows: df = df.T + # replace the remaining temporary placeholder string for np.nan after pivoting + df.replace("SUPERSET_PANDAS_NAN", np.nan, inplace=True) + df.rename( + index={"SUPERSET_PANDAS_NAN": np.nan}, + columns={"SUPERSET_PANDAS_NAN": np.nan}, + inplace=True, + ) + return df diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 77243d41f8d7c..9215e1545bb21 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -1688,10 +1688,10 @@ def _normalize_prequery_result_type( if isinstance(value, np.generic): value = value.item() - column_ = columns_by_name[dimension] + column_ = columns_by_name.get(dimension) db_extra: dict[str, Any] = self.database.get_extra() - if column_.type and column_.is_temporal and isinstance(value, str): + if column_ and column_.type and column_.is_temporal and isinstance(value, str): sql = self.db_engine_spec.convert_dttm( column_.type, dateutil.parser.parse(value), db_extra=db_extra ) diff --git a/superset/views/core.py b/superset/views/core.py index 56221b646a39e..1a21934bf93c6 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -792,9 +792,16 @@ def dashboard( try: dashboard.raise_for_access() except SupersetSecurityException as ex: + # anonymous users should get the login screen, others should go to dashboard list + if g.user is None or g.user.is_anonymous: + redirect_url = f"{appbuilder.get_url_for_login}?next={request.url}" + warn_msg = "Users must be logged in to view this dashboard." + else: + redirect_url = "/dashboard/list/" + warn_msg = utils.error_msg_from_exception(ex) return redirect_with_flash( - url="/dashboard/list/", - message=utils.error_msg_from_exception(ex), + url=redirect_url, + message=warn_msg, category="danger", ) add_extra_log_payload( diff --git a/tests/unit_tests/charts/test_post_processing.py b/tests/unit_tests/charts/test_post_processing.py index 790c494516347..181b9f06352ab 100644 --- a/tests/unit_tests/charts/test_post_processing.py +++ b/tests/unit_tests/charts/test_post_processing.py @@ -78,10 +78,10 @@ def test_pivot_df_no_cols_no_rows_single_metric(): ) assert ( pivoted.to_markdown() - == f""" + == """ | | ('SUM(num)',) | |:-----------------|----------------:| -| ('{_("Total")} (Sum)',) | 8.06797e+07 | +| ('Total (Sum)',) | 8.06797e+07 | """.strip() ) @@ -407,6 +407,476 @@ def test_pivot_df_single_row_two_metrics(): ) +def test_pivot_df_single_row_null_values(): + """ + Pivot table when a single column and 2 metrics are selected. + """ + df = pd.DataFrame.from_dict( + { + "gender": {0: "girl", 1: "boy"}, + "SUM(num)": {0: 118065, 1: None}, + "MAX(num)": {0: 2588, 1: None}, + } + ) + assert ( + df.to_markdown() + == """ +| | gender | SUM(num) | MAX(num) | +|---:|:---------|-----------:|-----------:| +| 0 | girl | 118065 | 2588 | +| 1 | boy | nan | nan | + """.strip() + ) + + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=False, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | +|:----------|----------------:|----------------:| +| ('boy',) | nan | nan | +| ('girl',) | 118065 | 2588 | + """.strip() + ) + + # transpose_pivot + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=True, + combine_metrics=False, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)', 'boy') | ('SUM(num)', 'girl') | ('MAX(num)', 'boy') | ('MAX(num)', 'girl') | +|:-----------------|----------------------:|-----------------------:|----------------------:|-----------------------:| +| ('Total (Sum)',) | nan | 118065 | nan | 2588 | + """.strip() + ) + + # combine_metrics does nothing in this case + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=True, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | +|:----------|----------------:|----------------:| +| ('boy',) | nan | nan | +| ('girl',) | 118065 | 2588 | + """.strip() + ) + + # show totals + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=False, + show_rows_total=True, + show_columns_total=True, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | ('Total (Sum)',) | +|:-----------------|----------------:|----------------:|:-------------------| +| ('boy',) | nan | nan | nannan | +| ('girl',) | 118065 | 2588 | 120653.0 | +| ('Total (Sum)',) | 118065 | 2588 | 120653.0 | + """.strip() + ) + + # apply_metrics_on_rows + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=False, + show_rows_total=True, + show_columns_total=False, + apply_metrics_on_rows=True, + ) + assert ( + pivoted.to_markdown() + == f""" +| | ('{_("Total")} (Sum)',) | +|:-------------------------|-------------------:| +| ('SUM(num)', 'boy') | nan | +| ('SUM(num)', 'girl') | 118065 | +| ('SUM(num)', 'Subtotal') | 118065 | +| ('MAX(num)', 'boy') | nan | +| ('MAX(num)', 'girl') | 2588 | +| ('MAX(num)', 'Subtotal') | 2588 | +| ('{_("Total")} (Sum)', '') | 120653 | + """.strip() + ) + + # apply_metrics_on_rows with combine_metrics + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=True, + show_rows_total=True, + show_columns_total=True, + apply_metrics_on_rows=True, + ) + assert ( + pivoted.to_markdown() + == f""" +| | ('{_("Total")} (Sum)',) | +|:---------------------|-------------------:| +| ('boy', 'SUM(num)') | nan | +| ('boy', 'MAX(num)') | nan | +| ('boy', 'Subtotal') | 0 | +| ('girl', 'SUM(num)') | 118065 | +| ('girl', 'MAX(num)') | 2588 | +| ('girl', 'Subtotal') | 120653 | +| ('{_("Total")} (Sum)', '') | 120653 | + """.strip() + ) + + +def test_pivot_df_single_row_null_mix_values_strings(): + """ + Pivot table when a single column and 2 metrics are selected. + """ + df = pd.DataFrame.from_dict( + { + "gender": {0: "girl", 1: "boy"}, + "SUM(num)": {0: 118065, 1: "NULL"}, + "MAX(num)": {0: 2588, 1: None}, + } + ) + assert ( + df.to_markdown() + == """ +| | gender | SUM(num) | MAX(num) | +|---:|:---------|:-----------|-----------:| +| 0 | girl | 118065 | 2588 | +| 1 | boy | NULL | nan | + """.strip() + ) + + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=False, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | +|:----------|:----------------|----------------:| +| ('boy',) | NULL | nan | +| ('girl',) | 118065 | 2588 | + """.strip() + ) + + # transpose_pivot + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=True, + combine_metrics=False, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)', 'boy') | ('SUM(num)', 'girl') | ('MAX(num)', 'boy') | ('MAX(num)', 'girl') | +|:-----------------|:----------------------|-----------------------:|----------------------:|-----------------------:| +| ('Total (Sum)',) | NULL | 118065 | nan | 2588 | + + """.strip() + ) + + # combine_metrics does nothing in this case + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=True, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | +|:----------|:----------------|----------------:| +| ('boy',) | NULL | nan | +| ('girl',) | 118065 | 2588 | + """.strip() + ) + + # show totals + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=False, + show_rows_total=True, + show_columns_total=True, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | ('Total (Sum)',) | +|:-----------------|:----------------|----------------:|:-------------------| +| ('boy',) | NULL | nan | NULLnan | +| ('girl',) | 118065 | 2588 | 120653.0 | +| ('Total (Sum)',) | 118065.0 | 2588 | 120653.0 | + """.strip() + ) + + # apply_metrics_on_rows with combine_metrics + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=True, + show_rows_total=False, + show_columns_total=True, + apply_metrics_on_rows=True, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('Total (Sum)',) | +|:---------------------|:-------------------| +| ('boy', 'SUM(num)') | NULL | +| ('boy', 'MAX(num)') | nan | +| ('girl', 'SUM(num)') | 118065 | +| ('girl', 'MAX(num)') | 2588.0 | + """.strip() + ) + + +def test_pivot_df_single_row_null_mix_values_numbers(): + """ + Pivot table when a single column and 2 metrics are selected. + """ + df = pd.DataFrame.from_dict( + { + "gender": {0: "girl", 1: "boy"}, + "SUM(num)": {0: 118065, 1: 21}, + "MAX(num)": {0: 2588, 1: None}, + } + ) + assert ( + df.to_markdown() + == """ +| | gender | SUM(num) | MAX(num) | +|---:|:---------|-----------:|-----------:| +| 0 | girl | 118065 | 2588 | +| 1 | boy | 21 | nan | + """.strip() + ) + + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=False, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | +|:----------|----------------:|----------------:| +| ('boy',) | 21 | nan | +| ('girl',) | 118065 | 2588 | + """.strip() + ) + + # transpose_pivot + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=True, + combine_metrics=False, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)', 'boy') | ('SUM(num)', 'girl') | ('MAX(num)', 'boy') | ('MAX(num)', 'girl') | +|:-----------------|----------------------:|-----------------------:|----------------------:|-----------------------:| +| ('Total (Sum)',) | 21 | 118065 | nan | 2588 | """.strip() + ) + + # combine_metrics does nothing in this case + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=True, + show_rows_total=False, + show_columns_total=False, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | +|:----------|----------------:|----------------:| +| ('boy',) | 21 | nan | +| ('girl',) | 118065 | 2588 | + """.strip() + ) + + # show totals + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=False, + show_rows_total=False, + show_columns_total=True, + apply_metrics_on_rows=False, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('SUM(num)',) | ('MAX(num)',) | +|:-----------------|----------------:|----------------:| +| ('boy',) | 21 | nan | +| ('girl',) | 118065 | 2588 | +| ('Total (Sum)',) | 118086 | 2588 | + """.strip() + ) + + # apply_metrics_on_rows + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=False, + show_rows_total=False, + show_columns_total=True, + apply_metrics_on_rows=True, + ) + assert ( + pivoted.to_markdown() + == """ +| | ('Total (Sum)',) | +|:---------------------|-------------------:| +| ('SUM(num)', 'boy') | 21 | +| ('SUM(num)', 'girl') | 118065 | +| ('MAX(num)', 'boy') | nan | +| ('MAX(num)', 'girl') | 2588 | + """.strip() + ) + + # apply_metrics_on_rows with combine_metrics + pivoted = pivot_df( + df, + rows=["gender"], + columns=[], + metrics=["SUM(num)", "MAX(num)"], + aggfunc="Sum", + transpose_pivot=False, + combine_metrics=True, + show_rows_total=False, + show_columns_total=True, + apply_metrics_on_rows=True, + ) + assert ( + pivoted.to_markdown() + == f""" +| | ('{_("Total")} (Sum)',) | +|:---------------------|-------------------:| +| ('boy', 'SUM(num)') | 21 | +| ('boy', 'MAX(num)') | nan | +| ('girl', 'SUM(num)') | 118065 | +| ('girl', 'MAX(num)') | 2588 | + """.strip() + ) + + def test_pivot_df_complex(): """ Pivot table when a column, rows and 2 metrics are selected. @@ -1106,14 +1576,14 @@ def test_pivot_df_complex_null_values(): assert ( pivoted.to_markdown() == """ -| | ('SUM(num)', 'NULL') | ('MAX(num)', 'NULL') | -|:-------------------|-----------------------:|-----------------------:| -| ('boy', 'Edward') | 40685 | 1669 | -| ('boy', 'Tony') | 6438 | 845 | -| ('girl', 'Amy') | 60166 | 3081 | -| ('girl', 'Cindy') | 15367 | 1059 | -| ('girl', 'Dawn') | 16492 | 1618 | -| ('girl', 'Sophia') | 26040 | 3775 | +| | ('SUM(num)', nan) | ('MAX(num)', nan) | +|:-------------------|--------------------:|--------------------:| +| ('boy', 'Edward') | 40685 | 1669 | +| ('boy', 'Tony') | 6438 | 845 | +| ('girl', 'Amy') | 60166 | 3081 | +| ('girl', 'Cindy') | 15367 | 1059 | +| ('girl', 'Dawn') | 16492 | 1618 | +| ('girl', 'Sophia') | 26040 | 3775 | """.strip() ) @@ -1134,9 +1604,9 @@ def test_pivot_df_complex_null_values(): assert ( pivoted.to_markdown() == """ -| | ('SUM(num)', 'boy', 'Edward') | ('SUM(num)', 'boy', 'Tony') | ('SUM(num)', 'girl', 'Amy') | ('SUM(num)', 'girl', 'Cindy') | ('SUM(num)', 'girl', 'Dawn') | ('SUM(num)', 'girl', 'Sophia') | ('MAX(num)', 'boy', 'Edward') | ('MAX(num)', 'boy', 'Tony') | ('MAX(num)', 'girl', 'Amy') | ('MAX(num)', 'girl', 'Cindy') | ('MAX(num)', 'girl', 'Dawn') | ('MAX(num)', 'girl', 'Sophia') | -|:----------|--------------------------------:|------------------------------:|------------------------------:|--------------------------------:|-------------------------------:|---------------------------------:|--------------------------------:|------------------------------:|------------------------------:|--------------------------------:|-------------------------------:|---------------------------------:| -| ('NULL',) | 40685 | 6438 | 60166 | 15367 | 16492 | 26040 | 1669 | 845 | 3081 | 1059 | 1618 | 3775 | +| | ('SUM(num)', 'boy', 'Edward') | ('SUM(num)', 'boy', 'Tony') | ('SUM(num)', 'girl', 'Amy') | ('SUM(num)', 'girl', 'Cindy') | ('SUM(num)', 'girl', 'Dawn') | ('SUM(num)', 'girl', 'Sophia') | ('MAX(num)', 'boy', 'Edward') | ('MAX(num)', 'boy', 'Tony') | ('MAX(num)', 'girl', 'Amy') | ('MAX(num)', 'girl', 'Cindy') | ('MAX(num)', 'girl', 'Dawn') | ('MAX(num)', 'girl', 'Sophia') | +|:-------|--------------------------------:|------------------------------:|------------------------------:|--------------------------------:|-------------------------------:|---------------------------------:|--------------------------------:|------------------------------:|------------------------------:|--------------------------------:|-------------------------------:|---------------------------------:| +| (nan,) | 40685 | 6438 | 60166 | 15367 | 16492 | 26040 | 1669 | 845 | 3081 | 1059 | 1618 | 3775 | """.strip() ) @@ -1156,14 +1626,14 @@ def test_pivot_df_complex_null_values(): assert ( pivoted.to_markdown() == """ -| | ('NULL', 'SUM(num)') | ('NULL', 'MAX(num)') | -|:-------------------|-----------------------:|-----------------------:| -| ('boy', 'Edward') | 40685 | 1669 | -| ('boy', 'Tony') | 6438 | 845 | -| ('girl', 'Amy') | 60166 | 3081 | -| ('girl', 'Cindy') | 15367 | 1059 | -| ('girl', 'Dawn') | 16492 | 1618 | -| ('girl', 'Sophia') | 26040 | 3775 | +| | (nan, 'SUM(num)') | (nan, 'MAX(num)') | +|:-------------------|--------------------:|--------------------:| +| ('boy', 'Edward') | 40685 | 1669 | +| ('boy', 'Tony') | 6438 | 845 | +| ('girl', 'Amy') | 60166 | 3081 | +| ('girl', 'Cindy') | 15367 | 1059 | +| ('girl', 'Dawn') | 16492 | 1618 | +| ('girl', 'Sophia') | 26040 | 3775 | """.strip() ) @@ -1183,17 +1653,17 @@ def test_pivot_df_complex_null_values(): assert ( pivoted.to_markdown() == """ -| | ('SUM(num)', 'NULL') | ('SUM(num)', 'Subtotal') | ('MAX(num)', 'NULL') | ('MAX(num)', 'Subtotal') | ('Total (Sum)', '') | -|:---------------------|-----------------------:|---------------------------:|-----------------------:|---------------------------:|----------------------:| -| ('boy', 'Edward') | 40685 | 40685 | 1669 | 1669 | 42354 | -| ('boy', 'Tony') | 6438 | 6438 | 845 | 845 | 7283 | -| ('boy', 'Subtotal') | 47123 | 47123 | 2514 | 2514 | 49637 | -| ('girl', 'Amy') | 60166 | 60166 | 3081 | 3081 | 63247 | -| ('girl', 'Cindy') | 15367 | 15367 | 1059 | 1059 | 16426 | -| ('girl', 'Dawn') | 16492 | 16492 | 1618 | 1618 | 18110 | -| ('girl', 'Sophia') | 26040 | 26040 | 3775 | 3775 | 29815 | -| ('girl', 'Subtotal') | 118065 | 118065 | 9533 | 9533 | 127598 | -| ('Total (Sum)', '') | 165188 | 165188 | 12047 | 12047 | 177235 | +| | ('SUM(num)', nan) | ('SUM(num)', 'Subtotal') | ('MAX(num)', nan) | ('MAX(num)', 'Subtotal') | ('Total (Sum)', '') | +|:---------------------|--------------------:|---------------------------:|--------------------:|---------------------------:|----------------------:| +| ('boy', 'Edward') | 40685 | 40685 | 1669 | 1669 | 42354 | +| ('boy', 'Tony') | 6438 | 6438 | 845 | 845 | 7283 | +| ('boy', 'Subtotal') | 47123 | 47123 | 2514 | 2514 | 49637 | +| ('girl', 'Amy') | 60166 | 60166 | 3081 | 3081 | 63247 | +| ('girl', 'Cindy') | 15367 | 15367 | 1059 | 1059 | 16426 | +| ('girl', 'Dawn') | 16492 | 16492 | 1618 | 1618 | 18110 | +| ('girl', 'Sophia') | 26040 | 26040 | 3775 | 3775 | 29815 | +| ('girl', 'Subtotal') | 118065 | 118065 | 9533 | 9533 | 127598 | +| ('Total (Sum)', '') | 165188 | 165188 | 12047 | 12047 | 177235 | """.strip() ) @@ -1213,20 +1683,20 @@ def test_pivot_df_complex_null_values(): assert ( pivoted.to_markdown() == """ -| | ('NULL',) | -|:-------------------------------|------------:| -| ('SUM(num)', 'boy', 'Edward') | 40685 | -| ('SUM(num)', 'boy', 'Tony') | 6438 | -| ('SUM(num)', 'girl', 'Amy') | 60166 | -| ('SUM(num)', 'girl', 'Cindy') | 15367 | -| ('SUM(num)', 'girl', 'Dawn') | 16492 | -| ('SUM(num)', 'girl', 'Sophia') | 26040 | -| ('MAX(num)', 'boy', 'Edward') | 1669 | -| ('MAX(num)', 'boy', 'Tony') | 845 | -| ('MAX(num)', 'girl', 'Amy') | 3081 | -| ('MAX(num)', 'girl', 'Cindy') | 1059 | -| ('MAX(num)', 'girl', 'Dawn') | 1618 | -| ('MAX(num)', 'girl', 'Sophia') | 3775 | +| | (nan,) | +|:-------------------------------|---------:| +| ('SUM(num)', 'boy', 'Edward') | 40685 | +| ('SUM(num)', 'boy', 'Tony') | 6438 | +| ('SUM(num)', 'girl', 'Amy') | 60166 | +| ('SUM(num)', 'girl', 'Cindy') | 15367 | +| ('SUM(num)', 'girl', 'Dawn') | 16492 | +| ('SUM(num)', 'girl', 'Sophia') | 26040 | +| ('MAX(num)', 'boy', 'Edward') | 1669 | +| ('MAX(num)', 'boy', 'Tony') | 845 | +| ('MAX(num)', 'girl', 'Amy') | 3081 | +| ('MAX(num)', 'girl', 'Cindy') | 1059 | +| ('MAX(num)', 'girl', 'Dawn') | 1618 | +| ('MAX(num)', 'girl', 'Sophia') | 3775 | """.strip() ) @@ -1246,20 +1716,20 @@ def test_pivot_df_complex_null_values(): assert ( pivoted.to_markdown() == """ -| | ('NULL',) | -|:-------------------------------|------------:| -| ('boy', 'Edward', 'SUM(num)') | 40685 | -| ('boy', 'Edward', 'MAX(num)') | 1669 | -| ('boy', 'Tony', 'SUM(num)') | 6438 | -| ('boy', 'Tony', 'MAX(num)') | 845 | -| ('girl', 'Amy', 'SUM(num)') | 60166 | -| ('girl', 'Amy', 'MAX(num)') | 3081 | -| ('girl', 'Cindy', 'SUM(num)') | 15367 | -| ('girl', 'Cindy', 'MAX(num)') | 1059 | -| ('girl', 'Dawn', 'SUM(num)') | 16492 | -| ('girl', 'Dawn', 'MAX(num)') | 1618 | -| ('girl', 'Sophia', 'SUM(num)') | 26040 | -| ('girl', 'Sophia', 'MAX(num)') | 3775 | +| | (nan,) | +|:-------------------------------|---------:| +| ('boy', 'Edward', 'SUM(num)') | 40685 | +| ('boy', 'Edward', 'MAX(num)') | 1669 | +| ('boy', 'Tony', 'SUM(num)') | 6438 | +| ('boy', 'Tony', 'MAX(num)') | 845 | +| ('girl', 'Amy', 'SUM(num)') | 60166 | +| ('girl', 'Amy', 'MAX(num)') | 3081 | +| ('girl', 'Cindy', 'SUM(num)') | 15367 | +| ('girl', 'Cindy', 'MAX(num)') | 1059 | +| ('girl', 'Dawn', 'SUM(num)') | 16492 | +| ('girl', 'Dawn', 'MAX(num)') | 1618 | +| ('girl', 'Sophia', 'SUM(num)') | 26040 | +| ('girl', 'Sophia', 'MAX(num)') | 3775 | """.strip() ) @@ -1279,12 +1749,12 @@ def test_pivot_df_complex_null_values(): assert ( pivoted.to_markdown() == """ -| | ('boy', 'Edward') | ('boy', 'Tony') | ('boy', 'Subtotal') | ('girl', 'Amy') | ('girl', 'Cindy') | ('girl', 'Dawn') | ('girl', 'Sophia') | ('girl', 'Subtotal') | ('Total (Sum)', '') | -|:---------------------|--------------------:|------------------:|----------------------:|------------------:|--------------------:|-------------------:|---------------------:|-----------------------:|----------------------:| -| ('NULL', 'SUM(num)') | 40685 | 6438 | 47123 | 60166 | 15367 | 16492 | 26040 | 118065 | 165188 | -| ('NULL', 'MAX(num)') | 1669 | 845 | 2514 | 3081 | 1059 | 1618 | 3775 | 9533 | 12047 | -| ('NULL', 'Subtotal') | 42354 | 7283 | 49637 | 63247 | 16426 | 18110 | 29815 | 127598 | 177235 | -| ('Total (Sum)', '') | 42354 | 7283 | 49637 | 63247 | 16426 | 18110 | 29815 | 127598 | 177235 | +| | ('boy', 'Edward') | ('boy', 'Tony') | ('boy', 'Subtotal') | ('girl', 'Amy') | ('girl', 'Cindy') | ('girl', 'Dawn') | ('girl', 'Sophia') | ('girl', 'Subtotal') | ('Total (Sum)', '') | +|:--------------------|--------------------:|------------------:|----------------------:|------------------:|--------------------:|-------------------:|---------------------:|-----------------------:|----------------------:| +| (nan, 'SUM(num)') | 40685 | 6438 | 47123 | 60166 | 15367 | 16492 | 26040 | 118065 | 165188 | +| (nan, 'MAX(num)') | 1669 | 845 | 2514 | 3081 | 1059 | 1618 | 3775 | 9533 | 12047 | +| (nan, 'Subtotal') | 42354 | 7283 | 49637 | 63247 | 16426 | 18110 | 29815 | 127598 | 177235 | +| ('Total (Sum)', '') | 42354 | 7283 | 49637 | 63247 | 16426 | 18110 | 29815 | 127598 | 177235 | """.strip() ) @@ -1304,17 +1774,17 @@ def test_pivot_df_complex_null_values(): assert ( pivoted.to_markdown() == """ -| | ('SUM(num)', 'NULL') | ('MAX(num)', 'NULL') | -|:-------------------------------------------|-----------------------:|-----------------------:| -| ('boy', 'Edward') | 0.246295 | 0.138541 | -| ('boy', 'Tony') | 0.0389738 | 0.0701419 | -| ('boy', 'Subtotal') | 0.285269 | 0.208683 | -| ('girl', 'Amy') | 0.364227 | 0.255748 | -| ('girl', 'Cindy') | 0.0930273 | 0.0879057 | -| ('girl', 'Dawn') | 0.0998378 | 0.134307 | -| ('girl', 'Sophia') | 0.157639 | 0.313356 | -| ('girl', 'Subtotal') | 0.714731 | 0.791317 | -| ('Total (Sum as Fraction of Columns)', '') | 1 | 1 | +| | ('SUM(num)', nan) | ('MAX(num)', nan) | +|:-------------------------------------------|--------------------:|--------------------:| +| ('boy', 'Edward') | 0.246295 | 0.138541 | +| ('boy', 'Tony') | 0.0389738 | 0.0701419 | +| ('boy', 'Subtotal') | 0.285269 | 0.208683 | +| ('girl', 'Amy') | 0.364227 | 0.255748 | +| ('girl', 'Cindy') | 0.0930273 | 0.0879057 | +| ('girl', 'Dawn') | 0.0998378 | 0.134307 | +| ('girl', 'Sophia') | 0.157639 | 0.313356 | +| ('girl', 'Subtotal') | 0.714731 | 0.791317 | +| ('Total (Sum as Fraction of Columns)', '') | 1 | 1 | """.strip() ) diff --git a/tests/unit_tests/connectors/sqla/models_test.py b/tests/unit_tests/connectors/sqla/models_test.py index 3fa32228ca48e..013d03e7e4cff 100644 --- a/tests/unit_tests/connectors/sqla/models_test.py +++ b/tests/unit_tests/connectors/sqla/models_test.py @@ -15,13 +15,14 @@ # specific language governing permissions and limitations # under the License. +import pandas as pd import pytest from pytest_mock import MockerFixture from sqlalchemy import create_engine from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.session import Session -from superset.connectors.sqla.models import SqlaTable +from superset.connectors.sqla.models import SqlaTable, TableColumn from superset.daos.dataset import DatasetDAO from superset.exceptions import OAuth2RedirectError from superset.models.core import Database @@ -263,3 +264,26 @@ def test_dataset_uniqueness(session: Session) -> None: database, Table("table", "schema", "some_catalog"), ) + + +def test_normalize_prequery_result_type_custom_sql() -> None: + """ + Test that the `_normalize_prequery_result_type` can hanndle custom SQL. + """ + sqla_table = SqlaTable( + table_name="my_sqla_table", + columns=[], + metrics=[], + database=Database(database_name="my_db", sqlalchemy_uri="sqlite://"), + ) + row: pd.Series = { + "custom_sql": "Car", + } + dimension: str = "custom_sql" + columns_by_name: dict[str, TableColumn] = { + "product_line": TableColumn(column_name="product_line"), + } + assert ( + sqla_table._normalize_prequery_result_type(row, dimension, columns_by_name) + == "Car" + )