From b853f9d0e0972c1bde763b7c8bf4af5cc0b44482 Mon Sep 17 00:00:00 2001
From: Nick Fitzpatrick <nick@metabase.com>
Date: Fri, 18 Oct 2024 10:54:07 -0300
Subject: [PATCH] Fixing broken Configure Slack links (#48758)
* cleaning up links, adjusting tests
* guard rails
* linter fix
---
.../scenarios/sharing/alert/alert.cy.spec.js | 26 ++++++++++--
.../sharing/subscriptions.cy.spec.js | 12 +++++-
.../ChannelSetupMessage.jsx | 41 ++++++++++++++-----
.../ChannelSetupModal/ChannelSetupModal.jsx | 18 +++++---
.../pulse/components/PulseEditChannels.tsx | 1 -
.../pulse/components/WebhookChannelEdit.tsx | 13 ------
.../AlertModals/CreateAlertModalContent.jsx | 2 +-
.../sharing/components/NewPulseSidebar.tsx | 2 +-
8 files changed, 76 insertions(+), 39 deletions(-)
diff --git a/e2e/test/scenarios/sharing/alert/alert.cy.spec.js b/e2e/test/scenarios/sharing/alert/alert.cy.spec.js
index 9ff3e855a38..7e19af0a82c 100644
--- a/e2e/test/scenarios/sharing/alert/alert.cy.spec.js
+++ b/e2e/test/scenarios/sharing/alert/alert.cy.spec.js
@@ -5,6 +5,7 @@ import {
} from "e2e/support/cypress_sample_instance_data";
import {
mockSlackConfigured,
+ modal,
openSharingMenu,
popover,
restore,
@@ -29,10 +30,27 @@ describe("scenarios > alert", () => {
visitQuestion(ORDERS_QUESTION_ID);
openSharingMenu("Create alert");
- // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage
- cy.findByText(
- "To send alerts, you'll need to set up email or Slack integration.",
- );
+ modal().within(() => {
+ cy.findByText(
+ "To send alerts, you'll need to set up email, Slack or Webhook integration.",
+ );
+
+ cy.findByRole("link", { name: "Configure email" }).should(
+ "have.attr",
+ "href",
+ "/admin/settings/email",
+ );
+ cy.findByRole("link", { name: "Configure Slack" }).should(
+ "have.attr",
+ "href",
+ "/admin/settings/notifications/slack",
+ );
+ cy.findByRole("link", { name: "Configure webhook" }).should(
+ "have.attr",
+ "href",
+ "/admin/settings/notifications",
+ );
+ });
});
it("should say to non-admins that admin must add email credentials", () => {
diff --git a/e2e/test/scenarios/sharing/subscriptions.cy.spec.js b/e2e/test/scenarios/sharing/subscriptions.cy.spec.js
index 2fa5113b736..cda713890de 100644
--- a/e2e/test/scenarios/sharing/subscriptions.cy.spec.js
+++ b/e2e/test/scenarios/sharing/subscriptions.cy.spec.js
@@ -90,8 +90,16 @@ describe("scenarios > dashboard > subscriptions", () => {
it("should instruct user to connect email or slack", () => {
openDashboardSubscriptions();
// Look for the messaging about configuring slack and email
- cy.findByRole("link", { name: /set up email/i });
- cy.findByRole("link", { name: /configure Slack/i });
+ cy.findByRole("link", { name: /set up email/i }).should(
+ "have.attr",
+ "href",
+ "/admin/settings/email",
+ );
+ cy.findByRole("link", { name: /configure Slack/i }).should(
+ "have.attr",
+ "href",
+ "/admin/settings/notifications/slack",
+ );
});
});
diff --git a/frontend/src/metabase/components/ChannelSetupMessage/ChannelSetupMessage.jsx b/frontend/src/metabase/components/ChannelSetupMessage/ChannelSetupMessage.jsx
index 35075149e95..1211aaf095d 100644
--- a/frontend/src/metabase/components/ChannelSetupMessage/ChannelSetupMessage.jsx
+++ b/frontend/src/metabase/components/ChannelSetupMessage/ChannelSetupMessage.jsx
@@ -9,6 +9,21 @@ import ButtonsS from "metabase/css/components/buttons.module.css";
import CS from "metabase/css/core/index.css";
import Settings from "metabase/lib/settings";
+const CHANNEL_MAP = {
+ email: {
+ name: "email",
+ link: "/admin/settings/email",
+ },
+ slack: {
+ name: "Slack",
+ link: "/admin/settings/notifications/slack",
+ },
+ webhook: {
+ name: "Webhook",
+ link: "/admin/settings/notifications",
+ },
+};
+
export default class ChannelSetupMessage extends Component {
static propTypes = {
user: PropTypes.object.isRequired,
@@ -16,7 +31,7 @@ export default class ChannelSetupMessage extends Component {
};
static defaultProps = {
- channels: ["email", "Slack"],
+ channels: ["email", "Slack", "webhook"],
};
render() {
@@ -25,16 +40,20 @@ export default class ChannelSetupMessage extends Component {
if (user.is_superuser) {
content = (
<div>
- {channels.map(c => (
- <Link
- to={"/admin/settings/" + c.toLowerCase()}
- key={c.toLowerCase()}
- className={cx(ButtonsS.Button, ButtonsS.ButtonPrimary, CS.mr1)}
- target={window.OSX ? null : "_blank"}
- >
- {t`Configure`} {c}
- </Link>
- ))}
+ {channels.map(c => {
+ const config = CHANNEL_MAP[c.toLowerCase()];
+
+ return config ? (
+ <Link
+ to={CHANNEL_MAP[c.toLowerCase()].link}
+ key={c.toLowerCase()}
+ className={cx(ButtonsS.Button, ButtonsS.ButtonPrimary, CS.mr1)}
+ target={window.OSX ? null : "_blank"}
+ >
+ {t`Configure`} {c}
+ </Link>
+ ) : null;
+ })}
</div>
);
} else {
diff --git a/frontend/src/metabase/components/ChannelSetupModal/ChannelSetupModal.jsx b/frontend/src/metabase/components/ChannelSetupModal/ChannelSetupModal.jsx
index 8ccfd8a58d2..157a0d05492 100644
--- a/frontend/src/metabase/components/ChannelSetupModal/ChannelSetupModal.jsx
+++ b/frontend/src/metabase/components/ChannelSetupModal/ChannelSetupModal.jsx
@@ -6,6 +6,16 @@ import { t } from "ttag";
import ChannelSetupMessage from "metabase/components/ChannelSetupMessage";
import ModalContent from "metabase/components/ModalContent";
import { Flex } from "metabase/ui";
+
+const formatChannelString = channels => {
+ const lastChannel = channels[channels.length - 1];
+ const restChannels = channels.slice(0, -1);
+
+ return restChannels.length > 0
+ ? `${restChannels.join(", ")} ${t` or `} ${lastChannel}`
+ : lastChannel;
+};
+
export default class ChannelSetupModal extends Component {
static propTypes = {
onClose: PropTypes.func.isRequired,
@@ -26,12 +36,8 @@ export default class ChannelSetupModal extends Component {
onClose={onClose}
title={
user.is_superuser
- ? t`To send ${entityNamePlural}, you'll need to set up ${channels.join(
- t` or `,
- )} integration.`
- : t`To send ${entityNamePlural}, an admin needs to set up ${channels.join(
- t` or `,
- )} integration.`
+ ? t`To send ${entityNamePlural}, you'll need to set up ${formatChannelString(channels)} integration.`
+ : t`To send ${entityNamePlural}, an admin needs to set up ${formatChannelString(channels)} integration.`
}
>
<Flex justify="center">
diff --git a/frontend/src/metabase/pulse/components/PulseEditChannels.tsx b/frontend/src/metabase/pulse/components/PulseEditChannels.tsx
index 9c77324aec6..3b18d32a3c3 100644
--- a/frontend/src/metabase/pulse/components/PulseEditChannels.tsx
+++ b/frontend/src/metabase/pulse/components/PulseEditChannels.tsx
@@ -138,7 +138,6 @@ export const PulseEditChannels = ({
{notificationChannels.map(notification => (
<WebhookChannelEdit
key={`webhook-${notification.id}`}
- user={user}
toggleChannel={toggleChannel}
channelSpec={channels.http}
alert={pulse}
diff --git a/frontend/src/metabase/pulse/components/WebhookChannelEdit.tsx b/frontend/src/metabase/pulse/components/WebhookChannelEdit.tsx
index d1acbb5613b..e9c29f8184d 100644
--- a/frontend/src/metabase/pulse/components/WebhookChannelEdit.tsx
+++ b/frontend/src/metabase/pulse/components/WebhookChannelEdit.tsx
@@ -2,7 +2,6 @@ import cx from "classnames";
import { t } from "ttag";
import { useTestAlertMutation } from "metabase/api";
-import ChannelSetupMessage from "metabase/components/ChannelSetupMessage";
import { Ellipsified } from "metabase/core/components/Ellipsified";
import CS from "metabase/css/core/index.css";
import { useActionButtonLabel } from "metabase/hooks/use-action-button-label";
@@ -12,7 +11,6 @@ import type {
Alert,
ChannelSpec,
NotificationChannel,
- User,
} from "metabase-types/api";
export const WebhookChannelEdit = ({
@@ -20,7 +18,6 @@ export const WebhookChannelEdit = ({
alert,
notification,
toggleChannel,
- user,
}: {
channelSpec: ChannelSpec;
alert: Alert;
@@ -30,7 +27,6 @@ export const WebhookChannelEdit = ({
value: boolean,
notification: NotificationChannel,
) => void;
- user: User;
notification: NotificationChannel;
}) => {
const [testAlert, testAlertRequest] = useTestAlertMutation();
@@ -103,16 +99,7 @@ export const WebhookChannelEdit = ({
</Box>
</Flex>
</li>
-
- {/* {renderChannel(channel, channelSpec, channelIndex)} */}
</ul>
- ) : channel?.enabled && !channelSpec.configured ? (
- <div className={cx(CS.p4, CS.textCentered)}>
- <h3
- className={CS.mb2}
- >{t`${channelSpec.name} needs to be set up by an administrator.`}</h3>
- <ChannelSetupMessage user={user} channels={[channelSpec.name]} />
- </div>
) : null}
</li>
);
diff --git a/frontend/src/metabase/query_builder/components/AlertModals/CreateAlertModalContent.jsx b/frontend/src/metabase/query_builder/components/AlertModals/CreateAlertModalContent.jsx
index 22d9081880e..05e2b0178f1 100644
--- a/frontend/src/metabase/query_builder/components/AlertModals/CreateAlertModalContent.jsx
+++ b/frontend/src/metabase/query_builder/components/AlertModals/CreateAlertModalContent.jsx
@@ -104,7 +104,7 @@ class CreateAlertModalContentInner extends Component {
user={user}
onClose={onCancel}
entityNamePlural={t`alerts`}
- channels={isAdmin ? ["email", "Slack"] : ["email"]}
+ channels={isAdmin ? ["email", "Slack", "Webhook"] : ["email"]}
/>
);
}
diff --git a/frontend/src/metabase/sharing/components/NewPulseSidebar.tsx b/frontend/src/metabase/sharing/components/NewPulseSidebar.tsx
index 70e77bb3cd9..69366694522 100644
--- a/frontend/src/metabase/sharing/components/NewPulseSidebar.tsx
+++ b/frontend/src/metabase/sharing/components/NewPulseSidebar.tsx
@@ -108,7 +108,7 @@ export function NewPulseSidebar({
jt`First, you'll have to ${(
<Link
key="link"
- to="/admin/settings/slack"
+ to="/admin/settings/notifications/slack"
className={CS.link}
>
{t`configure Slack`}
--
GitLab