From 61102fe5addc70630e12663109789cb14d18e5ae Mon Sep 17 00:00:00 2001
From: Nelson Kopliku <nelson.kopliku@suse.com>
Date: Thu, 5 May 2022 09:40:16 +0200
Subject: [PATCH] Completely skip alerting feature steps when alerting is
 disabled (#494)

---
 .../application/usecases/alerting/alerting.ex | 46 ++++++++++++-------
 .../application/usecases/alerting_test.exs    | 32 +++++++++++++
 2 files changed, 62 insertions(+), 16 deletions(-)

diff --git a/lib/trento/application/usecases/alerting/alerting.ex b/lib/trento/application/usecases/alerting/alerting.ex
index 7aa3c0c844..3b3ab658e7 100644
--- a/lib/trento/application/usecases/alerting/alerting.ex
+++ b/lib/trento/application/usecases/alerting/alerting.ex
@@ -16,31 +16,53 @@ defmodule Trento.Application.UseCases.Alerting do
   require Logger
 
   @spec notify_heartbeat_failed(String.t()) :: :ok
-  def notify_heartbeat_failed(host_id) do
+  def notify_heartbeat_failed(host_id),
+    do: maybe_notify_heartbeat_failed(enabled?(), host_id)
+
+  @spec notify_critical_cluster_health(String.t()) :: :ok
+  def notify_critical_cluster_health(cluster_id),
+    do: maybe_notify_critical_cluster_health(enabled?(), cluster_id)
+
+  @spec notify_critical_database_health(String.t()) :: :ok
+  def notify_critical_database_health(id),
+    do: maybe_notify_critical_database_health(enabled?(), id)
+
+  @spec notify_critical_sap_system_health(String.t()) :: :ok
+  def notify_critical_sap_system_health(id),
+    do: maybe_notify_critical_sap_system_health(enabled?(), id)
+
+  defp enabled?, do: Application.fetch_env!(:trento, :alerting)[:enabled]
+
+  defp maybe_notify_heartbeat_failed(false, _), do: :ok
+
+  defp maybe_notify_heartbeat_failed(true, host_id) do
     %HostReadModel{hostname: hostname} = Trento.Repo.get!(HostReadModel, host_id)
 
     EmailAlert.alert("Host", "hostname", hostname, "heartbeat failed")
     |> deliver_notification()
   end
 
-  @spec notify_critical_cluster_health(String.t()) :: :ok
-  def notify_critical_cluster_health(cluster_id) do
+  defp maybe_notify_critical_cluster_health(false, _), do: :ok
+
+  defp maybe_notify_critical_cluster_health(true, cluster_id) do
     %ClusterReadModel{name: name} = Trento.Repo.get!(ClusterReadModel, cluster_id)
 
     EmailAlert.alert("Cluster", "name", name, "health is now in critical state")
     |> deliver_notification()
   end
 
-  @spec notify_critical_database_health(String.t()) :: :ok
-  def notify_critical_database_health(id) do
+  defp maybe_notify_critical_database_health(false, _), do: :ok
+
+  defp maybe_notify_critical_database_health(true, id) do
     %DatabaseReadModel{sid: sid} = Trento.Repo.get!(DatabaseReadModel, id)
 
     EmailAlert.alert("Database", "SID", sid, "health is now in critical state")
     |> deliver_notification()
   end
 
-  @spec notify_critical_sap_system_health(String.t()) :: :ok
-  def notify_critical_sap_system_health(id) do
+  defp maybe_notify_critical_sap_system_health(false, _), do: :ok
+
+  defp maybe_notify_critical_sap_system_health(true, id) do
     %SapSystemReadModel{sid: sid} = Trento.Repo.get!(SapSystemReadModel, id)
 
     EmailAlert.alert("Sap System", "SID", sid, "health is now in critical state")
@@ -48,15 +70,7 @@ defmodule Trento.Application.UseCases.Alerting do
   end
 
   @spec deliver_notification(Swoosh.Email.t()) :: :ok
-  defp deliver_notification(%Swoosh.Email{} = notification) do
-    maybe_deliver_notification(Application.fetch_env!(:trento, :alerting)[:enabled], notification)
-  end
-
-  @spec maybe_deliver_notification(false, Swoosh.Email.t()) :: :ok
-  defp maybe_deliver_notification(false, _), do: :ok
-
-  @spec maybe_deliver_notification(true, Swoosh.Email.t()) :: :ok
-  defp maybe_deliver_notification(true, %Swoosh.Email{subject: subject} = notification) do
+  defp deliver_notification(%Swoosh.Email{subject: subject} = notification) do
     notification
     |> Mailer.deliver()
     |> case do
diff --git a/test/trento/application/usecases/alerting_test.exs b/test/trento/application/usecases/alerting_test.exs
index 2705d37450..dc7032e105 100644
--- a/test/trento/application/usecases/alerting_test.exs
+++ b/test/trento/application/usecases/alerting_test.exs
@@ -10,6 +10,38 @@ defmodule Trento.Application.UseCases.AlertingTest do
 
   @moduletag :integration
 
+  describe "Enabling/Disabling Alerting Feature" do
+    setup do
+      on_exit(fn ->
+        Application.put_env(:trento, :alerting,
+          enabled: true,
+          recipient: "some.recipient@email.com"
+        )
+      end)
+    end
+
+    test "No email should be sent when alerting is disabled" do
+      Application.put_env(:trento, :alerting, enabled: false)
+      host_id = Faker.UUID.v4()
+
+      Alerting.notify_heartbeat_failed(host_id)
+
+      assert_no_email_sent()
+    end
+
+    test "An error should be raised when alerting is enabled but no recipient was provided" do
+      Application.put_env(:trento, :alerting, enabled: true)
+      sap_system_id = Faker.UUID.v4()
+      sap_system_projection(id: sap_system_id)
+
+      assert_raise ArgumentError,
+                   ~r/Unexpected tuple format, {"Trento Admin", nil} cannot be formatted into a Recipient./,
+                   fn -> Alerting.notify_critical_sap_system_health(sap_system_id) end
+
+      assert_no_email_sent()
+    end
+  end
+
   describe "Alerting the configured recipient about crucial facts with email notifications" do
     test "Notify Host heartbeating failure" do
       host_id = Faker.UUID.v4()