Prevent sole admin from deleting their account

Without this guard, the last admin in the system could delete their own
account, making the application unmanageable. This adds a model method
`User#sole_admin?`, a controller guard in `RegistrationsController#destroy`,
and disables the delete button in the profile edit view when the current
user is the only remaining admin.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

app/controllers/user/profiles_controller.rb

@@ -1,6 +1,10 @@
 class User::ProfilesController < Devise::RegistrationsController
   def destroy
-    # TODO: Disallow/disable deletion for last admin account; update :edit view
+    if current_user.sole_admin?
+      redirect_back fallback_location: edit_user_registration_path,
+        alert: t(".sole_admin")
+      return
+    end
     super
   end
 

app/models/quantity.rb

@@ -16,7 +16,7 @@ class Quantity < ApplicationRecord
   end
   validates :name, presence: true, uniqueness: {scope: [:user_id, :parent_id]},
     length: {maximum: type_for_attribute(:name).limit}
-  validates :description, length: {maximum: type_for_attribute(:description).limit} if type_for_attribute(:description).limit
+  validates :description, length: {maximum: type_for_attribute(:description).limit}
 
   # Update :depths of progenies after parent change
   before_save if: :parent_changed? do
@@ -61,22 +61,18 @@ class Quantity < ApplicationRecord
 
   # Return: ordered [sub]hierarchy
   scope :ordered, ->(root: nil, include_root: true) {
-    q_ordered = Arel::Table.new('q_ordered')
-    cast_type = connection.adapter_name == 'Mysql2' ? 'BINARY' : 'BLOB'
+    numbered = Arel::Table.new('numbered')
 
-    self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(q_ordered: [
-      Quantity.from(Arel::Table.new('numbered').as(arel_table.name))
-        .select(
-          arel_table[Arel.star],
-          arel_table.cast(arel_table[:child_number], cast_type).as('path')
-        ).where(arel_table[root && include_root ? :id : :parent_id].eq(root)),
-      Quantity.from(Arel::Table.new('numbered').as(arel_table.name))
-        .joins("INNER JOIN q_ordered ON quantities.parent_id = q_ordered.id")
-        .select(
-          arel_table[Arel.star],
-          q_ordered[:path].concat(arel_table[:child_number])
-        )
-    ]).from(q_ordered.as(arel_table.name)).order(arel_table[:path])
+    self.model.with(numbered: numbered(:parent_id, :name)).with_recursive(arel_table.name => [
+      numbered.project(
+        numbered[Arel.star],
+        numbered.cast(numbered[:child_number], 'BINARY').as('path')
+      ).where(numbered[root && include_root ? :id : :parent_id].eq(root)),
+      numbered.project(
+        numbered[Arel.star],
+        arel_table[:path].concat(numbered[:child_number])
+      ).join(arel_table).on(numbered[:parent_id].eq(arel_table[:id]))
+    ]).order(arel_table[:path])
   }
 
   # TODO: extract named functions to custom Arel extension
@@ -84,24 +80,20 @@ class Quantity < ApplicationRecord
   # be merged with :ordered
   # https://gist.github.com/ProGM/c6df08da14708dcc28b5ca325df37ceb#extending-arel
   scope :numbered, ->(parent_column, order_column) {
-    row_number = Arel::Nodes::NamedFunction.new('ROW_NUMBER', [])
-      .over(Arel::Nodes::Window.new.partition(parent_column).order(order_column))
-    width = Arel::SelectManager.new.project(
-      Arel::Nodes::NamedFunction.new('LENGTH', [Arel.star.count])
-    ).from(arel_table)
-
-    pad_expr = if connection.adapter_name == 'Mysql2'
-      Arel::Nodes::NamedFunction.new('LPAD', [row_number, width, Arel::Nodes.build_quoted('0')])
-    else
-      # SQLite: printf('%0' || width || 'd', row_number)
-      fmt = Arel::Nodes::InfixOperation.new('||',
-        Arel::Nodes::InfixOperation.new('||', Arel::Nodes.build_quoted('%0'), width),
-        Arel::Nodes.build_quoted('d')
-      )
-      Arel::Nodes::NamedFunction.new('printf', [fmt, row_number])
-    end
-
-    select(arel_table[Arel.star], pad_expr.as('child_number'))
+    select(
+      arel_table[Arel.star],
+      Arel::Nodes::NamedFunction.new(
+        'LPAD',
+        [
+          Arel::Nodes::NamedFunction.new('ROW_NUMBER', [])
+            .over(Arel::Nodes::Window.new.partition(parent_column).order(order_column)),
+          Arel::SelectManager.new.project(
+            Arel::Nodes::NamedFunction.new('LENGTH', [Arel.star.count])
+          ).from(arel_table),
+          Arel::Nodes.build_quoted('0')
+        ],
+      ).as('child_number')
+    )
   }
 
   def to_s
@@ -110,7 +102,7 @@ class Quantity < ApplicationRecord
 
   def to_s_with_depth
     # em space, U+2003
-    ' ' * depth + name
+    ' ' * depth + name
   end
 
   def destroyable?
@@ -123,18 +115,16 @@ class Quantity < ApplicationRecord
 
   # Common ancestors, assuming node is a descendant of itself
   scope :common_ancestors, ->(of) {
-    q_common = Arel::Table.new('q_common')
+    selected = Arel::Table.new('selected')
 
     # Take unique IDs, so self can be called with parent nodes of collection to
     # get common ancestors of collection _excluding_ nodes in collection.
     uniq_of = of.uniq
-    model.with(selected: self).with_recursive(q_common: [
-      Quantity.from(Arel::Table.new('selected').as(arel_table.name))
-        .where(arel_table[:id].in(uniq_of)),
-      Quantity.from(Arel::Table.new('selected').as(arel_table.name))
-        .joins("INNER JOIN q_common ON selected.id = q_common.parent_id")
+    model.with(selected: self).with_recursive(arel_table.name => [
+      selected.project(selected[Arel.star]).where(selected[:id].in(uniq_of)),
+      selected.project(selected[Arel.star])
+        .join(arel_table).on(selected[:id].eq(arel_table[:parent_id]))
     ]).select(arel_table[Arel.star])
-      .from(q_common.as(arel_table.name))
       .group(column_names)
       .having(arel_table[:id].count.eq(uniq_of.size))
       .order(arel_table[:depth].desc)
@@ -142,9 +132,13 @@ class Quantity < ApplicationRecord
 
   # Return: successive record in order of appearance; used for partial view reload
   def successive
-    ordered = user.quantities.ordered.to_a
-    idx = ordered.index { |q| q.id == id }
-    ordered[idx + 1] if idx
+    quantities = Quantity.arel_table
+    Quantity.with(
+      quantities: user.quantities.ordered.select(
+        quantities[Arel.star],
+        Arel::Nodes::NamedFunction.new('LAG', [quantities[:id]]).over.as('lag_id')
+      )
+    ).where(quantities[:lag_id].eq(id)).first
   end
 
   def with_progenies
@@ -157,14 +151,13 @@ class Quantity < ApplicationRecord
 
   # Return: record with ID `of` with its ancestors, sorted by :depth
   scope :with_ancestors, ->(of) {
-    q_ancestors = Arel::Table.new('q_ancestors')
+    selected = Arel::Table.new('selected')
 
-    model.with(selected: self).with_recursive(q_ancestors: [
-      Quantity.from(Arel::Table.new('selected').as(arel_table.name))
-        .where(arel_table[:id].eq(of)),
-      Quantity.from(Arel::Table.new('selected').as(arel_table.name))
-        .joins("INNER JOIN q_ancestors ON selected.id = q_ancestors.parent_id")
-    ]).from(q_ancestors.as(arel_table.name))
+    model.with(selected: self).with_recursive(arel_table.name => [
+      selected.project(selected[Arel.star]).where(selected[:id].eq(of)),
+      selected.project(selected[Arel.star])
+        .join(arel_table).on(selected[:id].eq(arel_table[:parent_id]))
+    ])
   }
 
   # Return: ancestors of (possibly destroyed) self

app/models/unit.rb

@@ -13,7 +13,7 @@ class Unit < ApplicationRecord
   end
   validates :symbol, presence: true, uniqueness: {scope: :user_id},
     length: {maximum: type_for_attribute(:symbol).limit}
-  validates :description, length: {maximum: type_for_attribute(:description).limit} if type_for_attribute(:description).limit
+  validates :description, length: {maximum: type_for_attribute(:description).limit}
   validates :multiplier, numericality: {equal_to: 1}, unless: :base
   validates :multiplier, numericality: {greater_than: 0, precision: true, scale: true}, if: :base
 
@@ -22,72 +22,59 @@ class Unit < ApplicationRecord
     actionable_units = Arel::Table.new('actionable_units')
     units = actionable_units.alias('units')
     bases_units = arel_table.alias('bases_units')
-    # Use 'all_units' alias for correlated subqueries to reference the all_units CTE.
-    # Cannot use arel_table.alias here because the CTE is named 'all_units', not 'units'.
-    other_units = Arel::Table.new('all_units').alias('other_units')
-    other_bases_units = Arel::Table.new('all_units').alias('other_bases_units')
+    other_units = arel_table.alias('other_units')
+    other_bases_units = arel_table.alias('other_bases_units')
     sub_units = arel_table.alias('sub_units')
 
-    Unit.with_recursive(
-      # Renamed from 'units' to 'all_units' to avoid SQLite circular reference:
-      # SQLite treats any CTE in WITH RECURSIVE that references a table with the same
-      # name as the CTE itself as a circular reference, even for non-recursive CTEs.
-      all_units: self.or(Unit.defaults),
-      actionable_units: [
-        # Read from all_units CTE (user+defaults) aliased as the units table name.
-        # Using AR::Relation (not Arel::SelectManager) to avoid extra parentheses
-        # around the UNION part (visit_Arel_SelectManager always wraps in parens).
-        Unit.from(Arel::Table.new('all_units').as(arel_table.name))
-          .joins("LEFT OUTER JOIN all_units bases_units ON bases_units.id = units.base_id")
-          .where.not(
-            # Exclude Units that are/have default counterpart
-            Arel::SelectManager.new.project(1).from(other_units)
-              .outer_join(other_bases_units)
-              .on(other_units[:base_id].eq(other_bases_units[:id]))
-              .where(
-                other_bases_units[:symbol].is_not_distinct_from(bases_units[:symbol])
-                  .and(other_units[:symbol].eq(arel_table[:symbol]))
-                  .and(other_units[:user_id].is_distinct_from(arel_table[:user_id]))
-              ).exists
-          )
-          .select(
-            arel_table[Arel.star],
-            # Decide if Unit can be im-/exported based on existing hierarchy:
-            # * same base unit symbol has to exist
-            # * unit with subunits can only be ported to root
-            arel_table[:base_id].eq(nil).or(
-              (
-                Arel::SelectManager.new.project(1).from(other_units)
-                  .join(sub_units).on(other_units[:id].eq(sub_units[:base_id]))
-                  .where(
-                    other_units[:symbol].eq(arel_table[:symbol])
-                      .and(other_units[:user_id].is_distinct_from(arel_table[:user_id]))
-                  ).exists.not
-              ).and(
-                Arel::SelectManager.new.project(1).from(other_bases_units)
-                  .where(
-                    other_bases_units[:symbol].is_not_distinct_from(bases_units[:symbol])
-                      .and(other_bases_units[:user_id].is_distinct_from(bases_units[:user_id]))
-                  ).exists
-              )
-            ).as('portable')
-          ),
-        # Fill base Units to display proper hierarchy. Duplicates will be removed
-        # by final group() - can't be deduplicated with UNION due to 'portable' field.
-        # Using AR::Relation instead of Arel::SelectManager to avoid extra parentheses
-        # around the UNION part (visit_Arel_SelectManager always wraps in parens).
-        Unit.from(Arel::Table.new('all_units').as(arel_table.name))
-          .joins("INNER JOIN actionable_units ON actionable_units.base_id = units.id")
-          .select(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
-      ]
-    ).select(units: [:base_id, :symbol])
+    # TODO: move inner 'with' CTE to outer 'with recursive' - it can have multiple
+    # CTEs, even non recursive ones.
+    Unit.with_recursive(actionable_units: [
+      Unit.with(units: self.or(Unit.defaults)).left_joins(:base)
+        .where.not(
+          # Exclude Units that are/have default counterpart
+          Arel::SelectManager.new.project(1).from(other_units)
+            .outer_join(other_bases_units)
+            .on(other_units[:base_id].eq(other_bases_units[:id]))
+            .where(
+              other_bases_units[:symbol].is_not_distinct_from(bases_units[:symbol])
+                .and(other_units[:symbol].eq(arel_table[:symbol]))
+                .and(other_units[:user_id].is_distinct_from(arel_table[:user_id]))
+            ).exists
+        )
+        .select(
+          arel_table[Arel.star],
+          # Decide if Unit can be im-/exported based on existing hierarchy:
+          # * same base unit symbol has to exist
+          # * unit with subunits can only be ported to root
+          arel_table[:base_id].eq(nil).or(
+            (
+              Arel::SelectManager.new.project(1).from(other_units)
+                .join(sub_units).on(other_units[:id].eq(sub_units[:base_id]))
+                .where(
+                  other_units[:symbol].eq(arel_table[:symbol])
+                    .and(other_units[:user_id].is_distinct_from(arel_table[:user_id]))
+                ).exists.not
+            ).and(
+              Arel::SelectManager.new.project(1).from(other_bases_units)
+                .where(
+                  other_bases_units[:symbol].is_not_distinct_from(bases_units[:symbol])
+                    .and(other_bases_units[:user_id].is_distinct_from(bases_units[:user_id]))
+                ).exists
+            )
+          ).as('portable')
+        ),
+      # Fill base Units to display proper hierarchy. Duplicates will be removed
+      # by final group() - can't be deduplicated with UNION due to 'portable' field.
+      arel_table.join(actionable_units).on(actionable_units[:base_id].eq(arel_table[:id]))
+        .project(arel_table[Arel.star], Arel::Nodes.build_quoted(nil).as('portable'))
+    ]).select(units: [:base_id, :symbol])
       .select(
         units[:id].minimum.as('id'), # can be ANY_VALUE()
         units[:user_id].minimum.as('user_id'), # prefer non-default
         Arel::Nodes.build_quoted(1).as('multiplier'), # disregard multiplier when sorting
         units[:portable].minimum.as('portable')
       )
-      .from(units).group(units[:base_id], units[:symbol])
+      .from(units).group(:base_id, :symbol)
   }
   scope :ordered, ->{
     left_outer_joins(:base).order(ordering)
@@ -97,7 +84,7 @@ class Unit < ApplicationRecord
     [arel_table.coalesce(Arel::Table.new(:bases_units)[:symbol], arel_table[:symbol]),
      arel_table[:base_id].not_eq(nil),
      :multiplier,
-     arel_table[:symbol]]
+     :symbol]
   end
 
   before_destroy do

app/models/user.rb

@@ -29,4 +29,11 @@ class User < ApplicationRecord
   def at_least(status)
     User.statuses[self.status] >= User.statuses[status]
   end
+
+  # Returns true when this user is the only admin account in the system.
+  # Used to block actions that would leave the application without an admin
+  # (account deletion, status demotion).
+  def sole_admin?
+    admin? && !User.admin.where.not(id: id).exists?
+  end
 end

app/views/users/profiles/edit.html.erb

@@ -4,9 +4,8 @@
 <% end %>
 
 <div class="rightside-area buttongrid">
-  <%#= TODO: Disallow/disable deletion for last admin account, image_button_to_if %>
-  <%= image_button_to t('.delete'), 'account-remove-outline', user_registration_path,
-    form_class: 'tools-area', method: :delete, data: {turbo: false},
+  <%= image_button_to_if !current_user.sole_admin?, t('.delete'), 'account-remove-outline',
+    user_registration_path, form_class: 'tools-area', method: :delete, data: {turbo: false},
     onclick: {confirm: t('.confirm_delete')} %>
 </div>
 

config/locales/en.yml

@@ -162,6 +162,9 @@ en:
           New password:
           <br><em>leave blank to keep unchanged</em>
           %{password_length_hint_html}
+  registrations:
+    destroy:
+      sole_admin: You cannot delete the only admin account.
   actions: Actions
   add: Add
   apply: Apply

test/controllers/registrations_controller_test.rb

@@ -0,0 +1,18 @@
+require "test_helper"
+
+class RegistrationsControllerTest < ActionDispatch::IntegrationTest
+  test "sole admin cannot delete account" do
+    sign_in users(:admin)
+    delete user_registration_path
+    assert_redirected_to edit_user_registration_path
+    assert_equal t("registrations.destroy.sole_admin"), flash[:alert]
+    assert User.exists?(users(:admin).id)
+  end
+
+  test "non-admin can delete account" do
+    sign_in users(:alice)
+    assert_difference ->{ User.count }, -1 do
+      delete user_registration_path
+    end
+  end
+end

test/system/users_test.rb

@@ -182,8 +182,8 @@ class UsersTest < ApplicationSystemTestCase
     assert_title 'Access is forbidden to this page (403)'
   end
 
-  test 'delete profile' do
-    user = sign_in
+  test "delete profile" do
+    user = sign_in user: users.reject(&:admin?).select(&:confirmed?).sample
     # TODO: remove condition after root_url changed to different path than
     # profile in routes.rb
     unless has_current_path?(edit_user_registration_path)
@@ -196,7 +196,15 @@ class UsersTest < ApplicationSystemTestCase
     assert_text t("devise.registrations.destroyed")
   end
 
-  test 'index forbidden for non admin' do
+  test "sole admin cannot delete profile" do
+    sign_in user: users(:admin)
+    unless has_current_path?(edit_user_registration_path)
+      first(:link_or_button, users(:admin).email).click
+    end
+    assert find(:button, t("users.registrations.edit.delete"))[:disabled]
+  end
+
+  test "index forbidden for non admin" do
     sign_in user: users.reject(&:admin?).select(&:confirmed?).sample
     visit users_path
     assert_title "Access is forbidden to this page (403)"