Collection of UI updates to the alert modal (#48431) (#48487)

* collection of UI updates to the alert modal

* remiving unused es lint directive

* adding tag to cypress test

Co-authored-by: Nick Fitzpatrick <nick@metabase.com>

e2e/support/helpers/e2e-notification-helpers.js → e2e/support/helpers/e2e-notification-helpers.ts

+import type { NotificationChannel } from "../../../frontend/src/metabase-types/api/notifications";
+
 export const getAlertChannel = name =>
   cy.findByRole("listitem", {
     name,
@@ -8,3 +10,18 @@ export const WEBHOOK_TEST_HOST = "http://127.0.0.1:9080";
 
 export const WEBHOOK_TEST_URL = `${WEBHOOK_TEST_HOST}/${WEBHOOK_TEST_SESSION_ID}`;
 export const WEBHOOK_TEST_DASHBOARD = `${WEBHOOK_TEST_HOST}/#/${WEBHOOK_TEST_SESSION_ID}`;
+
+export const setupNotificationChannel = (
+  opts: Partial<NotificationChannel>,
+) => {
+  cy.request("POST", "/api/channel", {
+    type: "channel/http",
+    details: {
+      url: `${WEBHOOK_TEST_HOST}/${WEBHOOK_TEST_SESSION_ID}`,
+      "fe-form-type": "none",
+      "auth-method": "none",
+      "auth-info": {},
+    },
+    ...opts,
+  });
+};

e2e/support/helpers/e2e-sharing-helpers.ts

@@ -7,3 +7,7 @@ export const openSharingMenu = (menuItemText?: string) => {
     sharingMenu().findByText(menuItemText).click();
   }
 };
+
+export const toggleAlertChannel = channel => {
+  cy.findByText(channel).parent().find("input").click({ force: true });
+};

e2e/test/scenarios/sharing/alert/alert.cy.spec.js

@@ -6,9 +6,12 @@ import {
 import {
   mockSlackConfigured,
   openSharingMenu,
+  popover,
   restore,
+  setupNotificationChannel,
   setupSMTP,
   sharingMenuButton,
+  toggleAlertChannel,
   visitModel,
   visitQuestion,
 } from "e2e/support/helpers";
@@ -92,6 +95,49 @@ describe("scenarios > alert", () => {
     });
   });
 
+  describe("with a webhook", { tags: ["@external"] }, () => {
+    beforeEach(() => {
+      setupNotificationChannel({
+        name: "Foo Hook",
+        description: "This is a hook",
+      });
+      setupNotificationChannel({
+        name: "Bar Hook",
+        description: "This is another hook",
+      });
+      cy.setCookie("metabase.SEEN_ALERT_SPLASH", "true");
+    });
+
+    it("should be able to create and delete alerts with webhooks enabled", () => {
+      visitQuestion(ORDERS_QUESTION_ID);
+      openSharingMenu("Create alert");
+
+      //Disable Email
+      toggleAlertChannel("Email");
+      toggleAlertChannel("Foo Hook");
+      toggleAlertChannel("Bar Hook");
+
+      cy.findByRole("button", { name: "Done" }).click();
+
+      openSharingMenu("Edit alerts");
+
+      popover().within(() => {
+        cy.findByText("You set up an alert").should("be.visible");
+        cy.findByText("Edit").click();
+      });
+
+      cy.findByRole("button", { name: "Delete this alert" }).click();
+
+      cy.log(
+        "Webhooks should render with their given names in delete modal metabase#48428",
+      );
+      cy.findByRole("checkbox", { name: /Channel Foo Hook/ }).click();
+      cy.findByRole("checkbox", { name: /Channel Bar Hook/ }).click();
+
+      cy.findByRole("button", { name: "Delete this alert" }).click();
+    });
+  });
+
   it("should not be offered for models (metabase#37893)", () => {
     visitModel(ORDERS_MODEL_ID);
     cy.findByTestId("view-footer").within(() => {

e2e/test/scenarios/sharing/alert/email-alert.cy.spec.js

 import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
 import { ORDERS_QUESTION_ID } from "e2e/support/cypress_sample_instance_data";
 import {
+  mockSlackConfigured,
   modal,
   openSharingMenu,
   openTable,
   popover,
   restore,
+  setupNotificationChannel,
   setupSMTP,
+  toggleAlertChannel,
   updateSetting,
   visitQuestion,
 } from "e2e/support/helpers";
@@ -35,6 +38,15 @@ describe("scenarios > alert > email_alert", { tags: "@external" }, () => {
 
   it("should set up an email alert", () => {
     openAlertForQuestion(ORDERS_QUESTION_ID);
+
+    cy.log(
+      "Should not display slack channel if it is not configured metabase#48407",
+    );
+    cy.findByTestId("alert-create").within(() => {
+      cy.findByTestId("loading-indicator").should("not.exist");
+      cy.findByRole("heading", { name: "Slack" }).should("not.exist");
+    });
+
     cy.button("Done").click();
 
     cy.wait("@savedAlert").then(({ response: { body } }) => {
@@ -47,28 +59,37 @@ describe("scenarios > alert > email_alert", { tags: "@external" }, () => {
   it("should respect email alerts toggled off (metabase#12349)", () => {
     updateSetting("report-timezone", "America/New_York");
 
+    //For this test, we need to pretend that slack is set up
+    mockSlackConfigured();
+    setupNotificationChannel({ name: "Webhook" });
+
     openAlertForQuestion(ORDERS_QUESTION_ID);
 
     cy.findByTestId("alert-create").within(() => {
       cy.findByText(/Emails will be sent at 12:00 AM ET/).should("exist");
 
       // Turn off email
-      toggleChannel("Email");
+      toggleAlertChannel("Email");
       cy.findByText(/Emails will be sent/).should("not.exist");
       cy.findByText(/Slack messages will be sent/).should("not.exist");
 
       // Turn on Slack
-      toggleChannel("Slack");
+      toggleAlertChannel("Slack");
+      cy.findByPlaceholderText(/Pick a user or channel/).click();
+    });
+
+    popover().findByText("#work").click();
 
+    cy.findByTestId("alert-create").within(() => {
       cy.findByText(/Slack messages will be sent at 12:00 AM ET/).should(
         "exist",
       );
 
-      toggleChannel("Email");
+      toggleAlertChannel("Email");
       cy.findByText(
         /Emails and Slack messages will be sent at 12:00 AM ET/,
       ).should("exist");
-      toggleChannel("Email");
+      toggleAlertChannel("Email");
 
       cy.button("Done").click();
     });
@@ -78,6 +99,19 @@ describe("scenarios > alert > email_alert", { tags: "@external" }, () => {
       expect(body.channels[0].channel_type).to.eq("email");
       expect(body.channels[0].enabled).to.eq(false);
     });
+
+    cy.log(
+      "ensure that when the alert is deleted, the delete modal is correct metabase#48402",
+    );
+    openSharingMenu("Edit alerts");
+    popover().within(() => {
+      cy.findByText("You set up an alert").should("be.visible");
+      cy.findByText("Edit").click();
+    });
+
+    cy.findByRole("button", { name: "Delete this alert" }).click();
+    cy.findByRole("checkbox", { name: /be emailed to / }).should("not.exist");
+    cy.findByRole("checkbox", { name: /Slack channel / }).should("exist");
   });
 
   it("should set up an email alert for newly created question", () => {
@@ -144,10 +178,6 @@ function openAlertForQuestion(id) {
   openSharingMenu("Create alert");
 }
 
-function toggleChannel(channel) {
-  cy.findByText(channel).parent().find("input").click({ force: true });
-}
-
 function saveAlert() {
   openSharingMenu();
 

frontend/src/metabase-types/api/notifications.ts

@@ -13,7 +13,7 @@ export type NotificationRecipient = {
 
 export type Channel = {
   channel_type: string;
-  details: Record<string, string>;
+  details?: Record<string, string>;
   enabled?: boolean;
   recipients?: User[];
   channel_id?: number;

frontend/src/metabase/lib/alert.js

@@ -18,6 +18,11 @@ export function channelIsValid(channel) {
   }
 }
 
+export function channelIsEnabled(channel) {
+  return channel.enabled;
+}
+
 export function alertIsValid(alert) {
-  return alert.channels.length > 0 && alert.channels.every(channelIsValid);
+  const enabledChannels = alert.channels.filter(channelIsEnabled);
+  return enabledChannels.length > 0 && enabledChannels.every(channelIsValid);
 }

frontend/src/metabase/lib/pulse.ts

@@ -184,13 +184,10 @@ export function createChannel(
   channelSpec: ChannelSpec,
   opts?: Partial<Channel>,
 ): Channel {
-  const details = {};
-
   return {
     channel_type: channelSpec.type,
     enabled: true,
     recipients: [],
-    details: details,
     schedule_type: channelSpec.schedules[0],
     schedule_day: "mon",
     schedule_hour: 8,

frontend/src/metabase/pulse/components/PulseEditChannels.tsx

@@ -126,13 +126,15 @@ export const PulseEditChannels = ({
         alert={pulse}
         invalidRecipientText={invalidRecipientText}
       />
-      <SlackChannelEdit
-        user={user}
-        toggleChannel={toggleChannel}
-        onChannelPropertyChange={onChannelPropertyChange}
-        channelSpec={channels.slack}
-        alert={pulse}
-      />
+      {channels.slack.configured && (
+        <SlackChannelEdit
+          user={user}
+          toggleChannel={toggleChannel}
+          onChannelPropertyChange={onChannelPropertyChange}
+          channelSpec={channels.slack}
+          alert={pulse}
+        />
+      )}
       {notificationChannels.map(notification => (
         <WebhookChannelEdit
           key={`webhook-${notification.id}`}

frontend/src/metabase/query_builder/components/AlertModals/DeleteAlertSection.jsx

-/* eslint-disable react/prop-types */
-import cx from "classnames";
-import { Component } from "react";
-import { jt, msgid, ngettext, t } from "ttag";
-
-import DeleteModalWithConfirm from "metabase/components/DeleteModalWithConfirm";
-import ModalWithTrigger from "metabase/components/ModalWithTrigger";
-import Button from "metabase/core/components/Button";
-import ButtonsS from "metabase/css/components/buttons.module.css";
-import CS from "metabase/css/core/index.css";
-
-import AlertModalsS from "./AlertModals.module.css";
-import { DangerZone } from "./AlertModals.styled";
-
-export class DeleteAlertSection extends Component {
-  getConfirmItems() {
-    // same as in PulseEdit but with some changes to copy
-    return this.props.alert.channels.map((c, index) =>
-      c.channel_type === "email" ? (
-        <span
-          key={`${c.channel_type}-${index}`}
-        >{jt`This alert will no longer be emailed to ${(
-          <strong>
-            {(n => ngettext(msgid`${n} address`, `${n} addresses`, n))(
-              c.recipients.length,
-            )}
-          </strong>
-        )}.`}</span>
-      ) : c.channel_type === "slack" ? (
-        <span>{jt`Slack channel ${(
-          <strong>{c.details && c.details.channel}</strong>
-        )} will no longer get this alert.`}</span>
-      ) : (
-        <span>{jt`Channel ${(
-          <strong>{c.channel_type}</strong>
-        )} will no longer receive this alert.`}</span>
-      ),
-    );
-  }
-
-  render() {
-    const { onDeleteAlert } = this.props;
-
-    return (
-      <DangerZone
-        className={cx(
-          AlertModalsS.AlertModalsBorder,
-          CS.bordered,
-          CS.mt4,
-          CS.pt4,
-          CS.mb2,
-          CS.p3,
-          CS.rounded,
-          CS.relative,
-        )}
-      >
-        <h3
-          className={cx(CS.textError, CS.absolute, CS.top, CS.bgWhite, CS.px1)}
-          style={{ marginTop: "-12px" }}
-        >{jt`Danger Zone`}</h3>
-        <div className={CS.ml1}>
-          <h4 className={cx(CS.textBold, CS.mb1)}>{jt`Delete this alert`}</h4>
-          <div className={CS.flex}>
-            <p
-              className={cx(CS.h4, CS.pr2)}
-            >{jt`Stop delivery and delete this alert. There's no undo, so be careful.`}</p>
-            <ModalWithTrigger
-              ref={ref => (this.deleteModal = ref)}
-              as={Button}
-              triggerClasses={cx(
-                ButtonsS.ButtonDanger,
-                CS.flexAlignRight,
-                CS.flexNoShrink,
-                CS.alignSelfEnd,
-              )}
-              triggerElement={t`Delete this alert`}
-            >
-              <DeleteModalWithConfirm
-                objectType="alert"
-                title={t`Delete this alert?`}
-                confirmItems={this.getConfirmItems()}
-                onClose={() => this.deleteModal.close()}
-                onDelete={onDeleteAlert}
-              />
-            </ModalWithTrigger>
-          </div>
-        </div>
-      </DangerZone>
-    );
-  }
-}

frontend/src/metabase/query_builder/components/AlertModals/DeleteAlertSection.tsx

+import cx from "classnames";
+import { useMemo, useRef } from "react";
+import { jt, msgid, ngettext, t } from "ttag";
+
+import { useListChannelsQuery } from "metabase/api/channel";
+import DeleteModalWithConfirm from "metabase/components/DeleteModalWithConfirm";
+import ModalWithTrigger from "metabase/components/ModalWithTrigger";
+import Button from "metabase/core/components/Button";
+import ButtonsS from "metabase/css/components/buttons.module.css";
+import CS from "metabase/css/core/index.css";
+import { channelIsEnabled } from "metabase/lib/alert";
+import type { Alert } from "metabase-types/api";
+
+import AlertModalsS from "./AlertModals.module.css";
+import { DangerZone } from "./AlertModals.styled";
+
+export const DeleteAlertSection = ({
+  onDeleteAlert,
+  alert,
+}: {
+  onDeleteAlert: () => void;
+  alert: Alert;
+}) => {
+  const deleteModal = useRef<any>(null);
+
+  const { data: notificationChannels = [] } = useListChannelsQuery();
+
+  const getConfirmItems = useMemo(() => {
+    // same as in PulseEdit but with some changes to copy
+    return alert.channels.filter(channelIsEnabled).map((channel, index) => {
+      switch (channel.channel_type) {
+        case "email": {
+          return (
+            <span
+              key={`${channel.channel_type}-${index}`}
+            >{jt`This alert will no longer be emailed to ${(
+              <strong>
+                {(n => ngettext(msgid`${n} address`, `${n} addresses`, n || 0))(
+                  channel.recipients?.length,
+                )}
+              </strong>
+            )}.`}</span>
+          );
+        }
+        case "slack": {
+          return (
+            <span>{jt`Slack channel ${(
+              <strong>{channel.details && channel.details.channel}</strong>
+            )} will no longer get this alert.`}</span>
+          );
+        }
+        case "http": {
+          const notification = notificationChannels.find(
+            notificationChannels =>
+              notificationChannels.id === channel.channel_id,
+          );
+          return (
+            <span>{jt`Channel ${(
+              <strong>{notification?.name || channel.channel_type}</strong>
+            )} will no longer receive this alert.`}</span>
+          );
+        }
+        default: {
+          return (
+            <span>{jt`Channel ${(
+              <strong>{channel.channel_type}</strong>
+            )} will no longer receive this alert.`}</span>
+          );
+        }
+      }
+    });
+  }, [notificationChannels, alert.channels]);
+
+  return (
+    <DangerZone
+      className={cx(
+        AlertModalsS.AlertModalsBorder,
+        CS.bordered,
+        CS.mt4,
+        CS.pt4,
+        CS.mb2,
+        CS.p3,
+        CS.rounded,
+        CS.relative,
+      )}
+    >
+      <h3
+        className={cx(CS.textError, CS.absolute, CS.top, CS.bgWhite, CS.px1)}
+        style={{ marginTop: "-12px" }}
+      >{jt`Danger Zone`}</h3>
+      <div className={CS.ml1}>
+        <h4 className={cx(CS.textBold, CS.mb1)}>{jt`Delete this alert`}</h4>
+        <div className={CS.flex}>
+          <p
+            className={cx(CS.h4, CS.pr2)}
+          >{jt`Stop delivery and delete this alert. There's no undo, so be careful.`}</p>
+          <ModalWithTrigger
+            ref={deleteModal}
+            as={Button}
+            triggerClasses={cx(
+              ButtonsS.ButtonDanger,
+              CS.flexAlignRight,
+              CS.flexNoShrink,
+              CS.alignSelfEnd,
+            )}
+            triggerElement={t`Delete this alert`}
+          >
+            <DeleteModalWithConfirm
+              objectType="alert"
+              title={t`Delete this alert?`}
+              confirmItems={getConfirmItems}
+              onClose={() => deleteModal.current?.close()}
+              onDelete={onDeleteAlert}
+            />
+          </ModalWithTrigger>
+        </div>
+      </div>
+    </DangerZone>
+  );
+};