From 657249250e436201b6dcf4db59fca5980b8eef81 Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Fri, 20 Aug 2021 13:09:40 -0400 Subject: [PATCH 1/9] code dry (#16358) --- .../HeaderReportActionsDropdown/index.tsx | 20 +------------------ .../src/dashboard/components/Header/index.jsx | 7 ------- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx index f9fdca066556e..e2b0540be5e5e 100644 --- a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx +++ b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState, useEffect, useRef } from 'react'; +import React, { useState, useEffect } from 'react'; import { useSelector, useDispatch } from 'react-redux'; import { t, SupersetTheme, css, useTheme } from '@superset-ui/core'; import Icons from 'src/components/Icons'; @@ -61,7 +61,6 @@ export default function HeaderReportActionsDropDown({ ] = useState(null); const theme = useTheme(); const [showModal, setShowModal] = useState(false); - const dashboardIdRef = useRef(dashboardId); const toggleActiveKey = async (data: AlertObject, checked: boolean) => { if (data?.id) { toggleActive(data, checked); @@ -103,23 +102,6 @@ export default function HeaderReportActionsDropDown({ } }, []); - useEffect(() => { - if ( - canAddReports() && - dashboardId && - dashboardId !== dashboardIdRef.current - ) { - dispatch( - fetchUISpecificReport({ - userId: user.userId, - filterField: 'dashboard_id', - creationMethod: 'dashboards', - resourceId: dashboardId, - }), - ); - } - }, [dashboardId]); - const menu = () => ( diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index e0a686629057f..fa1e6a1493f19 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -166,13 +166,6 @@ class Header extends React.PureComponent { this.startPeriodicRender(refreshFrequency * 1000); } - componentDidUpdate(prevProps) { - if (this.props.refreshFrequency !== prevProps.refreshFrequency) { - const { refreshFrequency } = this.props; - this.startPeriodicRender(refreshFrequency * 1000); - } - } - UNSAFE_componentWillReceiveProps(nextProps) { if ( UNDO_LIMIT - nextProps.undoLength <= 0 && From b8ef6e0ec26d126be4b5551dd0ceabf0baba54b1 Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Wed, 18 Aug 2021 15:58:10 -0400 Subject: [PATCH 2/9] pexdax refactor (#16333) --- .../explore/components/ExploreChartHeader.jsx | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/superset-frontend/src/explore/components/ExploreChartHeader.jsx b/superset-frontend/src/explore/components/ExploreChartHeader.jsx index 6bb61c8d32ded..feb174d85afde 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader.jsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader.jsx @@ -103,6 +103,24 @@ export class ExploreChartHeader extends React.PureComponent { }; this.openPropertiesModal = this.openPropertiesModal.bind(this); this.closePropertiesModal = this.closePropertiesModal.bind(this); +<<<<<<< HEAD +======= + this.showReportModal = this.showReportModal.bind(this); + this.hideReportModal = this.hideReportModal.bind(this); + } + + componentDidMount() { + if (this.canAddReports()) { + const { user, chart } = this.props; + // this is in the case that there is an anonymous user. + this.props.fetchUISpecificReport( + user.userId, + 'chart_id', + 'charts', + chart.id, + ); + } +>>>>>>> pexdax refactor (#16333) } getSliceName() { @@ -214,9 +232,25 @@ export class ExploreChartHeader extends React.PureComponent { status={CHART_STATUS_MAP[chartStatus]} /> + >>>>>> pexdax refactor (#16333) /> Date: Thu, 19 Aug 2021 12:19:33 -0500 Subject: [PATCH 3/9] refactor progress (#16339) --- .../explore/components/ExploreChartHeader.jsx | 34 ------------------- 1 file changed, 34 deletions(-) diff --git a/superset-frontend/src/explore/components/ExploreChartHeader.jsx b/superset-frontend/src/explore/components/ExploreChartHeader.jsx index feb174d85afde..6bb61c8d32ded 100644 --- a/superset-frontend/src/explore/components/ExploreChartHeader.jsx +++ b/superset-frontend/src/explore/components/ExploreChartHeader.jsx @@ -103,24 +103,6 @@ export class ExploreChartHeader extends React.PureComponent { }; this.openPropertiesModal = this.openPropertiesModal.bind(this); this.closePropertiesModal = this.closePropertiesModal.bind(this); -<<<<<<< HEAD -======= - this.showReportModal = this.showReportModal.bind(this); - this.hideReportModal = this.hideReportModal.bind(this); - } - - componentDidMount() { - if (this.canAddReports()) { - const { user, chart } = this.props; - // this is in the case that there is an anonymous user. - this.props.fetchUISpecificReport( - user.userId, - 'chart_id', - 'charts', - chart.id, - ); - } ->>>>>>> pexdax refactor (#16333) } getSliceName() { @@ -232,25 +214,9 @@ export class ExploreChartHeader extends React.PureComponent { status={CHART_STATUS_MAP[chartStatus]} /> - >>>>>> pexdax refactor (#16333) /> Date: Thu, 19 Aug 2021 13:23:42 -0400 Subject: [PATCH 4/9] fix: Header Actions test refactor (#16336) * fixed tests * Update index.tsx Co-authored-by: Elizabeth Thompson --- .../src/dashboard/components/Header/Header.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index 6773b0c1af2ec..5d1b6af995295 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -57,6 +57,7 @@ const createProps = () => ({ dashboardTitle: 'Dashboard Title', charts: {}, layout: {}, + reports: {}, expandedSlices: {}, css: '', customCss: '', From 97a0071b2d8d5daa9319c222f18f1f5dbad20345 Mon Sep 17 00:00:00 2001 From: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com> Date: Fri, 20 Aug 2021 12:51:29 -0500 Subject: [PATCH 5/9] Fetch bug fixed (#16376) --- .../components/ReportModal/HeaderReportActionsDropdown/index.tsx | 1 + .../src/dashboard/components/Header/Header.test.tsx | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx index e2b0540be5e5e..5582cbe34eef4 100644 --- a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx +++ b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx @@ -55,6 +55,7 @@ export default function HeaderReportActionsDropDown({ >(state => state.user || state.explore?.user); const reportsIds = Object.keys(reports || []); const report: AlertObject = reports?.[reportsIds[0]]; + console.log(report); const [ currentReportDeleting, setCurrentReportDeleting, diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index 5d1b6af995295..6773b0c1af2ec 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -57,7 +57,6 @@ const createProps = () => ({ dashboardTitle: 'Dashboard Title', charts: {}, layout: {}, - reports: {}, expandedSlices: {}, css: '', customCss: '', From 5e8bd9d1f708a460594df43da646df85a33ba47c Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Fri, 20 Aug 2021 18:04:57 -0400 Subject: [PATCH 6/9] continued refactoring (#16377) --- .../HeaderReportActionsDropdown/index.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx index 5582cbe34eef4..3003c00ae7598 100644 --- a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx +++ b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx @@ -55,7 +55,6 @@ export default function HeaderReportActionsDropDown({ >(state => state.user || state.explore?.user); const reportsIds = Object.keys(reports || []); const report: AlertObject = reports?.[reportsIds[0]]; - console.log(report); const [ currentReportDeleting, setCurrentReportDeleting, @@ -103,6 +102,19 @@ export default function HeaderReportActionsDropDown({ } }, []); + useEffect(() => { + if (canAddReports()) { + dispatch( + fetchUISpecificReport({ + userId: user.userId, + filterField: 'dashboard_id', + creationMethod: 'dashboards', + resourceId: dashboardId, + }), + ); + } + }, [dashboardId]); + const menu = () => ( From 8827a0439b0dacf92cd37c3c3dcf36758452744a Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Mon, 27 Sep 2021 15:16:30 -0400 Subject: [PATCH 7/9] refactor(reports): Arash/refactor reports (#16855) * pexdax refactor (#16333) * refactor progress (#16339) * fix: Header Actions test refactor (#16336) * fixed tests * Update index.tsx Co-authored-by: Elizabeth Thompson * code dry (#16358) * Fetch bug fixed (#16376) * continued refactoring (#16377) * refactor: Reports - ReportModal (#16622) * refactoring progress * removed consoles * Working, but with 2 fetches * report pickup Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com> Co-authored-by: Elizabeth Thompson --- .../HeaderReportActionsDropdown/index.tsx | 37 +++++++------------ .../src/components/ReportModal/index.tsx | 18 ++++----- .../src/dashboard/components/Header/index.jsx | 1 + superset/reports/api.py | 2 + 4 files changed, 23 insertions(+), 35 deletions(-) diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx index 3003c00ae7598..357bd5b3c8ad0 100644 --- a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx +++ b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx @@ -46,21 +46,23 @@ export default function HeaderReportActionsDropDown({ chart?: ChartState; }) { const dispatch = useDispatch(); - const reports: Record = useSelector( - state => state.reports, + const reports: any = useSelector(state => + Object.values(state.reports).filter((report: any) => + dashboardId + ? report.dashboard_id === dashboardId + : report.chart_id === chart?.id, + ), ); const user: UserWithPermissionsAndRoles = useSelector< any, UserWithPermissionsAndRoles >(state => state.user || state.explore?.user); - const reportsIds = Object.keys(reports || []); - const report: AlertObject = reports?.[reportsIds[0]]; const [ currentReportDeleting, setCurrentReportDeleting, ] = useState(null); const theme = useTheme(); - const [showModal, setShowModal] = useState(false); + const [showModal, setShowModal] = useState(false); const toggleActiveKey = async (data: AlertObject, checked: boolean) => { if (data?.id) { toggleActive(data, checked); @@ -102,27 +104,14 @@ export default function HeaderReportActionsDropDown({ } }, []); - useEffect(() => { - if (canAddReports()) { - dispatch( - fetchUISpecificReport({ - userId: user.userId, - filterField: 'dashboard_id', - creationMethod: 'dashboards', - resourceId: dashboardId, - }), - ); - } - }, [dashboardId]); - const menu = () => ( {t('Email reports active')} toggleActiveKey(report, checked)} + checked={reports?.active} + onClick={(checked: boolean) => toggleActiveKey(reports, checked)} size="small" css={{ marginLeft: theme.gridUnit * 2 }} /> @@ -131,7 +120,7 @@ export default function HeaderReportActionsDropDown({ {t('Edit email report')} setCurrentReportDeleting(report)} + onClick={() => setCurrentReportDeleting(reports)} css={deleteColor} > {t('Delete email report')} @@ -143,14 +132,14 @@ export default function HeaderReportActionsDropDown({ canAddReports() && ( <> setShowModal(false)} userId={user.userId} + showModal={showModal} + onHide={() => setShowModal(false)} userEmail={user.email} dashboardId={dashboardId} chart={chart} /> - {report ? ( + {reports ? ( <> void; addSuccessToast: (msg: string) => void; addReport: (report?: ReportObject) => {}; - onHide: () => {}; + onHide: () => void; onReportAdd: (report?: ReportObject) => {}; - show: boolean; + showModal: boolean; userId: number; userEmail: string; dashboardId?: number; chart?: ChartState; - props: any; + props?: any; } interface ReportPayloadType { @@ -154,7 +153,7 @@ const reportReducer = ( const ReportModal: FunctionComponent = ({ onReportAdd, onHide, - show = false, + showModal = false, dashboardId, chart, userId, @@ -291,7 +290,7 @@ const ReportModal: FunctionComponent = ({ return ( = ({ ); }; -const mapDispatchToProps = (dispatch: any) => - bindActionCreators({ addReport, editReport }, dispatch); - -export default connect(null, mapDispatchToProps)(withToasts(ReportModal)); +export default withToasts(ReportModal); diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx index fa1e6a1493f19..9449e258bd026 100644 --- a/superset-frontend/src/dashboard/components/Header/index.jsx +++ b/superset-frontend/src/dashboard/components/Header/index.jsx @@ -517,6 +517,7 @@ class Header extends React.PureComponent { )} Optional[Response]: "changed_by.last_name", "changed_on", "changed_on_delta_humanized", + "chart_id", "created_by.first_name", "created_by.last_name", "created_on", "creation_method", "crontab", "crontab_humanized", + "dashboard_id", "description", "id", "last_eval_dttm", From 6046f88424dd49dbbb19be9fb15b7d5e171bdfbc Mon Sep 17 00:00:00 2001 From: AAfghahi <48933336+AAfghahi@users.noreply.github.com> Date: Tue, 5 Oct 2021 16:09:40 -0400 Subject: [PATCH 8/9] refactor(reports): Arash/again refactor reports (#16872) * pexdax refactor (#16333) * refactor progress (#16339) * fix: Header Actions test refactor (#16336) * fixed tests * Update index.tsx Co-authored-by: Elizabeth Thompson * code dry (#16358) * Fetch bug fixed (#16376) * continued refactoring (#16377) * refactor: Reports - ReportModal (#16622) * refactoring progress * removed consoles * Working, but with 2 fetches * it is still not working Co-authored-by: Lyndsi Kay Williams <55605634+lyndsiWilliams@users.noreply.github.com> Co-authored-by: Elizabeth Thompson --- .../HeaderReportActionsDropdown/index.tsx | 23 ++++++++++++++----- .../src/views/CRUD/alert/types.ts | 1 + 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx index 357bd5b3c8ad0..c23d187b66d1a 100644 --- a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx +++ b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx @@ -46,13 +46,11 @@ export default function HeaderReportActionsDropDown({ chart?: ChartState; }) { const dispatch = useDispatch(); - const reports: any = useSelector(state => - Object.values(state.reports).filter((report: any) => - dashboardId - ? report.dashboard_id === dashboardId - : report.chart_id === chart?.id, - ), + const reports: Record = useSelector( + state => state.reports, ); + const report: AlertObject = Object.values(reports)[0]; + const hasReport = !!report; const user: UserWithPermissionsAndRoles = useSelector< any, UserWithPermissionsAndRoles @@ -104,6 +102,19 @@ export default function HeaderReportActionsDropDown({ } }, []); + useEffect(() => { + if (hasReport && report.dashboard_id !== dashboardId) { + dispatch( + fetchUISpecificReport({ + userId: user.userId, + filterField: dashboardId ? 'dashboard_id' : 'chart_id', + creationMethod: dashboardId ? 'dashboards' : 'charts', + resourceId: dashboardId || chart?.id, + }), + ); + } + }, [dashboardId]); + const menu = () => ( diff --git a/superset-frontend/src/views/CRUD/alert/types.ts b/superset-frontend/src/views/CRUD/alert/types.ts index 3f939a03bcfa1..24e5ddf13770a 100644 --- a/superset-frontend/src/views/CRUD/alert/types.ts +++ b/superset-frontend/src/views/CRUD/alert/types.ts @@ -66,6 +66,7 @@ export type AlertObject = { created_on?: string; crontab?: string; dashboard?: MetaObject; + dashboard_id?: number; database?: MetaObject; description?: string; grace_period?: number; From 7c902c97bc7b6a19190645c8139356b989a19f67 Mon Sep 17 00:00:00 2001 From: Arash Date: Tue, 5 Oct 2021 23:37:33 -0400 Subject: [PATCH 9/9] next changes --- .../HeaderReportActionsDropdown/index.tsx | 17 +++++++++++------ .../src/reports/reducers/reports.js | 3 +++ superset-frontend/src/views/CRUD/alert/types.ts | 1 + 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx index c23d187b66d1a..5eb4448e5b4a5 100644 --- a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx +++ b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx @@ -49,8 +49,13 @@ export default function HeaderReportActionsDropDown({ const reports: Record = useSelector( state => state.reports, ); - const report: AlertObject = Object.values(reports)[0]; - const hasReport = !!report; + const report: AlertObject = Object.values(reports).filter(report => { + if (dashboardId) { + return report.dashboard_id === dashboardId; + } + return report.chart_id === chart?.id; + })[0]; + const user: UserWithPermissionsAndRoles = useSelector< any, UserWithPermissionsAndRoles @@ -103,7 +108,7 @@ export default function HeaderReportActionsDropDown({ }, []); useEffect(() => { - if (hasReport && report.dashboard_id !== dashboardId) { + if (canAddReports()) { dispatch( fetchUISpecificReport({ userId: user.userId, @@ -121,8 +126,8 @@ export default function HeaderReportActionsDropDown({ {t('Email reports active')} toggleActiveKey(reports, checked)} + checked={report?.active} + onClick={(checked: boolean) => toggleActiveKey(report, checked)} size="small" css={{ marginLeft: theme.gridUnit * 2 }} /> @@ -131,7 +136,7 @@ export default function HeaderReportActionsDropDown({ {t('Edit email report')} setCurrentReportDeleting(reports)} + onClick={() => setCurrentReportDeleting(report)} css={deleteColor} > {t('Delete email report')} diff --git a/superset-frontend/src/reports/reducers/reports.js b/superset-frontend/src/reports/reducers/reports.js index 8b582d0d0cc1d..54cf493fd5cea 100644 --- a/superset-frontend/src/reports/reducers/reports.js +++ b/superset-frontend/src/reports/reducers/reports.js @@ -19,10 +19,13 @@ /* eslint-disable camelcase */ import { SET_REPORT, ADD_REPORT, EDIT_REPORT } from '../actions/reports'; +// Talk about the delete + export default function reportsReducer(state = {}, action) { const actionHandlers = { [SET_REPORT]() { return { + ...state, ...action.report.result.reduce( (obj, report) => ({ ...obj, [report.id]: report }), {}, diff --git a/superset-frontend/src/views/CRUD/alert/types.ts b/superset-frontend/src/views/CRUD/alert/types.ts index 24e5ddf13770a..b0319c9f83385 100644 --- a/superset-frontend/src/views/CRUD/alert/types.ts +++ b/superset-frontend/src/views/CRUD/alert/types.ts @@ -62,6 +62,7 @@ export type AlertObject = { chart?: MetaObject; changed_by?: user; changed_on_delta_humanized?: string; + chart_id: number; created_by?: user; created_on?: string; crontab?: string;