diff --git a/frontend/src/metabase/components/SendTestPulse.jsx b/frontend/src/metabase/components/SendTestPulse.jsx index eb696522e7805df6cc840c8960ad31e2e6d3e997..6a665938feea7af584306e5d248320b947dceba9 100644 --- a/frontend/src/metabase/components/SendTestPulse.jsx +++ b/frontend/src/metabase/components/SendTestPulse.jsx @@ -2,11 +2,13 @@ import React, { Component } from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; +import { cleanPulse } from "metabase/lib/pulse"; import ActionButton from "metabase/components/ActionButton"; export default class SendTestPulse extends Component { static propTypes = { channel: PropTypes.object.isRequired, + channelSpecs: PropTypes.object.isRequired, pulse: PropTypes.object.isRequired, testPulse: PropTypes.func.isRequired, disabled: PropTypes.bool.isRequired, @@ -16,8 +18,11 @@ export default class SendTestPulse extends Component { static defaultProps = {}; onTestPulseChannel = () => { - const { pulse, channel, testPulse } = this.props; - return testPulse({ ...pulse, channels: [channel] }); + const { pulse, channel, channelSpecs, testPulse } = this.props; + const channelPulse = { ...pulse, channels: [channel] }; + const cleanedPulse = cleanPulse(channelPulse, channelSpecs); + + return testPulse(cleanedPulse); }; render() { diff --git a/frontend/src/metabase/lib/pulse.js b/frontend/src/metabase/lib/pulse.js index 186e60a637d8f1847c020b04837b888a41a38913..941806ab8f25793f00886912e01178157e2f74c8 100644 --- a/frontend/src/metabase/lib/pulse.js +++ b/frontend/src/metabase/lib/pulse.js @@ -4,6 +4,7 @@ import MetabaseUtils from "metabase/lib/utils"; import { hasDefaultParameterValue, hasParameterValue, + normalizeParameterValue, } from "metabase/parameters/utils/parameter-values"; export const NEW_PULSE_TEMPLATE = { @@ -119,12 +120,33 @@ export function emailIsEnabled(pulse) { export function cleanPulse(pulse, channelSpecs) { return { ...pulse, - channels: pulse.channels.filter(c => - channelIsValid(c, channelSpecs && channelSpecs[c.channel_type]), - ), + channels: cleanPulseChannels(pulse.channels, channelSpecs), + parameters: cleanPulseParameters(getPulseParameters(pulse)), }; } +function cleanPulseChannels(channels, channelSpecs) { + return channels.filter(c => + channelIsValid(c, channelSpecs && channelSpecs[c.channel_type]), + ); +} + +function cleanPulseParameters(parameters) { + return parameters.map(parameter => { + const { default: defaultValue, name, slug, type, value, id } = parameter; + const normalizedValue = normalizeParameterValue(type, value); + + return { + default: defaultValue, + id, + name, + slug, + type, + value: normalizedValue, + }; + }); +} + export function getDefaultChannel(channelSpecs) { // email is the first choice if (channelSpecs.email && channelSpecs.email.configured) { diff --git a/frontend/src/metabase/sharing/components/AddEditSidebar.jsx b/frontend/src/metabase/sharing/components/AddEditSidebar.jsx index a383c626d51a4d0be1e3448470cb89e91b6ec7c1..da05da267adc876bd1444caaad28dd15d9cd28cc 100644 --- a/frontend/src/metabase/sharing/components/AddEditSidebar.jsx +++ b/frontend/src/metabase/sharing/components/AddEditSidebar.jsx @@ -131,6 +131,7 @@ function _AddEditEmailSidebar({ <div className="pt2 pb1"> <SendTestPulse channel={channel} + channelSpecs={formInput.channels} pulse={pulse} testPulse={testPulse} normalText={t`Send email now`} @@ -329,6 +330,7 @@ function _AddEditSlackSidebar({ <div className="pt2 pb1"> <SendTestPulse channel={channel} + channelSpecs={formInput.channels} pulse={pulse} testPulse={testPulse} normalText={t`Send to Slack now`} diff --git a/frontend/src/metabase/sharing/components/SharingSidebar.jsx b/frontend/src/metabase/sharing/components/SharingSidebar.jsx index fbc81ebd770341e33a374ff2a7bf10ee1ad3382f..0328b139d908a0a7bdb1e716c60aa0379d8abbb6 100644 --- a/frontend/src/metabase/sharing/components/SharingSidebar.jsx +++ b/frontend/src/metabase/sharing/components/SharingSidebar.jsx @@ -13,14 +13,12 @@ import { import Sidebar from "metabase/dashboard/components/Sidebar"; import Pulses from "metabase/entities/pulses"; import User from "metabase/entities/users"; -import { normalizeParameterValue } from "metabase/parameters/utils/parameter-values"; import { connect } from "react-redux"; import { cleanPulse, createChannel, - getPulseParameters, NEW_PULSE_TEMPLATE, } from "metabase/lib/pulse"; @@ -207,27 +205,6 @@ class SharingSidebar extends React.Component { const cleanedPulse = cleanPulse(pulse, formInput.channels); cleanedPulse.name = dashboard.name; - cleanedPulse.parameters = getPulseParameters(cleanedPulse).map( - parameter => { - const { - default: defaultValue, - name, - slug, - type, - value, - id, - } = parameter; - const normalizedValue = normalizeParameterValue(type, value); - return { - default: defaultValue, - id, - name, - slug, - type, - value: normalizedValue, - }; - }, - ); try { this.setState({ isSaving: true }); diff --git a/frontend/test/metabase/scenarios/sharing/reproductions/18669-test-email-with-parameters.cy.spec.js b/frontend/test/metabase/scenarios/sharing/reproductions/18669-test-email-with-parameters.cy.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..f50716277825128ea0fb69176dda1dbe11c18dc3 --- /dev/null +++ b/frontend/test/metabase/scenarios/sharing/reproductions/18669-test-email-with-parameters.cy.spec.js @@ -0,0 +1,82 @@ +import { + describeWithToken, + popover, + restore, + setupSMTP, + sidebar, +} from "__support__/e2e/cypress"; +import { USERS } from "__support__/e2e/cypress_data"; +import { SAMPLE_DATASET } from "__support__/e2e/cypress_sample_dataset"; + +const { admin } = USERS; +const { PRODUCTS_ID, PRODUCTS } = SAMPLE_DATASET; + +describeWithToken("issue 18669", () => { + beforeEach(() => { + restore(); + cy.signInAsAdmin(); + setupSMTP(); + + cy.createQuestionAndDashboard({ questionDetails, dashboardDetails }).then( + ({ body: card }) => { + cy.editDashboardCard(card, getFilterMapping(card)); + cy.visit(`/dashboard/${card.dashboard_id}`); + }, + ); + }); + + it("should send a test email with non-default parameters (metabase#18669)", () => { + cy.icon("share").click(); + cy.findByText("Dashboard subscriptions").click(); + cy.findByText("Email it").click(); + + cy.findByPlaceholderText("Enter user names or email addresses") + .click() + .type(`${admin.first_name} ${admin.last_name}{enter}`) + .blur(); + + sidebar().within(() => { + cy.findByText("Doohickey").click(); + }); + + popover().within(() => { + cy.findByText("Gizmo").click(); + cy.button("Update filter").click(); + }); + + cy.button("Send email now").click(); + cy.findByText("Email sent", { timeout: 10000 }); + }); +}); + +const questionDetails = { + name: "Product count", + database: 1, + type: "query", + query: { + "source-table": PRODUCTS_ID, + aggregation: [["count"]], + }, +}; + +const filterDetails = { + name: "Category", + slug: "category", + id: "c32a49e1", + type: "category", + default: ["Doohickey"], +}; + +const dashboardDetails = { + parameters: [filterDetails], +}; + +const getFilterMapping = card => ({ + parameter_mappings: [ + { + parameter_id: filterDetails.id, + card_id: card.card_id, + target: ["dimension", ["field", PRODUCTS.CATEGORY, null]], + }, + ], +});