From a7d42c9c93bff8f2ea59a2c1720b451486353f7c Mon Sep 17 00:00:00 2001
From: Stephen Nelson <stephen@katalyst.com.au>
Date: Thu, 21 Nov 2024 14:07:36 +1030
Subject: [PATCH] Use variants for admin users to change behaviour for self vs
 others

---
 .../admin/admin_users_controller.rb           |  2 ++
 .../admin/credentials_controller.rb           |  6 +++++-
 .../admin/admin_users/_fields.html+self.erb   |  3 +++
 app/views/admin/admin_users/_fields.html.erb  |  1 -
 .../admin/admin_users/show.html+self.erb      | 20 +++++++++++++++++++
 app/views/admin/admin_users/show.html.erb     | 20 ++++++++-----------
 .../admin/admin_users_controller_spec.rb      |  6 +++---
 7 files changed, 41 insertions(+), 17 deletions(-)
 create mode 100644 app/views/admin/admin_users/_fields.html+self.erb
 create mode 100644 app/views/admin/admin_users/show.html+self.erb

diff --git a/app/controllers/admin/admin_users_controller.rb b/app/controllers/admin/admin_users_controller.rb
index 63428c35d..5f038c0e4 100644
--- a/app/controllers/admin/admin_users_controller.rb
+++ b/app/controllers/admin/admin_users_controller.rb
@@ -72,6 +72,8 @@ def destroy
 
     def set_admin
       @admin = Admin::User.with_archived.find(params[:id])
+
+      request.variant << :self if @admin == current_admin_user
     end
 
     def admin_user_params
diff --git a/app/controllers/admin/credentials_controller.rb b/app/controllers/admin/credentials_controller.rb
index 7c8dc1ec8..9b4da2439 100644
--- a/app/controllers/admin/credentials_controller.rb
+++ b/app/controllers/admin/credentials_controller.rb
@@ -70,7 +70,11 @@ def credential_params
     def set_admin_user
       @admin_user = Admin::User.find(params[:admin_user_id])
 
-      head(:forbidden) unless current_admin == @admin_user
+      if current_admin == @admin_user
+        request.variant = :self
+      else
+        head(:forbidden)
+      end
     end
   end
 end
diff --git a/app/views/admin/admin_users/_fields.html+self.erb b/app/views/admin/admin_users/_fields.html+self.erb
new file mode 100644
index 000000000..55837792e
--- /dev/null
+++ b/app/views/admin/admin_users/_fields.html+self.erb
@@ -0,0 +1,3 @@
+<%= form.govuk_text_field :email %>
+<%= form.govuk_text_field :name %>
+<%= form.govuk_password_field :password, label: { text: "Password (optional)" } %>
diff --git a/app/views/admin/admin_users/_fields.html.erb b/app/views/admin/admin_users/_fields.html.erb
index 3dca79898..b3c44e24e 100644
--- a/app/views/admin/admin_users/_fields.html.erb
+++ b/app/views/admin/admin_users/_fields.html.erb
@@ -1,4 +1,3 @@
 <%= form.govuk_text_field :email %>
 <%= form.govuk_text_field :name %>
-<%= form.govuk_password_field :password, label: { text: "Password#{' (optional)' if form.object.persisted?}" } %>
 <%= form.govuk_check_box_field :archived if form.object.persisted? %>
diff --git a/app/views/admin/admin_users/show.html+self.erb b/app/views/admin/admin_users/show.html+self.erb
new file mode 100644
index 000000000..3a8b88b68
--- /dev/null
+++ b/app/views/admin/admin_users/show.html+self.erb
@@ -0,0 +1,20 @@
+<%# locals: (admin:) %>
+
+<% content_for :header do %>
+  <%= render Koi::Header::ShowComponent.new(resource: admin) %>
+<% end %>
+
+<%= render Koi::SummaryListComponent.new(model: admin, class: "item-table") do |builder| %>
+  <%= builder.text :name %>
+  <%= builder.text :email %>
+  <%= builder.date :created_at %>
+  <%= builder.date :last_sign_in_at, label: { text: "Last sign in" } %>
+<% end %>
+
+<h3>Passkeys</h3>
+
+<%= render "admin/credentials/credentials", admin: %>
+
+<div class="actions-group">
+  <%= kpop_link_to "Add this device", new_admin_admin_user_credential_path(admin), class: "button button--primary" %>
+</div>
diff --git a/app/views/admin/admin_users/show.html.erb b/app/views/admin/admin_users/show.html.erb
index 4913d1a49..8318c05f5 100644
--- a/app/views/admin/admin_users/show.html.erb
+++ b/app/views/admin/admin_users/show.html.erb
@@ -1,3 +1,5 @@
+<%# locals: (admin:) %>
+
 <% content_for :header do %>
   <%= render Koi::Header::ShowComponent.new(resource: admin) %>
 <% end %>
@@ -5,11 +7,15 @@
 <%= render Koi::SummaryListComponent.new(model: admin, class: "item-table") do |builder| %>
   <%= builder.text :name %>
   <%= builder.text :email %>
-  <%= builder.datetime :created_at %>
-  <%= builder.datetime :last_sign_in_at, label: { text: "Last sign in" } %>
+  <%= builder.date :created_at %>
+  <%= builder.date :last_sign_in_at, label: { text: "Last sign in" } %>
   <%= builder.boolean :archived? %>
 <% end %>
 
+<h3>Passkeys</h3>
+
+<%= render "admin/credentials/credentials", admin: %>
+
 <div class="actions">
   <% if admin.archived? %>
     <%= button_to "Delete", admin_admin_user_path(admin),
@@ -19,13 +25,3 @@
   <% end %>
   <%= button_to "Generate login link", admin_admin_user_tokens_path(admin), class: "button button--primary", form: { id: "invite" } %>
 </div>
-
-<h2>Authentication</h2>
-
-<%= render "admin/credentials/credentials", admin: %>
-
-<% if admin == current_admin %>
-  <div class="actions-group">
-    <%= kpop_link_to "Add this device", new_admin_admin_user_credential_path(admin), class: "button button--primary" %>
-  </div>
-<% end %>
diff --git a/spec/requests/admin/admin_users_controller_spec.rb b/spec/requests/admin/admin_users_controller_spec.rb
index 49d8117df..6f2e75dc5 100644
--- a/spec/requests/admin/admin_users_controller_spec.rb
+++ b/spec/requests/admin/admin_users_controller_spec.rb
@@ -42,7 +42,7 @@
 
   describe "POST /admin/admin_users" do
     let(:action) { post admin_admin_users_path, params: { admin: admin_params } }
-    let(:admin_params) { attributes_for(:admin) }
+    let(:admin_params) { attributes_for(:admin).except(:password) }
 
     it_behaves_like "requires admin"
 
@@ -94,14 +94,14 @@
     end
 
     it "updates password" do
-      expect { action }.not_to(change { admin.reload.password })
+      expect { action }.to(change { admin.reload.password_digest })
     end
 
     context "with empty password" do
       let(:admin_params) { { password: "" } }
 
       it "updates password" do
-        expect { action }.not_to(change { admin.reload.password })
+        expect { action }.not_to(change { admin.reload.password_digest })
       end
     end