diff --git a/.rubocop.yml b/.rubocop.yml index 6d67d96..2fee2c4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -11,3 +11,15 @@ inherit_gem: { rubocop-rails-omakase: rubocop.yml } # (Omakase defaults this to `indented_internal_methods`.) Layout/IndentationConsistency: EnforcedStyle: normal + +# Define nested classes/modules with a compact `class Foo::Bar` declaration. +# Rails-generated boot/framework boilerplate is excluded: it relies on the nested +# form to define its namespace (e.g. config/application.rb would fail to boot as +# `class CommunityFoundations::Application` since the module is defined nowhere else). +Style/ClassAndModuleChildren: + Enabled: true + EnforcedStyle: compact + Exclude: + - config/application.rb + - app/channels/application_cable/connection.rb + - test/test_helper.rb diff --git a/app/controllers/organization_memberships_controller.rb b/app/controllers/organization_memberships_controller.rb new file mode 100644 index 0000000..d4fa8d6 --- /dev/null +++ b/app/controllers/organization_memberships_controller.rb @@ -0,0 +1,25 @@ +class OrganizationMembershipsController < ApplicationController + before_action :require_member_management + before_action :set_membership, only: :update + + def index + @memberships = Current.organization.organization_memberships + .includes(:user).order(:created_at) + end + + def update + OrganizationMembership::RoleUpdater.new(@membership, actor: Current.user, role: params[:role]).call + redirect_to organization_memberships_path + end + + private + + def set_membership + @membership = Current.organization.organization_memberships.find(params[:id]) + end + + def require_member_management + redirect_to root_path, alert: "You don't have access to that." unless + Current.user.admin_of?(Current.organization) + end +end diff --git a/app/models/organization_membership.rb b/app/models/organization_membership.rb index 1b5e2e9..5c641b3 100644 --- a/app/models/organization_membership.rb +++ b/app/models/organization_membership.rb @@ -2,5 +2,8 @@ class OrganizationMembership < ApplicationRecord belongs_to :user belongs_to :organization + enum :role, { member: "member", admin: "admin", owner: "owner" }, default: :member + validates :user_id, uniqueness: { scope: :organization_id } + validates :role, presence: true end diff --git a/app/models/organization_membership/role_updater.rb b/app/models/organization_membership/role_updater.rb new file mode 100644 index 0000000..ee8b72c --- /dev/null +++ b/app/models/organization_membership/role_updater.rb @@ -0,0 +1,34 @@ +class OrganizationMembership::RoleUpdater + ASSIGNABLE_ROLES = %w[ admin member ].freeze + + Result = Data.define(:status) do + def updated? = status == :updated + def forbidden? = status == :forbidden + def rejected? = status == :rejected + def success? = updated? + end + + def initialize(membership, actor:, role:) + @membership = membership + @actor = actor + @role = role + end + + def call + return Result.new(:rejected) if @membership.owner? || ASSIGNABLE_ROLES.exclude?(@role) + return Result.new(:forbidden) if demotion? && !actor_owner? + + @membership.update(role: @role) + Result.new(:updated) + end + + private + + def demotion? + @role == "member" + end + + def actor_owner? + @actor.owner_of?(@membership.organization) + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 9d1aba4..9bd8518 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -28,6 +28,19 @@ def member_of?(organization) organization && organizations.exists?(organization.id) end + def membership_in(organization) + organization && organization_memberships.find_by(organization_id: organization.id) + end + + def admin_of?(organization) + membership = membership_in(organization) + membership&.admin? || membership&.owner? + end + + def owner_of?(organization) + membership_in(organization)&.owner? + end + def password_set? password_digest.present? end diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 8f2d5ba..344bafb 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -38,6 +38,9 @@ <%= link_to "Mailer previews", "/rails/mailers", class: "text-ink-soft hover:text-ink" %> <% end %> <% if authenticated? %> + <% if Current.user&.admin_of?(Current.organization) %> + <%= link_to "Members", organization_memberships_path, class: "text-ink-soft hover:text-ink" %> + <% end %> <%= link_to "Settings", users_password_path, class: "text-ink-soft hover:text-ink" %> <%= button_to "Sign out", session_path, method: :delete, class: "text-ink-soft hover:text-ink cursor-pointer" %> <% end %> diff --git a/app/views/organization_memberships/index.html.erb b/app/views/organization_memberships/index.html.erb new file mode 100644 index 0000000..4ff9468 --- /dev/null +++ b/app/views/organization_memberships/index.html.erb @@ -0,0 +1,38 @@ +<% content_for :title, "Members" %> + +
+
+

Members

+

+ Manage who belongs to <%= Current.organization.name %>. Admins and owners can promote members + to admins; only owners can demote admins. +

+
+ + <% owner = Current.user.owner_of?(Current.organization) %> + +
+ +
+
diff --git a/config/routes.rb b/config/routes.rb index 6333818..c3ad40d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,6 +13,8 @@ # Dev-only convenience: sign in as the first user without a password get "auto_sign_in", to: "auto_sign_in#create" if Rails.env.development? + resources :organization_memberships, only: %i[ index update ] + resources :scenarios do resource :name, only: %i[ show edit update ], module: :scenarios resources :allocations, only: %i[ create update destroy ] diff --git a/db/migrate/20260615203852_add_role_to_organization_memberships.rb b/db/migrate/20260615203852_add_role_to_organization_memberships.rb new file mode 100644 index 0000000..5a8bcf1 --- /dev/null +++ b/db/migrate/20260615203852_add_role_to_organization_memberships.rb @@ -0,0 +1,5 @@ +class AddRoleToOrganizationMemberships < ActiveRecord::Migration[8.1] + def change + add_column :organization_memberships, :role, :string, null: false, default: "member" + end +end diff --git a/db/schema.rb b/db/schema.rb index 8e4f44a..c230fde 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_06_15_120000) do +ActiveRecord::Schema[8.1].define(version: 2026_06_15_203852) do create_table "allocations", force: :cascade do |t| t.integer "scenario_id", null: false t.string "type", null: false @@ -28,6 +28,7 @@ t.integer "organization_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.string "role", default: "member", null: false t.index ["organization_id"], name: "index_organization_memberships_on_organization_id" t.index ["user_id", "organization_id"], name: "index_organization_memberships_on_user_id_and_organization_id", unique: true t.index ["user_id"], name: "index_organization_memberships_on_user_id" diff --git a/db/seeds.rb b/db/seeds.rb index 926be89..2e64516 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -8,19 +8,25 @@ # MovieGenre.find_or_create_by!(name: genre_name) # end -user = User.find_or_initialize_by(email_address: "user@example.com") -user.password = "password" -user.confirmed_at = Time.current -user.save! - arlington = Organization.find_or_create_by!(subdomain: "arlington") do |org| org.name = "Arlington Community Foundation" org.website = "https://www.arlcf.org/" end -OrganizationMembership.find_or_create_by!(user: user, organization: arlington) +# One user per role, all members of arlington. +%i[ owner admin member ].each do |role| + user = User.find_or_initialize_by(email_address: "#{role}@example.com") + user.password = "password" + user.confirmed_at = Time.current + user.save! + + membership = OrganizationMembership.find_or_create_by!(user: user, organization: arlington) + membership.update!(role: role) +end + +owner = User.find_by!(email_address: "owner@example.com") -balanced = user.scenarios.find_or_create_by!(organization: arlington, name: "Balanced giving") do |scenario| +balanced = owner.scenarios.find_or_create_by!(organization: arlington, name: "Balanced giving") do |scenario| scenario.total_giving_amount = 10_000 end @@ -31,7 +37,7 @@ balanced.one_time_allocations.create!(option: "Specific org", amount: 1_000) end -education = user.scenarios.find_or_create_by!(organization: arlington, name: "Education focus") do |scenario| +education = owner.scenarios.find_or_create_by!(organization: arlington, name: "Education focus") do |scenario| scenario.total_giving_amount = 5_000 end diff --git a/test/controllers/organization_memberships_controller_test.rb b/test/controllers/organization_memberships_controller_test.rb new file mode 100644 index 0000000..0d13442 --- /dev/null +++ b/test/controllers/organization_memberships_controller_test.rb @@ -0,0 +1,79 @@ +require "test_helper" + +class OrganizationMembershipsControllerTest < ActionDispatch::IntegrationTest + setup do + host! "arlington.localhost" + @owner = users(:one) + @admin = users(:admin) + @member = users(:passwordless) + @owner_membership = organization_memberships(:one_arlington) + @admin_membership = organization_memberships(:admin_arlington) + @member_membership = organization_memberships(:passwordless_arlington) + end + + # index + + test "owners and admins can view the members page" do + sign_in_as(@owner) + get organization_memberships_path + assert_response :success + + sign_in_as(@admin) + get organization_memberships_path + assert_response :success + end + + test "plain members are redirected away from the members page" do + sign_in_as(@member) + get organization_memberships_path + assert_redirected_to root_path + end + + # update — promote + + test "an admin can promote a member to admin" do + sign_in_as(@admin) + patch organization_membership_path(@member_membership), params: { role: "admin" } + assert_redirected_to organization_memberships_path + assert @member_membership.reload.admin? + end + + test "a member cannot promote anyone" do + sign_in_as(@member) + patch organization_membership_path(@admin_membership), params: { role: "admin" } + assert_redirected_to root_path + assert @admin_membership.reload.admin? + end + + # update — demote + + test "an owner can demote an admin to member" do + sign_in_as(@owner) + patch organization_membership_path(@admin_membership), params: { role: "member" } + assert_redirected_to organization_memberships_path + assert @admin_membership.reload.member? + end + + test "an admin cannot demote another admin" do + sign_in_as(@admin) + patch organization_membership_path(@admin_membership), params: { role: "member" } + assert_redirected_to organization_memberships_path + assert @admin_membership.reload.admin? + end + + # update — owner rows and roles are protected + + test "owners cannot be changed via update" do + sign_in_as(@owner) + patch organization_membership_path(@owner_membership), params: { role: "member" } + assert_redirected_to organization_memberships_path + assert @owner_membership.reload.owner? + end + + test "owner is not an assignable role" do + sign_in_as(@owner) + patch organization_membership_path(@member_membership), params: { role: "owner" } + assert_redirected_to organization_memberships_path + assert @member_membership.reload.member? + end +end diff --git a/test/fixtures/organization_memberships.yml b/test/fixtures/organization_memberships.yml index b9bac45..706b8c1 100644 --- a/test/fixtures/organization_memberships.yml +++ b/test/fixtures/organization_memberships.yml @@ -1,11 +1,19 @@ one_arlington: user: one organization: arlington + role: owner + +admin_arlington: + user: admin + organization: arlington + role: admin two_boston: user: two organization: boston + role: owner passwordless_arlington: user: passwordless organization: arlington + role: member diff --git a/test/fixtures/users.yml b/test/fixtures/users.yml index 217441e..8d556d3 100644 --- a/test/fixtures/users.yml +++ b/test/fixtures/users.yml @@ -15,6 +15,11 @@ unconfirmed: password_digest: <%= password_digest %> confirmed_at: <%= nil %> +admin: + email_address: admin@example.com + password_digest: <%= password_digest %> + confirmed_at: <%= Time.current.to_fs(:db) %> + # Passwordless (magic-link) member of arlington — no password_digest. passwordless: email_address: passwordless@example.com diff --git a/test/models/organization_membership/role_updater_test.rb b/test/models/organization_membership/role_updater_test.rb new file mode 100644 index 0000000..b14b65f --- /dev/null +++ b/test/models/organization_membership/role_updater_test.rb @@ -0,0 +1,49 @@ +require "test_helper" + +class OrganizationMembership::RoleUpdaterTest < ActiveSupport::TestCase + setup do + @owner = users(:one) + @admin = users(:admin) + @member_membership = organization_memberships(:passwordless_arlington) + @admin_membership = organization_memberships(:admin_arlington) + @owner_membership = organization_memberships(:one_arlington) + end + + test "an admin can promote a member to admin" do + result = OrganizationMembership::RoleUpdater.new(@member_membership, actor: @admin, role: "admin").call + assert result.updated? + assert result.success? + assert @member_membership.reload.admin? + end + + test "an owner can demote an admin to member" do + result = OrganizationMembership::RoleUpdater.new(@admin_membership, actor: @owner, role: "member").call + assert result.updated? + assert @admin_membership.reload.member? + end + + test "an admin cannot demote and the role is unchanged" do + result = OrganizationMembership::RoleUpdater.new(@admin_membership, actor: @admin, role: "member").call + assert result.forbidden? + assert_not result.success? + assert @admin_membership.reload.admin? + end + + test "owner rows are rejected" do + result = OrganizationMembership::RoleUpdater.new(@owner_membership, actor: @owner, role: "member").call + assert result.rejected? + assert @owner_membership.reload.owner? + end + + test "owner is not an assignable role" do + result = OrganizationMembership::RoleUpdater.new(@member_membership, actor: @owner, role: "owner").call + assert result.rejected? + assert @member_membership.reload.member? + end + + test "unknown roles are rejected" do + result = OrganizationMembership::RoleUpdater.new(@member_membership, actor: @owner, role: "wizard").call + assert result.rejected? + assert @member_membership.reload.member? + end +end diff --git a/test/models/organization_membership_test.rb b/test/models/organization_membership_test.rb index 27d168e..7d4b0eb 100644 --- a/test/models/organization_membership_test.rb +++ b/test/models/organization_membership_test.rb @@ -18,4 +18,23 @@ class OrganizationMembershipTest < ActiveSupport::TestCase assert_not duplicate.valid? assert_includes duplicate.errors[:user_id], "has already been taken" end + + test "defaults to the member role" do + membership = OrganizationMembership.new(user: users(:one), organization: organizations(:boston)) + assert membership.member? + assert_equal "member", membership.role + end + + test "exposes role predicates" do + assert organization_memberships(:one_arlington).owner? + assert organization_memberships(:admin_arlington).admin? + assert organization_memberships(:passwordless_arlington).member? + end + + test "requires a role" do + membership = OrganizationMembership.new(user: users(:one), organization: organizations(:boston)) + membership.role = nil + assert_not membership.valid? + assert_includes membership.errors[:role], "can't be blank" + end end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index f879a9b..3c4a3e8 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -16,6 +16,23 @@ class UserTest < ActiveSupport::TestCase assert_not users(:one).member_of?(nil) end + test "admin_of? is true for admins and owners only" do + arlington = organizations(:arlington) + assert users(:one).admin_of?(arlington) # owner + assert users(:admin).admin_of?(arlington) # admin + assert_not users(:passwordless).admin_of?(arlington) # member + assert_not users(:two).admin_of?(arlington) # non-member + assert_not users(:one).admin_of?(nil) + end + + test "owner_of? is true for owners only" do + arlington = organizations(:arlington) + assert users(:one).owner_of?(arlington) # owner + assert_not users(:admin).owner_of?(arlington) # admin + assert_not users(:passwordless).owner_of?(arlington) # member + assert_not users(:two).owner_of?(arlington) # non-member + end + test "is valid without a password" do user = User.new(email_address: "passwordless@example.org") assert user.valid?