From 5ef1aca1c6171da4f9241ce4ddb0e711f8e34286 Mon Sep 17 00:00:00 2001 From: Midigo Frank <39288959+midigofrank@users.noreply.github.com> Date: Thu, 31 Oct 2024 15:07:36 +0300 Subject: [PATCH] Limit retention period (#2618) * Add Extension for limiting data retention periods * allow message to be nil * remove placeholder text * make credo happy * update changelog * centralize data retention options --- CHANGELOG.md | 2 + lib/lightning/extensions/project_hook.ex | 4 ++ lib/lightning/extensions/project_hooking.ex | 3 + lib/lightning/extensions/usage_limiter.ex | 8 +++ lib/lightning/extensions/usage_limiting.ex | 7 ++ lib/lightning/pipeline/failure_alerter.ex | 4 +- lib/lightning/projects.ex | 5 +- lib/lightning/projects/project.ex | 11 ++-- .../projects/project_alerts_limiter.ex | 16 ----- lib/lightning/projects/project_limiter.ex | 50 ++++++++++++++ .../projects/project_users_limiter.ex | 24 ------- lib/lightning/services/project_hook.ex | 6 ++ lib/lightning/services/usage_limiter.ex | 10 +++ lib/lightning_web/hooks.ex | 13 ++++ .../invite_collaborator_component.ex | 4 +- .../new_collaborator_component.ex | 4 +- .../live/project_live/settings.ex | 8 +-- .../live/project_live/settings.html.heex | 26 ++++++-- test/lightning/projects_test.exs | 29 +++++++++ test/lightning_web/live/project_live_test.exs | 65 +++++++++++++++++++ test/support/conn_case.ex | 5 ++ test/support/data_case.ex | 5 ++ test/support/stub_usage_limiter.ex | 8 +++ test/test_helper.exs | 6 +- 24 files changed, 263 insertions(+), 60 deletions(-) delete mode 100644 lib/lightning/projects/project_alerts_limiter.ex create mode 100644 lib/lightning/projects/project_limiter.ex delete mode 100644 lib/lightning/projects/project_users_limiter.ex diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ebbd81f57..748c695f5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ and this project adheres to - Enforcing MFA for a project can be enforced by the usage limiter [#2607](https://github.com/OpenFn/lightning/pull/2607) +- Add extensions for limiting retention period + [#2618](https://github.com/OpenFn/lightning/pull/2618) ### Fixed diff --git a/lib/lightning/extensions/project_hook.ex b/lib/lightning/extensions/project_hook.ex index ed81c77b78..de767367b4 100644 --- a/lib/lightning/extensions/project_hook.ex +++ b/lib/lightning/extensions/project_hook.ex @@ -42,4 +42,8 @@ defmodule Lightning.Extensions.ProjectHook do Repo.delete(project) end + + @spec handle_project_validation(Changeset.t(Project.t())) :: + Changeset.t(Project.t()) + def handle_project_validation(changeset), do: changeset end diff --git a/lib/lightning/extensions/project_hooking.ex b/lib/lightning/extensions/project_hooking.ex index cd5ce177f3..122d047307 100644 --- a/lib/lightning/extensions/project_hooking.ex +++ b/lib/lightning/extensions/project_hooking.ex @@ -10,4 +10,7 @@ defmodule Lightning.Extensions.ProjectHooking do @callback handle_delete_project(Project.t()) :: {:ok, Project.t()} | {:error, Changeset.t()} + + @callback handle_project_validation(Ecto.Changeset.t(Project.t())) :: + Ecto.Changeset.t(Project.t()) end diff --git a/lib/lightning/extensions/usage_limiter.ex b/lib/lightning/extensions/usage_limiter.ex index 4bec4824e2..486b3cae42 100644 --- a/lib/lightning/extensions/usage_limiter.ex +++ b/lib/lightning/extensions/usage_limiter.ex @@ -20,4 +20,12 @@ defmodule Lightning.Extensions.UsageLimiter do run_timeout_ms: Lightning.Config.default_max_run_duration() * 1000 ] end + + @impl true + def get_data_retention_periods(_context) do + Lightning.Projects.Project.data_retention_options() + end + + @impl true + def get_data_retention_message(_context), do: nil end diff --git a/lib/lightning/extensions/usage_limiting.ex b/lib/lightning/extensions/usage_limiting.ex index 40a1b9c26c..df2a8da4f0 100644 --- a/lib/lightning/extensions/usage_limiting.ex +++ b/lib/lightning/extensions/usage_limiting.ex @@ -89,4 +89,11 @@ defmodule Lightning.Extensions.UsageLimiting do """ @callback get_run_options(context :: Context.t()) :: Lightning.Runs.RunOptions.keyword_list() + + @callback get_data_retention_periods(context :: Context.t()) :: [ + pos_integer(), + ... + ] + + @callback get_data_retention_message(context :: Context.t()) :: message() | nil end diff --git a/lib/lightning/pipeline/failure_alerter.ex b/lib/lightning/pipeline/failure_alerter.ex index b5b0c7fb53..4d7b533413 100644 --- a/lib/lightning/pipeline/failure_alerter.ex +++ b/lib/lightning/pipeline/failure_alerter.ex @@ -3,7 +3,7 @@ defmodule Lightning.FailureAlerter do use LightningWeb, :verified_routes - alias Lightning.Projects.ProjectAlertsLimiter + alias Lightning.Projects.ProjectLimiter alias Lightning.Run def alert_on_failure(nil), do: nil @@ -14,7 +14,7 @@ defmodule Lightning.FailureAlerter do def alert_on_failure(%Run{} = run) do workflow = run.work_order.workflow - if :ok == ProjectAlertsLimiter.limit_failure_alert(workflow.project_id) do + if :ok == ProjectLimiter.limit_failure_alert(workflow.project_id) do Lightning.Accounts.get_users_to_alert_for_project(%{ id: workflow.project_id }) diff --git a/lib/lightning/projects.ex b/lib/lightning/projects.ex index 7facecf0ac..0def3b1ff7 100644 --- a/lib/lightning/projects.ex +++ b/lib/lightning/projects.ex @@ -287,7 +287,10 @@ defmodule Lightning.Projects do """ def update_project(%Project{} = project, attrs) do - changeset = Project.changeset(project, attrs) + changeset = + project + |> Project.changeset(attrs) + |> ProjectHook.handle_project_validation() case Repo.update(changeset) do {:ok, updated_project} -> diff --git a/lib/lightning/projects/project.ex b/lib/lightning/projects/project.ex index 7a824bb1f7..fd54e6ad19 100644 --- a/lib/lightning/projects/project.ex +++ b/lib/lightning/projects/project.ex @@ -17,8 +17,6 @@ defmodule Lightning.Projects.Project do @type retention_policy_type :: :retain_all | :retain_with_errors | :erase_all - @retention_periods [7, 14, 30, 90, 180, 365] - schema "projects" do field :name, :string field :description, :string @@ -45,6 +43,11 @@ defmodule Lightning.Projects.Project do timestamps() end + @spec data_retention_options() :: [pos_integer(), ...] + def data_retention_options do + [7, 14, 30, 90, 180, 365] + end + @doc false # TODO: schedule_deletion shouldn't be changed by user input def changeset(project, attrs) do @@ -67,9 +70,9 @@ defmodule Lightning.Projects.Project do |> validate_length(:description, max: 240) |> validate_required([:name]) |> validate_format(:name, ~r/^[a-z\-\d]+$/) - |> validate_inclusion(:history_retention_period, @retention_periods) - |> validate_inclusion(:dataclip_retention_period, @retention_periods) |> validate_dataclip_retention_period() + |> validate_inclusion(:history_retention_period, data_retention_options()) + |> validate_inclusion(:dataclip_retention_period, data_retention_options()) end defp validate_dataclip_retention_period(changeset) do diff --git a/lib/lightning/projects/project_alerts_limiter.ex b/lib/lightning/projects/project_alerts_limiter.ex deleted file mode 100644 index e974546060..0000000000 --- a/lib/lightning/projects/project_alerts_limiter.ex +++ /dev/null @@ -1,16 +0,0 @@ -defmodule Lightning.Projects.ProjectAlertsLimiter do - @moduledoc false - - alias Lightning.Extensions.UsageLimiting - alias Lightning.Extensions.UsageLimiting.Action - alias Lightning.Extensions.UsageLimiting.Context - alias Lightning.Services.UsageLimiter - - @spec limit_failure_alert(project_id :: Ecto.UUID.t()) :: - :ok | UsageLimiting.error() - def limit_failure_alert(project_id) do - UsageLimiter.limit_action(%Action{type: :alert_failure}, %Context{ - project_id: project_id - }) - end -end diff --git a/lib/lightning/projects/project_limiter.ex b/lib/lightning/projects/project_limiter.ex new file mode 100644 index 0000000000..575c5a2a1a --- /dev/null +++ b/lib/lightning/projects/project_limiter.ex @@ -0,0 +1,50 @@ +defmodule Lightning.Projects.ProjectLimiter do + @moduledoc false + + alias Lightning.Extensions.UsageLimiting + alias Lightning.Extensions.UsageLimiting.Action + alias Lightning.Extensions.UsageLimiting.Context + alias Lightning.Services.UsageLimiter + + @spec limit_failure_alert(project_id :: Ecto.UUID.t()) :: + :ok | UsageLimiting.error() + def limit_failure_alert(project_id) do + UsageLimiter.limit_action(%Action{type: :alert_failure}, %Context{ + project_id: project_id + }) + end + + @spec request_new_user( + project_id :: Ecto.UUID.t(), + user_count :: non_neg_integer() + ) :: :ok | UsageLimiting.error() + def request_new_user(project_id, user_count) do + UsageLimiter.limit_action( + %Action{ + type: :new_user, + amount: user_count + }, + %Context{ + project_id: project_id + } + ) + end + + @spec get_data_retention_periods(project_id :: Ecto.UUID.t()) :: [ + pos_integer(), + ... + ] + def get_data_retention_periods(project_id) do + UsageLimiter.get_data_retention_periods(%Context{ + project_id: project_id + }) + end + + @spec get_data_retention_message(project_id :: Ecto.UUID.t()) :: + Lightning.Extensions.Message.t() + def get_data_retention_message(project_id) do + UsageLimiter.get_data_retention_message(%Context{ + project_id: project_id + }) + end +end diff --git a/lib/lightning/projects/project_users_limiter.ex b/lib/lightning/projects/project_users_limiter.ex deleted file mode 100644 index 8efb125ea7..0000000000 --- a/lib/lightning/projects/project_users_limiter.ex +++ /dev/null @@ -1,24 +0,0 @@ -defmodule Lightning.Projects.ProjectUsersLimiter do - @moduledoc false - - alias Lightning.Extensions.UsageLimiting - alias Lightning.Extensions.UsageLimiting.Action - alias Lightning.Extensions.UsageLimiting.Context - alias Lightning.Services.UsageLimiter - - @spec request_new( - project_id :: Ecto.UUID.t(), - user_count :: non_neg_integer() - ) :: :ok | UsageLimiting.error() - def request_new(project_id, user_count) do - UsageLimiter.limit_action( - %Action{ - type: :new_user, - amount: user_count - }, - %Context{ - project_id: project_id - } - ) - end -end diff --git a/lib/lightning/services/project_hook.ex b/lib/lightning/services/project_hook.ex index af2ad2532b..9920682baa 100644 --- a/lib/lightning/services/project_hook.ex +++ b/lib/lightning/services/project_hook.ex @@ -21,5 +21,11 @@ defmodule Lightning.Services.ProjectHook do adapter().handle_delete_project(project) end + @spec handle_project_validation(Changeset.t(Project.t())) :: + Changeset.t(Project.t()) + def handle_project_validation(changeset) do + adapter().handle_project_validation(changeset) + end + defp adapter, do: adapter(:project_hook) end diff --git a/lib/lightning/services/usage_limiter.ex b/lib/lightning/services/usage_limiter.ex index 0848780155..8753aacdd7 100644 --- a/lib/lightning/services/usage_limiter.ex +++ b/lib/lightning/services/usage_limiter.ex @@ -26,5 +26,15 @@ defmodule Lightning.Services.UsageLimiter do adapter().get_run_options(context) end + @impl true + def get_data_retention_periods(context) do + adapter().get_data_retention_periods(context) + end + + @impl true + def get_data_retention_message(context) do + adapter().get_data_retention_message(context) + end + defp adapter, do: adapter(:usage_limiter) end diff --git a/lib/lightning_web/hooks.ex b/lib/lightning_web/hooks.ex index a4a9f3740f..009252b6c6 100644 --- a/lib/lightning_web/hooks.ex +++ b/lib/lightning_web/hooks.ex @@ -11,6 +11,7 @@ defmodule LightningWeb.Hooks do alias Lightning.Extensions.UsageLimiting.Context alias Lightning.Policies.Permissions alias Lightning.Policies.ProjectUsers + alias Lightning.Projects.ProjectLimiter alias Lightning.Services.UsageLimiter alias Lightning.VersionControl.VersionControlUsageLimiter @@ -102,4 +103,16 @@ defmodule LightningWeb.Hooks do {:cont, assign(socket, mfa_banner: component, can_require_mfa: false)} end end + + def on_mount(:limit_retention_periods, _params, _session, socket) do + %{project: project} = socket.assigns + retention_periods = ProjectLimiter.get_data_retention_periods(project.id) + retention_message = ProjectLimiter.get_data_retention_message(project.id) + + {:cont, + assign(socket, + data_retention_periods: retention_periods, + data_retention_limit_message: retention_message + )} + end end diff --git a/lib/lightning_web/live/project_live/invite_collaborator_component.ex b/lib/lightning_web/live/project_live/invite_collaborator_component.ex index 3569d0558e..50815395e5 100644 --- a/lib/lightning_web/live/project_live/invite_collaborator_component.ex +++ b/lib/lightning_web/live/project_live/invite_collaborator_component.ex @@ -3,7 +3,7 @@ defmodule LightningWeb.ProjectLive.InviteCollaboratorComponent do use LightningWeb, :live_component alias Lightning.Projects - alias Lightning.Projects.ProjectUsersLimiter + alias Lightning.Projects.ProjectLimiter alias LightningWeb.ProjectLive.InvitedCollaborators alias Phoenix.LiveView.JS @@ -91,7 +91,7 @@ defmodule LightningWeb.ProjectLive.InviteCollaboratorComponent do project_users = Ecto.Changeset.get_embed(changeset, :invited_collaborators) - case ProjectUsersLimiter.request_new( + case ProjectLimiter.request_new_user( socket.assigns.project.id, Enum.count(project_users) ) do diff --git a/lib/lightning_web/live/project_live/new_collaborator_component.ex b/lib/lightning_web/live/project_live/new_collaborator_component.ex index a17f5ab13f..ab27997b4e 100644 --- a/lib/lightning_web/live/project_live/new_collaborator_component.ex +++ b/lib/lightning_web/live/project_live/new_collaborator_component.ex @@ -4,7 +4,7 @@ defmodule LightningWeb.ProjectLive.NewCollaboratorComponent do use LightningWeb, :live_component alias Lightning.Projects - alias Lightning.Projects.ProjectUsersLimiter + alias Lightning.Projects.ProjectLimiter alias LightningWeb.ProjectLive.Collaborators alias Phoenix.LiveView.JS @@ -130,7 +130,7 @@ defmodule LightningWeb.ProjectLive.NewCollaboratorComponent do project_users = Ecto.Changeset.get_embed(changeset, :collaborators) - case ProjectUsersLimiter.request_new( + case ProjectLimiter.request_new_user( socket.assigns.project.id, Enum.count(project_users) ) do diff --git a/lib/lightning_web/live/project_live/settings.ex b/lib/lightning_web/live/project_live/settings.ex index 7994709390..27fa1bce8d 100644 --- a/lib/lightning_web/live/project_live/settings.ex +++ b/lib/lightning_web/live/project_live/settings.ex @@ -13,9 +13,8 @@ defmodule LightningWeb.ProjectLive.Settings do alias Lightning.Policies.Permissions alias Lightning.Policies.ProjectUsers alias Lightning.Projects - alias Lightning.Projects.ProjectAlertsLimiter + alias Lightning.Projects.ProjectLimiter alias Lightning.Projects.ProjectUser - alias Lightning.Projects.ProjectUsersLimiter alias Lightning.VersionControl alias Lightning.WebhookAuthMethods alias Lightning.Workflows.WebhookAuthMethod @@ -28,6 +27,7 @@ defmodule LightningWeb.ProjectLive.Settings do on_mount {LightningWeb.Hooks, :project_scope} on_mount {LightningWeb.Hooks, :limit_github_sync} on_mount {LightningWeb.Hooks, :limit_mfa} + on_mount {LightningWeb.Hooks, :limit_retention_periods} @impl true def mount(_params, _session, socket) do @@ -119,7 +119,7 @@ defmodule LightningWeb.ProjectLive.Settings do ) can_receive_failure_alerts = - :ok == ProjectAlertsLimiter.limit_failure_alert(project.id) + :ok == ProjectLimiter.limit_failure_alert(project.id) repo_connection = VersionControl.get_repo_connection_for_project(project.id) @@ -685,7 +685,7 @@ defmodule LightningWeb.ProjectLive.Settings do end defp get_collaborator_limit_error(project) do - case ProjectUsersLimiter.request_new(project.id, 1) do + case ProjectLimiter.request_new_user(project.id, 1) do :ok -> nil diff --git a/lib/lightning_web/live/project_live/settings.html.heex b/lib/lightning_web/live/project_live/settings.html.heex index 8cfe2f5878..146499bd93 100644 --- a/lib/lightning_web/live/project_live/settings.html.heex +++ b/lib/lightning_web/live/project_live/settings.html.heex @@ -898,19 +898,37 @@ -
+
<.input type="select" prompt="Select Period" options={ - Enum.map([7, 14, 30, 90, 180, 365], fn days -> + Enum.map(@data_retention_periods, fn days -> {"#{days} Days", days} end) } - disabled={!@can_edit_data_retention} + disabled={ + !@can_edit_data_retention || + Enum.count(@data_retention_periods) == 1 + } field={f[:history_retention_period]} class="h-4 w-4 border-gray-300 text-indigo-600 focus:ring-indigo-600" /> +
+ <%= case assigns[:data_retention_limit_message] do %> + <% %{function: func} when is_function(func) -> %> + <%= Phoenix.LiveView.TagEngine.component( + @data_retention_limit_message.function, + @data_retention_limit_message.attrs, + {__ENV__.module, __ENV__.function, __ENV__.file, + __ENV__.line} + ) %> + <% %{text: text} when is_binary(text) -> %> + <%= text %> + <% _other -> %> + + <% end %> +