Commit 17bee986 authored by Gosia Ksionek's avatar Gosia Ksionek Committed by James Lopez

Add cr remarks

Chnage method used in model to make it
more efficient database-wise
Add additional spec
parent 702f1826
...@@ -228,22 +228,21 @@ class Group < Namespace ...@@ -228,22 +228,21 @@ class Group < Namespace
def has_owner?(user) def has_owner?(user)
return false unless user return false unless user
members_with_parents.owners.where(user_id: user).any? members_with_parents.owners.exists?(user_id: user)
end end
def has_maintainer?(user) def has_maintainer?(user)
return false unless user return false unless user
members_with_parents.maintainers.where(user_id: user).any? members_with_parents.maintainers.exists?(user_id: user)
end end
# @deprecated # @deprecated
alias_method :has_master?, :has_maintainer? alias_method :has_master?, :has_maintainer?
# Check if user is a last owner of the group. # Check if user is a last owner of the group.
# Parent owners are ignored for nested groups.
def last_owner?(user) def last_owner?(user)
owners.include?(user) && owners.size == 1 has_owner?(user) && members_with_parents.owners.size == 1
end end
def ldap_synced? def ldap_synced?
......
---
title: Allow removing last owner from subgroup if parent group has owners
merge_request: 26718
author:
type: changed
...@@ -364,6 +364,32 @@ describe Group do ...@@ -364,6 +364,32 @@ describe Group do
it { expect(group.has_maintainer?(nil)).to be_falsey } it { expect(group.has_maintainer?(nil)).to be_falsey }
end end
describe '#last_owner?' do
before do
@members = setup_group_members(group)
end
it { expect(group.last_owner?(@members[:owner])).to be_truthy }
context 'with two owners' do
before do
create(:group_member, :owner, group: group)
end
it { expect(group.last_owner?(@members[:owner])).to be_falsy }
end
context 'with owners from a parent', :postgresql do
before do
parent_group = create(:group)
create(:group_member, :owner, group: parent_group)
group.update(parent: parent_group)
end
it { expect(group.last_owner?(@members[:owner])).to be_falsy }
end
end
describe '#lfs_enabled?' do describe '#lfs_enabled?' do
context 'LFS enabled globally' do context 'LFS enabled globally' do
before do before do
......
# frozen_string_literal: true
require 'spec_helper'
describe GroupMemberPolicy do
let(:guest) { create(:user) }
let(:owner) { create(:user) }
let(:group) { create(:group, :private) }
before do
group.add_guest(guest)
group.add_owner(owner)
end
let(:member_related_permissions) do
[:update_group_member, :destroy_group_member]
end
let(:membership) { current_user.members.first }
subject { described_class.new(current_user, membership) }
def expect_allowed(*permissions)
permissions.each { |p| is_expected.to be_allowed(p) }
end
def expect_disallowed(*permissions)
permissions.each { |p| is_expected.not_to be_allowed(p) }
end
context 'with guest user' do
let(:current_user) { guest }
it do
expect_disallowed(:member_related_permissions)
end
end
context 'with one owner' do
let(:current_user) { owner }
it do
expect_disallowed(:destroy_group_member)
expect_disallowed(:update_group_member)
end
end
context 'with more than one owner' do
let(:current_user) { owner }
before do
group.add_owner(create(:user))
end
it do
expect_allowed(:destroy_group_member)
expect_allowed(:update_group_member)
end
end
context 'with the group parent', :postgresql do
let(:current_user) { create :user }
let(:subgroup) { create(:group, :private, parent: group)}
before do
group.add_owner(owner)
subgroup.add_owner(current_user)
end
it do
expect_allowed(:destroy_group_member)
expect_allowed(:update_group_member)
end
end
context 'without group parent' do
let(:current_user) { create :user }
let(:subgroup) { create(:group, :private)}
before do
subgroup.add_owner(current_user)
end
it do
expect_disallowed(:destroy_group_member)
expect_disallowed(:update_group_member)
end
end
context 'without group parent with two owners' do
let(:current_user) { create :user }
let(:other_user) { create :user }
let(:subgroup) { create(:group, :private)}
before do
subgroup.add_owner(current_user)
subgroup.add_owner(other_user)
end
it do
expect_allowed(:destroy_group_member)
expect_allowed(:update_group_member)
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment