diff --git a/Makefile b/Makefile index 85027a175f58b..ef0d3edbec13f 100644 --- a/Makefile +++ b/Makefile @@ -58,7 +58,7 @@ update-py: update-js: # Install js packages - cd superset-frontend; npm install + cd superset-frontend; npm ci venv: # Create a virtual environment and activate it (recommended) @@ -81,3 +81,9 @@ py-lint: pre-commit js-format: cd superset-frontend; npm run prettier + +flask-app: + flask run -p 8088 --with-threads --reload --debugger + +node-app: + cd superset-frontend; npm run dev-server diff --git a/docs/src/pages/docs/installation/configuring.mdx b/docs/src/pages/docs/installation/configuring.mdx index 2c141d4dd50c8..707aaeef7209e 100644 --- a/docs/src/pages/docs/installation/configuring.mdx +++ b/docs/src/pages/docs/installation/configuring.mdx @@ -201,6 +201,25 @@ CUSTOM_SECURITY_MANAGER = CustomSsoSecurityManager ] ``` +### Flask app Configuration Hook + +`FLASK_APP_MUTATOR` is a configuration function that can be provided in your environment, receives +the app object and can alter it in any way. For example, add `FLASK_APP_MUTATOR` into your +`superset_config.py` to setup session cookie expiration time to 24 hours: + +``` +def make_session_permanent(): + ''' + Enable maxAge for the cookie 'session' + ''' + session.permanent = True + +# Set up max age of session to 24 hours +PERMANENT_SESSION_LIFETIME = timedelta(hours=24) +def FLASK_APP_MUTATOR(app: Flask) -> None: + app.before_request_funcs.setdefault(None, []).append(make_session_permanent) +``` + ### Feature Flags To support a diverse set of users, Superset has some features that are not enabled by default. For diff --git a/helm/superset/Chart.yaml b/helm/superset/Chart.yaml index 8f39c30543620..30a409e083d8e 100644 --- a/helm/superset/Chart.yaml +++ b/helm/superset/Chart.yaml @@ -22,7 +22,7 @@ maintainers: - name: craig-rueda email: craig@craigrueda.com url: https://github.com/craig-rueda -version: 0.3.5 +version: 0.3.6 dependencies: - name: postgresql version: 10.2.0 diff --git a/helm/superset/templates/deployment-beat.yaml b/helm/superset/templates/deployment-beat.yaml index e5705fa08b097..2775d287498c8 100644 --- a/helm/superset/templates/deployment-beat.yaml +++ b/helm/superset/templates/deployment-beat.yaml @@ -86,6 +86,9 @@ spec: - name: superset-config mountPath: {{ .Values.configMountPath | quote }} readOnly: true + {{- with .Values.extraVolumeMounts }} + {{- tpl (toYaml .) $ | nindent 12 -}} + {{- end }} resources: {{ toYaml .Values.resources | indent 12 }} {{- with .Values.nodeSelector }} @@ -108,4 +111,7 @@ spec: - name: superset-config secret: secretName: {{ tpl .Values.configFromSecret . }} + {{- with .Values.extraVolumes }} + {{- tpl (toYaml .) $ | nindent 8 -}} + {{- end }} {{- end -}} diff --git a/helm/superset/templates/deployment-worker.yaml b/helm/superset/templates/deployment-worker.yaml index c228b35dc96d1..8bb2cc81c15d0 100644 --- a/helm/superset/templates/deployment-worker.yaml +++ b/helm/superset/templates/deployment-worker.yaml @@ -87,6 +87,9 @@ spec: - name: superset-config mountPath: {{ .Values.configMountPath | quote }} readOnly: true + {{- with .Values.extraVolumeMounts }} + {{- tpl (toYaml .) $ | nindent 12 -}} + {{- end }} resources: {{ toYaml .Values.resources | indent 12 }} {{- with .Values.nodeSelector }} @@ -109,3 +112,6 @@ spec: - name: superset-config secret: secretName: {{ tpl .Values.configFromSecret . }} + {{- with .Values.extraVolumes }} + {{- tpl (toYaml .) $ | nindent 8 -}} + {{- end }} diff --git a/helm/superset/templates/deployment.yaml b/helm/superset/templates/deployment.yaml index 919078299525f..ec6b4f453ea58 100644 --- a/helm/superset/templates/deployment.yaml +++ b/helm/superset/templates/deployment.yaml @@ -95,6 +95,9 @@ spec: mountPath: {{ .Values.extraConfigMountPath | quote }} readOnly: true {{- end }} + {{- with .Values.extraVolumeMounts }} + {{- tpl (toYaml .) $ | nindent 12 -}} + {{- end }} ports: - name: http containerPort: {{ .Values.service.port }} @@ -127,3 +130,6 @@ spec: configMap: name: {{ template "superset.fullname" . }}-extra-config {{- end }} + {{- with .Values.extraVolumes }} + {{- tpl (toYaml .) $ | nindent 8 -}} + {{- end }} diff --git a/helm/superset/templates/init-job.yaml b/helm/superset/templates/init-job.yaml index 45ab2235b7a5f..fb30abc8cef23 100644 --- a/helm/superset/templates/init-job.yaml +++ b/helm/superset/templates/init-job.yaml @@ -60,6 +60,9 @@ spec: mountPath: {{ .Values.extraConfigMountPath | quote }} readOnly: true {{- end }} + {{- with .Values.extraVolumeMounts }} + {{- tpl (toYaml .) $ | nindent 10 -}} + {{- end }} command: {{ tpl (toJson .Values.init.command) . }} resources: {{ toYaml .Values.init.resources | indent 10 }} @@ -76,5 +79,8 @@ spec: configMap: name: {{ template "superset.fullname" . }}-extra-config {{- end }} + {{- with .Values.extraVolumes }} + {{- tpl (toYaml .) $ | nindent 8 -}} + {{- end }} restartPolicy: Never {{- end }} diff --git a/helm/superset/values.yaml b/helm/superset/values.yaml index c6ad3e10ff33a..c8577774afb8e 100644 --- a/helm/superset/values.yaml +++ b/helm/superset/values.yaml @@ -82,6 +82,22 @@ extraConfigs: {} extraSecrets: {} +extraVolumes: [] + # - name: customConfig + # configMap: + # name: '{{ template "superset.fullname" . }}-custom-config' + # - name: additionalSecret + # secret: + # secretName: my-secret + # defaultMode: 0600 + +extraVolumeMounts: [] + # - name: customConfig + # mountPath: /mnt/config + # readOnly: true + # - name: additionalSecret: + # mountPath: /mnt/secret + # A dictionary of overrides to append at the end of superset_config.py - the name does not matter # WARNING: the order is not guaranteed configOverrides: {} diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 4930c8b04715d..1ef93bce0e862 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -36,8 +36,8 @@ "@superset-ui/legacy-plugin-chart-sunburst": "^0.17.85", "@superset-ui/legacy-plugin-chart-treemap": "^0.17.85", "@superset-ui/legacy-plugin-chart-world-map": "^0.17.85", - "@superset-ui/legacy-preset-chart-big-number": "^0.17.85", - "@superset-ui/legacy-preset-chart-deckgl": "^0.4.10", + "@superset-ui/legacy-preset-chart-big-number": "^0.17.86", + "@superset-ui/legacy-preset-chart-deckgl": "^0.4.11", "@superset-ui/legacy-preset-chart-nvd3": "^0.17.85", "@superset-ui/plugin-chart-echarts": "^0.17.85", "@superset-ui/plugin-chart-pivot-table": "^0.17.85", @@ -12480,9 +12480,9 @@ } }, "node_modules/@superset-ui/legacy-preset-chart-big-number": { - "version": "0.17.85", - "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-big-number/-/legacy-preset-chart-big-number-0.17.85.tgz", - "integrity": "sha512-Txuk8L/flifPESHWNpZ8vwXinHAPCXXEp2fZdB/6CRyB6TVfMs2RmKqnEyzO5xBGgqeTfdH6xeuVn+8Hfdcvrg==", + "version": "0.17.86", + "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-big-number/-/legacy-preset-chart-big-number-0.17.86.tgz", + "integrity": "sha512-b9TZ7VJ3IK+ii6VDllfOCDujLR++rYNXcnNW/XgCZP9+ZwbHBIzGqC7APdUid7qDJM/PZwN6USXSnYf5MIYPLw==", "dependencies": { "@data-ui/xy-chart": "^0.0.84", "@superset-ui/chart-controls": "0.17.85", @@ -12497,9 +12497,9 @@ } }, "node_modules/@superset-ui/legacy-preset-chart-deckgl": { - "version": "0.4.10", - "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-deckgl/-/legacy-preset-chart-deckgl-0.4.10.tgz", - "integrity": "sha512-UGgzzDjy6N+vZvHlaSbihEyblm41jS2eL/41RWAEAABrn8g27V33a1VdVR/5zLUAtjqvP/dZqqkebF0h4ebmXA==", + "version": "0.4.11", + "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-deckgl/-/legacy-preset-chart-deckgl-0.4.11.tgz", + "integrity": "sha512-N4l8zavJ3kQUyoPFqG6zXQT8EELFcXtD/GNRy3aJzSgHObRhK8+aqFaWIn3ncQrBuEiUzGDnHLNni/LRAxz7vA==", "dependencies": { "@math.gl/web-mercator": "^3.2.2", "@types/d3-array": "^2.0.0", @@ -61935,9 +61935,9 @@ } }, "@superset-ui/legacy-preset-chart-big-number": { - "version": "0.17.85", - "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-big-number/-/legacy-preset-chart-big-number-0.17.85.tgz", - "integrity": "sha512-Txuk8L/flifPESHWNpZ8vwXinHAPCXXEp2fZdB/6CRyB6TVfMs2RmKqnEyzO5xBGgqeTfdH6xeuVn+8Hfdcvrg==", + "version": "0.17.86", + "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-big-number/-/legacy-preset-chart-big-number-0.17.86.tgz", + "integrity": "sha512-b9TZ7VJ3IK+ii6VDllfOCDujLR++rYNXcnNW/XgCZP9+ZwbHBIzGqC7APdUid7qDJM/PZwN6USXSnYf5MIYPLw==", "requires": { "@data-ui/xy-chart": "^0.0.84", "@superset-ui/chart-controls": "0.17.85", @@ -61949,9 +61949,9 @@ } }, "@superset-ui/legacy-preset-chart-deckgl": { - "version": "0.4.10", - "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-deckgl/-/legacy-preset-chart-deckgl-0.4.10.tgz", - "integrity": "sha512-UGgzzDjy6N+vZvHlaSbihEyblm41jS2eL/41RWAEAABrn8g27V33a1VdVR/5zLUAtjqvP/dZqqkebF0h4ebmXA==", + "version": "0.4.11", + "resolved": "https://registry.npmjs.org/@superset-ui/legacy-preset-chart-deckgl/-/legacy-preset-chart-deckgl-0.4.11.tgz", + "integrity": "sha512-N4l8zavJ3kQUyoPFqG6zXQT8EELFcXtD/GNRy3aJzSgHObRhK8+aqFaWIn3ncQrBuEiUzGDnHLNni/LRAxz7vA==", "requires": { "@math.gl/web-mercator": "^3.2.2", "@types/d3-array": "^2.0.0", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 4b394600d6a12..20e1afc52bb4d 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -88,8 +88,8 @@ "@superset-ui/legacy-plugin-chart-sunburst": "^0.17.85", "@superset-ui/legacy-plugin-chart-treemap": "^0.17.85", "@superset-ui/legacy-plugin-chart-world-map": "^0.17.85", - "@superset-ui/legacy-preset-chart-big-number": "^0.17.85", - "@superset-ui/legacy-preset-chart-deckgl": "^0.4.10", + "@superset-ui/legacy-preset-chart-big-number": "^0.17.86", + "@superset-ui/legacy-preset-chart-deckgl": "^0.4.11", "@superset-ui/legacy-preset-chart-nvd3": "^0.17.85", "@superset-ui/plugin-chart-echarts": "^0.17.85", "@superset-ui/plugin-chart-pivot-table": "^0.17.85", diff --git a/superset-frontend/spec/fixtures/mockReportState.js b/superset-frontend/spec/fixtures/mockReportState.js new file mode 100644 index 0000000000000..075af8bfe0962 --- /dev/null +++ b/superset-frontend/spec/fixtures/mockReportState.js @@ -0,0 +1,38 @@ +/** + * 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 dashboardInfo from './mockDashboardInfo'; +import { user } from '../javascripts/sqllab/fixtures'; + +export default { + active: true, + creation_method: 'dashboards', + crontab: '0 12 * * 1', + dashboard: dashboardInfo.id, + name: 'Weekly Report', + owners: [user.userId], + recipients: [ + { + recipient_config_json: { + target: user.email, + }, + type: 'Email', + }, + ], + type: 'Report', +}; diff --git a/superset-frontend/spec/fixtures/mockStateWithoutUser.tsx b/superset-frontend/spec/fixtures/mockStateWithoutUser.tsx new file mode 100644 index 0000000000000..bc92df4df75d0 --- /dev/null +++ b/superset-frontend/spec/fixtures/mockStateWithoutUser.tsx @@ -0,0 +1,46 @@ +/** + * 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 datasources from 'spec/fixtures/mockDatasource'; +import messageToasts from 'spec/javascripts/messageToasts/mockMessageToasts'; +import { + nativeFiltersInfo, + mockDataMaskInfo, +} from 'spec/javascripts/dashboard/fixtures/mockNativeFilters'; +import chartQueries from 'spec/fixtures/mockChartQueries'; +import { dashboardLayout } from 'spec/fixtures/mockDashboardLayout'; +import dashboardInfo from 'spec/fixtures/mockDashboardInfo'; +import { emptyFilters } from 'spec/fixtures/mockDashboardFilters'; +import dashboardState from 'spec/fixtures/mockDashboardState'; +import { sliceEntitiesForChart } from 'spec/fixtures/mockSliceEntities'; +import reports from 'spec/fixtures/mockReportState'; + +export default { + datasources, + sliceEntities: sliceEntitiesForChart, + charts: chartQueries, + nativeFilters: nativeFiltersInfo, + dataMask: mockDataMaskInfo, + dashboardInfo, + dashboardFilters: emptyFilters, + dashboardState, + dashboardLayout, + messageToasts, + impressionId: 'mock_impression_id', + reports, +}; diff --git a/superset-frontend/spec/helpers/reducerIndex.ts b/superset-frontend/spec/helpers/reducerIndex.ts index e84b7f6f5119a..113368389509a 100644 --- a/superset-frontend/spec/helpers/reducerIndex.ts +++ b/superset-frontend/spec/helpers/reducerIndex.ts @@ -30,6 +30,7 @@ import saveModal from 'src/explore/reducers/saveModalReducer'; import explore from 'src/explore/reducers/exploreReducer'; import sqlLab from 'src/SqlLab/reducers/sqlLab'; import localStorageUsageInKilobytes from 'src/SqlLab/reducers/localStorageUsage'; +import reports from 'src/reports/reducers/reports'; const impressionId = (state = '') => state; @@ -53,5 +54,6 @@ export default { explore, sqlLab, localStorageUsageInKilobytes, + reports, common: () => common, }; diff --git a/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx deleted file mode 100644 index c6eda704e16ec..0000000000000 --- a/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx +++ /dev/null @@ -1,87 +0,0 @@ -/** - * 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 React from 'react'; -import { shallow } from 'enzyme'; - -import { ExploreChartHeader } from 'src/explore/components/ExploreChartHeader'; -import ExploreActionButtons from 'src/explore/components/ExploreActionButtons'; -import EditableTitle from 'src/components/EditableTitle'; - -const saveSliceStub = jest.fn(); -const updateChartTitleStub = jest.fn(); -const fetchUISpecificReportStub = jest.fn(); -const mockProps = { - actions: { - saveSlice: saveSliceStub, - updateChartTitle: updateChartTitleStub, - }, - can_overwrite: true, - can_download: true, - isStarred: true, - slice: { - form_data: { - viz_type: 'line', - }, - }, - table_name: 'foo', - form_data: { - viz_type: 'table', - }, - user: { - createdOn: '2021-04-27T18:12:38.952304', - email: 'admin', - firstName: 'admin', - isActive: true, - lastName: 'admin', - permissions: {}, - roles: { Admin: Array(173) }, - userId: 1, - username: 'admin', - }, - timeout: 1000, - chart: { - id: 0, - queryResponse: {}, - }, - fetchUISpecificReport: fetchUISpecificReportStub, - chartHeight: '30px', -}; - -describe('ExploreChartHeader', () => { - let wrapper; - beforeEach(() => { - wrapper = shallow(); - }); - - it('is valid', () => { - expect(React.isValidElement()).toBe( - true, - ); - }); - - it('renders', () => { - expect(wrapper.find(EditableTitle)).toExist(); - expect(wrapper.find(ExploreActionButtons)).toExist(); - }); - - it('should update title but not save', () => { - const editableTitle = wrapper.find(EditableTitle); - expect(editableTitle.props().onSaveTitle).toBe(updateChartTitleStub); - }); -}); diff --git a/superset-frontend/src/components/ReportModal/index.test.tsx b/superset-frontend/src/components/ReportModal/index.test.tsx index 99b1eadcc4970..44e3d0ef65c51 100644 --- a/superset-frontend/src/components/ReportModal/index.test.tsx +++ b/superset-frontend/src/components/ReportModal/index.test.tsx @@ -55,13 +55,17 @@ describe('Email Report Modal', () => { (featureFlag: FeatureFlag) => featureFlag === FeatureFlag.ALERT_REPORTS, ); }); + + beforeEach(() => { + render(, { useRedux: true }); + }); + afterAll(() => { // @ts-ignore isFeatureEnabledMock.restore(); }); - it('inputs respond correctly', () => { - render(, { useRedux: true }); + it('inputs respond correctly', () => { // ----- Report name textbox // Initial value const reportNameTextbox = screen.getByTestId('report-name-test'); @@ -86,4 +90,21 @@ describe('Email Report Modal', () => { const crontabInputs = screen.getAllByRole('combobox'); expect(crontabInputs).toHaveLength(5); }); + + it('does not allow user to create a report without a name', () => { + // Grab name textbox and add button + const reportNameTextbox = screen.getByTestId('report-name-test'); + const addButton = screen.getByRole('button', { name: /add/i }); + + // Add button should be enabled while name textbox has text + expect(reportNameTextbox).toHaveDisplayValue('Weekly Report'); + expect(addButton).toBeEnabled(); + + // Clear the text from the name textbox + userEvent.clear(reportNameTextbox); + + // Add button should now be disabled, blocking user from creation + expect(reportNameTextbox).toHaveDisplayValue(''); + expect(addButton).toBeDisabled(); + }); }); diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx index 292d3b7414b2e..e24d1d756c63f 100644 --- a/superset-frontend/src/components/ReportModal/index.tsx +++ b/superset-frontend/src/components/ReportModal/index.tsx @@ -53,7 +53,7 @@ import { StyledRadioGroup, } from './styles'; -interface ReportObject { +export interface ReportObject { id?: number; active: boolean; crontab: string; diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index 68af9380e9b42..8a9ecdb5be46f 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -19,7 +19,12 @@ import React from 'react'; import { render, screen, fireEvent } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; +import sinon from 'sinon'; import fetchMock from 'fetch-mock'; +import * as actions from 'src/reports/actions/reports'; +import * as featureFlags from 'src/featureFlags'; +import { ReportObject } from 'src/components/ReportModal'; +import mockState from 'spec/fixtures/mockStateWithoutUser'; import { HeaderProps } from './types'; import Header from '.'; @@ -40,15 +45,16 @@ const createProps = () => ({ }, user: { createdOn: '2021-04-27T18:12:38.952304', - email: 'admin', + email: 'admin@test.com', firstName: 'admin', isActive: true, lastName: 'admin', permissions: {}, - roles: { Admin: Array(173) }, + roles: { Admin: [['menu_access', 'Manage']] }, userId: 1, username: 'admin', }, + reports: {}, dashboardTitle: 'Dashboard Title', charts: {}, layout: {}, @@ -107,8 +113,10 @@ const redoProps = { redoLength: 1, }; +const REPORT_ENDPOINT = 'glob:*/api/v1/report*'; + fetchMock.get('glob:*/csstemplateasyncmodelview/api/read', {}); -fetchMock.get('glob:*/api/v1/report*', {}); +fetchMock.get(REPORT_ENDPOINT, {}); function setup(props: HeaderProps) { return ( @@ -315,3 +323,144 @@ test('should refresh the charts', async () => { userEvent.click(screen.getByText('Refresh dashboard')); expect(mockedProps.onRefresh).toHaveBeenCalledTimes(1); }); + +describe('Email Report Modal', () => { + let isFeatureEnabledMock: any; + let dispatch: any; + + beforeEach(async () => { + isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation(() => true); + dispatch = sinon.spy(); + }); + + afterAll(() => { + isFeatureEnabledMock.mockRestore(); + }); + + it('creates a new email report', async () => { + // ---------- Render/value setup ---------- + const mockedProps = createProps(); + render(setup(mockedProps), { useRedux: true }); + + const reportValues = { + active: true, + creation_method: 'dashboards', + crontab: '0 12 * * 1', + dashboard: mockedProps.dashboardInfo.id, + name: 'Weekly Report', + owners: [mockedProps.user.userId], + recipients: [ + { + recipient_config_json: { + target: mockedProps.user.email, + }, + type: 'Email', + }, + ], + type: 'Report', + }; + // This is needed to structure the reportValues to match the fetchMock return + const stringyReportValues = `{"active":true,"creation_method":"dashboards","crontab":"0 12 * * 1","dashboard":${mockedProps.dashboardInfo.id},"name":"Weekly Report","owners":[${mockedProps.user.userId}],"recipients":[{"recipient_config_json":{"target":"${mockedProps.user.email}"},"type":"Email"}],"type":"Report"}`; + // Watch for report POST + fetchMock.post(REPORT_ENDPOINT, reportValues); + + screen.logTestingPlaygroundURL(); + // ---------- Begin tests ---------- + // Click calendar icon to open email report modal + const emailReportModalButton = screen.getByRole('button', { + name: /schedule email report/i, + }); + userEvent.click(emailReportModalButton); + + // Click "Add" button to create a new email report + const addButton = screen.getByRole('button', { name: /add/i }); + userEvent.click(addButton); + + // Mock addReport from Redux + const makeRequest = () => { + const request = actions.addReport(reportValues as ReportObject); + return request(dispatch); + }; + + return makeRequest().then(() => { + // 🐞 ----- There are 2 POST calls at this point ----- 🐞 + + // addReport's mocked POST return should match the mocked values + expect(fetchMock.lastOptions()?.body).toEqual(stringyReportValues); + // Dispatch should be called once for addReport + expect(dispatch.callCount).toBe(2); + const reportCalls = fetchMock.calls(REPORT_ENDPOINT); + expect(reportCalls).toHaveLength(2); + }); + }); + + it('edits an existing email report', async () => { + // TODO (lyndsiWilliams): This currently does not work, see TODOs below + // The modal does appear with the edit title, but the PUT call is not registering + + // ---------- Render/value setup ---------- + const mockedProps = createProps(); + const editedReportValues = { + active: true, + creation_method: 'dashboards', + crontab: '0 12 * * 1', + dashboard: mockedProps.dashboardInfo.id, + name: 'Weekly Report edit', + owners: [mockedProps.user.userId], + recipients: [ + { + recipient_config_json: { + target: mockedProps.user.email, + }, + type: 'Email', + }, + ], + type: 'Report', + }; + + // getMockStore({ reports: reportValues }); + render(setup(mockedProps), { + useRedux: true, + initialState: mockState, + }); + // TODO (lyndsiWilliams): currently fetchMock detects this PUT + // address as 'glob:*/api/v1/report/undefined', is not detected + // on fetchMock.calls() + fetchMock.put(`glob:*/api/v1/report*`, editedReportValues); + + // Mock fetchUISpecificReport from Redux + // const makeFetchRequest = () => { + // const request = actions.fetchUISpecificReport( + // mockedProps.user.userId, + // 'dashboard_id', + // 'dashboards', + // mockedProps.dashboardInfo.id, + // ); + // return request(dispatch); + // }; + + // makeFetchRequest(); + + dispatch(actions.setReport(editedReportValues)); + + // ---------- Begin tests ---------- + // Click calendar icon to open email report modal + const emailReportModalButton = screen.getByRole('button', { + name: /schedule email report/i, + }); + userEvent.click(emailReportModalButton); + + const nameTextbox = screen.getByTestId('report-name-test'); + userEvent.type(nameTextbox, ' edit'); + + const saveButton = screen.getByRole('button', { name: /save/i }); + userEvent.click(saveButton); + + // TODO (lyndsiWilliams): There should be a report in state at this porint, + // which would render the HeaderReportActionsDropDown under the calendar icon + // BLOCKER: I cannot get report to populate, as its data is handled through redux + expect.anything(); + }); +}); diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx index c59b6ad70b8fa..e11b8310bb75c 100644 --- a/superset-frontend/src/datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/datasource/DatasourceEditor.jsx @@ -324,15 +324,19 @@ class DatasourceEditor extends React.PureComponent { datasource: { ...props.datasource, metrics: props.datasource.metrics?.map(metric => { + const { + certified_by: certifiedByMetric, + certification_details: certificationDetails, + } = metric; const { certification: { details, certified_by: certifiedBy } = {}, warning_markdown: warningMarkdown, } = JSON.parse(metric.extra || '{}') || {}; return { ...metric, - certification_details: details || '', + certification_details: certificationDetails || details, warning_markdown: warningMarkdown || '', - certified_by: certifiedBy, + certified_by: certifiedBy || certifiedByMetric, }; }), }, @@ -493,11 +497,14 @@ class DatasourceEditor extends React.PureComponent { schema_name: datasource.schema, table_name: datasource.table_name, }; - const endpoint = `/datasource/external_metadata_by_name/?q=${rison.encode( + Object.entries(params).forEach(([key, value]) => { // rison can't encode the undefined value - Object.keys(params).map(key => - params[key] === undefined ? null : params[key], - ), + if (value === undefined) { + params[key] = null; + } + }); + const endpoint = `/datasource/external_metadata_by_name/?q=${rison.encode( + params, )}`; this.setState({ metadataLoading: true }); @@ -932,7 +939,6 @@ class DatasourceEditor extends React.PureComponent { const { datasource } = this.state; const { metrics } = datasource; const sortedMetrics = metrics?.length ? this.sortMetrics(metrics) : []; - return ( { export const ADD_REPORT = 'ADD_REPORT'; -export const addReport = report => dispatch => { +export const addReport = report => dispatch => SupersetClient.post({ endpoint: `/api/v1/report/`, jsonPayload: report, @@ -111,14 +111,13 @@ export const addReport = report => dispatch => { const parsedError = await getClientErrorObject(e); const errorMessage = parsedError.message; const errorArr = Object.keys(errorMessage); - const error = errorMessage[errorArr[0]][0]; + const error = errorMessage[errorArr[0]]; dispatch( addDangerToast( t('An error occurred while editing this report: %s', error), ), ); }); -}; export const EDIT_REPORT = 'EDIT_REPORT'; diff --git a/superset/charts/commands/importers/v1/__init__.py b/superset/charts/commands/importers/v1/__init__.py index 0e2b5b3a8adf7..5c2b535a7b34a 100644 --- a/superset/charts/commands/importers/v1/__init__.py +++ b/superset/charts/commands/importers/v1/__init__.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. +import json from typing import Any, Dict, Set from marshmallow import Schema @@ -95,4 +96,10 @@ def _import( } ) config["params"].update({"datasource": dataset.uid}) + if config["query_context"]: + # TODO (betodealmeida): export query_context as object, not string + query_context = json.loads(config["query_context"]) + query_context["datasource"] = {"id": dataset.id, "type": "table"} + config["query_context"] = json.dumps(query_context) + import_chart(session, config, overwrite=overwrite) diff --git a/superset/charts/post_processing.py b/superset/charts/post_processing.py index 9ef976bdec62b..3ad0a8570c6fe 100644 --- a/superset/charts/post_processing.py +++ b/superset/charts/post_processing.py @@ -296,7 +296,9 @@ def apply_post_process( query["coltypes"] = extract_dataframe_dtypes(processed_df) query["rowcount"] = len(processed_df.index) - # flatten columns/index so we can encode data as JSON + # Flatten hierarchical columns/index since they are represented as + # `Tuple[str]`. Otherwise encoding to JSON later will fail because + # maps cannot have tuples as their keys in JSON. processed_df.columns = [ " ".join(str(name) for name in column).strip() if isinstance(column, tuple) diff --git a/superset/dashboards/commands/importers/v1/__init__.py b/superset/dashboards/commands/importers/v1/__init__.py index 6e17aef9e5431..1720e01ab8bcc 100644 --- a/superset/dashboards/commands/importers/v1/__init__.py +++ b/superset/dashboards/commands/importers/v1/__init__.py @@ -67,7 +67,9 @@ def _import( for file_name, config in configs.items(): if file_name.startswith("dashboards/"): chart_uuids.update(find_chart_uuids(config["position"])) - dataset_uuids.update(find_native_filter_datasets(config["metadata"])) + dataset_uuids.update( + find_native_filter_datasets(config.get("metadata", {})) + ) # discover datasets associated with charts for file_name, config in configs.items(): diff --git a/superset/examples/deck.py b/superset/examples/deck.py index c29413e2c5051..a3d137bb06d71 100644 --- a/superset/examples/deck.py +++ b/superset/examples/deck.py @@ -287,6 +287,7 @@ def load_deck_dash() -> None: slices.append(slc) slice_data = { + "autozoom": False, "spatial": {"type": "latlong", "lonCol": "LON", "latCol": "LAT"}, "row_limit": 5000, "mapbox_style": "mapbox://styles/mapbox/satellite-streets-v9", diff --git a/superset/reports/commands/base.py b/superset/reports/commands/base.py index bb4064d22cde7..3582767ef65f2 100644 --- a/superset/reports/commands/base.py +++ b/superset/reports/commands/base.py @@ -22,9 +22,12 @@ from superset.charts.dao import ChartDAO from superset.commands.base import BaseCommand from superset.dashboards.dao import DashboardDAO +from superset.models.reports import ReportCreationMethodType from superset.reports.commands.exceptions import ( ChartNotFoundValidationError, + ChartNotSavedValidationError, DashboardNotFoundValidationError, + DashboardNotSavedValidationError, ReportScheduleChartOrDashboardValidationError, ) @@ -47,6 +50,17 @@ def validate_chart_dashboard( """ Validate chart or dashboard relation """ chart_id = self._properties.get("chart") dashboard_id = self._properties.get("dashboard") + creation_method = self._properties.get("creation_method") + + if creation_method == ReportCreationMethodType.CHARTS and not chart_id: + # User has not saved chart yet in Explore view + exceptions.append(ChartNotSavedValidationError()) + return + + if creation_method == ReportCreationMethodType.DASHBOARDS and not dashboard_id: + exceptions.append(DashboardNotSavedValidationError()) + return + if chart_id and dashboard_id: exceptions.append(ReportScheduleChartOrDashboardValidationError()) if chart_id: diff --git a/superset/reports/commands/exceptions.py b/superset/reports/commands/exceptions.py index c59ba3e8b4c95..41b2e5605ca69 100644 --- a/superset/reports/commands/exceptions.py +++ b/superset/reports/commands/exceptions.py @@ -80,6 +80,32 @@ def __init__(self) -> None: super().__init__(_("Choose a chart or dashboard not both"), field_name="chart") +class ChartNotSavedValidationError(ValidationError): + """ + Marshmallow validation error for charts that haven't been saved yet + """ + + def __init__(self) -> None: + super().__init__( + _("Please save your chart first, then try creating a new email report."), + field_name="chart", + ) + + +class DashboardNotSavedValidationError(ValidationError): + """ + Marshmallow validation error for dashboards that haven't been saved yet + """ + + def __init__(self) -> None: + super().__init__( + _( + "Please save your dashboard first, then try creating a new email report." + ), + field_name="dashboard", + ) + + class ReportScheduleInvalidError(CommandInvalidError): message = _("Report Schedule parameters are invalid.") diff --git a/superset/utils/csv.py b/superset/utils/csv.py index 3c362f7fac432..42d2c557832e9 100644 --- a/superset/utils/csv.py +++ b/superset/utils/csv.py @@ -96,10 +96,14 @@ def get_chart_dataframe( result = simplejson.loads(content.decode("utf-8")) df = pd.DataFrame.from_dict(result["result"][0]["data"]) + + # rebuild hierarchical columns and index df.columns = pd.MultiIndex.from_tuples( - tuple(colname) for colname in result["result"][0]["colnames"] + tuple(colname) if isinstance(colname, list) else (colname,) + for colname in result["result"][0]["colnames"] ) df.index = pd.MultiIndex.from_tuples( - tuple(indexname) for indexname in result["result"][0]["indexnames"] + tuple(indexname) if isinstance(indexname, list) else (indexname,) + for indexname in result["result"][0]["indexnames"] ) return df diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py index cd6e01f3dc1d4..238a54e29c7f4 100644 --- a/tests/integration_tests/charts/commands_tests.py +++ b/tests/integration_tests/charts/commands_tests.py @@ -191,6 +191,34 @@ def test_import_v1_chart(self): ) assert dataset.table_name == "imported_dataset" assert chart.table == dataset + assert json.loads(chart.query_context) == { + "datasource": {"id": dataset.id, "type": "table"}, + "force": False, + "queries": [ + { + "time_range": " : ", + "filters": [], + "extras": { + "time_grain_sqla": None, + "having": "", + "having_druid": [], + "where": "", + }, + "applied_time_extras": {}, + "columns": [], + "metrics": [], + "annotation_layers": [], + "row_limit": 5000, + "timeseries_limit": 0, + "order_desc": True, + "url_params": {}, + "custom_params": {}, + "custom_form_data": {}, + } + ], + "result_format": "json", + "result_type": "full", + } database = ( db.session.query(Database).filter_by(uuid=database_config["uuid"]).one() diff --git a/tests/integration_tests/fixtures/importexport.py b/tests/integration_tests/fixtures/importexport.py index 951ecf9bb4350..78f643c587af1 100644 --- a/tests/integration_tests/fixtures/importexport.py +++ b/tests/integration_tests/fixtures/importexport.py @@ -444,6 +444,7 @@ }, "viz_type": "deck_path", }, + "query_context": '{"datasource":{"id":12,"type":"table"},"force":false,"queries":[{"time_range":" : ","filters":[],"extras":{"time_grain_sqla":null,"having":"","having_druid":[],"where":""},"applied_time_extras":{},"columns":[],"metrics":[],"annotation_layers":[],"row_limit":5000,"timeseries_limit":0,"order_desc":true,"url_params":{},"custom_params":{},"custom_form_data":{}}],"result_format":"json","result_type":"full"}', "cache_timeout": None, "uuid": "0c23747a-6528-4629-97bf-e4b78d3b9df1", "version": "1.0.0", diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py index 3c306490d1389..55fce65162ea9 100644 --- a/tests/integration_tests/reports/api_tests.py +++ b/tests/integration_tests/reports/api_tests.py @@ -734,6 +734,62 @@ def test_create_report_schedule_schema(self): assert data["result"]["timezone"] == "America/Los_Angeles" assert rv.status_code == 201 + @pytest.mark.usefixtures( + "load_birth_names_dashboard_with_slices", "create_report_schedules" + ) + def test_unsaved_report_schedule_schema(self): + """ + ReportSchedule Api: Test create report schedule with unsaved chart + """ + self.login(username="admin") + chart = db.session.query(Slice).first() + dashboard = db.session.query(Dashboard).first() + example_db = get_example_database() + + report_schedule_data = { + "type": ReportScheduleType.REPORT, + "name": "name3", + "description": "description", + "creation_method": ReportCreationMethodType.CHARTS, + "crontab": "0 9 * * *", + "chart": 0, + } + uri = "api/v1/report/" + rv = self.client.post(uri, json=report_schedule_data) + data = json.loads(rv.data.decode("utf-8")) + assert rv.status_code == 422 + assert ( + data["message"]["chart"] + == "Please save your chart first, then try creating a new email report." + ) + + @pytest.mark.usefixtures( + "load_birth_names_dashboard_with_slices", "create_report_schedules" + ) + def test_no_dashboard_report_schedule_schema(self): + """ + ReportSchedule Api: Test create report schedule with not dashboard id + """ + self.login(username="admin") + chart = db.session.query(Slice).first() + dashboard = db.session.query(Dashboard).first() + example_db = get_example_database() + report_schedule_data = { + "type": ReportScheduleType.REPORT, + "name": "name3", + "description": "description", + "creation_method": ReportCreationMethodType.DASHBOARDS, + "crontab": "0 9 * * *", + } + uri = "api/v1/report/" + rv = self.client.post(uri, json=report_schedule_data) + data = json.loads(rv.data.decode("utf-8")) + assert rv.status_code == 422 + assert ( + data["message"]["dashboard"] + == "Please save your dashboard first, then try creating a new email report." + ) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_create_report_schedule_chart_dash_validation(self): """