Validate alerts before evaluation them. Deactivate alert if not valid

lib/sanbase/alerts/evaluator/evaluator.ex

@@ -9,7 +9,8 @@ defmodule Sanbase.Alert.Evaluator do
   """
 
   alias Sanbase.Cache
-  alias Sanbase.Alert.{UserTrigger, Trigger}
+  alias Sanbase.Alert.UserTrigger
+  alias Sanbase.Alert.Trigger
 
   require Logger
 
@@ -36,6 +37,33 @@ defmodule Sanbase.Alert.Evaluator do
   end
 
   defp evaluate(%UserTrigger{trigger: trigger} = user_trigger) do
+    case Vex.validate(trigger.settings) do
+      {:ok, _} ->
+        do_evaluate(user_trigger)
+
+      {:error, [{:error, _field, _, error_msg}]} ->
+        # Do not evaluate invalid triggers but directly disactivate them
+        Logger.info("Auto disable alert with id #{user_trigger.id}. Reason: #{error_msg}")
+        _ = UserTrigger.update_is_active(user_trigger.id, user_trigger.user_id, false)
+
+        user_trigger
+        |> put_in(
+          [Access.key!(:trigger), Access.key!(:settings), Access.key!(:triggered?)],
+          false
+        )
+
+      error ->
+        Logger.error("Unknown and unexpected error during evaluation: #{inspect(error)}")
+
+        user_trigger
+        |> put_in(
+          [Access.key!(:trigger), Access.key!(:settings), Access.key!(:triggered?)],
+          false
+        )
+    end
+  end
+
+  defp do_evaluate(%UserTrigger{trigger: trigger} = user_trigger) do
     %{cooldown: cooldown, last_triggered: last_triggered} = trigger
 
     # Along with the trigger settings (the `cache_key`) take into account also

lib/sanbase/alerts/user_trigger.ex

@@ -215,7 +215,7 @@ defmodule Sanbase.Alert.UserTrigger do
     end
   end
 
-  def get_trigger_by_if_owner(user_id, trigger_id) do
+  def get_trigger_if_owner(user_id, trigger_id) do
     case by_user_and_id(user_id, trigger_id) do
       {:ok, %UserTrigger{user_id: ^user_id} = user_trigger} ->
         {:ok, user_trigger}
@@ -289,7 +289,7 @@ defmodule Sanbase.Alert.UserTrigger do
           {:ok, %__MODULE__{}} | {:error, String.t()} | {:error, %Ecto.Changeset{}}
   def create_user_trigger(%User{id: user_id} = _user, %{settings: settings} = params) do
     with {_, false} <- {:nil?, is_nil(settings)},
-         :ok <- valid?(settings) do
+         :ok <- validate_settings(settings) do
       changeset = %UserTrigger{} |> create_changeset(%{user_id: user_id, trigger: params})
 
       case Repo.insert(changeset) do
@@ -325,9 +325,9 @@ defmodule Sanbase.Alert.UserTrigger do
       when is_integer(user_id) and user_id > 0 do
     settings = Map.get(params, :settings)
 
-    with {_, :ok} <- {:valid?, valid_or_nil?(settings)},
+    with {_, :ok} <- {:validate_settings, validate_settings_or_nil(settings)},
          {_, {:ok, %__MODULE__{} = struct}} <-
-           {:get_trigger, get_trigger_by_if_owner(user_id, trigger_id)} do
+           {:get_trigger, get_trigger_if_owner(user_id, trigger_id)} do
       update_result =
         struct
         |> update_changeset(%{trigger: clean_params(params)})
@@ -346,7 +346,7 @@ defmodule Sanbase.Alert.UserTrigger do
         {:error,
          "Trigger with id #{trigger_id} does not exist or is not owned by the current user"}
 
-      {:valid?, {:error, error}} ->
+      {:validate_settings, {:error, error}} ->
         {:error,
          "Trigger structure is invalid. Key `settings` is not valid. Reason: #{inspect(error)}"}
     end
@@ -361,7 +361,7 @@ defmodule Sanbase.Alert.UserTrigger do
   @spec remove_user_trigger(%User{}, trigger_id) ::
           {:ok, %__MODULE__{}} | {:error, String.t()} | {:error, Ecto.Changeset.t()}
   def remove_user_trigger(%User{} = user, trigger_id) do
-    case get_trigger_by_if_owner(user.id, trigger_id) do
+    case get_trigger_if_owner(user.id, trigger_id) do
       {:ok, %__MODULE__{} = user_trigger} ->
         Repo.delete(user_trigger)
         |> emit_event(:delete_alert, %{})
@@ -457,21 +457,18 @@ defmodule Sanbase.Alert.UserTrigger do
     |> Enum.map(&trigger_in_struct/1)
   end
 
-  defp valid_or_nil?(nil), do: :ok
-  defp valid_or_nil?(trigger), do: valid?(trigger)
-
-  defp valid?(trigger) do
-    with {_, {:ok, trigger_struct}} <- {:load_in_struct?, load_in_struct_if_valid(trigger)},
-         {_, true} <- {:valid?, Vex.valid?(trigger_struct)},
+  defp validate_settings(settings) do
+    with {_, {:ok, trigger_struct}} <- {:load_in_struct?, load_in_struct_if_valid(settings)},
+         {_, true} <- {:validate_settings, Vex.valid?(trigger_struct)},
          {_, {:ok, _trigger_map}} <- {:map_from_struct, map_from_struct(trigger_struct)} do
       :ok
     else
       {:load_in_struct?, {:error, error}} ->
         Logger.warning("UserTrigger struct is not valid. Reason: #{inspect(error)}")
         {:error, error}
 
-      {:valid?, false} ->
-        {:ok, trigger_struct} = load_in_struct(trigger)
+      {:validate_settings, false} ->
+        {:ok, trigger_struct} = load_in_struct(settings)
         errors = Vex.errors(trigger_struct)
 
         errors_text =
@@ -490,6 +487,9 @@ defmodule Sanbase.Alert.UserTrigger do
     end
   end
 
+  defp validate_settings_or_nil(nil), do: :ok
+  defp validate_settings_or_nil(settings), do: validate_settings(settings)
+
   defp create_event(user_trigger, changeset, event_type) do
     TimelineEvent.maybe_create_event_async(event_type, user_trigger, changeset)
     {:ok, user_trigger}

lib/sanbase/utils/validation.ex

@@ -138,7 +138,8 @@ defmodule Sanbase.Validation do
   defp time_window_sanity_check(time_window) do
     case str_to_sec(time_window) do
       seconds when seconds >= @year_in_seconds ->
-        {:error, "The time_window parameter must not be bigger than 1 year"}
+        {:error,
+         "The time_window parameter must not be bigger than 1 year. Provided value: #{time_window}"}
 
       _ ->
         :ok

lib/sanbase_web/graphql/resolvers/signals/user_trigger_resolver.ex

@@ -64,7 +64,7 @@ defmodule SanbaseWeb.Graphql.Resolvers.UserTriggerResolver do
         context: %{auth: %{current_user: current_user}}
       }) do
     with {:ok, %UserTrigger{} = user_trigger} <-
-           UserTrigger.get_trigger_by_if_owner(current_user.id, args.id),
+           UserTrigger.get_trigger_if_owner(current_user.id, args.id),
          false <- UserTrigger.frozen?(user_trigger) do
       UserTrigger.update_user_trigger(current_user.id, args)
       |> handle_result("update")

test/sanbase/alerts/metric/intraday_metric_trigger_settings_test.exs

@@ -399,4 +399,36 @@ defmodule Sanbase.Alert.MetricTriggerSettingsTest do
       end)
     end
   end
+
+  test "alert with invalid time_window does not run and is disabled" do
+    user = insert(:user)
+
+    trigger_settings = %{
+      type: "metric_signal",
+      metric: "active_addresses_24h",
+      target: %{slug: "santiment"},
+      channel: "telegram",
+      time_window: "11111111d",
+      operation: %{percent_up: 200.0}
+    }
+
+    # To bypass the validation check on insertion use the Factory directly
+    user_trigger =
+      insert(:user_trigger,
+        user: user,
+        trigger: %{title: "Generic title", is_public: false, settings: trigger_settings}
+      )
+
+    assert user_trigger.trigger.is_active == true
+
+    triggered =
+      MetricTriggerSettings.type()
+      |> UserTrigger.get_active_triggers_by_type()
+      |> Evaluator.run()
+
+    assert triggered == []
+
+    {:ok, user_trigger} = UserTrigger.by_user_and_id(user_trigger.user_id, user_trigger.id)
+    assert user_trigger.trigger.is_active == false
+  end
 end

test/sanbase/alerts/trigger_test.exs

@@ -385,4 +385,29 @@ defmodule Sanbase.Alert.TriggersTest do
     assert trigger.trigger.is_public == true
     assert trigger.trigger.cooldown == "1h"
   end
+
+  @tag capture_log: true
+  test "too big time_window is not valid" do
+    user = insert(:user)
+
+    trigger_settings = %{
+      type: "metric_signal",
+      metric: "active_addresses_24h",
+      target: %{slug: "santiment"},
+      channel: "telegram",
+      time_window: "250000000000m",
+      operation: %{percent_up: 200.0}
+    }
+
+    assert {:error, error_msg} =
+             UserTrigger.create_user_trigger(user, %{
+               title: "Generic title",
+               is_public: true,
+               cooldown: "1d",
+               settings: trigger_settings
+             })
+
+    assert error_msg =~ "The time_window parameter must not be bigger than 1 year"
+    assert error_msg =~ "Provided value: #{trigger_settings.time_window}"
+  end
 end