Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Disables email reports for unsaved charts #23588

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
import React, { useState, useEffect } from 'react';
import { useSelector, useDispatch } from 'react-redux';
import { isEmpty } from 'lodash';
import {
t,
SupersetTheme,
Expand Down Expand Up @@ -103,6 +104,9 @@ export interface HeaderReportProps {
showReportSubMenu?: boolean;
}

// Same instance to be used in useEffects
const EMPTY_OBJECT = {};

export default function HeaderReportDropDown({
dashboardId,
chart,
Expand All @@ -116,7 +120,10 @@ export default function HeaderReportDropDown({
const resourceType = dashboardId
? CreationMethod.DASHBOARDS
: CreationMethod.CHARTS;
return reportSelector(state, resourceType, dashboardId || chart?.id);
return (
reportSelector(state, resourceType, dashboardId || chart?.id) ||
EMPTY_OBJECT
);
});

const isReportActive: boolean = report?.active || false;
Expand All @@ -133,6 +140,12 @@ export default function HeaderReportDropDown({
// this is in the case that there is an anonymous user.
return false;
}

// Cannot add reports if the resource is not saved
if (!(dashboardId || chart?.id)) {
return false;
}

const roles = Object.keys(user.roles || []);
const permissions = roles.map(key =>
user.roles[key].filter(
Expand Down Expand Up @@ -200,7 +213,21 @@ export default function HeaderReportDropDown({
};

const textMenu = () =>
report ? (
isEmpty(report) ? (
<Menu selectable={false} css={onMenuHover}>
<Menu.Item onClick={handleShowMenu}>
{DropdownItemExtension ? (
<StyledDropdownItemWithIcon>
<div>{t('Set up an email report')}</div>
<DropdownItemExtension />
</StyledDropdownItemWithIcon>
) : (
t('Set up an email report')
)}
</Menu.Item>
<Menu.Divider />
</Menu>
) : (
isDropdownVisible && (
<Menu selectable={false} css={{ border: 'none' }}>
<Menu.Item
Expand All @@ -220,20 +247,6 @@ export default function HeaderReportDropDown({
</Menu.Item>
</Menu>
)
) : (
<Menu selectable={false} css={onMenuHover}>
<Menu.Item onClick={handleShowMenu}>
{DropdownItemExtension ? (
<StyledDropdownItemWithIcon>
<div>{t('Set up an email report')}</div>
<DropdownItemExtension />
</StyledDropdownItemWithIcon>
) : (
t('Set up an email report')
)}
</Menu.Item>
<Menu.Divider />
</Menu>
);
const menu = () => (
<Menu selectable={false} css={{ width: '200px' }}>
Expand All @@ -260,7 +273,17 @@ export default function HeaderReportDropDown({
);

const iconMenu = () =>
report ? (
isEmpty(report) ? (
<span
role="button"
title={t('Schedule email report')}
tabIndex={0}
className="action-button action-schedule-report"
onClick={() => setShowModal(true)}
>
<Icons.Calendar />
</span>
) : (
<>
<NoAnimationDropdown
overlay={menu()}
Expand All @@ -278,16 +301,6 @@ export default function HeaderReportDropDown({
</span>
</NoAnimationDropdown>
</>
) : (
<span
role="button"
title={t('Schedule email report')}
tabIndex={0}
className="action-button action-schedule-report"
onClick={() => setShowModal(true)}
>
<Icons.Calendar />
</span>
);

return (
Expand Down
8 changes: 7 additions & 1 deletion superset-frontend/src/components/ReportModal/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ type ReportObjectState = Partial<ReportObject> & {
isSubmitting?: boolean;
};

// Same instance to be used in useEffects
const EMPTY_OBJECT = {};

function ReportModal({
onHide,
show = false,
Expand Down Expand Up @@ -147,7 +150,10 @@ function ReportModal({
const resourceType = dashboardId
? CreationMethod.DASHBOARDS
: CreationMethod.CHARTS;
return reportSelector(state, resourceType, dashboardId || chart?.id);
return (
reportSelector(state, resourceType, dashboardId || chart?.id) ||
EMPTY_OBJECT
);
});
const isEditMode = report && Object.keys(report).length;

Expand Down
4 changes: 3 additions & 1 deletion superset-frontend/src/reports/actions/reports.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ export function toggleActive(report, isActive) {
};
}

export const DELETE_REPORT = 'DELETE_REPORT';

export function deleteActiveReport(report) {
return function deleteActiveReportThunk(dispatch) {
return SupersetClient.delete({
Expand All @@ -152,7 +154,7 @@ export function deleteActiveReport(report) {
dispatch(addDangerToast(t('Your report could not be deleted')));
})
.finally(() => {
dispatch(structureFetchAction);
dispatch({ type: DELETE_REPORT, report });
dispatch(addSuccessToast(t('Deleted: %s', report.name)));
});
};
Expand Down
19 changes: 18 additions & 1 deletion superset-frontend/src/reports/reducers/reports.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,13 @@
*/
/* eslint-disable camelcase */
// eslint-disable-next-line import/no-extraneous-dependencies
import { SET_REPORT, ADD_REPORT, EDIT_REPORT } from '../actions/reports';
import { omit } from 'lodash';
import {
SET_REPORT,
ADD_REPORT,
EDIT_REPORT,
DELETE_REPORT,
} from '../actions/reports';

export default function reportsReducer(state = {}, action) {
const actionHandlers = {
Expand Down Expand Up @@ -78,6 +84,17 @@ export default function reportsReducer(state = {}, action) {
},
};
},

[DELETE_REPORT]() {
const { report } = action;
const reportTypeId = report.dashboard || report.chart;
return {
...state,
[report.creation_method]: {
...omit(state[report.creation_method], reportTypeId),
},
};
},
};

if (action.type in actionHandlers) {
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/views/CRUD/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -867,5 +867,5 @@ export const reportSelector = (
if (resourceId) {
return state.reports[resourceType]?.[resourceId];
}
return {};
return null;
};