From e167798b62e3669bc2a5258e873c3bd2c3739048 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Sun, 8 Aug 2021 20:57:50 -0500 Subject: [PATCH 01/10] Email Report Modal validation testing --- .../src/components/ReportModal/index.test.tsx | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/components/ReportModal/index.test.tsx b/superset-frontend/src/components/ReportModal/index.test.tsx index 27488dcdff155..79f960b8808ab 100644 --- a/superset-frontend/src/components/ReportModal/index.test.tsx +++ b/superset-frontend/src/components/ReportModal/index.test.tsx @@ -48,13 +48,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'); @@ -79,4 +83,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(); + }); }); From 82753844596e23178ffde4fd9e176735eaffa366 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 9 Aug 2021 11:29:55 -0500 Subject: [PATCH 02/10] Starting RTL testing for email report --- .../components/ExploreChartHeader_spec.jsx | 12 +++++ .../components/Header/Header.test.tsx | 47 +++++++++++++++++++ .../src/dashboard/components/Header/index.jsx | 21 ++++----- 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx index c6eda704e16ec..89023803aaca5 100644 --- a/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx @@ -1,3 +1,5 @@ +/* eslint-disable no-only-tests/no-only-tests */ +/* eslint-disable jest/no-focused-tests */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -17,6 +19,8 @@ * under the License. */ import React from 'react'; +import { render, screen } from 'spec/helpers/testing-library'; +// import userEvent from '@testing-library/user-event'; import { shallow } from 'enzyme'; import { ExploreChartHeader } from 'src/explore/components/ExploreChartHeader'; @@ -85,3 +89,11 @@ describe('ExploreChartHeader', () => { expect(editableTitle.props().onSaveTitle).toBe(updateChartTitleStub); }); }); + +describe.only('RTL', () => { + it('renders the calendar icon', () => { + render(, { useRedux: true }); + screen.logTestingPlaygroundURL(); + expect.anything(); + }); +}); diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index 68af9380e9b42..fb926c7f8afc6 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -1,3 +1,5 @@ +/* eslint-disable no-only-tests/no-only-tests */ +/* eslint-disable jest/no-focused-tests */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -20,6 +22,8 @@ import React from 'react'; import { render, screen, fireEvent } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import fetchMock from 'fetch-mock'; +import * as featureFlags from 'src/featureFlags'; +import Icons from 'src/components/Icons'; import { HeaderProps } from './types'; import Header from '.'; @@ -315,3 +319,46 @@ test('should refresh the charts', async () => { userEvent.click(screen.getByText('Refresh dashboard')); expect(mockedProps.onRefresh).toHaveBeenCalledTimes(1); }); + +describe('Email Report Modal', () => { + let isFeatureEnabledMock: any; + beforeEach(async () => { + isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation(() => true); + }); + + afterAll(() => { + isFeatureEnabledMock.mockRestore(); + }); + + it.only('Email Report Modal-create', async () => { + const mockedProps = createProps(); + const mockedUserPermsProps = { + ...mockedProps, + canAddReports: true, + }; + render( +
+
+ {}} + data-test="schedule-email-report-button-test" + > + + +
+
, + ); + // await openActionsDropdown(); + const emailReportModalButton = screen.getByRole('button'); + screen.debug(emailReportModalButton); + + screen.logTestingPlaygroundURL(); + expect.anything(); + }); +}); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 8d396e371ac9d..d17a86124652e 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -393,17 +393,16 @@ class Header extends React.PureComponent { deleteActiveReport={this.props.deleteActiveReport} /> ) : ( - <> - - - - + + + )) ); } From db16c25b7646c406782783e9c755b987b7c8984e Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 9 Aug 2021 16:18:17 -0500 Subject: [PATCH 03/10] Calendar icon now rendering! --- .../components/Header/Header.test.tsx | 23 ++++++------------- .../src/dashboard/components/Header/index.jsx | 15 +++++++++--- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index fb926c7f8afc6..e2bb8c84d58b4 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -23,7 +23,6 @@ import { render, screen, fireEvent } from 'spec/helpers/testing-library'; import userEvent from '@testing-library/user-event'; import fetchMock from 'fetch-mock'; import * as featureFlags from 'src/featureFlags'; -import Icons from 'src/components/Icons'; import { HeaderProps } from './types'; import Header from '.'; @@ -49,10 +48,11 @@ const createProps = () => ({ isActive: true, lastName: 'admin', permissions: {}, - roles: { Admin: Array(173) }, + roles: { Admin: [['can_add', 'AlertModelView']] }, userId: 1, username: 'admin', }, + reports: {}, dashboardTitle: 'Dashboard Title', charts: {}, layout: {}, @@ -332,7 +332,7 @@ describe('Email Report Modal', () => { isFeatureEnabledMock.mockRestore(); }); - it.only('Email Report Modal-create', async () => { + it.only('creates a new email report', async () => { const mockedProps = createProps(); const mockedUserPermsProps = { ...mockedProps, @@ -340,22 +340,13 @@ describe('Email Report Modal', () => { }; render(
-
- {}} - data-test="schedule-email-report-button-test" - > - - -
+
, ); // await openActionsDropdown(); - const emailReportModalButton = screen.getByRole('button'); + const emailReportModalButton = screen.getByTestId( + 'schedule-email-report-button-test', + ); screen.debug(emailReportModalButton); screen.logTestingPlaygroundURL(); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index d17a86124652e..d708153a358dd 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -384,6 +384,8 @@ class Header extends React.PureComponent { renderReportModal() { const attachedReportExists = !!Object.keys(this.props.reports).length; const canAddReports = isFeatureEnabled(FeatureFlag.ALERT_REPORTS); + console.warn('canAddReports', canAddReports); + console.warn('attachedReportExists', attachedReportExists); return ( (canAddReports || null) && (attachedReportExists ? ( @@ -415,10 +417,14 @@ class Header extends React.PureComponent { } const roles = Object.keys(user.roles || []); const permissions = roles.map(key => - user.roles[key].filter( - perms => perms[0] === 'can_add' && perms[1] === 'AlertModelView', - ), + user.roles[key].filter(perms => { + console.log('perms', perms); + return perms[0] === 'can_add' && perms[1] === 'AlertModelView'; + }), ); + console.warn('permissions', permissions); + console.warn('roles', roles); + console.warn('user.roles', user.roles); return permissions[0].length > 0; } @@ -454,6 +460,9 @@ class Header extends React.PureComponent { const userCanShare = dashboardInfo.dash_share_perm; const userCanSaveAs = dashboardInfo.dash_save_perm; const shouldShowReport = !editMode && this.canAddReports(); + console.warn('shouldShowReport', shouldShowReport); + console.warn('editMode', editMode); + console.warn('this.canAddReports', this.canAddReports()); const refreshLimit = dashboardInfo.common.conf.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT; const refreshWarning = From 47254ab6ee82a031c1e713018a37572ba809eb88 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 11 Aug 2021 17:50:27 -0500 Subject: [PATCH 04/10] Create report testing in dashboard --- .../spec/fixtures/mockReportState.js | 38 +++++ .../spec/fixtures/mockStateWithoutUser.tsx | 46 ++++++ .../spec/helpers/reducerIndex.ts | 2 + .../components/ExploreChartHeader_spec.jsx | 28 +++- .../src/components/ReportModal/index.tsx | 2 +- .../components/Header/Header.test.tsx | 138 ++++++++++++++++-- .../src/dashboard/components/Header/index.jsx | 15 +- .../src/reports/actions/reports.js | 3 +- 8 files changed, 242 insertions(+), 30 deletions(-) create mode 100644 superset-frontend/spec/fixtures/mockReportState.js create mode 100644 superset-frontend/spec/fixtures/mockStateWithoutUser.tsx 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 index 89023803aaca5..54e96c24aa869 100644 --- a/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx @@ -21,6 +21,10 @@ import React from 'react'; import { render, screen } 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 { shallow } from 'enzyme'; import { ExploreChartHeader } from 'src/explore/components/ExploreChartHeader'; @@ -49,15 +53,16 @@ const mockProps = { }, 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: [['can_add', 'AlertModelView']] }, userId: 1, username: 'admin', }, + reports: {}, timeout: 1000, chart: { id: 0, @@ -90,7 +95,24 @@ describe('ExploreChartHeader', () => { }); }); -describe.only('RTL', () => { +describe('RTL', () => { + // TODO (lyndsiWilliams): Cannot get ExploreChartHeader to render at all - errors in both RTL and enzyme + // ERROR: TypeError: Cannot read property 'datasource' of undefined (also cannot find 'userId' nor 'find') + // Possible an issue mocking redux/state? + + let isFeatureEnabledMock; + // let dispatch; + beforeEach(async () => { + isFeatureEnabledMock = jest + .spyOn(featureFlags, 'isFeatureEnabled') + .mockImplementation(() => true); + // dispatch = sinon.spy(); + }); + + afterAll(() => { + isFeatureEnabledMock.mockRestore(); + }); + it('renders the calendar icon', () => { render(, { useRedux: true }); screen.logTestingPlaygroundURL(); diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx index 148b03376ebc0..d7d53a2f40b87 100644 --- a/superset-frontend/src/components/ReportModal/index.tsx +++ b/superset-frontend/src/components/ReportModal/index.tsx @@ -47,7 +47,7 @@ import { SectionHeaderStyle, } 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 e2bb8c84d58b4..f0176c385d8d7 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -21,8 +21,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 '.'; @@ -43,7 +47,7 @@ const createProps = () => ({ }, user: { createdOn: '2021-04-27T18:12:38.952304', - email: 'admin', + email: 'admin@test.com', firstName: 'admin', isActive: true, lastName: 'admin', @@ -111,8 +115,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 ( @@ -322,32 +328,140 @@ test('should refresh the charts', async () => { 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.only('creates a new email report', async () => { + it('creates a new email report', async () => { + // ---------- Render/value setup ---------- const mockedProps = createProps(); - const mockedUserPermsProps = { - ...mockedProps, - canAddReports: true, + 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', }; - render( -
-
-
, + // 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); + + // ---------- Begin tests ---------- + // Click calendar icon to open email report modal + const emailReportModalButton = screen.getByTestId( + 'schedule-email-report-button-test', ); - // await openActionsDropdown(); + 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.getByTestId( 'schedule-email-report-button-test', ); - screen.debug(emailReportModalButton); + userEvent.click(emailReportModalButton); + + const nameTextbox = screen.getByTestId('report-name-test'); + userEvent.type(nameTextbox, ' edit'); + + const addButton = screen.getByRole('button', { name: /add/i }); + userEvent.click(addButton); + + // 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 screen.logTestingPlaygroundURL(); expect.anything(); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index d708153a358dd..d17a86124652e 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -384,8 +384,6 @@ class Header extends React.PureComponent { renderReportModal() { const attachedReportExists = !!Object.keys(this.props.reports).length; const canAddReports = isFeatureEnabled(FeatureFlag.ALERT_REPORTS); - console.warn('canAddReports', canAddReports); - console.warn('attachedReportExists', attachedReportExists); return ( (canAddReports || null) && (attachedReportExists ? ( @@ -417,14 +415,10 @@ class Header extends React.PureComponent { } const roles = Object.keys(user.roles || []); const permissions = roles.map(key => - user.roles[key].filter(perms => { - console.log('perms', perms); - return perms[0] === 'can_add' && perms[1] === 'AlertModelView'; - }), + user.roles[key].filter( + perms => perms[0] === 'can_add' && perms[1] === 'AlertModelView', + ), ); - console.warn('permissions', permissions); - console.warn('roles', roles); - console.warn('user.roles', user.roles); return permissions[0].length > 0; } @@ -460,9 +454,6 @@ class Header extends React.PureComponent { const userCanShare = dashboardInfo.dash_share_perm; const userCanSaveAs = dashboardInfo.dash_save_perm; const shouldShowReport = !editMode && this.canAddReports(); - console.warn('shouldShowReport', shouldShowReport); - console.warn('editMode', editMode); - console.warn('this.canAddReports', this.canAddReports()); const refreshLimit = dashboardInfo.common.conf.SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT; const refreshWarning = diff --git a/superset-frontend/src/reports/actions/reports.js b/superset-frontend/src/reports/actions/reports.js index 04073837aad58..fd25b16da09a1 100644 --- a/superset-frontend/src/reports/actions/reports.js +++ b/superset-frontend/src/reports/actions/reports.js @@ -97,7 +97,7 @@ const structureFetchAction = (dispatch, getState) => { 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,7 +111,6 @@ export const addReport = report => dispatch => { addDangerToast(t('An error occurred while creating this report.')), ), ); -}; export const EDIT_REPORT = 'EDIT_REPORT'; From 7c8e799047c7edd430b60b58b4057ac5f651228e Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 16 Aug 2021 11:55:29 -0500 Subject: [PATCH 05/10] make linter happy --- .../javascripts/explore/components/ExploreChartHeader_spec.jsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx b/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx index 54e96c24aa869..0030f318115d1 100644 --- a/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx @@ -1,5 +1,3 @@ -/* eslint-disable no-only-tests/no-only-tests */ -/* eslint-disable jest/no-focused-tests */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file From d3cef2f8db0a2a6ff9fada68f4ae6669188193f1 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 16 Aug 2021 11:59:04 -0500 Subject: [PATCH 06/10] Fixing weird error --- superset-frontend/src/reports/actions/reports.js | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/reports/actions/reports.js b/superset-frontend/src/reports/actions/reports.js index fda4167795c22..843bc51a3bf54 100644 --- a/superset-frontend/src/reports/actions/reports.js +++ b/superset-frontend/src/reports/actions/reports.js @@ -118,7 +118,6 @@ export const addReport = report => dispatch => ), ); }); -}; export const EDIT_REPORT = 'EDIT_REPORT'; From 00616cca0365d1520db968775547b411568d3429 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 18 Aug 2021 12:28:27 -0500 Subject: [PATCH 07/10] Removed ExploreChartHeader_spec --- .../components/ExploreChartHeader_spec.jsx | 119 ------------------ 1 file changed, 119 deletions(-) delete mode 100644 superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx 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 0030f318115d1..0000000000000 --- a/superset-frontend/spec/javascripts/explore/components/ExploreChartHeader_spec.jsx +++ /dev/null @@ -1,119 +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 { render, screen } 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 { 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@test.com', - firstName: 'admin', - isActive: true, - lastName: 'admin', - permissions: {}, - roles: { Admin: [['can_add', 'AlertModelView']] }, - userId: 1, - username: 'admin', - }, - reports: {}, - 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); - }); -}); - -describe('RTL', () => { - // TODO (lyndsiWilliams): Cannot get ExploreChartHeader to render at all - errors in both RTL and enzyme - // ERROR: TypeError: Cannot read property 'datasource' of undefined (also cannot find 'userId' nor 'find') - // Possible an issue mocking redux/state? - - let isFeatureEnabledMock; - // let dispatch; - beforeEach(async () => { - isFeatureEnabledMock = jest - .spyOn(featureFlags, 'isFeatureEnabled') - .mockImplementation(() => true); - // dispatch = sinon.spy(); - }); - - afterAll(() => { - isFeatureEnabledMock.mockRestore(); - }); - - it('renders the calendar icon', () => { - render(, { useRedux: true }); - screen.logTestingPlaygroundURL(); - expect.anything(); - }); -}); From 040156159af56ca7523d5140763de494c665ca74 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 18 Aug 2021 16:11:23 -0500 Subject: [PATCH 08/10] Fixed dashboard header test --- .../src/dashboard/components/Header/Header.test.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index f0176c385d8d7..2aaeba4680820 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -52,7 +52,7 @@ const createProps = () => ({ isActive: true, lastName: 'admin', permissions: {}, - roles: { Admin: [['can_add', 'AlertModelView']] }, + roles: { Admin: [['menu_access', 'Manage']] }, userId: 1, username: 'admin', }, @@ -456,14 +456,12 @@ describe('Email Report Modal', () => { const nameTextbox = screen.getByTestId('report-name-test'); userEvent.type(nameTextbox, ' edit'); - const addButton = screen.getByRole('button', { name: /add/i }); - userEvent.click(addButton); + 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 - - screen.logTestingPlaygroundURL(); expect.anything(); }); }); From d6941575d2b490b2237b01278f5e09630fc1e586 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Thu, 19 Aug 2021 10:17:47 -0700 Subject: [PATCH 09/10] revert changes from merge --- .../src/dashboard/components/Header/index.jsx | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index 2ac678fdebfe2..9198601ae685d 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -402,27 +402,24 @@ class Header extends React.PureComponent { renderReportModal() { const attachedReportExists = !!Object.keys(this.props.reports).length; - const canAddReports = isFeatureEnabled(FeatureFlag.ALERT_REPORTS); - return ( - (canAddReports || null) && - (attachedReportExists ? ( - - ) : ( + return attachedReportExists ? ( + + ) : ( + <> - )) + ); } From 1607fc6fdf59d010f81d8f3b2584670798028af9 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Thu, 19 Aug 2021 13:11:04 -0500 Subject: [PATCH 10/10] Fix tests --- .../dashboard/components/Header/Header.test.tsx | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index 2aaeba4680820..8a9ecdb5be46f 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -1,5 +1,3 @@ -/* eslint-disable no-only-tests/no-only-tests */ -/* eslint-disable jest/no-focused-tests */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -368,11 +366,12 @@ describe('Email Report Modal', () => { // Watch for report POST fetchMock.post(REPORT_ENDPOINT, reportValues); + screen.logTestingPlaygroundURL(); // ---------- Begin tests ---------- // Click calendar icon to open email report modal - const emailReportModalButton = screen.getByTestId( - 'schedule-email-report-button-test', - ); + const emailReportModalButton = screen.getByRole('button', { + name: /schedule email report/i, + }); userEvent.click(emailReportModalButton); // Click "Add" button to create a new email report @@ -448,9 +447,9 @@ describe('Email Report Modal', () => { // ---------- Begin tests ---------- // Click calendar icon to open email report modal - const emailReportModalButton = screen.getByTestId( - 'schedule-email-report-button-test', - ); + const emailReportModalButton = screen.getByRole('button', { + name: /schedule email report/i, + }); userEvent.click(emailReportModalButton); const nameTextbox = screen.getByTestId('report-name-test');