diff --git a/app/controllers/organization_memberships_controller.rb b/app/controllers/admin/organization_memberships_controller.rb similarity index 68% rename from app/controllers/organization_memberships_controller.rb rename to app/controllers/admin/organization_memberships_controller.rb index efe7c78..6c4c2d3 100644 --- a/app/controllers/organization_memberships_controller.rb +++ b/app/controllers/admin/organization_memberships_controller.rb @@ -1,7 +1,6 @@ -class OrganizationMembershipsController < ApplicationController +class Admin::OrganizationMembershipsController < Admin::ApplicationController PER_PAGE = 50 - before_action :require_member_management before_action :set_membership, only: :update def index @@ -19,7 +18,7 @@ def index def update OrganizationMembership::RoleUpdater.new(@membership, actor: Current.user, role: params[:role]).call - redirect_to organization_memberships_path(q: params[:q], roles: params[:roles], page: params[:page]) + redirect_to admin_organization_memberships_path(q: params[:q], roles: params[:roles], page: params[:page]) end private @@ -27,9 +26,4 @@ def update 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/helpers/scenarios_helper.rb b/app/helpers/scenarios_helper.rb index e371015..b63146e 100644 --- a/app/helpers/scenarios_helper.rb +++ b/app/helpers/scenarios_helper.rb @@ -1,5 +1,5 @@ module ScenariosHelper - CHART_COLORS = ["#E07B39", "#D9632B", "#C99A2E", "#B8842B", "#E0954F", "#C76A28"].freeze + CHART_COLORS = [ "#E07B39", "#D9632B", "#C99A2E", "#B8842B", "#E0954F", "#C76A28" ].freeze def allocation_chart_color(index) CHART_COLORS[index % CHART_COLORS.length] diff --git a/app/views/admin/dashboards/show.html.erb b/app/views/admin/dashboards/show.html.erb index c8b636c..092ad67 100644 --- a/app/views/admin/dashboards/show.html.erb +++ b/app/views/admin/dashboards/show.html.erb @@ -20,5 +20,11 @@

Allocation categories

Manage the categories used in allocations.

<% end %> + + <%= link_to admin_organization_memberships_path, + class: "block rounded-lg border border-line bg-surface p-5 hover:bg-surface-soft transition" do %> +

Members

+

View and manage who belongs to the organization.

+ <% end %> diff --git a/app/views/admin/organization_memberships/index.html.erb b/app/views/admin/organization_memberships/index.html.erb new file mode 100644 index 0000000..2b3efa1 --- /dev/null +++ b/app/views/admin/organization_memberships/index.html.erb @@ -0,0 +1,92 @@ +<% content_for :title, "Members" %> + +
+
+
+

Admin

+

Members

+ <% if owner? %> + <%= link_to "Edit organization", edit_organization_path, class: "mt-1 inline-block text-sm text-ink-soft hover:text-ink hover:underline" %> + <% end %> +
+
+ <%= form_with url: admin_organization_memberships_path, method: :get, + data: { controller: "debounced-form", action: "input->debounced-form#submit change->debounced-form#submit", + turbo_frame: "memberships_list", turbo_action: "advance" } do |form| %> +
+ <%= form.search_field :q, value: @query, + placeholder: "Filter by member…", autocomplete: "off", + class: "block w-64 rounded-md border border-line bg-surface px-3 py-1.5 text-sm text-ink placeholder:text-ink-faint focus:border-ink-soft focus:outline-none" %> +
+ <% %w[owner admin member].each do |role| %> + + <% end %> +
+
+ <% end %> +
+
+ + <%= turbo_frame_tag "memberships_list", class: "mt-5 block", data: { turbo_action: "advance" } do %> +
+ + + + + + + + + + <% @memberships.each do |membership| %> + + + + + + <% end %> + <% if @total.zero? %> + + + + <% end %> + +
MemberRoleActions
+

<%= membership.user.display_name %>

+ <% if membership.user.name.present? %> +

<%= membership.user.email_address %>

+ <% end %> + 		<%= membership.role.capitalize %> + <% if membership.member? %> + <%= button_to "Make admin", admin_organization_membership_path(membership), method: :patch, params: { role: "admin", q: @query, roles: @roles, page: @page }, + class: "text-brand hover:underline cursor-pointer bg-transparent p-0" %> + <% elsif membership.admin? && owner? %> + <%= button_to "Make member", admin_organization_membership_path(membership), method: :patch, params: { role: "member", q: @query, roles: @roles, page: @page }, + class: "text-ink-soft hover:underline cursor-pointer bg-transparent p-0" %> + <% else %> + — + <% end %> +
+ <%= @query.present? || @roles.any? ? "No members match your filters." : "No members yet." %> +
+
+ + <% if @total.positive? %> +
+ Showing <%= @offset + 1 %>–<%= @offset + @memberships.size %> of <%= @total %> +
+ <% if @page > 1 %> + <%= link_to "← Prev", admin_organization_memberships_path(q: @query, roles: @roles, page: @page - 1), class: "rounded border border-line px-2.5 py-1 text-ink-soft hover:bg-surface-soft hover:text-ink" %> + <% end %> + <% if @has_more %> + <%= link_to "Next →", admin_organization_memberships_path(q: @query, roles: @roles, page: @page + 1), class: "rounded border border-line px-2.5 py-1 text-ink-soft hover:bg-surface-soft hover:text-ink" %> + <% end %> +
+
+ <% end %> + <% end %> +
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index aafde84..01b44fd 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -44,7 +44,6 @@ <%= link_to "Explore options", scenarios_path, class: "text-ink-soft hover:text-ink" %> <% if Current.user&.admin_of?(Current.organization) %> <%= link_to "Admin Dashboard", admin_dashboard_path, class: "text-ink-soft hover:text-ink" %> - <%= link_to "Members", organization_memberships_path, class: "text-ink-soft hover:text-ink" %> <% end %> <%= link_to "Profile", users_profile_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" %> diff --git a/app/views/organization_memberships/index.html.erb b/app/views/organization_memberships/index.html.erb deleted file mode 100644 index 706fc05..0000000 --- a/app/views/organization_memberships/index.html.erb +++ /dev/null @@ -1,89 +0,0 @@ -<% content_for :title, "Members" %> - -
-
-
- <% if Current.organization.logo.attached? %> - <%= image_tag url_for(Current.organization.logo), alt: "#{Current.organization.name} logo", class: "h-10 w-auto" %> - <% end %> -

Members

-
-

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

- <% if owner? %> - <%= link_to "Edit organization", edit_organization_path, class: "mt-3 inline-block text-brand hover:underline" %> - <% end %> -
- -
- <%= form_with url: organization_memberships_path, method: :get, - data: { controller: "debounced-form", action: "input->debounced-form#submit change->debounced-form#submit", - turbo_frame: "memberships_list", turbo_action: "advance" } do |form| %> -
- <%= form.search_field :q, value: @query, - placeholder: "Filter by member…", autocomplete: "off", - class: "block w-64 rounded-md border border-line bg-surface px-3 py-1.5 text-sm text-ink placeholder:text-ink-faint focus:border-ink-soft focus:outline-none" %> -
- <% %w[owner admin member].each do |role| %> - - <% end %> -
-
- <% end %> -
- - <%= turbo_frame_tag "memberships_list", class: "mt-4 block", data: { turbo_action: "advance" } do %> -
- -
- - <% if @total.positive? %> -
- Showing <%= @offset + 1 %>–<%= @offset + @memberships.size %> of <%= @total %> -
- <% if @page > 1 %> - <%= link_to "← Prev", organization_memberships_path(q: @query, roles: @roles, page: @page - 1), class: "rounded border border-line px-2.5 py-1 text-ink-soft hover:bg-surface-soft hover:text-ink" %> - <% end %> - <% if @has_more %> - <%= link_to "Next →", organization_memberships_path(q: @query, roles: @roles, page: @page + 1), class: "rounded border border-line px-2.5 py-1 text-ink-soft hover:bg-surface-soft hover:text-ink" %> - <% end %> -
-
- <% end %> - <% end %> -
diff --git a/app/views/organizations/edit.html.erb b/app/views/organizations/edit.html.erb index d725654..d57ec65 100644 --- a/app/views/organizations/edit.html.erb +++ b/app/views/organizations/edit.html.erb @@ -40,7 +40,7 @@
<%= form.submit "Save changes", class: "rounded-md bg-accent px-3.5 py-2 font-medium text-white hover:bg-[#444] transition cursor-pointer" %> - <%= link_to "Back to members", organization_memberships_path, class: "text-ink-soft hover:text-ink" %> + <%= link_to "Back to members", admin_organization_memberships_path, class: "text-ink-soft hover:text-ink" %>
<% end %> diff --git a/config/routes.rb b/config/routes.rb index 88fc795..f6a6cc8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,7 +18,6 @@ get "auto_sign_in", to: "auto_sign_in#create" if Rails.env.development? resource :organization, only: %i[ edit update ] - resources :organization_memberships, only: %i[ index update ] resources :scenarios do resource :name, only: %i[ show edit update ], module: :scenarios resource :total_giving_amount, only: %i[ show edit update ], module: :scenarios @@ -29,6 +28,7 @@ resource :dashboard, only: :show resources :scenarios, only: :index resources :allocation_categories, only: %i[ index new create edit update destroy ] + resources :organization_memberships, only: %i[ index update ] end # Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500. diff --git a/test/controllers/admin/organization_memberships_controller_test.rb b/test/controllers/admin/organization_memberships_controller_test.rb new file mode 100644 index 0000000..275bb00 --- /dev/null +++ b/test/controllers/admin/organization_memberships_controller_test.rb @@ -0,0 +1,177 @@ +require "test_helper" + +class Admin::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 admin_organization_memberships_path + assert_response :success + + sign_in_as(@admin) + get admin_organization_memberships_path + assert_response :success + end + + test "plain members are redirected away from the members page" do + sign_in_as(@member) + get admin_organization_memberships_path + assert_redirected_to root_path + end + + test "lists every member in the organization" do + sign_in_as(@owner) + get admin_organization_memberships_path + + assert_select "turbo-frame tbody tr", text: /#{@owner.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@admin.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@member.email_address}/ + end + + test "does not show members from another organization" do + sign_in_as(@owner) + get admin_organization_memberships_path + + assert_select "turbo-frame tbody tr", text: /two@example.com/, count: 0 + end + + test "filters members by email on the server" do + sign_in_as(@owner) + get admin_organization_memberships_path(q: "admin") + + assert_select "turbo-frame tbody tr", text: /#{@admin.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@owner.email_address}/, count: 0 + end + + test "filters members by user name on the server" do + @member.update!(name: "Zelda Fitzgerald") + sign_in_as(@owner) + get admin_organization_memberships_path(q: "zelda") + + assert_select "turbo-frame tbody tr", text: /Zelda Fitzgerald/ + assert_select "turbo-frame tbody tr", text: /#{@owner.email_address}/, count: 0 + end + + test "filters members by role" do + sign_in_as(@owner) + get admin_organization_memberships_path(roles: [ "admin" ]) + + assert_select "turbo-frame tbody tr", text: /#{@admin.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@owner.email_address}/, count: 0 + assert_select "turbo-frame tbody tr", text: /#{@member.email_address}/, count: 0 + end + + test "filters members by multiple roles" do + sign_in_as(@owner) + get admin_organization_memberships_path(roles: [ "owner", "member" ]) + + assert_select "turbo-frame tbody tr", text: /#{@owner.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@member.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@admin.email_address}/, count: 0 + end + + test "combines the user search and role filters" do + sign_in_as(@owner) + get admin_organization_memberships_path(q: "example.com", roles: [ "owner" ]) + + assert_select "turbo-frame tbody tr", text: /#{@owner.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@admin.email_address}/, count: 0 + end + + test "ignores unknown role values" do + sign_in_as(@owner) + get admin_organization_memberships_path(roles: [ "superuser" ]) + + # No valid roles selected → no role filter applied, every member shows. + assert_select "turbo-frame tbody tr", text: /#{@owner.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@admin.email_address}/ + assert_select "turbo-frame tbody tr", text: /#{@member.email_address}/ + end + + test "shows an empty state when no members match" do + sign_in_as(@owner) + get admin_organization_memberships_path(q: "nobody-matches-this") + + assert_select "turbo-frame tbody tr", text: /No members match/ + end + + test "paginates results" do + sign_in_as(@owner) + arlington = organizations(:arlington) + # Create more members than fit on one page so a second page exists. + (Admin::OrganizationMembershipsController::PER_PAGE + 5).times do |i| + user = User.create!(email_address: "bulk#{i}@example.com", confirmed_at: Time.current) + arlington.organization_memberships.create!(user: user, role: "member") + end + + get admin_organization_memberships_path + assert_select "turbo-frame tbody tr", count: Admin::OrganizationMembershipsController::PER_PAGE + + get admin_organization_memberships_path(page: 2) + assert_select "turbo-frame tbody tr", minimum: 1 + end + + # update — promote + + test "an admin can promote a member to admin" do + sign_in_as(@admin) + patch admin_organization_membership_path(@member_membership), params: { role: "admin" } + assert_redirected_to admin_organization_memberships_path + assert @member_membership.reload.admin? + end + + test "update preserves the active filter and page on redirect" do + sign_in_as(@admin) + patch admin_organization_membership_path(@member_membership), params: { role: "admin", q: "passwordless", page: "2" } + assert_redirected_to admin_organization_memberships_path(q: "passwordless", page: "2") + end + + test "a member cannot promote anyone" do + sign_in_as(@member) + patch admin_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 admin_organization_membership_path(@admin_membership), params: { role: "member" } + assert_redirected_to admin_organization_memberships_path + assert @admin_membership.reload.member? + end + + test "an admin cannot demote another admin" do + sign_in_as(@admin) + patch admin_organization_membership_path(@admin_membership), params: { role: "member" } + assert_redirected_to admin_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 admin_organization_membership_path(@owner_membership), params: { role: "member" } + assert_redirected_to admin_organization_memberships_path + assert @owner_membership.reload.owner? + end + + test "owner is not an assignable role" do + sign_in_as(@owner) + patch admin_organization_membership_path(@member_membership), params: { role: "owner" } + assert_redirected_to admin_organization_memberships_path + assert @member_membership.reload.member? + end +end diff --git a/test/controllers/organization_memberships_controller_test.rb b/test/controllers/organization_memberships_controller_test.rb deleted file mode 100644 index e44c29e..0000000 --- a/test/controllers/organization_memberships_controller_test.rb +++ /dev/null @@ -1,177 +0,0 @@ -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 - - test "lists every member in the organization" do - sign_in_as(@owner) - get organization_memberships_path - - assert_select "turbo-frame li", text: /#{@owner.email_address}/ - assert_select "turbo-frame li", text: /#{@admin.email_address}/ - assert_select "turbo-frame li", text: /#{@member.email_address}/ - end - - test "does not show members from another organization" do - sign_in_as(@owner) - get organization_memberships_path - - assert_select "turbo-frame li", text: /two@example.com/, count: 0 - end - - test "filters members by email on the server" do - sign_in_as(@owner) - get organization_memberships_path(q: "admin") - - assert_select "turbo-frame li", text: /#{@admin.email_address}/ - assert_select "turbo-frame li", text: /#{@owner.email_address}/, count: 0 - end - - test "filters members by user name on the server" do - @member.update!(name: "Zelda Fitzgerald") - sign_in_as(@owner) - get organization_memberships_path(q: "zelda") - - assert_select "turbo-frame li", text: /Zelda Fitzgerald/ - assert_select "turbo-frame li", text: /#{@owner.email_address}/, count: 0 - end - - test "filters members by role" do - sign_in_as(@owner) - get organization_memberships_path(roles: [ "admin" ]) - - assert_select "turbo-frame li", text: /#{@admin.email_address}/ - assert_select "turbo-frame li", text: /#{@owner.email_address}/, count: 0 - assert_select "turbo-frame li", text: /#{@member.email_address}/, count: 0 - end - - test "filters members by multiple roles" do - sign_in_as(@owner) - get organization_memberships_path(roles: [ "owner", "member" ]) - - assert_select "turbo-frame li", text: /#{@owner.email_address}/ - assert_select "turbo-frame li", text: /#{@member.email_address}/ - assert_select "turbo-frame li", text: /#{@admin.email_address}/, count: 0 - end - - test "combines the user search and role filters" do - sign_in_as(@owner) - get organization_memberships_path(q: "example.com", roles: [ "owner" ]) - - assert_select "turbo-frame li", text: /#{@owner.email_address}/ - assert_select "turbo-frame li", text: /#{@admin.email_address}/, count: 0 - end - - test "ignores unknown role values" do - sign_in_as(@owner) - get organization_memberships_path(roles: [ "superuser" ]) - - # No valid roles selected → no role filter applied, every member shows. - assert_select "turbo-frame li", text: /#{@owner.email_address}/ - assert_select "turbo-frame li", text: /#{@admin.email_address}/ - assert_select "turbo-frame li", text: /#{@member.email_address}/ - end - - test "shows an empty state when no members match" do - sign_in_as(@owner) - get organization_memberships_path(q: "nobody-matches-this") - - assert_select "turbo-frame li", text: /No members match/ - end - - test "paginates results" do - sign_in_as(@owner) - arlington = organizations(:arlington) - # Create more members than fit on one page so a second page exists. - (OrganizationMembershipsController::PER_PAGE + 5).times do |i| - user = User.create!(email_address: "bulk#{i}@example.com", confirmed_at: Time.current) - arlington.organization_memberships.create!(user: user, role: "member") - end - - get organization_memberships_path - assert_select "turbo-frame li", count: OrganizationMembershipsController::PER_PAGE - - get organization_memberships_path(page: 2) - assert_select "turbo-frame li", minimum: 1 - 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 "update preserves the active filter and page on redirect" do - sign_in_as(@admin) - patch organization_membership_path(@member_membership), params: { role: "admin", q: "passwordless", page: "2" } - assert_redirected_to organization_memberships_path(q: "passwordless", page: "2") - 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