Merge pull request #1761 from basecamp/flavorjones/save-the-owner

Introduce an "owner" role, and prevent it from being administered

AGENTS.md

@@ -68,7 +68,7 @@ Fizzy uses **URL path-based multi-tenancy**:
 
 **Passwordless magic link authentication**:
 - Global `Identity` (email-based) can have `Users` in multiple Accounts
-- Users belong to an Account and have roles: admin, member, system
+- Users belong to an Account and have roles: owner, admin, member, system
 - Sessions managed via signed cookies
 - Board-level access control via `Access` records
 
@@ -84,7 +84,7 @@ Fizzy uses **URL path-based multi-tenancy**:
 
 **User** → Account membership
 - Belongs to Account and Identity
-- Has role (admin/member/system)
+- Has role (owner/admin/member/system)
 - Board access via explicit `Access` records
 
 **Board** → Primary organizational unit

app/helpers/users_helper.rb

@@ -0,0 +1,8 @@
+module UsersHelper
+  def role_display_name(user)
+    case user.role
+    when "admin" then "Administrator"
+    else user.role.titleize
+    end
+  end
+end

app/models/account.rb

@@ -17,10 +17,10 @@ class Account < ApplicationRecord
   validates :name, presence: true
 
   class << self
-    def create_with_admin_user(account:, owner:)
+    def create_with_owner(account:, owner:)
       create!(**account).tap do |account|
         account.users.create!(role: :system, name: "System")
-        account.users.create!(**owner.reverse_merge(role: "admin"))
+        account.users.create!(**owner.reverse_merge(role: "owner"))
       end
     end
   end

app/models/account/seedeable.rb

@@ -2,6 +2,6 @@ module Account::Seedeable
   extend ActiveSupport::Concern
 
   def setup_customer_template
-    Account::Seeder.new(self, users.active.where(role: :admin).first).seed
+    Account::Seeder.new(self, users.admin.first).seed
   end
 end

app/models/signup.rb

@@ -54,7 +54,7 @@ class Signup
     end
 
     def create_account
-      @account = Account.create_with_admin_user(
+      @account = Account.create_with_owner(
         account: {
           external_account_id: @tenant,
           name: generate_account_name
@@ -64,7 +64,7 @@ class Signup
           identity: identity
         }
       )
-      @user = @account.users.find_by!(role: :admin)
+      @user = @account.users.find_by!(role: :owner)
       @account.setup_customer_template
     end
 

app/models/user/role.rb

@@ -2,18 +2,24 @@ module User::Role
   extend ActiveSupport::Concern
 
   included do
-    enum :role, %i[ admin member system ].index_by(&:itself), scopes: false
+    enum :role, %i[ owner admin member system ].index_by(&:itself), scopes: false
 
-    scope :member, -> { where(role: :member) }
-    scope :active, -> { where(active: true, role: %i[ admin member ]) }
+    scope :owner, -> { where(active: true, role: :owner) }
+    scope :admin, -> { where(active: true, role: %i[ owner admin ]) }
+    scope :member, -> { where(active: true, role: :member) }
+    scope :active, -> { where(active: true, role: %i[ owner admin member ]) }
+
+    def admin?
+      super || owner?
+    end
   end
 
   def can_change?(other)
-    admin? || other == self
+    (admin? && !other.owner?) || other == self
   end
 
   def can_administer?(other)
-    admin? && other != self
+    admin? && !other.owner? && other != self
   end
 
   def can_administer_board?(board)

app/views/account/settings/_user.html.erb

@@ -10,11 +10,13 @@
   <hr class="separator--horizontal flex-item-grow" style="--border-color: var(--color-ink-medium); --border-style: dashed" aria-hidden="true">
 
   <%= form_with model: user, url: user_role_path(user), data: { controller: "form" }, method: :patch do | form | %>
-    <label class="btn btn--circle" for="<%= dom_id(user, :role) %>" arial-label="Role: <%= user.admin? ? "Administrator" : "Member" %>">
-      <%= icon_tag "crown" %>
-      <span class="for-screen-reader">Role: <%= user.admin? ? "Administrator" : "Member" %></span>
-      <%= form.check_box :role, { data: { action: "form#submit" }, disabled: !Current.user.can_administer?(user), hidden: true, id: dom_id(user, :role) }, "admin", "member" %>
-    </label>
+    <span title="<%= role_display_name(user) %>">
+      <label class="btn btn--circle" for="<%= dom_id(user, :role) %>" aria-label="Role: <%= role_display_name(user) %>">
+        <%= icon_tag "crown" %>
+        <span class="for-screen-reader">Role: <%= role_display_name(user) %></span>
+        <%= form.check_box :role, { data: { action: "form#submit" }, disabled: !Current.user.can_administer?(user), checked: user.admin?, hidden: true, id: dom_id(user, :role) }, "admin", "member" %>
+      </label>
+    </span>
   <% end %>
 
   <%# FIXME: Move this Current.user check to a stimulus controller that just checks for admin? or the like we so we can cache user list %>

app/views/admin/stats/show.html.erb

@@ -70,7 +70,7 @@
 
     <ul class="margin-block-half">
       <% @top_accounts.each do |account| %>
-        <% admin_user = account.users.find { |u| u.role == "admin" } %>
+        <% admin_user = account.users.owner.first %>
         <li class="flex align-start gap-half margin-block-start-half">
           <div
             class="flex-item-grow min-width overflow-ellipsis"

db/migrate/20251129175717_promote_first_admin_to_owner.rb

@@ -0,0 +1,14 @@
+class PromoteFirstAdminToOwner < ActiveRecord::Migration[8.2]
+  def up
+    Account.find_each do |account|
+      next if account.users.exists?(role: :owner)
+
+      first_admin = account.users.where(role: :admin).order(:created_at).first
+      first_admin&.update!(role: :owner)
+    end
+  end
+
+  def down
+    User.where(role: :owner).update_all(role: :admin)
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.2].define(version: 2025_11_29_110120) do
+ActiveRecord::Schema[8.2].define(version: 2025_11_29_175717) do
   create_table "accesses", id: :uuid, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
     t.datetime "accessed_at"
     t.uuid "account_id", null: false

db/schema_sqlite.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.2].define(version: 2025_11_29_110120) do
+ActiveRecord::Schema[8.2].define(version: 2025_11_29_175717) do
   create_table "accesses", id: :uuid, force: :cascade do |t|
     t.datetime "accessed_at"
     t.uuid "account_id", null: false

db/seeds.rb

@@ -17,7 +17,7 @@ else
     identity = Identity.find_or_create_by!(email_address: email_address)
 
     unless account = Account.find_by(external_account_id: tenant_id)
-      account = Account.create_with_admin_user(
+      account = Account.create_with_owner(
         account: {
           external_account_id: tenant_id,
           name: signal_account_name

script/import-sqlite-database.rb

@@ -122,7 +122,7 @@ class Import
 
     def setup_account
       step("Setting up account", "Account set up in %{duration}") do
-        oldest_admin = import.users.order(id: :asc).where(role: :admin, active: true).first
+        oldest_admin = import.users.order(id: :asc).admin.first
         raise "No admin user found in the database" unless oldest_admin
 
         membership = untenanted.memberships.find(oldest_admin.membership_id)
@@ -133,7 +133,7 @@ class Import
         if Account.all.exists?(external_account_id: account.external_account_id)
           raise AccountExistsError, "Account already exists"
         else
-          @account = Account.create_with_admin_user(
+          @account = Account.create_with_owner(
             account: {
               external_account_id: account.external_account_id,
               name: account.name.truncate(255, omission: "")
@@ -144,7 +144,7 @@ class Import
             }
           )
           @tenant = @account.external_account_id
-          @admin = @account.users.find_by(role: :admin)
+          @admin = @account.users.find_by(role: :owner)
         end
 
         old_join_code = import.account_join_codes.sole

script/load-prod-db-in-dev.rb

@@ -37,11 +37,11 @@ tenant = ActiveRecord::FixtureSet.identify(identifier)
 ApplicationRecord.with_tenant(tenant) do |tenant|
   Current.account.destroy!
 
-  Account.create_with_admin_user \
+  Account.create_with_owner \
     account: { name: "Company #{identifier}" },
     owner: { name: "Developer #{identifier}", email_address: "dev-#{identifier}@example.com" }
 
-  user = User.find_by(role: :admin)
+  user = User.find_by(role: :owner)
   identity = Identity.find_or_create_by(email_address: user.email_address)
   identity.link_to(user.tenant)
   Board.find_each do |board|

test/controllers/users/roles_controller_test.rb

@@ -18,5 +18,29 @@ class Users::RolesControllerTest < ActionDispatch::IntegrationTest
     assert_no_changes -> { users(:david).reload.role } do
       put user_role_path(users(:david)), params: { user: { role: "system" } }
     end
+
+    assert_no_changes -> { users(:david).reload.role } do
+      put user_role_path(users(:david)), params: { user: { role: "owner" } }
+    end
+  end
+
+  test "admin cannot demote the owner" do
+    assert users(:jason).owner?
+
+    assert_no_changes -> { users(:jason).reload.role } do
+      put user_role_path(users(:jason)), params: { user: { role: "admin" } }
+    end
+
+    assert_response :forbidden
+  end
+
+  test "admin cannot change owner role to member" do
+    assert users(:jason).owner?
+
+    assert_no_changes -> { users(:jason).reload.role } do
+      put user_role_path(users(:jason)), params: { user: { role: "member" } }
+    end
+
+    assert_response :forbidden
   end
 end

test/controllers/users_controller_test.rb

@@ -41,6 +41,20 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     assert_nil User.active.find_by(id: users(:david).id)
   end
 
+  test "admin cannot deactivate the owner" do
+    sign_in_as :kevin
+
+    assert users(:jason).owner?
+    assert users(:jason).active
+
+    assert_no_difference -> { User.active.count } do
+      delete user_path(users(:jason))
+    end
+
+    assert_response :forbidden
+    assert users(:jason).reload.active
+  end
+
   test "non-admins cannot perform actions" do
     sign_in_as :jz
 

test/fixtures/identities.yml

@@ -5,6 +5,10 @@ david:
 jz:
   email_address: jz@37signals.com
 
+jason:
+  email_address: jason@37signals.com
+  staff: true
+
 kevin:
   email_address: kevin@37signals.com
   staff: true

test/fixtures/users.yml

@@ -12,6 +12,13 @@ jz:
   identity: jz
   account: 37s_uuid
 
+jason:
+  id: <%= ActiveRecord::FixtureSet.identify("jason", :uuid) %>
+  name: Jason
+  role: owner
+  identity: jason
+  account: 37s_uuid
+
 kevin:
   id: <%= ActiveRecord::FixtureSet.identify("kevin", :uuid) %>
   name: Kevin

test/models/account_test.rb

@@ -12,17 +12,17 @@ class AccountTest < ActiveSupport::TestCase
     assert_equal "/#{account.external_account_id}", account.slug
   end
 
-  test ".create_with_admin_user creates a new local account" do
+  test ".create_with_owner creates a new local account" do
     Current.without_account do
       identity = identities(:david)
       account = nil
 
       assert_changes -> { Account.count }, +1 do
         assert_changes -> { User.count }, +2 do
-          account = Account.create_with_admin_user(
+          account = Account.create_with_owner(
             account: {
-              external_account_id: ActiveRecord::FixtureSet.identify("account-create-with-admin-user-test"),
-              name: "Account Create With Admin"
+              external_account_id: ActiveRecord::FixtureSet.identify("account-create-with-owner-test"),
+              name: "Account Create With Owner"
             },
             owner: {
               name: "David",
@@ -34,13 +34,14 @@ class AccountTest < ActiveSupport::TestCase
 
       assert_not_nil account
       assert account.persisted?
-      assert_equal ActiveRecord::FixtureSet.identify("account-create-with-admin-user-test"), account.external_account_id
-      assert_equal "Account Create With Admin", account.name
+      assert_equal ActiveRecord::FixtureSet.identify("account-create-with-owner-test"), account.external_account_id
+      assert_equal "Account Create With Owner", account.name
 
-      admin = account.users.find_by(role: "admin")
-      assert_equal "David", admin.name
-      assert_equal "david@37signals.com", admin.identity.email_address
-      assert_equal "admin", admin.role
+      owner = account.users.find_by(role: "owner")
+      assert_equal "David", owner.name
+      assert_equal "david@37signals.com", owner.identity.email_address
+      assert_equal "owner", owner.role
+      assert owner.admin?, "owner should also be considered an admin"
 
       assert_predicate account.system_user, :present?
     end

test/models/user/role_test.rb

@@ -8,6 +8,56 @@ class User::RoleTest < ActiveSupport::TestCase
     assert_not users(:jz).can_administer?(users(:kevin))
   end
 
+  test "owner can administer admins and members" do
+    assert users(:jason).can_administer?(users(:kevin))
+    assert users(:jason).can_administer?(users(:david))
+    assert users(:jason).can_administer?(users(:jz))
+  end
+
+  test "owner cannot administer themselves" do
+    assert_not users(:jason).can_administer?(users(:jason))
+  end
+
+  test "admin cannot administer the owner" do
+    assert_not users(:kevin).can_administer?(users(:jason))
+  end
+
+  test "owner is included in active scope" do
+    active_users = User.active
+    assert_includes active_users, users(:jason)
+    assert_includes active_users, users(:kevin)
+    assert_includes active_users, users(:david)
+    assert_not_includes active_users, users(:system)
+  end
+
+  test "owner is also considered an admin" do
+    assert users(:jason).owner?
+    assert users(:jason).admin?
+
+    assert users(:kevin).admin?
+    assert_not users(:kevin).owner?
+  end
+
+  test "owner scope returns only active owners" do
+    owners = accounts("37s").users.owner
+    assert_includes owners, users(:jason)
+    assert_not_includes owners, users(:kevin)
+    assert_not_includes owners, users(:david)
+
+    users(:jason).update!(active: false)
+    assert_not_includes accounts("37s").users.owner, users(:jason)
+  end
+
+  test "admin scope returns active owners and admins" do
+    admins = accounts("37s").users.admin
+    assert_includes admins, users(:jason)
+    assert_includes admins, users(:kevin)
+    assert_not_includes admins, users(:david)
+
+    users(:kevin).update!(active: false)
+    assert_not_includes accounts("37s").users.admin, users(:kevin)
+  end
+
   test "can administer board?" do
     writebook_board = boards(:writebook)
     private_board = boards(:private)