Delete organisation members and unaccepted invitations (#711)

Co-authored-by: Shivam Singhal <[email protected]>
Co-authored-by: Samrat Man Singh <[email protected]>

app/components/v2/header_component.html.erb

@@ -9,38 +9,40 @@
 
       <div class="flex flex-wrap justify-between items-center py-3 px-4 pb-5 sm:pb-3 lg:px-6">
         <div class="flex items-center mb-2 sm:mb-0 gap-x-2">
-          <%= render V2::ButtonComponent.new(scheme: :light,
-                                             options: edit_accounts_organization_path(current_organization),
-                                             type: :link,
-                                             size: :xxs) do |b| %>
-            <% b.with_tooltip("Organization Settings") %>
-            <% b.with_icon("v2/building.svg", size: :md) %>
-          <% end %>
+          <% if current_organization %>
+            <%= render V2::ButtonComponent.new(scheme: :light,
+                                               options: edit_accounts_organization_path(current_organization),
+                                               type: :link,
+                                               size: :xxs) do |b| %>
+              <% b.with_tooltip("Organization Settings") %>
+              <% b.with_icon("v2/building.svg", size: :md) %>
+            <% end %>
 
-          <%= render V2::DropdownComponent.new(authz: false) do |dropdown| %>
-            <% button = dropdown.with_button(size: :xs) %>
-            <% button.with_icon("art/org_default.png", size: :xxl) %>
-            <% button.with_title_text do %>
-              <div class="text-left">
-                <div class="text-sm font-semibold leading-none text-main dark:text-white">
-                  <%= current_organization.name %>
+            <%= render V2::DropdownComponent.new(authz: false) do |dropdown| %>
+              <% button = dropdown.with_button(size: :xs) %>
+              <% button.with_icon("art/org_default.png", size: :xxl) %>
+              <% button.with_title_text do %>
+                <div class="text-left">
+                  <div class="text-sm font-semibold leading-none text-main dark:text-white">
+                    <%= current_organization.name %>
+                  </div>
                 </div>
-              </div>
-            <% end %>
+              <% end %>
 
-            <% dropdown.with_item_group do |group| %>
-              <% current_user.organizations.each do |organization| %>
-                <% group.with_item(link: { path: switch_accounts_organization_path(organization), "data-turbo": false },
-                                   selected: organization.id == current_organization.id) do %>
-                  <div class="text-left">
-                    <div class="font-medium leading-none mb-0.5 text-sm">
-                      <%= organization.name %>
-                    </div>
+              <% dropdown.with_item_group do |group| %>
+                <% current_user.organizations.each do |organization| %>
+                  <% group.with_item(link: { path: switch_accounts_organization_path(organization), "data-turbo": false },
+                                     selected: organization.id == current_organization.id) do %>
+                    <div class="text-left">
+                      <div class="font-medium leading-none mb-0.5 text-sm">
+                        <%= organization.name %>
+                      </div>
 
-                    <div class="text-xs text-secondary dark:text-secondary-50">
-                      Created <%= time_format organization.created_at, with_year: true, with_time: false %>
+                      <div class="text-xs text-secondary dark:text-secondary-50">
+                        Created <%= time_format organization.created_at, with_year: true, with_time: false %>
+                      </div>
                     </div>
-                  </div>
+                  <% end %>
                 <% end %>
               <% end %>
             <% end %>

app/controllers/accounts/invitations_controller.rb

@@ -8,8 +8,8 @@
 # |------------+-----------------------------------+-----------------------------------+--------------------------------------------|
 
 class Accounts::InvitationsController < SignedInApplicationController
-  before_action :require_write_access!, only: %i[create]
-  before_action :set_organization
+  before_action :require_write_access!, only: %i[create destroy]
+  before_action :set_organization, only: [:create]
 
   def create
     @invite = Accounts::Invite.new(invite_params)
@@ -24,6 +24,18 @@ def create
     end
   end
 
+  def destroy
+    @invite = current_organization.pending_invites.find_by(id: params[:id])
+
+    if @invite&.destroy
+      redirect_to accounts_organization_teams_path(current_organization),
+        notice: "Invitation to #{@invite.email} has been cancelled"
+    else
+      redirect_to accounts_organization_teams_path(current_organization),
+        flash: {error: "Could not cancel the invitation."}
+    end
+  end
+
   protected
 
   def invite_params

app/controllers/accounts/memberships_controller.rb

@@ -0,0 +1,30 @@
+class Accounts::MembershipsController < SignedInApplicationController
+  before_action :require_write_access!, only: %i[destroy]
+
+  def destroy
+    @membership = current_organization.memberships.find_by(id: params[:id])
+
+    if @membership.blank?
+      redirect_to teams_accounts_organization_path(current_organization),
+        flash: {error: "Could not find the member to remove."}
+      return
+    end
+
+    unless helpers.can_current_user_remove_member?(@membership.user)
+      redirect_to teams_accounts_organization_path(current_organization),
+        flash: {error: "You don't have permission to remove this member"}
+      return
+    end
+
+    if @membership.discarded?
+      redirect_to teams_accounts_organization_path(current_organization),
+        flash: {error: "Member was already removed"}
+    elsif @membership.discard
+      redirect_to teams_accounts_organization_path(current_organization),
+        notice: "Member #{@membership.user.email} has been removed"
+    else
+      redirect_to teams_accounts_organization_path(current_organization),
+        flash: {error: @membership.errors.full_messages.to_sentence}
+    end
+  end
+end

app/controllers/authentication/email/invite_confirmations_controller.rb

@@ -1,6 +1,7 @@
 class Authentication::Email::InviteConfirmationsController < ApplicationController
   before_action :set_invite_token, only: [:new, :create]
   before_action :set_invite, only: [:new, :create]
+  before_action :check_valid_invitation, only: [:new]
   before_action :check_accepted_invitation, only: [:new]
   helper_method :current_user
 
@@ -28,6 +29,12 @@ def create
 
   private
 
+  def check_valid_invitation
+    if @invite.nil?
+      redirect_to root_path, flash: {error: t("invitation.flash.invalid_or_expired")}
+    end
+  end
+
   def check_accepted_invitation
     redirect_to root_path, notice: t("invitation.flash.already_accepted") if @invite.accepted?
   end

app/controllers/authentication/email/registrations_controller.rb

@@ -10,6 +10,13 @@ class Authentication::Email::RegistrationsController < Devise::RegistrationsCont
 
   def new
     if @token.present?
+      # Check if invite exists and is valid
+      if @invite.nil?
+        flash.clear
+        flash[:alert] = t("invitation.flash.invalid_or_expired")
+        redirect_to new_email_authentication_session_path and return
+      end
+
       flash[:notice] = t("invitation.flash.signup_before", org: @invite.organization.name)
     end
 

app/controllers/signed_in_application_controller.rb

@@ -7,7 +7,7 @@ class SignedInApplicationController < ApplicationController
   Action = Coordinators::Actions
 
   layout -> { ensure_supported_layout("signed_in_application") }
-
+  before_action :require_organization!
   before_action :authenticate_sso_request!, if: :sso_authentication_signed_in?
   before_action :turbo_frame_request_variant
   before_action :set_currents
@@ -176,7 +176,7 @@ def default_app
   end
 
   def new_app
-    current_organization.apps.new
+    current_organization&.apps&.new
   end
 
   def vcs_provider_logo
@@ -214,4 +214,11 @@ def turbo_frame_request_variant
   def stream_flash
     turbo_stream.update("flash_stream", V2::FlashComponent.new(flash))
   end
+
+  def require_organization!
+    if current_organization.blank?
+      flash.now[:error] = "You are not a member of any organization"
+      render template: "shared/no_organization", status: :unauthorized and return
+    end
+  end
 end

app/helpers/application_helper.rb

@@ -156,6 +156,6 @@ def page_title(page_name, current_organization, app, release)
       page_name || middle_section
     end
 
-    [prefix.titleize, middle_section.titleize, suffix.titleize].compact.join(" | ")
+    [prefix&.titleize, middle_section&.titleize, suffix&.titleize].compact.join(" | ")
   end
 end

app/helpers/organizations_helper.rb

@@ -10,4 +10,10 @@ def can_current_user_edit_role?(member)
   def available_roles_for_member(member_role)
     (member_role == "viewer") ? Accounts::Membership.allowed_roles : Accounts::Membership.all_roles
   end
+
+  def can_current_user_remove_member?(member)
+    return false if current_user.id == member.id  # Users can't remove themselves
+    return false if current_user_role != Accounts::Membership.roles[:owner]
+    true
+  end
 end

app/models/accounts/membership.rb

@@ -3,6 +3,7 @@
 # Table name: memberships
 #
 #  id              :uuid             not null, primary key
+#  discarded_at    :datetime         indexed
 #  role            :string           not null, indexed, indexed => [user_id, organization_id]
 #  created_at      :datetime         not null
 #  updated_at      :datetime         not null
@@ -11,6 +12,7 @@
 #  user_id         :uuid             indexed => [organization_id, role]
 #
 class Accounts::Membership < ApplicationRecord
+  include Discard::Model
   include Roleable
   has_paper_trail
 
@@ -22,6 +24,17 @@ class Accounts::Membership < ApplicationRecord
   validate :team_can_only_be_set_once
   validate :valid_role_change, on: :update
 
+  before_discard :ensure_at_least_one_owner_remains
+
+  def ensure_at_least_one_owner_remains
+    return true unless role == Accounts::Membership.roles[:owner]
+
+    remaining_owners = organization.memberships.kept.where(role: Accounts::Membership.roles[:owner]).where.not(id: id)
+    if remaining_owners.empty?
+      errors.add(:base, "Organization must have at least one owner")
+    end
+  end
+
   def self.allowed_roles
     roles.except(:owner).transform_keys(&:titleize).to_a
   end

app/models/accounts/organization.rb

@@ -22,7 +22,7 @@ class Accounts::Organization < ApplicationRecord
   extend FriendlyId
   has_paper_trail
 
-  has_many :memberships, dependent: :delete_all, inverse_of: :organization
+  has_many :memberships, -> { kept }, dependent: :delete_all, inverse_of: :organization
   has_many :teams, -> { sequential }, dependent: :delete_all, inverse_of: :organization
   has_many :users, through: :memberships, dependent: :delete_all
   has_many :apps, -> { sequential }, dependent: :destroy, inverse_of: :organization

app/models/accounts/user.rb

@@ -23,7 +23,7 @@ class Accounts::User < ApplicationRecord
   validates :preferred_name, length: {maximum: 70, message: :too_long}
   validates :unique_authn_id, uniqueness: {message: :already_taken, case_sensitive: false}
 
-  has_many :memberships, dependent: :delete_all, inverse_of: :user
+  has_many :memberships, -> { kept }, dependent: :delete_all, inverse_of: :user
   has_many :organizations, -> { where(status: :active).sequential }, through: :memberships
   has_many :all_organizations, through: :memberships, source: :organization
   has_many :sent_invites, class_name: "Invite", foreign_key: "sender_id", inverse_of: :sender, dependent: :destroy
@@ -163,7 +163,11 @@ def add_via_email(invite)
   end
 
   def role_for(organization)
-    access_for(organization).role
+    access_for(organization)&.role
+  end
+
+  def membership_for(organization)
+    access_for(organization)
   end
 
   def team_for(organization)
@@ -172,11 +176,11 @@ def team_for(organization)
 
   # TODO: [nplus1]
   def writer_for?(organization)
-    access_for(organization).writer?
+    access_for(organization)&.writer?
   end
 
   def owner_for?(organization)
-    access_for(organization).owner?
+    access_for(organization)&.owner?
   end
 
   def successful_invite_for(organization)

app/views/accounts/organizations/_teams.html.erb

@@ -102,9 +102,9 @@
         <% end %>
         <% user_role = current_user_role %>
         <% member_role = member.role_for(current_organization) %>
-        <% if can_current_user_edit_role?(member) %>
-          <% row.with_cell(wrap: true) do %>
-            <div class="flex gap-x-2 justify-end">
+        <% row.with_cell(wrap: true) do %>
+          <div class="flex gap-x-2 justify-end">
+            <% if can_current_user_edit_role?(member) %>
               <%= render V2::ModalComponent.new(title: "Edit role") do |modal| %>
                 <% modal.with_button(scheme: :light,
                                     type: :action,
@@ -114,13 +114,22 @@
                 <% modal.with_body do %>
                   <%= render partial: "accounts/invitations/edit_form", locals: { email: member.email, member_role: member_role } %>
                 <% end %>
-              </div>
               <% end %>
-          <% end %>
+            <% end %>
+            <% if can_current_user_remove_member?(member) %>
+              <%= render V2::ButtonComponent.new(
+                scheme: :danger,
+                type: :button,
+                size: :xxs,
+                options: accounts_organization_membership_path(current_organization, member.membership_for(current_organization)),
+                html_options: { method: :delete, data: { turbo_method: :delete, turbo_confirm: "Are you sure you want to remove #{member.email} from the organization?" } }) do |b|
+                b.with_icon("v2/trash.svg")
+              end %>
+            <% end %>
+          </div>
         <% end %>
       <% end %>
     <% end %>
-
     <% @invited_users.each do |invite| %>
       <% table.with_row do |row| %>
         <% row.with_cell do %>
@@ -149,6 +158,18 @@
         <% row.with_cell do %>
           <%= invite.sender&.email || "–" %>
         <% end %>
+        <% row.with_cell(wrap: true) do %>
+          <div class="flex gap-x-2 justify-end">
+            <%= render V2::ButtonComponent.new(
+              scheme: :danger,
+              type: :button,
+              size: :xxs,
+              options: accounts_organization_invitation_path(current_organization, invite),
+              html_options: { method: :delete, data: { turbo_method: :delete, turbo_confirm: "Are you sure you want to cancel the invitation for #{invite.email}?" } }) do |b|
+              b.with_icon("v2/trash.svg")
+            end %>
+          </div>
+        <% end %>
       <% end %>
     <% end %>
   <% end %>

app/views/shared/no_organization.html.erb

@@ -0,0 +1,4 @@
+<%= render V2::EmptyStateComponent.new(title: "Oops! Looks like you don't belong to any organization",
+                                       text: "Please contact your admin to add you to an organization.",
+                                       type: :subdued,
+                                       banner_image: "v2/building.svg") %>

compose.yml

@@ -11,6 +11,7 @@ x-app: &app
   tmpfs:
     - /tmp
     - /app/tmp/pids
+    - /rails/tmp/pids
 
 x-backend: &backend
   <<: *app

config/locales/en.yml

@@ -563,6 +563,7 @@ en:
       accepted: "Invitation was accepted. Please sign in to continue."
       already_accepted: "Invitation was already accepted!"
       failed: "Failed to accept your invitation. Please contact support!"
+      invalid_or_expired: "This invitation link is no longer valid. Please contact the organization admin for a new invitation."
     invite_mailer:
       existing_user:
         subject: "%{sender} invited you to join the %{org} team on Tramline"

config/routes.rb

@@ -49,7 +49,8 @@
       end
 
       resources :teams, only: %i[create update destroy]
-      resources :invitations, only: [:create]
+      resources :invitations, only: %i[create destroy]
+      resources :memberships, only: [:destroy]
     end
 
     resource :user, only: [:edit, :update] do

db/migrate/20241102204836_add_discarded_at_to_memberships.rb

@@ -0,0 +1,8 @@
+class AddDiscardedAtToMemberships < ActiveRecord::Migration[7.2]
+  disable_ddl_transaction!
+
+  def change
+    add_column :memberships, :discarded_at, :datetime
+    add_index :memberships, :discarded_at, algorithm: :concurrently
+  end
+end