Organisation ownership transfer (#152)

* migration - AddOwnerToOrganizations

* associations

* transfer ownership model

* fix existing tests

* set owner on creating org

* basic controller to transfer ownership

* authorize_organization_owner!

* link to transfer

* fix transfer method

* lint

* prep for adding tests

* Update transfer.rb

* styling transfer views

* try to fix net-pop

gem uninstall net-pop

bundle update net-pop

* Update transfer_test.rb

* Update transfers_controller_test.rb

* transfer form - frontend validation

Gemfile.lock

@@ -190,6 +190,7 @@ GEM
       date
       net-protocol
     net-pop (0.1.2)
+      net-protocol
     net-protocol (0.2.2)
       timeout
     net-scp (4.0.0)

app/controllers/organizations/base_controller.rb

@@ -16,6 +16,10 @@ class Organizations::BaseController < ApplicationController
   #   raise Pundit::NotAuthorizedError unless @current_membership.admin?
   # end
 
+  def authorize_organization_owner!
+    redirect_to organization_path(@organization), alert: "You are not authorized to perform this action." unless @organization.owner?(current_user)
+  end
+
   private
 
   def set_organization

app/controllers/organizations/transfers_controller.rb

@@ -0,0 +1,14 @@
+class Organizations::TransfersController < Organizations::BaseController
+  before_action :authorize_organization_owner!
+
+  def show
+  end
+
+  def update
+    if @organization.transfer_ownership(params[:user_id])
+      redirect_to @organization, notice: "Ownership transferred successfully."
+    else
+      render :show, status: :unprocessable_entity
+    end
+  end
+end

app/controllers/organizations_controller.rb

@@ -22,6 +22,7 @@ def edit
   # POST /organizations
   def create
     @organization = Organization.new(organization_params)
+    @organization.owner = current_user
     @organization.memberships.build(user: current_user, role: Membership.roles[:admin])
 
     if @organization.save

app/models/organization.rb

@@ -1,6 +1,10 @@
 class Organization < ApplicationRecord
   has_many :memberships, dependent: :destroy
   has_many :users, through: :memberships
+  belongs_to :owner, class_name: "User"
+
+  include Transfer
+
   has_many :inboxes, dependent: :destroy
 
   validates :name, presence: true

app/models/organization/transfer.rb

@@ -0,0 +1,28 @@
+module Organization::Transfer
+  extend ActiveSupport::Concern
+
+  def transfer_ownership(user_id)
+    # previous_owner = owner
+
+    membership = memberships.find_by(user_id: user_id)
+    return false unless membership
+
+    new_owner = membership.user
+
+    ApplicationRecord.transaction do
+      membership.update!(role: Membership.roles[:admin])
+      update!(owner: new_owner)
+    end
+  rescue => e
+    Rails.logger.error("Ownership transfer failed: #{e.message}")
+    false
+  end
+
+  def owner?(user)
+    owner_id == user.id
+  end
+
+  def can_transfer?(user)
+    owner?(user) && users.size >= 2
+  end
+end

app/models/user.rb

@@ -5,4 +5,5 @@ class User < ApplicationRecord
          :recoverable, :rememberable, :validatable
   has_many :memberships, dependent: :destroy
   has_many :organizations, through: :memberships
+  has_many :owned_organizations, class_name: "Organization", foreign_key: :owner_id, inverse_of: :owner, dependent: :destroy
 end

app/views/organizations/show.html.erb

@@ -1,5 +1,6 @@
 <%= render PageComponent.new(title: "Organizations / #{@organization.name}") do |component| %>
   <% component.with_action_list do %>
+    <%= link_to "Transfer", organization_transfer_path(@organization), class: "btn btn-danger" if @organization.can_transfer?(current_user) %>
     <%= link_to "Edit", edit_organization_path(@organization), class: "btn btn-secondary" if policy(@organization).edit? %>
     <div class="inline-block">
       <%= button_to "Destroy", @organization, method: :delete, data: { turbo_confirm: "Are you sure?" }, class: "btn btn-danger" if policy(@organization).destroy? %>

app/views/organizations/transfers/_form.html.erb

@@ -0,0 +1,11 @@
+<%= form_with url: organization_transfer_path(current_organization), method: :patch, class: "contents space-y-4" do |form| %>
+  <div class="">
+    <%= form.label :user_id %>
+    <%= form.collection_select :user_id, current_organization.users.where.not(users: { id: current_user.id }), :id, :email, { include_blank: true, required: true }, { class: "form-input" } %>
+  </div>
+
+  <div class="inline">
+    <%#= form.button, data: { turbo_confirm: "Are you sure?" } %>
+    <%= form.submit class: "btn btn-primary" %>
+  </div>
+<% end %>

app/views/organizations/transfers/show.html.erb

@@ -0,0 +1,4 @@
+<%= render PageComponent.new(title: "Transfer organization", content_container: true, full_width: false) do |component| %>
+  <%= render "organizations/transfers/form" %>
+  <%= link_to "Cancel", organization_path(current_organization), class: "btn btn-secondary" %>
+<% end %>

config/routes.rb

@@ -3,6 +3,7 @@
 
   resources :organizations do
     resources :memberships, module: :organizations, except: %i[show]
+    resource :transfer, module: :organizations, only: %i[show update]
     resources :inboxes, module: :organizations
   end
 

db/migrate/20241208203403_add_owner_to_organizations.rb

@@ -0,0 +1,12 @@
+class AddOwnerToOrganizations < ActiveRecord::Migration[8.0]
+  def change
+    add_reference :organizations, :owner, foreign_key: { to_table: :users }
+
+    Organization.all.each do |organization|
+      user = organization.memberships.admin.first&.user
+      organization.update(owner: user)
+    end
+
+    change_column_null :organizations, :owner_id, false
+  end
+end

test/controllers/organizations/transfers_controller_test.rb

@@ -0,0 +1,52 @@
+require "test_helper"
+
+class Organizations::TransfersControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = users(:one)
+    @organization = organizations(:one)
+    @membership = memberships(:one)
+
+    @user2 = users(:two)
+  end
+
+  test "#show" do
+    sign_in @user
+
+    get organization_transfer_path(@organization)
+    assert_response :success
+  end
+
+  test "admin can transfer organization" do
+    @membership2 = @organization.memberships.create!(user: @user2, role: Membership.roles[:member])
+    sign_in @user
+
+    patch organization_transfer_path(@organization), params: { user_id: @user2.id }
+    assert_redirected_to organization_path(@organization)
+    assert_equal @user2, @organization.reload.owner
+  end
+
+  test "regular_user cannot transfer organization" do
+    @membership2 = @organization.memberships.create!(user: @user2, role: Membership.roles[:member])
+    sign_in @user2
+
+    patch organization_transfer_path(@organization), params: { user_id: @user2.id }
+    assert_redirected_to organization_path(@organization)
+    assert_equal @user, @organization.reload.owner
+  end
+
+  test "transfer organization to non-member" do
+    sign_in @user
+
+    patch organization_transfer_path(@organization), params: { user_id: @user2.id }
+    assert_response :unprocessable_entity
+    assert_equal @user, @organization.reload.owner
+  end
+
+  test "transfer organization that user is not a member of" do
+    sign_in @user2
+
+    patch organization_transfer_path(@organization), params: { user_id: @user2.id }
+    assert_response :not_found
+    assert_equal @user, @organization.reload.owner
+  end
+end

test/fixtures/organizations.yml

@@ -2,6 +2,8 @@
 
 one:
   name: Organization 1
+  owner: one
 
 two:
   name: Organization 2
+  owner: two

test/fixtures/users.yml

@@ -9,3 +9,6 @@ one:
 
 two:
   email: [email protected]
+
+unassociated:
+  email: [email protected]

test/models/organization/transfer_test.rb

@@ -0,0 +1,16 @@
+require "test_helper"
+
+class OrganizationTransferTest < ActiveSupport::TestCase
+  test "transfer_ownership" do
+    organization = organizations(:one)
+    user = users(:two)
+
+    assert_not organization.transfer_ownership(user.id)
+    assert_not organization.owner?(user)
+
+    membership = organization.memberships.create!(user: user, role: Membership.roles[:member])
+    assert organization.transfer_ownership(user.id)
+    assert organization.owner?(user)
+    assert membership.reload.admin?
+  end
+end

test/models/user_test.rb

@@ -2,11 +2,11 @@
 
 class UserTest < ActiveSupport::TestCase
   # TODO: memberships with associated downstream records should not be deletable
-  test "destroying a user destroys associated memberships" do
+  test "destroying organization owner destroys organization" do
     organization = organizations(:one)
     user = users(:one)
 
-    assert_no_difference "Organization.count" do
+    assert_difference "Organization.count", -1 do
       assert_difference "Membership.count", -1 do
         user.destroy
       end
@@ -15,4 +15,18 @@ class UserTest < ActiveSupport::TestCase
     # TODO: this should not be a valid state!
     assert organization.memberships.none?
   end
+
+  test "destroying organization member (non-owner) does not destroy organization" do
+    organization = organizations(:one)
+    user = users(:unassociated)
+    organization.memberships.create!(user:, role: Membership.roles[:admin])
+
+    assert_no_difference "Organization.count" do
+      assert_difference "Membership.count", -1 do
+        user.destroy
+      end
+    end
+
+    assert organization.memberships.any?
+  end
 end