Activity log pagination and filtering (#2786)

* Pull in Flop as a dependency .. and add config * Setup schema level config and flop defaults * Adds some tests * Updates test test now uses factory inserted columns and Enum operations on these for assertions. * Implements pagination endpoint WIP * Address PR comment Makes use of `insert_list` * Adds test for pagination Changes query params for supporting pagination; removes unused field in view. * Fixes credo * Updates view/test - Some renaming - Removes one test * mix format * General refactor; Addresses PR comments - Simplifies implementation of parse_params according to suggestion - Moves parse_params function to context module - Removes some dead/unused code - Adds error response of 400 bad request - Some general refactoring/default removal * Adds some docs According to PR comment * Refactors schema According to PR comment * Update lib/trento/activity_log.ex Co-authored-by: Nelson Kopliku <[email protected]> * Update lib/trento/activity_log.ex PR suggestion. Co-authored-by: Emanuele De Cupis <[email protected]> * Fixes openapi schema Needs to be nested and in specific order. Re-adds a removed field (id). * Fixes/Rebase clean-up for a rebase * Adds test for date ranges * Small refactor of the context module - prevents type errors - corrects incorrect comment - minor refactoring * Renaming According to PR comments * Removes unnecessary guard * More tests; removes some response fields According to PR comments * Disables api-bc-check dependency Temporary, to allow CI runs, to be reverted * Revert "Disables api-bc-check dependency" This reverts commit 28aa22c. * Makes function private * Makes function private --------- Co-authored-by: Emanuele De Cupis <[email protected]> Co-authored-by: Nelson Kopliku <[email protected]>
trento-project · Jul 26, 2024 · 33a61f4 · 33a61f4
1 parent fed0401
commit 33a61f4
Show file tree

Hide file tree

Showing 12 changed files with 437 additions and 78 deletions.
diff --git a/config/config.exs b/config/config.exs
@@ -213,6 +213,8 @@ config :bodyguard,
   # The second element of the {:error, reason} tuple returned on auth failure
   default_error: :forbidden
 
+config :flop, repo: Trento.Repo
+
 # Import environment specific config. This must remain at the bottom
 # of this file so it overrides the configuration defined above.
 import_config "#{config_env()}.exs"
diff --git a/lib/trento/activity_log.ex b/lib/trento/activity_log.ex
@@ -43,16 +43,6 @@ defmodule Trento.ActivityLog do
     end
   end
 
-  @spec list_activity_log() :: list(ActivityLog.t())
-  def list_activity_log do
-    # This will be made filterable/paginatable in a later PR
-    query =
-      from activity in ActivityLog,
-        order_by: [desc: activity.inserted_at]
-
-    Repo.all(query)
-  end
-
   @spec clear_expired_logs() :: :ok | {:error, any()}
   def clear_expired_logs do
     with {:ok, %{retention_time: retention_time}} <- Trento.ActivityLog.get_settings(),
@@ -62,6 +52,61 @@ defmodule Trento.ActivityLog do
     end
   end
 
+  @spec list_activity_log(map()) ::
+          {:ok, list(ActivityLog.t()), Flop.Meta.t()} | {:error, :activity_log_fetch_error}
+  def list_activity_log(params) do
+    parsed_params = parse_params(params)
+
+    case Flop.validate_and_run(ActivityLog, parsed_params, for: ActivityLog) do
+      {:ok, {activity_log_entries, meta}} ->
+        {:ok, activity_log_entries, meta}
+
+      error ->
+        Logger.error("Activity log fetch error: #{inspect(error)}")
+        {:error, :activity_log_fetch_error}
+    end
+  end
+
+  # ''&& false' is a workaround until we reach OTP 27 that allows doc tag for private functions;
+  # we get a compile warning without this with OTP 26
+  @doc """
+       Parses the query parameters and returns a map with the parsed values as expected by the Flop library.
+       Some parameters are recognized by Flop and are used as is (example: last, first, after, before);
+       some other parameters are used to build filters with custom operator logic (example: from_date, to_date, actor, type).
+       ## Examples
+             iex> parse_params([{:from_date, "2021-01-31"}, {:to_date, "2021-01-01"}, last: 10])
+       %{
+          filters: [
+                     %{value: "2021-01-31", op: :<=, field: :inserted_at}, 
+                     %{value: "2021-01-01", op: :>=, field: :inserted_at}], 
+          last: 10
+       }
+       """ && false
+  @spec parse_params(map()) :: map()
+  defp parse_params(query_params) when query_params == %{} do
+    # Implies
+    # %{first: 25, order_by: [:inserted_at], order_directions: [:desc]}
+    %{}
+  end
+
+  defp parse_params(query_params) do
+    query_params
+    |> Enum.map(fn
+      {:from_date, v} -> {:filters, %{field: :inserted_at, op: :<=, value: v}}
+      {:to_date, v} -> {:filters, %{field: :inserted_at, op: :>=, value: v}}
+      {:actor, v} -> {:filters, %{field: :actor, op: :ilike_or, value: v}}
+      {:type, v} -> {:filters, %{field: :type, op: :ilike_or, value: v}}
+      param -> param
+    end)
+    |> Enum.reduce(%{filters: []}, fn
+      {:filters, filter}, acc ->
+        Map.put(acc, :filters, [filter | acc.filters])
+
+      {k, v}, acc ->
+        Map.put(acc, k, v)
+    end)
+  end
+
   defp log_error({:error, _} = error, message) do
     Logger.error("#{message}: #{inspect(error)}")
     error

diff --git a/lib/trento/activity_logging/activity_log.ex b/lib/trento/activity_logging/activity_log.ex
@@ -7,6 +7,20 @@ defmodule Trento.ActivityLog.ActivityLog do
 
   @type t() :: %__MODULE__{}
 
+  @derive {
+    Flop.Schema,
+    filterable: [:type, :actor, :inserted_at],
+    sortable: [:type, :actor, :inserted_at],
+    max_limit: 100,
+    default_limit: 25,
+    default_order: %{
+      order_by: [:inserted_at],
+      order_directions: [:desc]
+    },
+    pagination_types: [:first, :last],
+    default_pagination_type: :first
+  }
+
   @primary_key {:id, :binary_id, autogenerate: true}
   schema "activity_logs" do
     field :type, :string

diff --git a/lib/trento_web/controllers/fallback_controller.ex b/lib/trento_web/controllers/fallback_controller.ex
@@ -3,6 +3,13 @@ defmodule TrentoWeb.FallbackController do
 
   alias TrentoWeb.ErrorView
 
+  def call(conn, {:error, :activity_log_fetch_error}) do
+    conn
+    |> put_status(:bad_request)
+    |> put_view(ErrorView)
+    |> render(:"400", reason: "Activity Log fetch error.")
+  end
+
   def call(conn, {:error, :invalid_credentials}) do
     conn
     |> put_status(:unauthorized)

diff --git a/lib/trento_web/controllers/v1/activity_log_controller.ex b/lib/trento_web/controllers/v1/activity_log_controller.ex
@@ -11,14 +11,59 @@ defmodule TrentoWeb.V1.ActivityLogController do
   operation :get_activity_log,
     summary: "Fetches the Activity Log entries.",
     tags: ["Platform"],
+    parameters: [
+      first: [
+        in: :query,
+        schema: %OpenApiSpex.Schema{type: :integer},
+        required: false
+      ],
+      last: [
+        in: :query,
+        schema: %OpenApiSpex.Schema{type: :integer},
+        required: false
+      ],
+      after: [
+        in: :query,
+        schema: %OpenApiSpex.Schema{type: :string},
+        required: false
+      ],
+      before: [
+        in: :query,
+        schema: %OpenApiSpex.Schema{type: :string},
+        required: false
+      ],
+      from_date: [
+        in: :query,
+        schema: %OpenApiSpex.Schema{type: :string},
+        required: false
+      ],
+      to_date: [
+        in: :query,
+        schema: %OpenApiSpex.Schema{type: :string},
+        required: false
+      ],
+      actor: [
+        in: :query,
+        schema: %OpenApiSpex.Schema{type: :array},
+        required: false
+      ],
+      type: [
+        in: :query,
+        schema: %OpenApiSpex.Schema{type: :array},
+        required: false
+      ]
+    ],
     responses: [
-      ok: {"Activity Log settings fetched successfully", "application/json", Schema.ActivityLog}
+      ok:
+        {"Activity Log settings fetched successfully", "application/json",
+         Schema.ActivityLog.ActivityLog}
     ]
 
-  def get_activity_log(conn, _) do
-    with activity_log_entries <- ActivityLog.list_activity_log() do
+  def get_activity_log(conn, params) do
+    with {:ok, activity_log_entries, meta} <- ActivityLog.list_activity_log(params) do
       render(conn, "activity_log.json", %{
-        activity_log: activity_log_entries
+        activity_log: activity_log_entries,
+        pagination: meta
       })
     end
   end

diff --git a/lib/trento_web/openapi/v1/schema/activity_log.ex b/lib/trento_web/openapi/v1/schema/activity_log.ex
@@ -4,20 +4,19 @@ defmodule TrentoWeb.OpenApi.V1.Schema.ActivityLog do
   require OpenApiSpex
   alias OpenApiSpex.Schema
 
-  OpenApiSpex.schema(
-    %{
-      title: "ActivityLog",
-      description: "Activity Log for the current installation.",
+  defmodule ActivityLogEntries do
+    @moduledoc false
+    OpenApiSpex.schema(%{
       type: :array,
       items: %Schema{
-        title: "ActivityLogEntry",
+        title: "ActivityLogEntries",
         type: :object,
         additionalProperties: false,
         properties: %{
           id: %Schema{
             type: :string,
-            description: "Identifier of Activity Log entry.",
-            format: :uuid
+            format: :uuid,
+            description: "UUID (v4) of Activity Log entry."
           },
           type: %Schema{
             type: :string,
@@ -37,7 +36,26 @@ defmodule TrentoWeb.OpenApi.V1.Schema.ActivityLog do
         },
         required: [:id, :type, :actor, :metadata, :occurred_on]
       }
-    },
-    struct?: false
-  )
+    })
+  end
+
+  defmodule ActivityLog do
+    @moduledoc false
+
+    OpenApiSpex.schema(
+      %{
+        title: "ActivityLog",
+        description: "Activity Log for the current installation.",
+        type: :object,
+        properties: %{
+          data: ActivityLogEntries,
+          pagination: %Schema{
+            type: :object
+          }
+        },
+        required: [:data, :pagination]
+      },
+      struct?: false
+    )
+  end
 end
diff --git a/lib/trento_web/views/v1/activity_log_view.ex b/lib/trento_web/views/v1/activity_log_view.ex
@@ -1,8 +1,11 @@
 defmodule TrentoWeb.V1.ActivityLogView do
   use TrentoWeb, :view
 
-  def render("activity_log.json", %{activity_log: entries}) do
-    render_many(entries, __MODULE__, "activity_log_entry.json", as: :activity_log_entry)
+  def render("activity_log.json", %{activity_log: entries, pagination: meta}) do
+    %{
+      data: render_many(entries, __MODULE__, "activity_log_entry.json", as: :activity_log_entry),
+      pagination: render_one(meta, __MODULE__, "pagination.json", pagination: meta)
+    }
   end
 
   def render("activity_log_entry.json", %{activity_log_entry: entry}) do
@@ -15,4 +18,23 @@ defmodule TrentoWeb.V1.ActivityLogView do
       occurred_on: entry.inserted_at
     }
   end
+
+  def render("pagination.json", %{pagination: pagination}) do
+    %{
+      end_cursor: end_cursor,
+      start_cursor: start_cursor,
+      flop: %{
+        first: first,
+        last: last
+      }
+    } =
+      pagination
+
+    %{
+      start_cursor: start_cursor,
+      end_cursor: end_cursor,
+      first: first,
+      last: last
+    }
+  end
 end
diff --git a/mix.exs b/mix.exs
@@ -71,6 +71,7 @@ defmodule Trento.MixProject do
       {:excoveralls, "~> 0.10", only: :test},
       {:faker, "~> 0.17", only: [:dev, :test]},
       {:floki, ">= 0.30.0", only: :test},
+      {:flop, "~> 0.25.0"},
       {:fun_with_flags, "~> 1.8.1"},
       {:fun_with_flags_ui, "~> 0.8.0"},
       {:gettext, "~> 0.18"},

diff --git a/mix.lock b/mix.lock
@@ -41,6 +41,7 @@
   "faker": {:hex, :faker, "0.17.0", "671019d0652f63aefd8723b72167ecdb284baf7d47ad3a82a15e9b8a6df5d1fa", [:mix], [], "hexpm", "a7d4ad84a93fd25c5f5303510753789fc2433ff241bf3b4144d3f6f291658a6a"},
   "file_system": {:hex, :file_system, "0.2.10", "fb082005a9cd1711c05b5248710f8826b02d7d1784e7c3451f9c1231d4fc162d", [:mix], [], "hexpm", "41195edbfb562a593726eda3b3e8b103a309b733ad25f3d642ba49696bf715dc"},
   "floki": {:hex, :floki, "0.35.2", "87f8c75ed8654b9635b311774308b2760b47e9a579dabf2e4d5f1e1d42c39e0b", [:mix], [], "hexpm", "6b05289a8e9eac475f644f09c2e4ba7e19201fd002b89c28c1293e7bd16773d9"},
+  "flop": {:hex, :flop, "0.25.0", "0667f3c65f140b2ed7d64dad01b8e0e3dcc071addf8a556d11b36cad1619a6c1", [:mix], [{:ecto, "~> 3.11", [hex: :ecto, repo: "hexpm", optional: false]}, {:nimble_options, "~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}], "hexpm", "7e4b01b76412c77691e9aaea6903c5a8212fb7243198e1d2ba74fa15f2aec34c"},
   "fsm": {:hex, :fsm, "0.3.1", "087aa9b02779a84320dc7a2d8464452b5308e29877921b2bde81cdba32a12390", [:mix], [], "hexpm", "fbf0d53f89e9082b326b0b5828b94b4c549ff9d1452bbfd00b4d1ac082208e96"},
   "fun_with_flags": {:hex, :fun_with_flags, "1.8.1", "5999dd64d2e97646554244baa9b1da8ca33d656eed37307238b2dda8c17dba45", [:mix], [{:ecto_sql, "~> 3.0", [hex: :ecto_sql, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: true]}, {:redix, "~> 1.0", [hex: :redix, repo: "hexpm", optional: true]}], "hexpm", "02222ddffccbb1fa339f6ee3f14fac88ba44121b167c72bce93c2390013b3adc"},
   "fun_with_flags_ui": {:hex, :fun_with_flags_ui, "0.8.1", "43fb6ff46eb47bdd2eeb3eccb0f15ef25e23dcbf136cc1ec3132abab53c20c3d", [:mix], [{:cowboy, ">= 2.0.0", [hex: :cowboy, repo: "hexpm", optional: true]}, {:fun_with_flags, "~> 1.8", [hex: :fun_with_flags, repo: "hexpm", optional: false]}, {:plug, "~> 1.12", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, ">= 2.0.0", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "709db0e00ec803a54d7592f04a2bea0676e2394bd30bdd43a62c33af3bdc228e"},
@@ -63,6 +64,7 @@
   "mime": {:hex, :mime, "2.0.6", "8f18486773d9b15f95f4f4f1e39b710045fa1de891fada4516559967276e4dc2", [:mix], [], "hexpm", "c9945363a6b26d747389aac3643f8e0e09d30499a138ad64fe8fd1d13d9b153e"},
   "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
   "mox": {:hex, :mox, "1.1.0", "0f5e399649ce9ab7602f72e718305c0f9cdc351190f72844599545e4996af73c", [:mix], [], "hexpm", "d44474c50be02d5b72131070281a5d3895c0e7a95c780e90bc0cfe712f633a13"},
+  "nimble_options": {:hex, :nimble_options, "1.1.1", "e3a492d54d85fc3fd7c5baf411d9d2852922f66e69476317787a7b2bb000a61b", [:mix], [], "hexpm", "821b2470ca9442c4b6984882fe9bb0389371b8ddec4d45a9504f00a66f650b44"},
   "nimble_parsec": {:hex, :nimble_parsec, "1.4.0", "51f9b613ea62cfa97b25ccc2c1b4216e81df970acd8e16e8d1bdc58fef21370d", [:mix], [], "hexpm", "9c565862810fb383e9838c1dd2d7d2c437b3d13b267414ba6af33e50d2d1cf28"},
   "nimble_totp": {:hex, :nimble_totp, "1.0.0", "79753bae6ce59fd7cacdb21501a1dbac249e53a51c4cd22b34fa8438ee067283", [:mix], [], "hexpm", "6ce5e4c068feecdb782e85b18237f86f66541523e6bad123e02ee1adbe48eda9"},
   "open_api_spex": {:hex, :open_api_spex, "3.19.1", "65ccb5d06e3d664d1eec7c5ea2af2289bd2f37897094a74d7219fb03fc2b5994", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}, {:poison, "~> 3.0 or ~> 4.0 or ~> 5.0", [hex: :poison, repo: "hexpm", optional: true]}, {:ymlr, "~> 2.0 or ~> 3.0 or ~> 4.0 or ~> 5.0", [hex: :ymlr, repo: "hexpm", optional: true]}], "hexpm", "392895827ce2984a3459c91a484e70708132d8c2c6c5363972b4b91d6bbac3dd"},