fix(alerts): email destination form fixes (#15580)

tests/ui-testing/playwright-tests/Alerts/dl-email-destinations.spec.js

@@ -682,19 +682,10 @@ test.describe('Email destinations and distribution lists', () => {
 
   }); // end delivery
 
-  // ══ REGRESSION TESTS FOR OPEN DEFECTS (#2471) ════════════════════════════
-  // These assert the CORRECT behaviour and therefore fail on today's build.
-  // Skipped rather than left failing, since none of the defects is P0/blocking.
-  // Un-skip the matching test as each issue closes — do not soften the assertion.
-  // (#2471 B4, semicolon-separated lists, no longer has a subject: recipients are
-  // picked from accounts, so no separator is ever parsed.)
-
-  test.skip('UI-13 · the email form shows no Skip TLS Verify control [#2471 B5]', {
+  test('UI-13 · the email form shows no Skip TLS Verify control [#2471 B5]', {
     tag: ['@dlEmailDestinations', '@email', '@regression', '@P3', '@all'],
   }, async () => {
-    // BLOCKED BY #2471 (B5): the toggle is rendered on the email form but has no
-    // effect — an email destination carries only recipients, and SMTP transport
-    // security is server configuration (ZO_SMTP_ENCRYPTION).
+    // SMTP transport security is server configuration (ZO_SMTP_ENCRYPTION), so the toggle would do nothing.
     await pm.alertDestinationsPage.clickNewDestination();
     await pm.alertDestinationsPage.selectDestinationType('email');
 
@@ -702,12 +693,9 @@ test.describe('Email destinations and distribution lists', () => {
       'a control with no effect on email must not be shown').toBe(false);
   });
 
-  test.skip('UI-14 · Preview shows the configured sender and the entered recipients [#2471 B7]', {
+  test('UI-14 · Preview shows the entered recipients [#2471 B7]', {
     tag: ['@dlEmailDestinations', '@email', '@regression', '@P3', '@all'],
   }, async () => {
-    // BLOCKED BY #2471 (B7): the dialog renders hardcoded
-    // From: alerts@openobserve.ai / To: admin@example.com, so it answers
-    // "will this reach the right people?" wrongly.
     const destName = `auto_dest_dl_preview_${RUN}`;
     await fillEmailForm(pm, destName, ORG_USER);
     const preview = await pm.alertDestinationsPage.openPreviewAndGetText();
@@ -716,6 +704,13 @@ test.describe('Email destinations and distribution lists', () => {
     expect(preview, 'Preview must not show a placeholder recipient').not.toContain('admin@example.com');
   });
 
+  // ══ REGRESSION TESTS FOR OPEN DEFECTS (#2471) ════════════════════════════
+  // These assert the CORRECT behaviour and therefore fail on today's build.
+  // Skipped rather than left failing, since none of the defects is P0/blocking.
+  // Un-skip the matching test as each issue closes — do not soften the assertion.
+  // (#2471 B4, semicolon-separated lists, no longer has a subject: recipients are
+  // picked from accounts, so no separator is ever parsed.)
+
   // BLOCKED BY #2471 (B2): membership is validated in destinations.rs save()
   // and test_email() only; the send path never re-checks, so alerts keep
   // reaching a removed user while the destination can no longer be saved.

web/src/components/alerts/AddDestination.spec.ts

@@ -143,7 +143,7 @@ function mountComp(props: Record<string, any> = {}) {
         DestinationPreview: {
           name: "DestinationPreview",
           template: '<div data-test="destination-preview-stub"></div>',
-          props: ["modelValue", "type", "templateContent"],
+          props: ["modelValue", "type", "templateContent", "recipients"],
         },
         AppTabs: {
           template: '<div data-test="app-tabs-stub"></div>',
@@ -1367,3 +1367,70 @@ describe("AddDestination — email recipients are org users", () => {
     ]);
   });
 });
+
+describe("AddDestination — email destinations", () => {
+  const TEMPLATES = [
+    { name: "email_tpl", type: "email", body: "" },
+    { name: "raw_http_tpl", type: "http", kind: "custom", body: "" },
+    { name: "content_tpl", type: "http", kind: "content", body: "" },
+  ];
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    vi.mocked(usersService.orgUsers).mockResolvedValue({ data: { data: [] } } as any);
+    vi.mocked(usersService.getRoles).mockResolvedValue({ data: [] } as any);
+  });
+
+  const toPrebuiltEmail = async (w: any) => {
+    const form = getForm(w);
+    form.setFieldValue("destination_type", "email");
+    form.setFieldValue("type", "email");
+    form.setFieldValue("credentials", { recipients: ["a@acme.io", "b@acme.io"] });
+    await flushPromises();
+    return form;
+  };
+
+  it("offers content templates alongside email templates, on both paths", async () => {
+    wrapper = mountComp({ templates: TEMPLATES });
+    const form = getForm(wrapper);
+    form.setFieldValue("destination_type", "custom");
+    form.setFieldValue("type", "email");
+    await nextTick();
+    expect(wrapper.vm.getFormattedTemplates).toEqual(["email_tpl", "content_tpl"]);
+
+    await toPrebuiltEmail(wrapper);
+    expect(wrapper.vm.prebuiltTemplateOptions.map((o: any) => o.value)).toEqual([
+      "prebuilt_email",
+      "email_tpl",
+      "content_tpl",
+    ]);
+  });
+
+  it("hides Additional Settings for prebuilt email, keeps it for other prebuilt types", async () => {
+    wrapper = mountComp();
+    await toPrebuiltEmail(wrapper);
+    expect(wrapper.find('[data-test="prebuilt-additional-settings"]').exists()).toBe(false);
+    expect(wrapper.find('[data-test="add-destination-skip-tls-verify-toggle"]').exists()).toBe(
+      false,
+    );
+
+    const form = getForm(wrapper);
+    form.setFieldValue("destination_type", "teams");
+    form.setFieldValue("type", "http");
+    form.setFieldValue("credentials", {});
+    await flushPromises();
+    expect(wrapper.find('[data-test="prebuilt-additional-settings"]').exists()).toBe(true);
+    expect(wrapper.find('[data-test="add-destination-skip-tls-verify-toggle"]').exists()).toBe(
+      true,
+    );
+  });
+
+  it("passes the entered recipients to the preview", async () => {
+    wrapper = mountComp();
+    await toPrebuiltEmail(wrapper);
+    expect(wrapper.findComponent({ name: "DestinationPreview" }).props("recipients")).toEqual([
+      "a@acme.io",
+      "b@acme.io",
+    ]);
+  });
+});

web/src/components/alerts/AddDestination.vue

@@ -140,9 +140,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
               </div>
             </div>
 
-            <!-- Additional Settings for Prebuilt Destinations -->
+            <!-- None of these settings apply to email destinations -->
             <div
-              v-if="showPrebuiltAdvancedSettings"
+              v-if="showPrebuiltAdvancedSettings && dtVal !== 'email'"
               class="mt-3 w-full"
               data-test="prebuilt-additional-settings"
             >
@@ -150,8 +150,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
                 {{ t("alert_destinations.additional_settings") }}
               </div>
 
-              <!-- Custom Headers (hidden for email destinations) -->
-              <div v-if="dtVal !== 'email'" class="py-2">
+              <!-- Custom Headers -->
+              <div class="py-2">
                 <div class="pb-1 text-sm font-medium">
                   {{ t("alert_destinations.custom_headers") }}
                 </div>
@@ -479,6 +479,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
       v-model="showPreviewModal"
       :type="dtVal"
       :template-content="previewContent"
+      :recipients="previewRecipients"
       data-test="destination-preview-modal"
     />
   </OPageLayout>
@@ -709,6 +710,9 @@ const prebuiltCredentials = form.useStore(
 const destinationSearchQuery = ref("");
 const showPreviewModal = ref(false);
 const previewContent = ref("");
+const previewRecipients = computed(() =>
+  Array.isArray(prebuiltCredentials.value.recipients) ? prebuiltCredentials.value.recipients : [],
+);
 
 // A test result is stale once anything it was sent with changes; an in-flight
 // test is discarded too.
@@ -1150,7 +1154,8 @@ const extractPrebuiltCredentials = (typeId: string): Record<string, any> => {
 const getFormattedTemplates = computed(() =>
   props.templates
     .filter((template) => {
-      if (typeVal.value === "email" && template.type === "email") return true;
+      if (typeVal.value === "email" && (template.type === "email" || template.kind === "content"))
+        return true;
       else if (typeVal.value !== "email") return true;
       return false;
     })
@@ -1171,13 +1176,13 @@ const defaultPrebuiltTemplateName = computed(() => templateNameFor(dtVal.value))
 
 // Template choices for a prebuilt destination: the API-sourced prebuilt
 // template for this type as the first (default) option, followed by any
-// user-created custom templates of the matching kind (email vs http).
+// user-created templates that fit the channel (email or content for email).
 // Other prebuilt types are excluded to prevent cross-type mismatches.
 const prebuiltTemplateOptions = computed(() => {
   const isEmailType = dtVal.value === "email";
   const matching = props.templates.filter((template) => {
     if (template.isPrebuilt) return false;
-    if (isEmailType) return template.type === "email";
+    if (isEmailType) return template.type === "email" || template.kind === "content";
     return template.type !== "email";
   });
 

web/src/components/alerts/DestinationPreview.spec.ts

@@ -199,10 +199,15 @@ describe("DestinationPreview", () => {
     expect(subject.text()).toContain("OpenObserve Alert Notification");
   });
 
-  it("should display Email from address", () => {
+  it("shows the entered recipients as To and no hardcoded From", () => {
+    wrapper = mountComponent({ type: "email", recipients: ["a@acme.io", "b@acme.io"] });
+    expect(wrapper.find('[data-test="email-to"]').text()).toBe("To: a@acme.io, b@acme.io");
+    expect(wrapper.find('[data-test="email-from"]').exists()).toBe(false);
+  });
+
+  it("shows no To line before any recipient is picked", () => {
     wrapper = mountComponent({ type: "email" });
-    const from = wrapper.find('[data-test="email-from"]');
-    expect(from.text()).toContain("alerts@openobserve.ai");
+    expect(wrapper.find('[data-test="email-to"]').exists()).toBe(false);
   });
 
   it("should display Email body", () => {

web/src/components/alerts/DestinationPreview.vue

@@ -191,11 +191,12 @@ limitations under the License.
           >
             {{ raw("Subject: 🚨 OpenObserve Alert Notification") }}
           </div>
-          <div data-test="email-from" class="email-from text-brand-msg-meta mb-1 text-sm">
-            {{ raw("From: alerts@openobserve.ai") }}
-          </div>
-          <div class="email-to text-brand-msg-meta mb-1 text-sm">
-            {{ raw("To: admin@example.com") }}
+          <div
+            v-if="recipients.length"
+            data-test="email-to"
+            class="email-to text-brand-msg-meta mb-1 text-sm"
+          >
+            {{ raw(`To: ${recipients.join(", ")}`) }}
           </div>
           <div class="email-time text-brand-msg-meta mb-1 text-sm">
             {{ getCurrentTime() }}
@@ -443,11 +444,13 @@ interface Props {
   modelValue?: boolean;
   type: string;
   templateContent?: string;
+  recipients?: string[];
 }
 
 const props = withDefaults(defineProps<Props>(), {
   modelValue: false,
   templateContent: "",
+  recipients: () => [],
 });
 
 const emit = defineEmits(["update:modelValue"]);

web/src/components/alerts/ImportDestination.spec.ts

@@ -264,6 +264,24 @@ describe("ImportDestination", () => {
       expect(result).toBe(true);
     });
 
+    it("accepts and offers a content template for an email destination", async () => {
+      await wrapper.setProps({
+        templates: [
+          ...defaultProps.templates,
+          { name: "content-template", type: "http", kind: "content" },
+        ],
+      });
+      const result = await wrapper.vm.validateDestinationInputs(
+        { name: "email-dest", type: "email", template: "content-template", emails: ["recipient"] },
+        1,
+      );
+      expect(result).toBe(true);
+
+      wrapper.vm.jsonArrayOfObj = [{ type: "email" }];
+      wrapper.vm.destinationErrorsToDisplay = [[]];
+      expect(wrapper.vm.getFormattedTemplates).toEqual(["email-template", "content-template"]);
+    });
+
     it("returns true for a valid email destination", async () => {
       const result = await wrapper.vm.validateDestinationInputs(
         { name: "email-dest", type: "email", template: "email-template", emails: ["a@b.com"] },

web/src/components/alerts/ImportDestination.vue

@@ -348,7 +348,10 @@ export default defineComponent({
           const currentDestinationType =
             jsonArrayOfObj.value[destinationErrorsToDisplay.value.length - 1]?.type;
 
-          if (currentDestinationType === "email" && template.type === "email") {
+          if (
+            currentDestinationType === "email" &&
+            (template.type === "email" || template.kind === "content")
+          ) {
             return true;
           } else if (currentDestinationType !== "email") {
             return true;
@@ -640,7 +643,11 @@ export default defineComponent({
       if (input.template) {
         const availableTemplates = props.templates
           .filter((template: any) => {
-            if (input.type === "email" && template.type === "email") return true;
+            if (
+              input.type === "email" &&
+              (template.type === "email" || template.kind === "content")
+            )
+              return true;
             else if (input.type !== "email") return true;
             return false;
           })

web/src/composables/usePrebuiltDestinations.spec.ts

@@ -904,6 +904,53 @@ describe("usePrebuiltDestinations", () => {
     });
   });
 
+  // The http interceptor already shows "Access Required" on a 403.
+  describe("save on 403", () => {
+    const forbidden = { response: { status: 403, data: { message: "Forbidden" } } };
+
+    it.each([
+      [
+        "createDestination",
+        (inst: any) => inst.createDestination("slack", "s", makeSlackCredentials()),
+      ],
+      [
+        "updateDestination",
+        (inst: any) => inst.updateDestination("slack", "s", "s", makeSlackCredentials()),
+      ],
+      ["convertToPrebuilt", (inst: any) => inst.convertToPrebuilt("s", "slack")],
+    ])("%s rethrows without its own error toast", async (_name, save) => {
+      mockDestCreate.mockRejectedValue(forbidden);
+      mockDestUpdate.mockRejectedValue(forbidden);
+      mockDestGetByName.mockResolvedValue({
+        data: { name: "s", url: "", headers: {}, metadata: {} },
+      });
+
+      await expect(save(usePrebuiltDestinations())).rejects.toBe(forbidden);
+
+      expect(vi.mocked(toast)).not.toHaveBeenCalledWith(
+        expect.objectContaining({ variant: "error" }),
+      );
+    });
+
+    it.each([
+      [
+        "createDestination",
+        (inst: any) => inst.createDestination("slack", "s", makeSlackCredentials()),
+      ],
+      [
+        "updateDestination",
+        (inst: any) => inst.updateDestination("slack", "s", "s", makeSlackCredentials()),
+      ],
+    ])("%s still toasts and rethrows a non-object rejection", async (_name, save) => {
+      mockDestCreate.mockRejectedValue(undefined);
+      mockDestUpdate.mockRejectedValue(undefined);
+
+      await expect(save(usePrebuiltDestinations())).rejects.toBeUndefined();
+
+      expect(vi.mocked(toast)).toHaveBeenCalledWith(expect.objectContaining({ variant: "error" }));
+    });
+  });
+
   // -------------------------------------------------------------------------
   // generatePreview
   // -------------------------------------------------------------------------

web/src/composables/usePrebuiltDestinations.ts

@@ -693,7 +693,8 @@ export function usePrebuiltDestinations() {
         variant: "success",
         message: t("alert_destinations.saved"),
       });
-    } catch (error: unknown) {
+    } catch (error: any) {
+      if (error?.response?.status == 403) throw error;
       toast({
         variant: "error",
         message: requestErrorMessage(error, t("alerts.prebuilt.testFailedUnknownError")),
@@ -800,7 +801,8 @@ export function usePrebuiltDestinations() {
         variant: "success",
         message: t("alert_destinations.saved"),
       });
-    } catch (error: unknown) {
+    } catch (error: any) {
+      if (error?.response?.status == 403) throw error;
       toast({
         variant: "error",
         message: requestErrorMessage(error, t("alerts.prebuilt.testFailedUnknownError")),
@@ -893,6 +895,7 @@ export function usePrebuiltDestinations() {
         message: t("alerts.prebuilt.conversionSuccess"),
       });
     } catch (error: any) {
+      if (error?.response?.status == 403) throw error;
       console.error("Failed to convert destination:", error);
       toast({
         variant: "error",