From 12800961833cf03232d72191d5d17d71b1aa2935 Mon Sep 17 00:00:00 2001
From: Alexander Polyankin <alexander.polyankin@metabase.com>
Date: Fri, 24 Sep 2021 21:55:31 +0300
Subject: [PATCH] Fix approved domains validation for alerts and dashboard
subscriptions (#18049)
---
.../advanced_config/models/pulse_channel.clj | 3 +-
.../metabase/components/form/FormMessage.jsx | 30 ++++--
.../metabase/dashboard/components/Sidebar.jsx | 20 +++-
.../dashboard/components/Sidebar.styled.jsx | 11 +++
.../src/metabase/query_builder/actions.js | 1 -
.../query_builder/components/AlertModals.jsx | 92 ++++++++++++-------
.../components/AlertModals.styled.jsx | 14 +++
.../query_builder/containers/QueryBuilder.jsx | 3 +-
.../sharing/components/AddEditSidebar.jsx | 12 ++-
.../sharing/components/SharingSidebar.jsx | 26 ++++--
10 files changed, 156 insertions(+), 56 deletions(-)
create mode 100644 frontend/src/metabase/dashboard/components/Sidebar.styled.jsx
create mode 100644 frontend/src/metabase/query_builder/components/AlertModals.styled.jsx
diff --git a/enterprise/backend/src/metabase_enterprise/advanced_config/models/pulse_channel.clj b/enterprise/backend/src/metabase_enterprise/advanced_config/models/pulse_channel.clj
index 58f544822b3..754c819686d 100644
--- a/enterprise/backend/src/metabase_enterprise/advanced_config/models/pulse_channel.clj
+++ b/enterprise/backend/src/metabase_enterprise/advanced_config/models/pulse_channel.clj
@@ -39,5 +39,4 @@
(pr-str domain)
(str/join ", " allowed-domains))
{:email email
- :allowed-domains allowed-domains
- :status-code 403})))))))
+ :allowed-domains allowed-domains})))))))
diff --git a/frontend/src/metabase/components/form/FormMessage.jsx b/frontend/src/metabase/components/form/FormMessage.jsx
index f7b0bf04057..7508327fabb 100644
--- a/frontend/src/metabase/components/form/FormMessage.jsx
+++ b/frontend/src/metabase/components/form/FormMessage.jsx
@@ -11,15 +11,9 @@ export default class FormMessage extends Component {
if (!message) {
if (formError) {
- if (formError.data && formError.data.message) {
- message = formError.data.message;
- } else if (formError.status >= 400) {
- message = SERVER_ERROR_MESSAGE;
- } else {
- message = UNKNOWN_ERROR_MESSAGE;
- }
- } else if (formSuccess && formSuccess.data.message) {
- message = formSuccess.data.message;
+ message = getErrorMessage(formError);
+ } else if (formSuccess) {
+ message = getSuccessMessage(formSuccess);
}
}
@@ -32,3 +26,21 @@ export default class FormMessage extends Component {
return <span className={classes}>{message}</span>;
}
}
+
+export const getErrorMessage = formError => {
+ if (formError) {
+ if (formError.data && formError.data.message) {
+ return formError.data.message;
+ } else if (formError.status >= 400) {
+ return SERVER_ERROR_MESSAGE;
+ } else {
+ return UNKNOWN_ERROR_MESSAGE;
+ }
+ }
+};
+
+export const getSuccessMessage = formSuccess => {
+ if (formSuccess && formSuccess.data.message) {
+ return formSuccess.data.message;
+ }
+};
diff --git a/frontend/src/metabase/dashboard/components/Sidebar.jsx b/frontend/src/metabase/dashboard/components/Sidebar.jsx
index 2e75eec5f5c..0779f400586 100644
--- a/frontend/src/metabase/dashboard/components/Sidebar.jsx
+++ b/frontend/src/metabase/dashboard/components/Sidebar.jsx
@@ -1,12 +1,23 @@
/* eslint-disable react/prop-types */
import React from "react";
+import PropTypes from "prop-types";
import { t } from "ttag";
import Button from "metabase/components/Button";
+import { getErrorMessage } from "metabase/components/form/FormMessage";
+import { SidebarError, SidebarFooter } from "./Sidebar.styled";
const WIDTH = 384;
-function Sidebar({ onClose, onCancel, closeIsDisabled, children }) {
+const propTypes = {
+ closeIsDisabled: PropTypes.bool,
+ formError: PropTypes.object,
+ children: PropTypes.node,
+ onClose: PropTypes.func,
+ onCancel: PropTypes.func,
+};
+
+function Sidebar({ closeIsDisabled, formError, children, onClose, onCancel }) {
return (
<aside
style={{ width: WIDTH, minWidth: WIDTH }}
@@ -39,8 +50,15 @@ function Sidebar({ onClose, onCancel, closeIsDisabled, children }) {
)}
</div>
)}
+ {formError && (
+ <SidebarFooter>
+ <SidebarError>{getErrorMessage(formError)}</SidebarError>
+ </SidebarFooter>
+ )}
</aside>
);
}
+Sidebar.propTypes = propTypes;
+
export default Sidebar;
diff --git a/frontend/src/metabase/dashboard/components/Sidebar.styled.jsx b/frontend/src/metabase/dashboard/components/Sidebar.styled.jsx
new file mode 100644
index 00000000000..82233ec8db6
--- /dev/null
+++ b/frontend/src/metabase/dashboard/components/Sidebar.styled.jsx
@@ -0,0 +1,11 @@
+import styled from "styled-components";
+import { color } from "metabase/lib/colors";
+import { space } from "metabase/styled-components/theme";
+
+export const SidebarFooter = styled.div`
+ padding: 0 ${space(3)} ${space(2)};
+`;
+
+export const SidebarError = styled.div`
+ color: ${color("error")};
+`;
diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js
index 7584e5ac047..5928764385d 100644
--- a/frontend/src/metabase/query_builder/actions.js
+++ b/frontend/src/metabase/query_builder/actions.js
@@ -1045,7 +1045,6 @@ export const apiUpdateQuestion = question => {
// so we want the databases list to be re-fetched next time we hit "New Question" so it shows up
dispatch(setRequestUnloaded(["entities", "databases"]));
- dispatch(updateUrl(updatedQuestion.card(), { dirty: false }));
MetabaseAnalytics.trackEvent(
"QueryBuilder",
"Update Card",
diff --git a/frontend/src/metabase/query_builder/components/AlertModals.jsx b/frontend/src/metabase/query_builder/components/AlertModals.jsx
index f3d2e346320..03d0db7b68a 100644
--- a/frontend/src/metabase/query_builder/components/AlertModals.jsx
+++ b/frontend/src/metabase/query_builder/components/AlertModals.jsx
@@ -15,12 +15,14 @@ import Icon from "metabase/components/Icon";
import ChannelSetupModal from "metabase/components/ChannelSetupModal";
import ButtonWithStatus from "metabase/components/ButtonWithStatus";
import PulseEditChannels from "metabase/pulse/components/PulseEditChannels";
+import { getErrorMessage } from "metabase/components/form/FormMessage";
+import { AlertModalFooter, AlertModalError } from "./AlertModals.styled";
import User from "metabase/entities/users";
// actions
import { createAlert, deleteAlert, updateAlert } from "metabase/alert/alert";
-import { apiUpdateQuestion } from "metabase/query_builder/actions";
+import { apiUpdateQuestion, updateUrl } from "metabase/query_builder/actions";
import { fetchPulseFormInput } from "metabase/pulse/actions";
// selectors
@@ -71,7 +73,7 @@ const textStyle = {
hasConfiguredAnyChannel: hasConfiguredAnyChannelSelector(state),
hasConfiguredEmailChannel: hasConfiguredEmailChannelSelector(state),
}),
- { createAlert, fetchPulseFormInput, apiUpdateQuestion },
+ { createAlert, fetchPulseFormInput, apiUpdateQuestion, updateUrl },
)
export class CreateAlertModalContent extends Component {
props: {
@@ -87,6 +89,7 @@ export class CreateAlertModalContent extends Component {
this.state = {
hasSeenEducationalScreen: MetabaseCookies.getHasSeenAlertSplash(),
alert: getDefaultAlert(question, user, visualizationSettings),
+ formError: null,
};
}
@@ -113,21 +116,28 @@ export class CreateAlertModalContent extends Component {
onAlertChange = alert => this.setState({ alert });
onCreateAlert = async () => {
- const { createAlert, apiUpdateQuestion, onAlertCreated } = this.props;
+ const {
+ question,
+ createAlert,
+ apiUpdateQuestion,
+ updateUrl,
+ onAlertCreated,
+ } = this.props;
const { alert } = this.state;
- // Resave the question here (for persisting the x/y axes; see #6749)
- await apiUpdateQuestion();
+ try {
+ this.setState({ formError: null });
- await createAlert(alert);
+ await apiUpdateQuestion(question);
+ await createAlert(alert);
+ await updateUrl(question.card(), { dirty: false });
- // should close be triggered manually like this
- // but the creation notification would appear automatically ...?
- // OR should the modal visibility be part of QB redux state
- // (maybe check how other modals are implemented)
- onAlertCreated();
-
- MetabaseAnalytics.trackEvent("Alert", "Create", alert.alert_condition);
+ onAlertCreated();
+ MetabaseAnalytics.trackEvent("Alert", "Create", alert.alert_condition);
+ } catch (e) {
+ this.setState({ formError: e });
+ throw e;
+ }
};
proceedFromEducationalScreen = () => {
@@ -146,7 +156,7 @@ export class CreateAlertModalContent extends Component {
user,
hasLoadedChannelInfo,
} = this.props;
- const { alert, hasSeenEducationalScreen } = this.state;
+ const { alert, hasSeenEducationalScreen, formError } = this.state;
const channelRequirementsMet = isAdmin
? hasConfiguredAnyChannel
@@ -186,14 +196,16 @@ export class CreateAlertModalContent extends Component {
alert={alert}
onAlertChange={this.onAlertChange}
/>
- <div className="flex align-center mt4">
- <div className="flex-full" />
+ <AlertModalFooter>
+ {formError && (
+ <AlertModalError>{getErrorMessage(formError)}</AlertModalError>
+ )}
<Button onClick={onCancel} className="mr2">{t`Cancel`}</Button>
<ButtonWithStatus
titleForState={{ default: t`Done` }}
onClickOperation={this.onCreateAlert}
/>
- </div>
+ </AlertModalFooter>
</div>
</ModalContent>
);
@@ -290,7 +302,7 @@ export class AlertEducationalScreen extends Component {
question: getQuestion(state),
visualizationSettings: getVisualizationSettings(state),
}),
- { apiUpdateQuestion, updateAlert, deleteAlert },
+ { apiUpdateQuestion, updateAlert, deleteAlert, updateUrl },
)
export class UpdateAlertModalContent extends Component {
props: {
@@ -306,26 +318,40 @@ export class UpdateAlertModalContent extends Component {
super();
this.state = {
modifiedAlert: props.alert,
+ formError: null,
};
}
onAlertChange = modifiedAlert => this.setState({ modifiedAlert });
onUpdateAlert = async () => {
- const { apiUpdateQuestion, updateAlert, onAlertUpdated } = this.props;
+ const {
+ question,
+ apiUpdateQuestion,
+ updateAlert,
+ updateUrl,
+ onAlertUpdated,
+ } = this.props;
const { modifiedAlert } = this.state;
- // Resave the question here (for persisting the x/y axes; see #6749)
- await apiUpdateQuestion();
+ try {
+ this.setState({ formError: null });
- await updateAlert(modifiedAlert);
- onAlertUpdated();
+ await apiUpdateQuestion();
+ await updateAlert(modifiedAlert);
+ await updateUrl(question.card(), { dirty: false });
- MetabaseAnalytics.trackEvent(
- "Alert",
- "Update",
- modifiedAlert.alert_condition,
- );
+ onAlertUpdated();
+
+ MetabaseAnalytics.trackEvent(
+ "Alert",
+ "Update",
+ modifiedAlert.alert_condition,
+ );
+ } catch (e) {
+ this.setState({ formError: e });
+ throw e;
+ }
};
onDeleteAlert = async () => {
@@ -343,7 +369,7 @@ export class UpdateAlertModalContent extends Component {
user,
isAdmin,
} = this.props;
- const { modifiedAlert } = this.state;
+ const { modifiedAlert, formError } = this.state;
const isCurrentUser = alert.creator.id === user.id;
const title = isCurrentUser ? t`Edit your alert` : t`Edit alert`;
@@ -367,14 +393,16 @@ export class UpdateAlertModalContent extends Component {
/>
)}
- <div className="flex align-center mt4">
- <div className="flex-full" />
+ <AlertModalFooter>
+ {formError && (
+ <AlertModalError>{getErrorMessage(formError)}</AlertModalError>
+ )}
<Button onClick={onCancel} className="mr2">{t`Cancel`}</Button>
<ButtonWithStatus
titleForState={{ default: t`Save changes` }}
onClickOperation={this.onUpdateAlert}
/>
- </div>
+ </AlertModalFooter>
</div>
</ModalContent>
);
diff --git a/frontend/src/metabase/query_builder/components/AlertModals.styled.jsx b/frontend/src/metabase/query_builder/components/AlertModals.styled.jsx
new file mode 100644
index 00000000000..ed338861bfe
--- /dev/null
+++ b/frontend/src/metabase/query_builder/components/AlertModals.styled.jsx
@@ -0,0 +1,14 @@
+import styled from "styled-components";
+import { space } from "metabase/styled-components/theme";
+import { color } from "metabase/lib/colors";
+
+export const AlertModalFooter = styled.div`
+ display: flex;
+ justify-content: right;
+ align-items: center;
+ margin-top: ${space(3)};
+`;
+
+export const AlertModalError = styled.div`
+ color: ${color("error")};
+`;
diff --git a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
index 87049a712c2..6e5017cf152 100644
--- a/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
+++ b/frontend/src/metabase/query_builder/containers/QueryBuilder.jsx
@@ -253,9 +253,10 @@ export default class QueryBuilder extends Component {
};
handleSave = async card => {
- const { question, apiUpdateQuestion } = this.props;
+ const { question, apiUpdateQuestion, updateUrl } = this.props;
const questionWithUpdatedCard = question.setCard(card);
await apiUpdateQuestion(questionWithUpdatedCard);
+ await updateUrl(questionWithUpdatedCard.card(), { dirty: false });
if (this.props.fromUrl) {
this.props.onChangeLocation(this.props.fromUrl);
diff --git a/frontend/src/metabase/sharing/components/AddEditSidebar.jsx b/frontend/src/metabase/sharing/components/AddEditSidebar.jsx
index efe7be6cddf..89674b075e8 100644
--- a/frontend/src/metabase/sharing/components/AddEditSidebar.jsx
+++ b/frontend/src/metabase/sharing/components/AddEditSidebar.jsx
@@ -56,6 +56,7 @@ export const AddEditSlackSidebar = connect(mapStateToProps)(
function _AddEditEmailSidebar({
pulse,
formInput,
+ formError,
channel,
channelSpec,
users,
@@ -76,10 +77,10 @@ function _AddEditEmailSidebar({
}) {
return (
<Sidebar
+ closeIsDisabled={!dashboardPulseIsValid(pulse, formInput.channels)}
+ formError={formError}
onClose={handleSave}
onCancel={onCancel}
- className="text-dark"
- closeIsDisabled={!dashboardPulseIsValid(pulse, formInput.channels)}
>
<div className="pt4 px4 flex align-center">
<Icon name="mail" className="mr1" size={21} />
@@ -187,6 +188,7 @@ function _AddEditEmailSidebar({
_AddEditEmailSidebar.propTypes = {
pulse: PropTypes.object.isRequired,
formInput: PropTypes.object.isRequired,
+ formError: PropTypes.object,
channel: PropTypes.object.isRequired,
channelSpec: PropTypes.object.isRequired,
users: PropTypes.array,
@@ -269,6 +271,7 @@ function getConfirmItems(pulse) {
function _AddEditSlackSidebar({
pulse,
formInput,
+ formError,
channel,
channelSpec,
parameters,
@@ -286,10 +289,10 @@ function _AddEditSlackSidebar({
}) {
return (
<Sidebar
+ closeIsDisabled={!dashboardPulseIsValid(pulse, formInput.channels)}
+ formError={formError}
onClose={handleSave}
onCancel={onCancel}
- className="text-dark"
- closeIsDisabled={!dashboardPulseIsValid(pulse, formInput.channels)}
>
<div className="pt4 flex align-center px4 mb3">
<Icon name="slack" className="mr1" size={21} />
@@ -368,6 +371,7 @@ function _AddEditSlackSidebar({
_AddEditSlackSidebar.propTypes = {
pulse: PropTypes.object.isRequired,
formInput: PropTypes.object.isRequired,
+ formError: PropTypes.object,
channel: PropTypes.object.isRequired,
channelSpec: PropTypes.object.isRequired,
users: PropTypes.array,
diff --git a/frontend/src/metabase/sharing/components/SharingSidebar.jsx b/frontend/src/metabase/sharing/components/SharingSidebar.jsx
index d2d3713cec7..ee7f79c3bb8 100644
--- a/frontend/src/metabase/sharing/components/SharingSidebar.jsx
+++ b/frontend/src/metabase/sharing/components/SharingSidebar.jsx
@@ -120,6 +120,8 @@ class SharingSidebar extends React.Component {
editingMode: "list-pulses",
// use this to know where to go "back" to
returnMode: [],
+ isSaving: false,
+ formError: null,
};
static propTypes = {
@@ -200,6 +202,11 @@ class SharingSidebar extends React.Component {
handleSave = async () => {
const { pulse, dashboard, formInput } = this.props;
+ const { isSaving } = this.state;
+
+ if (isSaving) {
+ return;
+ }
const cleanedPulse = cleanPulse(pulse, formInput.channels);
cleanedPulse.name = dashboard.name;
@@ -225,11 +232,16 @@ class SharingSidebar extends React.Component {
},
);
- await this.props.updateEditingPulse(cleanedPulse);
-
- // The order below matters; it hides the "Done" button faster and prevents two pulses from being made if it's double-clicked
- this.setState({ editingMode: "list-pulses", returnMode: [] });
- await this.props.saveEditingPulse();
+ try {
+ this.setState({ isSaving: true, formError: null });
+ await this.props.updateEditingPulse(cleanedPulse);
+ await this.props.saveEditingPulse();
+ this.setState({ editingMode: "list-pulses", returnMode: [] });
+ } catch (e) {
+ this.setState({ formError: e });
+ } finally {
+ this.setState({ isSaving: false });
+ }
};
createSubscription = () => {
@@ -275,7 +287,7 @@ class SharingSidebar extends React.Component {
};
render() {
- const { editingMode } = this.state;
+ const { editingMode, formError } = this.state;
const {
pulse,
pulses,
@@ -321,6 +333,7 @@ class SharingSidebar extends React.Component {
<AddEditEmailSidebar
pulse={pulse}
formInput={formInput}
+ formError={formError}
channel={channel}
channelSpec={channelSpec}
handleSave={this.handleSave}
@@ -363,6 +376,7 @@ class SharingSidebar extends React.Component {
<AddEditSlackSidebar
pulse={pulse}
formInput={formInput}
+ formError={formError}
channel={channel}
channelSpec={channelSpec}
handleSave={this.handleSave}
--
GitLab