Organizations Members CRUD actions

app/controllers/organizations/members_controller.rb

@@ -0,0 +1,74 @@
+class Organizations::MembersController < ApplicationController
+  before_action :redirect_to_signin, only: :index, unless: :signed_in?
+  before_action :redirect_to_new_mfa, only: :index, if: :mfa_required_not_yet_enabled?
+  before_action :redirect_to_settings_strong_mfa_required, only: :index, if: :mfa_required_weak_level_enabled?
+
+  before_action :find_organization, only: %i[create update destroy]
+  before_action :find_membership, only: %i[update destroy]
+
+  layout "subject"
+
+  def index
+    @organization = Organization.find_by_handle!(params[:organization_id])
+    authorize @organization, :list_memberships?
+
+    @memberships = @organization.memberships.includes(:user)
+    @memberships_count = @organization.memberships.count
+  end
+
+  def create
+    username, role = create_membership_params.require([:username, :role])
+    # we can open this up in the future to handle email too via find_by_name,
+    # but it will need to use an invite process to handle non-existing users.
+    member = User.find_by(handle: username)
+    if member
+      membership = authorize @organization.memberships.build(user: member, role:)
+      if membership.save
+        flash[:notice] = t(".success", username: member.name)
+      else
+        flash[:error] = t(".failure", error: membership.errors.full_messages.to_sentence)
+      end
+    else
+      flash[:error] = t(".failure", error: t(".user_not_found"))
+    end
+    redirect_to organization_members_path(@organization)
+  end
+
+  def update
+    @membership.attributes = update_membership_params
+    authorize @membership
+    if @membership.save
+      flash[:notice] = t(".success")
+    else
+      flash[:error] = t(".failure", error: membership.errors.full_messages.to_sentence)
+    end
+    redirect_to organization_members_path(@organization)
+  end
+
+  def destroy
+    authorize @membership
+    flash[:notice] = t(".success") if @membership.destroy
+    redirect_to organization_members_path(@organization)
+  end
+
+  private
+
+  def find_organization
+    @organization = Organization.find_by_handle!(params[:organization_id])
+    authorize @organization, :manage_memberships?
+  end
+
+  def find_membership
+    handle = params.permit(:id).require(:id)
+    @member = User.find_by_slug!(handle)
+    @membership = @organization.memberships.find_by!(user: @member)
+  end
+
+  def create_membership_params
+    params.permit(membership: %i[username role]).require(:membership)
+  end
+
+  def update_membership_params
+    params.permit(membership: %i[role]).require(:membership)
+  end
+end

app/views/organizations/_subject.html.erb

@@ -18,7 +18,7 @@
   <%= nav.link t("layouts.application.header.dashboard"), organization_path(@organization), name: :dashboard, icon: "space-dashboard" %>
   <%= nav.link t("organizations.show.history"), organization_path(@organization), name: :subscriptions, icon: "notifications" %>
   <%= nav.link t("organizations.show.gems"), organization_gems_path(@organization), name: :gems, icon: "gems" %>
-  <%= nav.link t("organizations.show.members"), organization_path(@organization), name: :organizations, icon: "organizations" %>
+  <%= nav.link t("organizations.show.members"), organization_members_path(@organization), name: :members, icon: "organizations" %>
   <% if policy(@organization).edit? %>
     <%= nav.link t("layouts.application.header.settings"), edit_organization_path(@organization), name: :settings, icon: "settings" %>
   <% end %>

app/views/organizations/members/index.html.erb

@@ -0,0 +1,32 @@
+<%
+  add_breadcrumb t("breadcrumbs.org_name", name: @organization.handle), organization_path(@organization)
+  add_breadcrumb t("breadcrumbs.members")
+%>
+
+<% content_for :subject do %>
+  <%= render "organizations/subject", organization: @organization, current: :members %>
+<% end %>
+
+<h1 class="text-h2 mb-10"><%= t("organizations.show.members") %></h1>
+
+<%= render CardComponent.new do |c| %>
+  <%= c.head do %>
+    <%= c.title t("organizations.show.members"), icon: :organizations %>
+  <% end %>
+  <% if @memberships.empty? %>
+    <%= prose do %>
+      <i><%= t('organizations.show.no_members') %></i>
+    <% end %>
+  <% else %>
+    <%= c.divided_list do %>
+      <% @memberships.each do |membership| %>
+        <%= c.list_item_to(profile_path(membership.user.handle)) do %>
+          <div class="flex justify-between">
+            <p class="text-neutral-800 dark:text-white"><%= membership.user.name %></p>
+            <p class="text-neutral-500 capitalize"><%= membership.role %></p>
+          </div>
+        <% end %>
+      <% end %>
+    <% end %>
+  <% end %>
+<% end %>

config/locales/en.yml

@@ -428,6 +428,16 @@ en:
       members: Members
       no_history: No events yet
       no_gems: No gems yet
+    members:
+      create:
+        failure: "Failed to add member: %{error}"
+        success: "Member added!"
+        user_not_found: "User not found"
+      destroy:
+        success: "User was removed from the organization"
+      update:
+        failure: "Failed to update member: %{error}"
+        success: "User was updated"
   pages:
     about:
       contributors_amount: "%{count} Rubyists"

config/routes.rb

@@ -290,6 +290,7 @@
     end
     resources :organizations, only: %i[index show edit update], constraints: { id: Patterns::ROUTE_PATTERN } do
       resources :gems, only: :index, controller: 'organizations/gems'
+      resources :members, only: %i[index create update destroy], controller: 'organizations/members'
     end
   end
 

test/integration/organizations/members_test.rb

@@ -0,0 +1,153 @@
+require "test_helper"
+
+class Organizations::MembersTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = create(:user, remember_token_expires_at: Gemcutter::REMEMBER_FOR.from_now)
+    post session_path(session: { who: @user.handle, password: PasswordHelpers::SECURE_TEST_PASSWORD })
+  end
+
+  test "index should render Not Found org" do
+    get "/organizations/notfound/members"
+
+    assert_response :not_found
+  end
+
+  test "index should render Forbidden" do
+    create(:organization, handle: "chaos")
+
+    get "/organizations/chaos/members"
+
+    assert_response :forbidden
+  end
+
+  test "should get index" do
+    create(:organization, owners: [@user], handle: "chaos")
+
+    get "/organizations/chaos/members"
+
+    assert_response :success
+    assert page.has_content?("Members")
+  end
+
+  test "create should return Not Found org" do
+    post "/organizations/notfound/members", params: { membership: { role: "owner" } }
+
+    assert_response :not_found
+  end
+
+  test "create should return Forbidden when trying to create your own membership" do
+    create(:organization, handle: "chaos")
+
+    post "/organizations/chaos/members", params: { membership: { username: @user.id, role: "maintainer" } }
+
+    assert_response :forbidden
+  end
+
+  test "create membership with bad role should not work" do
+    organization = create(:organization, owners: [@user], handle: "chaos")
+    bdfl = create(:user, handle: "bdfl")
+
+    post "/organizations/chaos/members", params: { membership: { username: bdfl.handle, role: "bdfl" } }
+
+    assert_redirected_to organization_members_path(organization)
+    follow_redirect!
+
+    assert page.has_content?("Failed to add member: Role is not included in the list")
+    assert_nil organization.unconfirmed_memberships.find_by(user_id: bdfl.id)
+  end
+
+  test "create membership by email should not work (yet)" do
+    organization = create(:organization, owners: [@user], handle: "chaos")
+    maintainer = create(:user, handle: "maintainer")
+
+    post "/organizations/chaos/members", params: { membership: { username: maintainer.email, role: "maintainer" } }
+
+    assert_redirected_to organization_members_path(organization)
+    follow_redirect!
+
+    assert page.has_content?("Failed to add member: User not found")
+    assert_nil organization.unconfirmed_memberships.find_by(user_id: maintainer.id)
+  end
+
+  test "should create a membership by handle" do
+    organization = create(:organization, owners: [@user], handle: "chaos")
+    maintainer = create(:user, handle: "maintainer")
+
+    post "/organizations/chaos/members", params: { membership: { username: maintainer.handle, role: "maintainer" } }
+
+    assert_redirected_to organization_members_path(organization)
+    membership = organization.unconfirmed_memberships.find_by(user_id: maintainer.id)
+
+    assert membership
+    assert_predicate membership, :maintainer?
+    refute_predicate membership, :confirmed?
+  end
+
+  test "update should return Not Found org" do
+    patch "/organizations/notfound/members/notfound", params: { membership: { role: "owner" } }
+
+    assert_response :not_found
+  end
+
+  test "update should return Not Found membership" do
+    create(:organization, owners: [@user], handle: "chaos")
+
+    patch "/organizations/chaos/members/notfound", params: { membership: { role: "owner" } }
+
+    assert_response :not_found
+  end
+
+  test "update should return Forbidden" do
+    organization = create(:organization, handle: "chaos")
+    membership = create(:membership, :maintainer, user: @user, organization: organization)
+
+    patch "/organizations/chaos/members/#{@user.handle}", params: { membership: { role: "owner" } }
+
+    assert_response :forbidden
+  end
+
+  test "should update" do
+    organization = create(:organization, owners: [@user], handle: "chaos")
+    maintainer = create(:user, handle: "maintainer")
+    membership = create(:membership, :maintainer, user: maintainer, organization: organization)
+
+    patch "/organizations/chaos/members/#{maintainer.handle}", params: { membership: { role: "owner" } }
+
+    assert_redirected_to organization_members_path(organization)
+    assert_predicate membership.reload, :owner?
+  end
+
+  test "destroy should return Not Found org" do
+    delete "/organizations/notfound/members/notfound"
+
+    assert_response :not_found
+  end
+
+  test "destroy should return Not Found membership" do
+    create(:organization, owners: [@user], handle: "chaos")
+
+    delete "/organizations/chaos/members/notfound"
+
+    assert_response :not_found
+  end
+
+  test "destroy should return Forbidden" do
+    organization = create(:organization, handle: "chaos")
+    membership = create(:membership, :maintainer, user: @user, organization: organization)
+
+    delete "/organizations/chaos/members/#{@user.handle}"
+
+    assert_response :forbidden
+  end
+
+  test "should destroy a membership" do
+    organization = create(:organization, handle: "chaos", owners: [@user])
+    maintainer = create(:user, handle: "maintainer")
+    membership = create(:membership, :maintainer, user: maintainer, organization: organization)
+
+    delete "/organizations/chaos/members/#{maintainer.handle}"
+
+    assert_redirected_to organization_members_path(organization)
+    assert_nil Membership.find_by(id: membership.id)
+  end
+end