From c9cad42d1e74a9131737e1c80318614b2dff8d51 Mon Sep 17 00:00:00 2001 From: Rogerio Pontual <44991200+jyeshe@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:25:46 +0100 Subject: [PATCH] Allow different rules and action for delete user (#2714) * Refactor user profile for remaining components * Rename components module * Remove unnecessary vertical bar between table actions in UserLive.Components * Replace not working private with metadata * Address the user directly yet once more * Fix use of metadata modal * Fix cancel scheduled deletion on admin users * Revert change * There is pre-existing bug that needs to be fixed when clicking outside the modal * Fix test cases and revert to attrs string keys * Rename menu context to current user --------- Co-authored-by: Elias W. BA --- CHANGELOG.md | 3 + config/config.exs | 5 +- lib/lightning/accounts.ex | 13 +- .../live/components/user_deletion_modal.ex | 9 +- .../live/profile_live/components.ex | 90 ++++++++++++ lib/lightning_web/live/profile_live/edit.ex | 9 ++ .../live/profile_live/edit.html.heex | 31 ++-- .../profile_live/form_component.html.heex | 38 +---- .../live/user_live/components.ex | 135 ++++++++++++++++++ lib/lightning_web/live/user_live/index.ex | 95 +++--------- .../live/user_live/index.html.heex | 58 +------- lib/lightning_web/router.ex | 2 - test/lightning/accounts/user_test.exs | 13 +- test/lightning_web/live/profile_live_test.exs | 4 +- test/lightning_web/live/user_live_test.exs | 12 +- 15 files changed, 311 insertions(+), 206 deletions(-) create mode 100644 lib/lightning_web/live/profile_live/components.ex create mode 100644 lib/lightning_web/live/user_live/components.ex diff --git a/CHANGELOG.md b/CHANGELOG.md index e34973384b..3be67a0a4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ and this project adheres to ### Added +- Allow different rules and action for delete user. + [#2500](https://github.com/OpenFn/lightning/issues/2500) + ### Changed ### Fixed diff --git a/config/config.exs b/config/config.exs index 305d30f627..0505e78723 100644 --- a/config/config.exs +++ b/config/config.exs @@ -40,7 +40,10 @@ config :lightning, Lightning.Extensions, config :lightning, Lightning.Extensions.Routing, session_opts: [on_mount: LightningWeb.InitAssigns], routes: [ - {"/projects", LightningWeb.DashboardLive.Index, :index, []} + {"/projects", LightningWeb.DashboardLive.Index, :index, []}, + {"/profile", LightningWeb.ProfileLive.Edit, :edit, + metadata: %{delete_modal: LightningWeb.Components.UserDeletionModal}}, + {"/settings/users", LightningWeb.UserLive.Index, :index, []} ] # TODO: don't use this value in production diff --git a/lib/lightning/accounts.ex b/lib/lightning/accounts.ex index 2ddccfcad6..3776ee7517 100644 --- a/lib/lightning/accounts.ex +++ b/lib/lightning/accounts.ex @@ -449,11 +449,13 @@ defmodule Lightning.Accounts do end def cancel_scheduled_deletion(user_id) do - get_user!(user_id) - |> update_user_details(%{ - scheduled_deletion: nil, - disabled: false + user_id + |> get_user!() + |> User.details_changeset(%{ + "scheduled_deletion" => nil, + "disabled" => false }) + |> Repo.update() end @doc """ @@ -671,7 +673,8 @@ defmodule Lightning.Accounts do integer -> DateTime.utc_now() |> Timex.shift(days: integer) end - User.scheduled_deletion_changeset(user, %{ + user + |> User.scheduled_deletion_changeset(%{ "scheduled_deletion" => date, "disabled" => true, "scheduled_deletion_email" => email diff --git a/lib/lightning_web/live/components/user_deletion_modal.ex b/lib/lightning_web/live/components/user_deletion_modal.ex index c141fdd40a..45f0bd31bd 100644 --- a/lib/lightning_web/live/components/user_deletion_modal.ex +++ b/lib/lightning_web/live/components/user_deletion_modal.ex @@ -14,6 +14,7 @@ defmodule LightningWeb.Components.UserDeletionModal do |> assign( delete_now?: !is_nil(user.scheduled_deletion), has_activity_in_projects?: Accounts.has_activity_in_projects?(user), + is_current_user: Map.get(assigns, :is_current_user, true), scheduled_deletion_changeset: Accounts.change_scheduled_deletion(user) ) |> assign(assigns)} @@ -106,7 +107,7 @@ defmodule LightningWeb.Components.UserDeletionModal do

- This user cannot be deleted until their auditable activities have also been purged. + <%= if @is_current_user, do: "Your account", else: "This user" %> cannot be deleted until their auditable activities have also been purged.

Audit trails are removed on a project-basis and may be controlled by the project owner or a superuser.

@@ -158,10 +159,12 @@ defmodule LightningWeb.Components.UserDeletionModal do >

- This user's account and credential data will be deleted. Please make sure none of these credentials are used in production workflows. + <%= if @is_current_user, do: "Your", else: "This user's" %> account and credential data will be deleted. Please make sure none of these credentials are used in production workflows.

- *Note that this user still has activity related to active projects. We may not be able to delete them entirely from the app until those projects are deleted. + *Note that <%= if @is_current_user, + do: "you still have", + else: "this user still has" %> activity related to active projects. We may not be able to delete them entirely from the app until those projects are deleted.


<.input diff --git a/lib/lightning_web/live/profile_live/components.ex b/lib/lightning_web/live/profile_live/components.ex new file mode 100644 index 0000000000..17f1d52ffe --- /dev/null +++ b/lib/lightning_web/live/profile_live/components.ex @@ -0,0 +1,90 @@ +defmodule LightningWeb.ProfileLive.Components do + use LightningWeb, :component + + attr :current_user, Lightning.Accounts.User, required: true + + def user_info(assigns) do + ~H""" +
+

+ <%= @current_user.first_name %> <%= @current_user.last_name %> +

+

+ Change name, email, password, and request deletion. +

+
+

+ Created: <%= @current_user.inserted_at |> Calendar.strftime("%c %Z") %> UTC +

+

+ Email: <%= @current_user.email %> +

+
+ """ + end + + attr :page_title, :string, required: true + attr :live_action, :atom, required: true + attr :current_user, Lightning.Accounts.User, required: true + + attr :user_deletion_modal, :atom, + default: LightningWeb.Components.UserDeletionModal + + attr :delete_user_url, :string, required: true + + def action_cards(assigns) do + ~H""" +
+ <.live_component + :if={@live_action == :delete} + module={@user_deletion_modal} + id={@current_user.id} + user={@current_user} + logout={true} + return_to={~p"/profile"} + /> + <.live_component + module={LightningWeb.ProfileLive.FormComponent} + id={@current_user.id} + title={@page_title} + action={@live_action} + user={@current_user} + return_to={~p"/profile"} + /> + <.live_component + module={LightningWeb.ProfileLive.MfaComponent} + id={"#{@current_user.id}_mfa_section"} + user={@current_user} + /> + <.live_component + module={LightningWeb.ProfileLive.GithubComponent} + id={"#{@current_user.id}_github_section"} + user={@current_user} + /> + <.delete_user_card url={@delete_user_url} /> +
+ """ + end + + attr :url, :string, required: true + + defp delete_user_card(assigns) do + ~H""" +
+
+ Delete account + + <.link navigate={@url}> + + + +
+
+ """ + end +end diff --git a/lib/lightning_web/live/profile_live/edit.ex b/lib/lightning_web/live/profile_live/edit.ex index 17bb1106f3..e295d193ad 100644 --- a/lib/lightning_web/live/profile_live/edit.ex +++ b/lib/lightning_web/live/profile_live/edit.ex @@ -3,6 +3,9 @@ defmodule LightningWeb.ProfileLive.Edit do LiveView for user profile page. """ use LightningWeb, :live_view + + import LightningWeb.ProfileLive.Components + alias Lightning.VersionControl on_mount {LightningWeb.Hooks, :assign_projects} @@ -67,9 +70,15 @@ defmodule LightningWeb.ProfileLive.Edit do ) if can_delete_account do + modal = + socket.router + |> Phoenix.Router.route_info("GET", ~p"/profile", nil) + |> Map.get(:delete_modal) + socket |> assign(:page_title, "User Profile") |> assign(:user, user) + |> assign(:user_deletion_modal, modal) else put_flash(socket, :error, "You can't perform this action") |> push_patch(to: ~p"/profile") diff --git a/lib/lightning_web/live/profile_live/edit.html.heex b/lib/lightning_web/live/profile_live/edit.html.heex index 0939ec18d1..8468b8c55a 100644 --- a/lib/lightning_web/live/profile_live/edit.html.heex +++ b/lib/lightning_web/live/profile_live/edit.html.heex @@ -6,28 +6,15 @@
-
-

- <%= @current_user.first_name %> <%= @current_user.last_name %> -

-

- Change name, email, password, and request deletion. -

-
-

- Created: <%= @current_user.inserted_at |> Calendar.strftime("%c %Z") %> UTC -

-

- Email: <%= @current_user.email %> -

-
- <.live_component - module={LightningWeb.ProfileLive.FormComponent} - id={@current_user.id} - title={@page_title} - action={@live_action} - user={@user} - return_to={~p"/profile"} + <.user_info current_user={@current_user} /> + <.action_cards + page_title={@page_title} + current_user={@current_user} + live_action={@live_action} + user_deletion_modal={assigns[:user_deletion_modal]} + delete_user_url={ + Routes.profile_edit_path(@socket, :delete, @current_user) + } />
diff --git a/lib/lightning_web/live/profile_live/form_component.html.heex b/lib/lightning_web/live/profile_live/form_component.html.heex index b804d20bcc..eb4cc4a75e 100644 --- a/lib/lightning_web/live/profile_live/form_component.html.heex +++ b/lib/lightning_web/live/profile_live/form_component.html.heex @@ -1,14 +1,4 @@ -
- <%= if @action == :delete do %> - <.live_component - module={LightningWeb.Components.UserDeletionModal} - id={@user.id} - user={@user} - logout={true} - return_to={~p"/profile"} - /> - <% end %> - +
<.form :let={f} as={:user} @@ -154,30 +144,4 @@ Update password - <.live_component - module={LightningWeb.ProfileLive.MfaComponent} - id={"#{@user.id}_mfa_section"} - user={@user} - /> - - <.live_component - module={LightningWeb.ProfileLive.GithubComponent} - id={"#{@user.id}_github_section"} - user={@user} - /> -
-
- Delete account - - <.link navigate={Routes.profile_edit_path(@socket, :delete, @user)}> - - - -
-
diff --git a/lib/lightning_web/live/user_live/components.ex b/lib/lightning_web/live/user_live/components.ex new file mode 100644 index 0000000000..585d7ebb44 --- /dev/null +++ b/lib/lightning_web/live/user_live/components.ex @@ -0,0 +1,135 @@ +defmodule LightningWeb.UserLive.Components do + use LightningWeb, :component + + import PetalComponents.Table + + attr :socket, :map, required: true + attr :users, :list, required: true + attr :live_action, :atom, required: true + + attr :user_deletion_modal, :atom, + default: LightningWeb.Components.UserDeletionModal + + attr :delete_user, Lightning.Accounts.User, default: nil + + def users_table(assigns) do + ~H""" + <.live_component + :if={@live_action == :delete} + module={@user_deletion_modal} + id={@delete_user.id} + user={@delete_user} + is_current_user={false} + logout={false} + return_to={Routes.user_index_path(@socket, :index)} + /> + <.table id="users"> + <.tr> + <.th>First name + <.th>Last name + <.th>Email + <.th>Role* + <.th>Enabled? + <.th>Scheduled Deletion + <.th>Actions + + <%= for user <- @users do %> + <.tr id={"user-#{user.id}"}> + <.td><%= user.first_name %> + <.td><%= user.last_name %> + <.td><%= user.email %> + <.td><%= user.role %> + <.td> + <%= if !user.disabled do %> + + <% end %> + + <.td><%= user.scheduled_deletion %> + <.td class="py-0.5"> + + <.link + class="table-action" + navigate={Routes.user_edit_path(@socket, :edit, user)} + > + Edit + + + <.delete_action + user={user} + delete_url={Routes.user_index_path(@socket, :delete, user)} + /> + + + <% end %> + +
+ <.p> + *Note that a superuser can access everything in a + Lightning installation across all projects, including this page. Most + day-to-day user management (adding and removing collaborators) will be + done by project "admins" via the project settings page. + + """ + end + + attr :delete_url, :string, required: true + attr :user, :string, required: true + + defp delete_action(%{user: %{role: :superuser}} = assigns) do + if assigns.user.scheduled_deletion do + ~H""" + <.cancel_deletion user={@user} /> | + + Delete now + + """ + else + ~H""" + + Delete + + """ + end + end + + defp delete_action(%{user: %{role: :user}} = assigns) do + if assigns.user.scheduled_deletion do + ~H""" + <.cancel_deletion user={@user} /> | + + <.link + id={"delete-now-#{@user.id}"} + class="table-action" + navigate={@delete_url} + > + Delete now + + + """ + else + ~H""" + + <.link id={"delete-#{@user.id}"} class="table-action" navigate={@delete_url}> + Delete + + + """ + end + end + + defp cancel_deletion(assigns) do + ~H""" + + <.link + id={"cancel-deletion-#{@user.id}"} + href="#" + phx-click="cancel_deletion" + phx-value-id={@user.id} + class="table-action" + > + Cancel deletion + + + """ + end +end diff --git a/lib/lightning_web/live/user_live/index.ex b/lib/lightning_web/live/user_live/index.ex index 03472ab688..51423efc80 100644 --- a/lib/lightning_web/live/user_live/index.ex +++ b/lib/lightning_web/live/user_live/index.ex @@ -4,7 +4,7 @@ defmodule LightningWeb.UserLive.Index do """ use LightningWeb, :live_view - import PetalComponents.Table + import LightningWeb.UserLive.Components alias Lightning.Accounts alias Lightning.Policies.Permissions @@ -17,10 +17,13 @@ defmodule LightningWeb.UserLive.Index do |> Permissions.can?(:access_admin_space, socket.assigns.current_user, {}) if can_access_admin_space do - {:ok, - assign(socket, :users, list_users()) - |> assign(:active_menu_item, :users), - layout: {LightningWeb.Layouts, :settings}} + socket = + assign(socket, + users: list_users(), + active_menu_item: :users + ) + + {:ok, socket, layout: {LightningWeb.Layouts, :settings}} else {:ok, put_flash(socket, :nav, :no_access) @@ -36,13 +39,13 @@ defmodule LightningWeb.UserLive.Index do defp apply_action(socket, :index, _params) do socket |> assign(:page_title, "Users") - |> assign(:user, nil) + |> assign(:delete_user, nil) end defp apply_action(socket, :delete, %{"id" => id}) do socket |> assign(:page_title, "Users") - |> assign(:user, Accounts.get_user!(id)) + |> assign(:delete_user, Accounts.get_user!(id)) end @impl true @@ -51,77 +54,21 @@ defmodule LightningWeb.UserLive.Index do %{"id" => user_id}, socket ) do - Accounts.cancel_scheduled_deletion(user_id) + case Accounts.cancel_scheduled_deletion(user_id) do + {:ok, _change} -> + {:noreply, + socket + |> put_flash(:info, "User deletion canceled") + |> push_navigate(to: ~p"/settings/users")} - {:noreply, - socket - |> put_flash(:info, "User deletion canceled") - |> push_patch(to: ~p"/settings/users")} + {:error, _reason} -> + {:noreply, + socket + |> put_flash(:error, "Cancel user deletion failed")} + end end defp list_users do Accounts.list_users() end - - def delete_action(%{user: %{role: :user}} = assigns) do - if assigns.user.scheduled_deletion do - ~H""" - <.cancel_deletion user={@user} /> | - - <.link - id={"delete-now-#{@user.id}"} - class="table-action" - navigate={Routes.user_index_path(@socket, :delete, @user)} - > - Delete now - - - """ - else - ~H""" - - <.link - id={"delete-#{@user.id}"} - class="table-action" - navigate={Routes.user_index_path(@socket, :delete, @user)} - > - Delete - - - """ - end - end - - def delete_action(%{user: %{role: :superuser}} = assigns) do - if assigns.user.scheduled_deletion do - ~H""" - <.cancel_deletion user={@user} /> | - - Delete now - - """ - else - ~H""" - - Delete - - """ - end - end - - defp cancel_deletion(assigns) do - ~H""" - - <.link - id={"cancel-deletion-#{@user.id}"} - href="#" - phx-click="cancel_deletion" - phx-value-id={@user.id} - class="table-action" - > - Cancel deletion - - - """ - end end diff --git a/lib/lightning_web/live/user_live/index.html.heex b/lib/lightning_web/live/user_live/index.html.heex index 80dae12695..c443c69115 100644 --- a/lib/lightning_web/live/user_live/index.html.heex +++ b/lib/lightning_web/live/user_live/index.html.heex @@ -13,57 +13,11 @@ - <%= if @live_action == :delete do %> - <.live_component - module={LightningWeb.Components.UserDeletionModal} - id={@user.id} - user={@user} - logout={false} - return_to={Routes.user_index_path(@socket, :index)} - /> - <% end %> - <.table id="users"> - <.tr> - <.th>First name - <.th>Last name - <.th>Email - <.th>Role* - <.th>Enabled? - <.th>Scheduled Deletion - <.th>Actions - - <%= for user <- @users do %> - <.tr id={"user-#{user.id}"}> - <.td><%= user.first_name %> - <.td><%= user.last_name %> - <.td><%= user.email %> - <.td><%= user.role %> - <.td> - <%= if !user.disabled do %> - - <% end %> - - <.td><%= user.scheduled_deletion %> - <.td class="py-0.5"> - - <.link - class="table-action" - navigate={Routes.user_edit_path(@socket, :edit, user)} - > - Edit - - - <.delete_action socket={@socket} user={user} /> - - - <% end %> - -
- <.p> - *Note that a superuser can access everything in a - Lightning installation across all projects, including this page. Most - day-to-day user management (adding and removing collaborators) will be - done by project "admins" via the project settings page. - + <.users_table + socket={@socket} + live_action={@live_action} + delete_user={assigns[:delete_user]} + users={@users} + /> diff --git a/lib/lightning_web/router.ex b/lib/lightning_web/router.ex index fd4e428b38..3842cc2fcf 100644 --- a/lib/lightning_web/router.ex +++ b/lib/lightning_web/router.ex @@ -156,7 +156,6 @@ defmodule LightningWeb.Router do live_session :settings, on_mount: LightningWeb.InitAssigns do live "/settings", SettingsLive.Index, :index - live "/settings/users", UserLive.Index, :index live "/settings/users/new", UserLive.Edit, :new live "/settings/users/:id", UserLive.Edit, :edit live "/settings/users/:id/delete", UserLive.Index, :delete @@ -198,7 +197,6 @@ defmodule LightningWeb.Router do live "/credentials/new", CredentialLive.Edit, :new live "/credentials/:id", CredentialLive.Edit, :edit - live "/profile", ProfileLive.Edit, :edit live "/profile/:id/delete", ProfileLive.Edit, :delete live "/profile/tokens", TokensLive.Index, :index diff --git a/test/lightning/accounts/user_test.exs b/test/lightning/accounts/user_test.exs index 7994f9190c..2329b0acd6 100644 --- a/test/lightning/accounts/user_test.exs +++ b/test/lightning/accounts/user_test.exs @@ -446,10 +446,15 @@ defmodule Lightning.Accounts.UserTest do test "email does match current users email" do errors = - User.scheduled_deletion_changeset(%User{email: "real@email.com"}, %{ - "id" => "86201fff-a699-4eca-bb53-8736228ff187", - "scheduled_deletion_email" => "real@email.com" - }) + User.scheduled_deletion_changeset( + %User{ + id: "86201fff-a699-4eca-bb53-8736228ff187", + email: "real@email.com" + }, + %{ + "scheduled_deletion_email" => "real@email.com" + } + ) |> errors_on() assert errors[:scheduled_deletion_email] == nil diff --git a/test/lightning_web/live/profile_live_test.exs b/test/lightning_web/live/profile_live_test.exs index 85966e4a36..491d6511a4 100644 --- a/test/lightning_web/live/profile_live_test.exs +++ b/test/lightning_web/live/profile_live_test.exs @@ -202,7 +202,7 @@ defmodule LightningWeb.ProfileLiveTest do ) assert html =~ - "This user's account and credential data will be deleted" + "Your account and credential data will be deleted" assert new_live |> form("#scheduled_deletion_form", @@ -258,7 +258,7 @@ defmodule LightningWeb.ProfileLiveTest do ) assert html =~ - "This user's account and credential data will be deleted" + "Your account and credential data will be deleted" {:ok, _new_live, html} = new_live diff --git a/test/lightning_web/live/user_live_test.exs b/test/lightning_web/live/user_live_test.exs index 47dc14a9c8..2c66c1358e 100644 --- a/test/lightning_web/live/user_live_test.exs +++ b/test/lightning_web/live/user_live_test.exs @@ -27,7 +27,7 @@ defmodule LightningWeb.UserLiveTest do @invalid_attrs %{email: nil, first_name: nil, last_name: nil, password: nil} @invalid_schedule_deletion_attrs %{ - scheduled_deletion_email: "invalid@email.com" + "scheduled_deletion_email" => "invalid@email.com" } describe "Index for super user" do @@ -246,7 +246,7 @@ defmodule LightningWeb.UserLiveTest do form_live |> form("#scheduled_deletion_form", user: %{ - scheduled_deletion_email: user.email + "scheduled_deletion_email" => user.email } ) |> render_submit() @@ -320,7 +320,11 @@ defmodule LightningWeb.UserLiveTest do assert index_live |> element("#user-#{user.id} a", "Cancel deletion") - |> render_click() =~ "User deletion canceled" + |> render_click() + + flash = assert_redirected(index_live, ~p"/settings/users") + + assert flash["info"] == "User deletion canceled" end test "retains a cancel deletion button for superusers pending deletion", %{ @@ -358,7 +362,7 @@ defmodule LightningWeb.UserLiveTest do form_live |> form("#scheduled_deletion_form", user: %{ - scheduled_deletion_email: user.email + "scheduled_deletion_email" => user.email } ) |> render_submit()