Commit 90daf0d4dbf22709ec2f6d03cc0aebb1b9551bf0

Thomas de Grivel 2022-01-28T10:54:04

[admin] user index: refactor column sort

diff --git a/lib/kmxgit/organisation_manager.ex b/lib/kmxgit/organisation_manager.ex
index ed9dda5..e9fed40 100644
--- a/lib/kmxgit/organisation_manager.ex
+++ b/lib/kmxgit/organisation_manager.ex
@@ -8,14 +8,11 @@ defmodule Kmxgit.OrganisationManager do
   alias Kmxgit.SlugManager.Slug
   alias Kmxgit.UserManager
 
-  @list_preload [:owned_repositories,
-                 :slug]
-
   def list_organisations(params \\ %IndexParams{}) do
     update_disk_usage()
     from(org in Organisation)
-    |> join(:inner, [org], s in Slug, on: s.organisation_id == org.id,)
-    |> preload(^@list_preload)
+    |> join(:inner, [org], s in Slug, on: s.organisation_id == org.id)
+    |> preload([:owned_repositories, :slug])
     |> index_order_by(params)
     |> Repo.all()
   end
diff --git a/lib/kmxgit/repository_manager.ex b/lib/kmxgit/repository_manager.ex
index 1eadb96..644edba 100644
--- a/lib/kmxgit/repository_manager.ex
+++ b/lib/kmxgit/repository_manager.ex
@@ -11,10 +11,6 @@ defmodule Kmxgit.RepositoryManager do
   alias Kmxgit.UserManager
   alias Kmxgit.UserManager.User
 
-  @list_preload [members: :slug,
-                 organisation: [:slug, users: :slug],
-                 user: :slug]
-
   def list_repositories(params \\ %IndexParams{}) do
     update_disk_usage()
     from(r in Repository)
@@ -23,7 +19,9 @@ defmodule Kmxgit.RepositoryManager do
     |> join(:full, [r, o, os], u in User, on: u.id == r.user_id)
     |> join(:full, [r, o, os, u], us in Slug, on: us.user_id == u.id)
     |> where([r, o, os, u, us], not is_nil(r))
-    |> preload(^@list_preload)
+    |> preload([members: :slug,
+               organisation: [:slug, users: :slug],
+               user: :slug])
     |> index_order_by(params)
     |> Repo.all()
   end
diff --git a/lib/kmxgit/user_manager.ex b/lib/kmxgit/user_manager.ex
index 469d716..a508950 100644
--- a/lib/kmxgit/user_manager.ex
+++ b/lib/kmxgit/user_manager.ex
@@ -5,117 +5,66 @@ defmodule Kmxgit.UserManager do
 
   import Ecto.Query, warn: false
 
+  alias Kmxgit.IndexParams
   alias Kmxgit.Repo
   alias Kmxgit.SlugManager.Slug
   alias Kmxgit.UserManager.{Avatar, User, UserToken, UserNotifier}
 
-  @list_preload [:owned_repositories,
-                 :slug]
-
-  def list_users do
+  def list_users(params \\ %IndexParams{}) do
     update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload
+    from(u in User)
+    |> join(:inner, [u], s in Slug, on: s.user_id == u.id)
+    |> preload([:owned_repositories, :slug])
+    |> index_order_by(params)
+    |> Repo.all()
   end
-  def list_users(%{column: "id", reverse: true}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [desc: :id]
+  def index_order_by(query, %IndexParams{column: "id", reverse: true}) do
+    order_by(query, [desc: :id])
   end
-  def list_users(%{column: "id"}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: :id
+  def index_order_by(query, %IndexParams{column: "id"}) do
+    order_by(query, :id)
   end
-  def list_users(%{column: "name", reverse: true}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [desc_nulls_last: fragment("lower(?)", user.name)]
+  def index_order_by(query, %IndexParams{column: "name", reverse: true}) do
+    order_by(query, [u, s], [desc_nulls_last: fragment("lower(?)", u.name)])
   end
-  def list_users(%{column: "name"}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [asc_nulls_last: fragment("lower(?)", user.name)]
+  def index_order_by(query, %IndexParams{column: "name"}) do
+    order_by(query, [u, s], [asc_nulls_last: fragment("lower(?)", u.name)])
   end
-  def list_users(%{column: "email", reverse: true}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [desc_nulls_last: fragment("lower(?)", user.email)]
+  def index_order_by(query, %IndexParams{column: "email", reverse: true}) do
+    order_by(query, [u, s], [desc_nulls_last: fragment("lower(?)", u.email)])
   end
-  def list_users(%{column: "email"}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [asc_nulls_last: fragment("lower(?)", user.email)]
+  def index_order_by(query, %IndexParams{column: "email"}) do
+    order_by(query, [u, s], [asc_nulls_last: fragment("lower(?)", u.email)])
   end
-  def list_users(%{column: "login", reverse: true}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      join: s in Slug,
-      on: s.user_id == user.id,
-      preload: ^@list_preload,
-      order_by: [desc_nulls_last: fragment("lower(?)", s.slug)]
+  def index_order_by(query, %IndexParams{column: "login", reverse: true}) do
+    order_by(query, [u, s], [desc_nulls_last: fragment("lower(?)", s.slug)])
   end
-  def list_users(%{column: "login"}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      join: s in Slug,
-      on: s.user_id == user.id,
-      preload: ^@list_preload,
-      order_by: [asc_nulls_last: fragment("lower(?)", s.slug)]
+  def index_order_by(query, %IndexParams{column: "login"}) do
+    order_by(query, [u, s], [asc_nulls_last: fragment("lower(?)", s.slug)])
   end
-  def list_users(%{column: "du", reverse: true}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [desc: :disk_usage]
+  def index_order_by(query, %IndexParams{column: "du", reverse: true}) do
+    order_by(query, [desc: :disk_usage])
   end
-  def list_users(%{column: "du"}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: :disk_usage
+  def index_order_by(query, %IndexParams{column: "du"}) do
+    order_by(query, :disk_usage)
   end
-  def list_users(%{column: "mfa", reverse: true}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [desc: user.totp_last == 0]
+  def index_order_by(query, %IndexParams{column: "mfa", reverse: true}) do
+    order_by(query, [u], [desc: u.totp_last == 0])
   end
-  def list_users(%{column: "mfa"}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: user.totp_last == 0
+  def index_order_by(query, %IndexParams{column: "mfa"}) do
+    order_by(query, [u], u.totp_last == 0)
   end
-  def list_users(%{column: "admin", reverse: true}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [desc: :is_admin]
+  def index_order_by(query, %IndexParams{column: "admin", reverse: true}) do
+    order_by(query, :is_admin)
   end
-  def list_users(%{column: "admin"}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: :is_admin
+  def index_order_by(query, %IndexParams{column: "admin"}) do
+    order_by(query, [desc: :is_admin])
   end
-  def list_users(%{column: "deploy", reverse: true}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: [desc: :deploy_only]
+  def index_order_by(query, %IndexParams{column: "deploy", reverse: true}) do
+    order_by(query, :deploy_only)
   end
-  def list_users(%{column: "deploy"}) do
-    update_disk_usage()
-    Repo.all from user in User,
-      preload: ^@list_preload,
-      order_by: :deploy_only
+  def index_order_by(query, %IndexParams{column: "deploy"}) do
+    order_by(query, [desc: :deploy_only])
   end
 
   def update_disk_usage() do
diff --git a/lib/kmxgit_web/controllers/admin/user_controller.ex b/lib/kmxgit_web/controllers/admin/user_controller.ex
index b527253..8890dfa 100644
--- a/lib/kmxgit_web/controllers/admin/user_controller.ex
+++ b/lib/kmxgit_web/controllers/admin/user_controller.ex
@@ -1,6 +1,7 @@
 defmodule KmxgitWeb.Admin.UserController do
   use KmxgitWeb, :controller
 
+  alias Kmxgit.IndexParams
   alias Kmxgit.GitManager
   alias Kmxgit.RepositoryManager
   alias Kmxgit.UserManager
@@ -8,11 +9,12 @@ defmodule KmxgitWeb.Admin.UserController do
   alias KmxgitWeb.ErrorView
 
   def index(conn, params) do
-    sort = KmxgitWeb.Admin.sort_param(params["sort"])
-    users = UserManager.list_users(sort)
+    index_params = %IndexParams{}
+    |> KmxgitWeb.Admin.sort_param(params["sort"])
+    users = UserManager.list_users(index_params)
     conn
+    |> assign(:index, index_params)
     |> assign(:page_title, gettext("Users"))
-    |> assign(:sort, sort)
     |> assign(:users, users)
     |> render("index.html")
   end
diff --git a/lib/kmxgit_web/templates/admin/user/index.html.heex b/lib/kmxgit_web/templates/admin/user/index.html.heex
index eba0a71..a0fff53 100644
--- a/lib/kmxgit_web/templates/admin/user/index.html.heex
+++ b/lib/kmxgit_web/templates/admin/user/index.html.heex
@@ -11,14 +11,14 @@
   <table class="table admin-index">
     <thead>
       <tr>
-        <th><%= link gettext("Id"), to: Routes.admin_user_path(@conn, :index, sort: "id#{if @sort.column == "id" && !@sort.reverse, do: "-"}") %><%= if @sort.column == "id" do %><%= if @sort.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
-        <th><%= link gettext("Name"), to: Routes.admin_user_path(@conn, :index, sort: "name#{if @sort.column == "name" && !@sort.reverse, do: "-"}") %><%= if @sort.column == "name" do %><%= if @sort.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
-        <th><%= link gettext("Email"), to: Routes.admin_user_path(@conn, :index, sort: "email#{if @sort.column == "email" && !@sort.reverse, do: "-"}") %><%= if @sort.column == "email" do %><%= if @sort.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
-        <th><%= link gettext("Login"), to: Routes.admin_user_path(@conn, :index, sort: "login#{if @sort.column == "login" && !@sort.reverse, do: "-"}") %><%= if @sort.column == "login" do %><%= if @sort.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
-        <th><%= link gettext("Disk usage"), to: Routes.admin_user_path(@conn, :index, sort: "du#{if @sort.column != "du" || (@sort.column == "du" && !@sort.reverse), do: "-"}") %><%= if @sort.column == "du" do %><%= if @sort.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
-        <th><%= link gettext("2FA"), to: Routes.admin_user_path(@conn, :index, sort: "mfa#{if @sort.column == "mfa" && !@sort.reverse, do: "-"}") %><%= if @sort.column == "mfa" do %><%= if @sort.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
-        <th><%= link gettext("Admin"), to: Routes.admin_user_path(@conn, :index, sort: "admin#{if @sort.column == "admin" && !@sort.reverse, do: "-"}") %><%= if @sort.column == "admin" do %><%= if @sort.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
-        <th><%= link gettext("Deploy"), to: Routes.admin_user_path(@conn, :index, sort: "deploy#{if @sort.column == "deploy" && !@sort.reverse, do: "-"}") %><%= if @sort.column == "deploy" do %><%= if @sort.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
+        <th><%= link gettext("Id"), to: Routes.admin_user_path(@conn, :index, sort: "id#{if @index.column == "id" && !@index.reverse, do: "-"}") %><%= if @index.column == "id" do %><%= if @index.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
+        <th><%= link gettext("Name"), to: Routes.admin_user_path(@conn, :index, sort: "name#{if @index.column == "name" && !@index.reverse, do: "-"}") %><%= if @index.column == "name" do %><%= if @index.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
+        <th><%= link gettext("Email"), to: Routes.admin_user_path(@conn, :index, sort: "email#{if @index.column == "email" && !@index.reverse, do: "-"}") %><%= if @index.column == "email" do %><%= if @index.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
+        <th><%= link gettext("Login"), to: Routes.admin_user_path(@conn, :index, sort: "login#{if @index.column == "login" && !@index.reverse, do: "-"}") %><%= if @index.column == "login" do %><%= if @index.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
+        <th><%= link gettext("Disk usage"), to: Routes.admin_user_path(@conn, :index, sort: "du#{if @index.column != "du" || (@index.column == "du" && !@index.reverse), do: "-"}") %><%= if @index.column == "du" do %><%= if @index.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
+        <th><%= link gettext("2FA"), to: Routes.admin_user_path(@conn, :index, sort: "mfa#{if @index.column == "mfa" && !@index.reverse, do: "-"}") %><%= if @index.column == "mfa" do %><%= if @index.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
+        <th><%= link gettext("Admin"), to: Routes.admin_user_path(@conn, :index, sort: "admin#{if @index.column == "admin" && !@index.reverse, do: "-"}") %><%= if @index.column == "admin" do %><%= if @index.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
+        <th><%= link gettext("Deploy"), to: Routes.admin_user_path(@conn, :index, sort: "deploy#{if @index.column == "deploy" && !@index.reverse, do: "-"}") %><%= if @index.column == "deploy" do %><%= if @index.reverse do %> <i class="fa fa-angle-down"></i><% else %> <i class="fa fa-angle-up"></i><% end %><% end %></th>
         <th><%= gettext "Actions" %></th>
       </tr>
     </thead>