Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete an organisation member #711

Merged
merged 18 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 29 additions & 27 deletions app/components/v2/header_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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 %>
Expand Down
16 changes: 14 additions & 2 deletions app/controllers/accounts/invitations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
30 changes: 30 additions & 0 deletions app/controllers/accounts/memberships_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 9 additions & 2 deletions app/controllers/signed_in_application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -176,7 +176,7 @@ def default_app
end

def new_app
current_organization.apps.new
current_organization&.apps&.new
end

def vcs_provider_logo
Expand Down Expand Up @@ -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
2 changes: 1 addition & 1 deletion app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
6 changes: 6 additions & 0 deletions app/helpers/organizations_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 13 additions & 0 deletions app/models/accounts/membership.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -11,6 +12,7 @@
# user_id :uuid indexed => [organization_id, role]
#
class Accounts::Membership < ApplicationRecord
include Discard::Model
include Roleable
has_paper_trail

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/models/accounts/organization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions app/models/accounts/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
33 changes: 27 additions & 6 deletions app/views/accounts/organizations/_teams.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 %>
Expand Down Expand Up @@ -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 %>
Expand Down
4 changes: 4 additions & 0 deletions app/views/shared/no_organization.html.erb
Original file line number Diff line number Diff line change
@@ -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") %>
1 change: 1 addition & 0 deletions compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ x-app: &app
tmpfs:
- /tmp
- /app/tmp/pids
- /rails/tmp/pids

x-backend: &backend
<<: *app
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 2 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions db/migrate/20241102204836_add_discarded_at_to_memberships.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading