You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2021/07/30 17:39:39 UTC
[superset] branch selfSubscribeReports updated: feat: add edit
modal to self subscribe reports (#15951)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch selfSubscribeReports
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/selfSubscribeReports by this push:
new 78033c3 feat: add edit modal to self subscribe reports (#15951)
78033c3 is described below
commit 78033c3fe74ae1d2c4a921bda17e207aa985d3b1
Author: AAfghahi <48...@users.noreply.github.com>
AuthorDate: Fri Jul 30 13:38:48 2021 -0400
feat: add edit modal to self subscribe reports (#15951)
* changed state structure
* added edit functionality
* changed report state and revisions
---
.../HeaderReportActionsDropdown/index.tsx | 18 ++---
.../src/components/ReportModal/index.tsx | 68 ++++++++++++-------
.../src/dashboard/components/Header/index.jsx | 3 +-
.../src/dashboard/containers/DashboardHeader.jsx | 6 +-
.../src/explore/components/ExploreChartHeader.jsx | 11 ++--
.../src/explore/components/ExploreChartPanel.jsx | 2 +-
.../explore/components/ExploreViewContainer.jsx | 4 +-
superset-frontend/src/explore/reducers/index.js | 4 +-
.../reports/actions/{reportState.js => reports.js} | 76 ++++++++++++----------
.../reducers/{reportState.js => reports.js} | 43 ++++--------
superset-frontend/src/views/store.ts | 4 +-
superset/models/slice.py | 2 +-
superset/reports/api.py | 2 +
tests/integration_tests/reports/api_tests.py | 2 +
14 files changed, 122 insertions(+), 123 deletions(-)
diff --git a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
index 2fad319..6e6544b 100644
--- a/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
+++ b/superset-frontend/src/components/ReportModal/HeaderReportActionsDropdown/index.tsx
@@ -31,20 +31,17 @@ const deleteColor = (theme: SupersetTheme) => css`
`;
export default function HeaderReportActionsDropDown({
- // showReportModal,
- // hideReportModal,
- // addDangerToast,
+ showReportModal,
toggleActive,
deleteActiveReport,
}: {
showReportModal: () => void;
- hideReportModal: () => void;
toggleActive: (data: AlertObject, checked: boolean) => void;
deleteActiveReport: (data: AlertObject) => void;
}) {
- const report = useSelector<any, AlertObject>(
- state => state.reportState.report.result[0],
- );
+ const reports = useSelector<any, AlertObject>(state => state.reports);
+ const reportsIds = Object.keys(reports);
+ const report = reports[reportsIds[0]];
const [
currentReportDeleting,
setCurrentReportDeleting,
@@ -72,12 +69,7 @@ export default function HeaderReportActionsDropDown({
size="small"
/>
</Menu.Item>
- <Menu.Item>
- <div role="button" tabIndex={0}>
- {' '}
- {t('Edit email report')}
- </div>
- </Menu.Item>
+ <Menu.Item onClick={showReportModal}>{t('Edit email report')}</Menu.Item>
<Menu.Item
onClick={() => setCurrentReportDeleting(report)}
css={deleteColor}
diff --git a/superset-frontend/src/components/ReportModal/index.tsx b/superset-frontend/src/components/ReportModal/index.tsx
index ffe7018..90511a7 100644
--- a/superset-frontend/src/components/ReportModal/index.tsx
+++ b/superset-frontend/src/components/ReportModal/index.tsx
@@ -18,7 +18,7 @@
*/
import React, {
useState,
- // useEffect,
+ useEffect,
useCallback,
useReducer,
Reducer,
@@ -26,9 +26,9 @@ import React, {
} from 'react';
import { t, SupersetTheme } from '@superset-ui/core';
import { bindActionCreators } from 'redux';
-import { connect, useDispatch } from 'react-redux';
-import { addReport } from 'src/reports/actions/reportState';
-
+import { connect, useDispatch, useSelector } from 'react-redux';
+import { addReport, editReport } from 'src/reports/actions/reports';
+import { AlertObject } from 'src/views/CRUD/alert/types';
import LabeledErrorBoundInput from 'src/components/Form/LabeledErrorBoundInput';
import TimezoneSelector from 'src/components/TimezoneSelector';
import Icons from 'src/components/Icons';
@@ -48,6 +48,7 @@ import {
} from './styles';
interface ReportObject {
+ id?: number;
active: boolean;
crontab: string;
dashboard?: number;
@@ -85,6 +86,7 @@ enum ActionType {
textChange,
inputChange,
fetched,
+ reset,
}
interface ReportPayloadType {
@@ -100,6 +102,9 @@ type ReportActionType =
| {
type: ActionType.fetched;
payload: Partial<ReportObject>;
+ }
+ | {
+ type: ActionType.reset;
};
const reportReducer = (
@@ -117,6 +122,12 @@ const reportReducer = (
...initialState,
[action.payload.name]: action.payload.value,
};
+ case ActionType.fetched:
+ return {
+ ...action.payload,
+ };
+ case ActionType.reset:
+ return null;
default:
return state;
}
@@ -136,31 +147,29 @@ const ReportModal: FunctionComponent<ReportProps> = ({
setCurrentReport({ type, payload });
}, []);
const [error, setError] = useState<CronError>();
- // ---------- comments on lines 21, 28, 125, 139-159 & 182 are being held for edit functionality
- // const [hasConnectedReport, setHasConnectedReport] = useState<boolean>(false);
// const [isLoading, setLoading] = useState<boolean>(false);
const dispatch = useDispatch();
-
// Report fetch logic
- // const {
- // state: { resource },
- // } = useSingleViewResource<ReportObject>(
- // 'report',
- // t('report'),
- // addDangerToast,
- // );
-
- // useEffect(() => {
- // if (resource?.dashboard) {
- // setHasConnectedReport(true);
- // }
- // }, [resource?.dashboard]);
-
+ const reports = useSelector<any, AlertObject>(state => state.reports);
+ const isEditMode = reports && Object.keys(reports).length;
+ useEffect(() => {
+ if (isEditMode) {
+ const reportsIds = Object.keys(reports);
+ const report = reports[reportsIds[0]];
+ setCurrentReport({
+ type: ActionType.fetched,
+ payload: report,
+ });
+ } else {
+ setCurrentReport({
+ type: ActionType.reset,
+ });
+ }
+ }, [reports]);
const onClose = () => {
// setLoading(false);
onHide();
};
-
const onSave = async () => {
// Create new Report
const newReportValues: Partial<ReportObject> = {
@@ -178,10 +187,17 @@ const ReportModal: FunctionComponent<ReportProps> = ({
],
type: 'Report',
creation_method: props.props.creationMethod,
+ active: true,
};
// setLoading(true);
- await dispatch(addReport(newReportValues as ReportObject));
+ if (isEditMode) {
+ await dispatch(
+ editReport(currentReport?.id, newReportValues as ReportObject),
+ );
+ } else {
+ await dispatch(addReport(newReportValues as ReportObject));
+ }
if (onReportAdd) {
onReportAdd();
@@ -193,7 +209,9 @@ const ReportModal: FunctionComponent<ReportProps> = ({
const wrappedTitle = (
<StyledIconWrapper>
<Icons.Calendar />
- <span className="text">{t('New Email Report')}</span>
+ <span className="text">
+ {isEditMode ? t('Edit Email Report') : t('New Email Report')}
+ </span>
</StyledIconWrapper>
);
@@ -305,6 +323,6 @@ const ReportModal: FunctionComponent<ReportProps> = ({
};
const mapDispatchToProps = (dispatch: any) =>
- bindActionCreators({ addReport }, dispatch);
+ bindActionCreators({ addReport, editReport }, dispatch);
export default connect(null, mapDispatchToProps)(withToasts(ReportModal));
diff --git a/superset-frontend/src/dashboard/components/Header/index.jsx b/superset-frontend/src/dashboard/components/Header/index.jsx
index 9f42dd1..9fa42e3 100644
--- a/superset-frontend/src/dashboard/components/Header/index.jsx
+++ b/superset-frontend/src/dashboard/components/Header/index.jsx
@@ -381,11 +381,10 @@ class Header extends React.PureComponent {
}
renderReportModal() {
- const attachedReportExists = this.props.report?.count > 0;
+ const attachedReportExists = !!Object.keys(this.props.reports).length;
return attachedReportExists ? (
<HeaderReportActionsDropdown
showReportModal={this.showReportModal}
- hideReportModal={this.hideReportModal}
toggleActive={this.props.toggleActive}
deleteActiveReport={this.props.deleteActiveReport}
/>
diff --git a/superset-frontend/src/dashboard/containers/DashboardHeader.jsx b/superset-frontend/src/dashboard/containers/DashboardHeader.jsx
index 1d2f59a..1ea2b7d 100644
--- a/superset-frontend/src/dashboard/containers/DashboardHeader.jsx
+++ b/superset-frontend/src/dashboard/containers/DashboardHeader.jsx
@@ -60,12 +60,12 @@ import {
fetchUISpecificReport,
toggleActive,
deleteActiveReport,
-} from '../../reports/actions/reportState';
+} from '../../reports/actions/reports';
function mapStateToProps({
dashboardLayout: undoableLayout,
dashboardState,
- reportState,
+ reports,
dashboardInfo,
charts,
dataMask,
@@ -100,7 +100,7 @@ function mapStateToProps({
editMode: !!dashboardState.editMode,
slug: dashboardInfo.slug,
metadata: dashboardInfo.metadata,
- report: reportState.report,
+ reports,
};
}
diff --git a/superset-frontend/src/explore/components/ExploreChartHeader.jsx b/superset-frontend/src/explore/components/ExploreChartHeader.jsx
index a8fca5f..da20c31 100644
--- a/superset-frontend/src/explore/components/ExploreChartHeader.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartHeader.jsx
@@ -28,7 +28,7 @@ import {
fetchUISpecificReport,
toggleActive,
deleteActiveReport,
-} from 'src/reports/actions/reportState';
+} from 'src/reports/actions/reports';
import HeaderReportActionsDropdown from 'src/components/ReportModal/HeaderReportActionsDropdown';
import { chartPropShape } from '../../dashboard/util/propShapes';
import ExploreActionButtons from './ExploreActionButtons';
@@ -111,6 +111,7 @@ export class ExploreChartHeader extends React.PureComponent {
this.closePropertiesModal = this.closePropertiesModal.bind(this);
this.showReportModal = this.showReportModal.bind(this);
this.hideReportModal = this.hideReportModal.bind(this);
+ this.renderReportModal = this.renderReportModal.bind(this);
}
componentDidMount() {
@@ -161,7 +162,7 @@ export class ExploreChartHeader extends React.PureComponent {
}
renderReportModal() {
- const attachedReportExists = this.props.report?.count > 0;
+ const attachedReportExists = !!Object.keys(this.props.reports).length;
return attachedReportExists ? (
<HeaderReportActionsDropdown
showReportModal={this.showReportModal}
@@ -196,7 +197,7 @@ export class ExploreChartHeader extends React.PureComponent {
perms => perms[0] === 'can_add' && perms[1] === 'AlertModelView',
),
);
- return this.props.report && permissions[0].length > 0;
+ return permissions[0].length > 0;
}
render() {
@@ -286,8 +287,8 @@ export class ExploreChartHeader extends React.PureComponent {
show={this.state.showingReportModal}
onHide={this.hideReportModal}
props={{
- userId: this.props.userId,
- userEmail: this.props.email,
+ userId: this.props.user.userId,
+ userEmail: this.props.user.email,
chartId: this.props.chart.id,
creationMethod: 'charts',
}}
diff --git a/superset-frontend/src/explore/components/ExploreChartPanel.jsx b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
index 8a09e69..aaf604b 100644
--- a/superset-frontend/src/explore/components/ExploreChartPanel.jsx
+++ b/superset-frontend/src/explore/components/ExploreChartPanel.jsx
@@ -255,7 +255,7 @@ const ExploreChartPanel = props => {
timeout={props.timeout}
chart={props.chart}
user={props.user}
- report={props.report}
+ reports={props.reports}
/>
);
diff --git a/superset-frontend/src/explore/components/ExploreViewContainer.jsx b/superset-frontend/src/explore/components/ExploreViewContainer.jsx
index dd22d50..fe4297a 100644
--- a/superset-frontend/src/explore/components/ExploreViewContainer.jsx
+++ b/superset-frontend/src/explore/components/ExploreViewContainer.jsx
@@ -572,7 +572,7 @@ function ExploreViewContainer(props) {
ExploreViewContainer.propTypes = propTypes;
function mapStateToProps(state) {
- const { explore, charts, impressionId, dataMask, reportState } = state;
+ const { explore, charts, impressionId, dataMask, reports } = state;
const form_data = getFormDataFromControls(explore.controls);
form_data.extra_form_data = mergeExtraFormData(
{ ...form_data.extra_form_data },
@@ -613,7 +613,7 @@ function mapStateToProps(state) {
impressionId,
userId: explore.user_id,
user: explore.user,
- report: reportState.report,
+ reports,
};
}
diff --git a/superset-frontend/src/explore/reducers/index.js b/superset-frontend/src/explore/reducers/index.js
index 12e854b..e387c49 100644
--- a/superset-frontend/src/explore/reducers/index.js
+++ b/superset-frontend/src/explore/reducers/index.js
@@ -18,7 +18,7 @@
*/
import { combineReducers } from 'redux';
-import reportState from 'src/reports/reducers/reportState';
+import reports from 'src/reports/reducers/reports';
import charts from '../../chart/chartReducer';
import saveModal from './saveModalReducer';
import explore from './exploreReducer';
@@ -34,5 +34,5 @@ export default combineReducers({
explore,
impressionId,
messageToasts,
- reportState,
+ reports,
});
diff --git a/superset-frontend/src/reports/actions/reportState.js b/superset-frontend/src/reports/actions/reports.js
similarity index 75%
rename from superset-frontend/src/reports/actions/reportState.js
rename to superset-frontend/src/reports/actions/reports.js
index 64f32bf..373dd99 100644
--- a/superset-frontend/src/reports/actions/reportState.js
+++ b/superset-frontend/src/reports/actions/reports.js
@@ -70,6 +70,31 @@ export function fetchUISpecificReport(
};
}
+const structureFetchAction = (dispatch, getState) => {
+ const state = getState();
+ const { user, dashboardInfo, charts, explore } = state;
+ if (dashboardInfo) {
+ dispatch(
+ fetchUISpecificReport(
+ user.userId,
+ 'dashboard_id',
+ 'dashboards',
+ dashboardInfo.id,
+ ),
+ );
+ } else {
+ const [chartArr] = Object.keys(charts);
+ dispatch(
+ fetchUISpecificReport(
+ explore.user.userId,
+ 'chart_id',
+ 'charts',
+ charts[chartArr].id,
+ ),
+ );
+ }
+};
+
export const ADD_REPORT = 'ADD_REPORT';
export const addReport = report => dispatch => {
@@ -77,8 +102,9 @@ export const addReport = report => dispatch => {
endpoint: `/api/v1/report/`,
jsonPayload: report,
})
- .then(() => {
- dispatch({ type: ADD_REPORT, report });
+ .then(response => {
+ const { json } = response;
+ dispatch({ type: ADD_REPORT, json });
})
.catch(() =>
dispatch(
@@ -86,15 +112,18 @@ export const addReport = report => dispatch => {
),
);
};
+
export const EDIT_REPORT = 'EDIT_REPORT';
-export function reportEditor(report) {
+export function editReport(id, report) {
return function (dispatch) {
SupersetClient.put({
- endpoint: `/api/v1/report/${report.id}`,
- postPayload: { report },
+ endpoint: `/api/v1/report/${id}`,
+ jsonPayload: report,
})
- .then(() => dispatch({ type: EDIT_REPORT, report }))
+ .then(({ json }) => {
+ dispatch({ type: EDIT_REPORT, json });
+ })
.catch(() =>
dispatch(
addDangerToast(t('An error occurred while editing this report.')),
@@ -103,10 +132,6 @@ export function reportEditor(report) {
};
}
-export const TOGGLE_ACTIVE = 'TOGGLE_ACTIVE';
-export function toggleActiveKey(report, isActive) {
- return { type: TOGGLE_ACTIVE, report, isActive };
-}
export function toggleActive(report, isActive) {
return function toggleActiveThunk(dispatch) {
return SupersetClient.put({
@@ -116,21 +141,21 @@ export function toggleActive(report, isActive) {
active: isActive,
}),
})
- .then(() => {
- dispatch(toggleActiveKey(report, isActive));
- })
.catch(() => {
dispatch(
addDangerToast(
t('We were unable to active or deactivate this report.'),
),
);
+ })
+ .finally(() => {
+ dispatch(structureFetchAction);
});
};
}
export function deleteActiveReport(report) {
- return function deleteActiveReportThunk(dispatch, getState) {
+ return function deleteActiveReportThunk(dispatch) {
return SupersetClient.delete({
endpoint: encodeURI(`/api/v1/report/${report.id}`),
})
@@ -138,28 +163,7 @@ export function deleteActiveReport(report) {
dispatch(addDangerToast(t('Your report could not be deleted')));
})
.finally(() => {
- const state = getState();
- const { user, dashboardInfo, charts, explore } = state;
- if (dashboardInfo) {
- dispatch(
- fetchUISpecificReport(
- user.userId,
- 'dashboard_id',
- 'dashboards',
- dashboardInfo.id,
- ),
- );
- } else {
- const [chartArr] = Object.keys(charts);
- dispatch(
- fetchUISpecificReport(
- explore.user.userId,
- 'chart_id',
- 'charts',
- charts[chartArr].id,
- ),
- );
- }
+ dispatch(structureFetchAction);
dispatch(addSuccessToast(t('Deleted: %s', report.name)));
});
};
diff --git a/superset-frontend/src/reports/reducers/reportState.js b/superset-frontend/src/reports/reducers/reports.js
similarity index 61%
rename from superset-frontend/src/reports/reducers/reportState.js
rename to superset-frontend/src/reports/reducers/reports.js
index 6493e0e..8b582d0 100644
--- a/superset-frontend/src/reports/reducers/reportState.js
+++ b/superset-frontend/src/reports/reducers/reports.js
@@ -17,51 +17,32 @@
* under the License.
*/
/* eslint-disable camelcase */
-import { alterInArr } from 'src/reduxUtils';
-import {
- SET_REPORT,
- ADD_REPORT,
- EDIT_REPORT,
- TOGGLE_ACTIVE,
-} from '../actions/reportState';
+import { SET_REPORT, ADD_REPORT, EDIT_REPORT } from '../actions/reports';
-export default function reportStateReducer(state = {}, action) {
+export default function reportsReducer(state = {}, action) {
const actionHandlers = {
[SET_REPORT]() {
return {
- ...state,
- report: action.report,
+ ...action.report.result.reduce(
+ (obj, report) => ({ ...obj, [report.id]: report }),
+ {},
+ ),
};
},
[ADD_REPORT]() {
+ const report = action.json.result;
+ report.id = action.json.id;
return {
...state,
- report: action.report,
+ [action.json.id]: report,
};
},
[EDIT_REPORT]() {
+ const report = action.json.result;
+ report.id = action.json.id;
return {
...state,
- report: action.report,
- };
- },
- [TOGGLE_ACTIVE]() {
- const { report, isActive } = action;
- const existing = state.report.result.find(
- result => result.id === report.id,
- );
- const newArr = alterInArr(
- state.report,
- 'result',
- existing,
- {
- active: isActive,
- },
- 'id',
- );
- return {
- ...state,
- report: newArr,
+ [action.json.id]: report,
};
},
};
diff --git a/superset-frontend/src/views/store.ts b/superset-frontend/src/views/store.ts
index d53462e..4704da8 100644
--- a/superset-frontend/src/views/store.ts
+++ b/superset-frontend/src/views/store.ts
@@ -22,7 +22,7 @@ import messageToastReducer from 'src/messageToasts/reducers';
import { initEnhancer } from 'src/reduxUtils';
import charts from 'src/chart/chartReducer';
import dataMask from 'src/dataMask/reducer';
-import reportState from 'src/reports/reducers/reportState';
+import reports from 'src/reports/reducers/reports';
import dashboardInfo from 'src/dashboard/reducers/dashboardInfo';
import dashboardState from 'src/dashboard/reducers/dashboardState';
import dashboardFilters from 'src/dashboard/reducers/dashboardFilters';
@@ -53,7 +53,7 @@ const dashboardReducers = {
dashboardState,
dashboardLayout,
sliceEntities,
- reportState,
+ reports,
};
// exported for tests
diff --git a/superset/models/slice.py b/superset/models/slice.py
index cf6addb..98d4ee9 100644
--- a/superset/models/slice.py
+++ b/superset/models/slice.py
@@ -55,7 +55,7 @@ logger = logging.getLogger(__name__)
class Slice(
Model, AuditMixinNullable, ImportExportMixin
-): # pylint: disable=too-many-public-methods
+): # pylint: disable=too-many-public-methods, too-many-instance-attributes
"""A slice is essentially a report or a view on data"""
diff --git a/superset/reports/api.py b/superset/reports/api.py
index 03d72bc..4224525 100644
--- a/superset/reports/api.py
+++ b/superset/reports/api.py
@@ -127,6 +127,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
"creation_method",
"crontab",
"crontab_humanized",
+ "description",
"id",
"last_eval_dttm",
"last_state",
@@ -165,6 +166,7 @@ class ReportScheduleRestApi(BaseSupersetModelRestApi):
order_columns = [
"active",
+ "description",
"created_by.first_name",
"changed_by.first_name",
"changed_on",
diff --git a/tests/integration_tests/reports/api_tests.py b/tests/integration_tests/reports/api_tests.py
index 06888e9..cfeca57 100644
--- a/tests/integration_tests/reports/api_tests.py
+++ b/tests/integration_tests/reports/api_tests.py
@@ -273,6 +273,7 @@ class TestReportSchedulesApi(SupersetTestCase):
"creation_method",
"crontab",
"crontab_humanized",
+ "description",
"id",
"last_eval_dttm",
"last_state",
@@ -312,6 +313,7 @@ class TestReportSchedulesApi(SupersetTestCase):
"changed_on_delta_humanized",
"created_on",
"crontab",
+ "description",
"last_eval_dttm",
"name",
"type",