Refactor activity log settings (#3223)

* Move activity log settings next to other system settings

* Move activity log settings operations to settings context

* Move activity log settings tests to settings related tests

* Adjust references to refactored activity log settings

* Properly group aliases in settings test

lib/trento/activity_log.ex

@@ -7,12 +7,11 @@ defmodule Trento.ActivityLog do
 
   require Logger
 
-  alias Trento.ActivityLog.RetentionTime
-  require Trento.ActivityLog.RetentionPeriodUnit, as: RetentionPeriodUnit
   alias Trento.ActivityLog.ActivityLog
   alias Trento.ActivityLog.MetadataQueryParser
-  alias Trento.ActivityLog.Settings
+  alias Trento.ActivityLog.RetentionTime
   alias Trento.Repo
+  alias Trento.Settings
 
   @user_management_log_types [
     "login_attempt",
@@ -22,45 +21,6 @@ defmodule Trento.ActivityLog do
     "profile_update"
   ]
 
-  @spec get_settings() ::
-          {:ok, Settings.t()} | {:error, :not_found}
-  def get_settings do
-    case Repo.one(Settings.base_query()) do
-      %Settings{} = settings -> {:ok, settings}
-      nil -> {:error, :not_found}
-    end
-  end
-
-  @spec change_retention_period(integer(), RetentionPeriodUnit.t()) ::
-          {:ok, Settings.t()}
-          | {:error, :activity_log_settings_not_configured}
-  def change_retention_period(value, unit) do
-    case get_settings() do
-      {:ok, settings} ->
-        settings
-        |> Settings.changeset(%{
-          retention_time: %{
-            value: value,
-            unit: unit
-          }
-        })
-        |> Repo.update()
-        |> log_error("Error while updating activity log retention period")
-
-      {:error, :not_found} ->
-        {:error, :activity_log_settings_not_configured}
-    end
-  end
-
-  @spec clear_expired_logs() :: :ok | {:error, any()}
-  def clear_expired_logs do
-    with {:ok, %{retention_time: retention_time}} <- Trento.ActivityLog.get_settings(),
-         expiration_date <- calculate_expiration_date(retention_time) do
-      delete_logs_before(expiration_date)
-      :ok
-    end
-  end
-
   @spec list_activity_log(map()) ::
           {:ok, list(ActivityLog.t()), Flop.Meta.t()} | {:error, :activity_log_fetch_error}
   def list_activity_log(params, include_all_log_types? \\ false) do
@@ -79,6 +39,15 @@ defmodule Trento.ActivityLog do
     end
   end
 
+  @spec clear_expired_logs() :: :ok | {:error, any()}
+  def clear_expired_logs do
+    with {:ok, %{retention_time: retention_time}} <- Settings.get_activity_log_settings(),
+         expiration_date <- calculate_expiration_date(retention_time) do
+      delete_logs_before(expiration_date)
+      :ok
+    end
+  end
+
   defp maybe_exclude_user_logs(ActivityLog = q, true = _include_all_log_types?), do: q
 
   defp maybe_exclude_user_logs(ActivityLog = q, false = _include_all_log_types?) do
@@ -154,13 +123,6 @@ defmodule Trento.ActivityLog do
     end)
   end
 
-  defp log_error({:error, _} = error, message) do
-    Logger.error("#{message}: #{inspect(error)}")
-    error
-  end
-
-  defp log_error(result, _), do: result
-
   defp calculate_expiration_date(%RetentionTime{value: value, unit: :day}),
     do: DateTime.add(DateTime.utc_now(), -value, :day)
 

lib/trento/release.ex

@@ -8,9 +8,11 @@ defmodule Trento.Release do
 
   alias Mix.Tasks.Phx.Gen.Cert
 
-  alias Trento.ActivityLog.Settings, as: ActivityLogSettings
-  alias Trento.Settings.ApiKeySettings
-  alias Trento.Settings.SSOCertificatesSettings
+  alias Trento.Settings.{
+    ActivityLogSettings,
+    ApiKeySettings,
+    SSOCertificatesSettings
+  }
 
   @app :trento
 

lib/trento/settings.ex

@@ -8,6 +8,7 @@ defmodule Trento.Settings do
   alias Trento.SoftwareUpdates.Discovery, as: SoftwareUpdatesDiscovery
 
   alias Trento.Settings.{
+    ActivityLogSettings,
     ApiKeySettings,
     InstallationSettings,
     SSOCertificatesSettings,
@@ -16,6 +17,8 @@ defmodule Trento.Settings do
 
   alias Trento.Support.DateService
 
+  require Trento.ActivityLog.RetentionPeriodUnit, as: RetentionPeriodUnit
+
   require Logger
 
   @type suse_manager_settings_save_submission :: %{
@@ -119,6 +122,38 @@ defmodule Trento.Settings do
     :ok
   end
 
+  # Activity log settings
+
+  @spec get_activity_log_settings() ::
+          {:ok, ActivityLogSettings.t()} | {:error, :activity_log_settings_not_configured}
+  def get_activity_log_settings do
+    case Repo.one(ActivityLogSettings.base_query()) do
+      %ActivityLogSettings{} = settings -> {:ok, settings}
+      nil -> {:error, :activity_log_settings_not_configured}
+    end
+  end
+
+  @spec change_activity_log_retention_period(integer(), RetentionPeriodUnit.t()) ::
+          {:ok, ActivityLogSettings.t()}
+          | {:error, :activity_log_settings_not_configured}
+  def change_activity_log_retention_period(value, unit) do
+    case get_activity_log_settings() do
+      {:ok, settings} ->
+        settings
+        |> ActivityLogSettings.changeset(%{
+          retention_time: %{
+            value: value,
+            unit: unit
+          }
+        })
+        |> Repo.update()
+        |> log_error("Error while updating activity log retention period")
+
+      error ->
+        error
+    end
+  end
+
   # Certificates settings
 
   @spec get_sso_certificates() :: [SSOCertificatesSettings.t()]

lib/trento/activity_logging/settings.ex → lib/trento/settings/activity_log_settings.ex

@@ -1,6 +1,6 @@
-defmodule Trento.ActivityLog.Settings do
+defmodule Trento.Settings.ActivityLogSettings do
   @moduledoc """
-  ActivityLog Settings is the STI projection of activity log related settings
+  ActivityLogSettings is the STI projection of activity log related settings
   """
 
   use Ecto.Schema
@@ -36,6 +36,6 @@ defmodule Trento.ActivityLog.Settings do
   end
 
   def with_default_retention_time do
-    changeset(%Settings{}, %{retention_time: RetentionTime.default()})
+    changeset(%ActivityLogSettings{}, %{retention_time: RetentionTime.default()})
   end
 end

lib/trento_web/controllers/v1/settings_controller.ex

@@ -2,7 +2,6 @@ defmodule TrentoWeb.V1.SettingsController do
   use TrentoWeb, :controller
   use OpenApiSpex.ControllerSpecs
 
-  alias Trento.ActivityLog
   alias Trento.Settings
   alias Trento.SoftwareUpdates
   alias TrentoWeb.OpenApi.V1.Schema
@@ -124,7 +123,7 @@ defmodule TrentoWeb.V1.SettingsController do
       OpenApiSpex.body_params(conn)
 
     with {:ok, updated_settings} <-
-           ActivityLog.change_retention_period(retention_period, retention_period_unit) do
+           Settings.change_activity_log_retention_period(retention_period, retention_period_unit) do
       render(conn, :activity_log_settings, %{
         activity_log_settings: updated_settings
       })
@@ -142,10 +141,9 @@ defmodule TrentoWeb.V1.SettingsController do
     ]
 
   def get_activity_log_settings(conn, _) do
-    with {:ok, settings} <- ActivityLog.get_settings() do
-      render(conn, :activity_log_settings, %{
-        activity_log_settings: settings
-      })
+    case Settings.get_activity_log_settings() do
+      {:ok, settings} -> render(conn, :activity_log_settings, %{activity_log_settings: settings})
+      {:error, :activity_log_settings_not_configured} -> {:error, :not_found}
     end
   end
 

priv/repo/seeds.exs

@@ -37,6 +37,6 @@
 })
 |> Trento.Repo.insert!(on_conflict: :nothing)
 
-Trento.Repo.insert!(Trento.ActivityLog.Settings.with_default_retention_time(),
+Trento.Repo.insert!(Trento.Settings.ActivityLogSettings.with_default_retention_time(),
   on_conflict: :nothing
 )

test/support/factory.ex

@@ -130,6 +130,7 @@ defmodule Trento.Factory do
   }
 
   alias Trento.Settings.{
+    ActivityLogSettings,
     ApiKeySettings,
     InstallationSettings,
     SSOCertificatesSettings,
@@ -138,7 +139,6 @@ defmodule Trento.Factory do
 
   alias Trento.ActivityLog.ActivityLog, as: ActivityLogEntry
   alias Trento.ActivityLog.RetentionTime
-  alias Trento.ActivityLog.Settings, as: ActivityLogSettings
 
   alias Trento.Abilities.{
     Ability,