diff options
author | Yorick Peterse <yorickpeterse@gmail.com> | 2016-05-26 13:38:28 +0200 |
---|---|---|
committer | Yorick Peterse <yorickpeterse@gmail.com> | 2016-06-01 16:22:35 +0200 |
commit | 580d250166d97bd5c2b0526be737d02806e577c2 (patch) | |
tree | 83f4266e3a09621eea233051eb7b08544791f183 /spec/models/ability_spec.rb | |
parent | 35e977d69b622e5a82be58c632ddc427d771cc09 (diff) | |
download | gitlab-ce-580d250166d97bd5c2b0526be737d02806e577c2.tar.gz |
Refactor Participable
There are several changes to this module:
1. The use of an explicit stack in Participable#participants
2. Proc behaviour has been changed
3. Batch permissions checking
== Explicit Stack
Participable#participants no longer uses recursion to process "self" and
all child objects, instead it uses an Array and processes objects in
breadth-first order. This allows us to for example create a single
Gitlab::ReferenceExtractor instance and pass this to any Procs. Re-using
a ReferenceExtractor removes the need for running potentially many SQL
queries every time a Proc is called on a new object.
== Proc Behaviour Changed
Previously a Proc in Participable was expected to return an Array of
User instances. This has been changed and instead it's now expected that
a Proc modifies the Gitlab::ReferenceExtractor passed to it. The return
value of the Proc is ignored.
== Permissions Checking
The method Participable#participants uses
Ability.users_that_can_read_project to check if the returned users have
access to the project of "self" _without_ running multiple SQL queries
for every user.
Diffstat (limited to 'spec/models/ability_spec.rb')
-rw-r--r-- | spec/models/ability_spec.rb | 117 |
1 files changed, 117 insertions, 0 deletions
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb new file mode 100644 index 00000000000..1acb5846fcf --- /dev/null +++ b/spec/models/ability_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper' + +describe Ability, lib: true do + describe '.users_that_can_read_project' do + context 'using a public project' do + it 'returns all the users' do + project = create(:project, :public) + user = build(:user) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + end + + context 'using an internal project' do + let(:project) { create(:project, :internal) } + + it 'returns users that are administrators' do + user = build(:user, admin: true) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + + it 'returns internal users while skipping external users' do + user1 = build(:user) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are the project owner' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project).to receive(:owner).twice.and_return(user1) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are project members' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project.team).to receive(:members).twice.and_return([user1]) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns an empty Array if all users are external users without access' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + end + + context 'using a private project' do + let(:project) { create(:project, :private) } + + it 'returns users that are administrators' do + user = build(:user, admin: true) + + expect(described_class.users_that_can_read_project([user], project)). + to eq([user]) + end + + it 'returns external users if they are the project owner' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project).to receive(:owner).twice.and_return(user1) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns external users if they are project members' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(project.team).to receive(:members).twice.and_return([user1]) + + expect(described_class.users_that_can_read_project(users, project)). + to eq([user1]) + end + + it 'returns an empty Array if all users are internal users without access' do + user1 = build(:user) + user2 = build(:user) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + + it 'returns an empty Array if all users are external users without access' do + user1 = build(:user, external: true) + user2 = build(:user, external: true) + users = [user1, user2] + + expect(described_class.users_that_can_read_project(users, project)). + to eq([]) + end + end + end +end |