Commit 6a05f246 authored by Rubén Dávila Santos's avatar Rubén Dávila Santos

Merge branch 'fix-rspec-failures-due-to-cached-permissions' into 'master'

Fix assorted rspec failures due to stale, cached user permissions

RequestStore is disabled in tests, but the Ability class was caching user permissions based on the user and project ID of previous test runs. Revise code to use RequestStore only if it is active.

See merge request !5919
parents a16ac37e 1954bb17
...@@ -166,10 +166,17 @@ class Ability ...@@ -166,10 +166,17 @@ class Ability
end end
def project_abilities(user, project) def project_abilities(user, project)
rules = []
key = "/user/#{user.id}/project/#{project.id}" key = "/user/#{user.id}/project/#{project.id}"
RequestStore.store[key] ||= begin if RequestStore.active?
RequestStore.store[key] ||= uncached_project_abilities(user, project)
else
uncached_project_abilities(user, project)
end
end
def uncached_project_abilities(user, project)
rules = []
# Push abilities on the users team role # Push abilities on the users team role
rules.push(*project_team_rules(project.team, user)) rules.push(*project_team_rules(project.team, user))
...@@ -196,8 +203,7 @@ class Ability ...@@ -196,8 +203,7 @@ class Ability
rules -= project_archived_rules rules -= project_archived_rules
end end
rules - project_disabled_features_rules(project) (rules - project_disabled_features_rules(project)).uniq
end
end end
def project_team_rules(team, user) def project_team_rules(team, user)
......
...@@ -171,6 +171,70 @@ describe Ability, lib: true do ...@@ -171,6 +171,70 @@ describe Ability, lib: true do
end end
end end
shared_examples_for ".project_abilities" do |enable_request_store|
before do
RequestStore.begin! if enable_request_store
end
after do
if enable_request_store
RequestStore.end!
RequestStore.clear!
end
end
describe '.project_abilities' do
let!(:project) { create(:empty_project, :public) }
let!(:user) { create(:user) }
it 'returns permissions for admin user' do
admin = create(:admin)
results = described_class.project_abilities(admin, project)
expect(results.count).to eq(68)
end
it 'returns permissions for an owner' do
results = described_class.project_abilities(project.owner, project)
expect(results.count).to eq(68)
end
it 'returns permissions for a master' do
project.team << [user, :master]
results = described_class.project_abilities(user, project)
expect(results.count).to eq(60)
end
it 'returns permissions for a developer' do
project.team << [user, :developer]
results = described_class.project_abilities(user, project)
expect(results.count).to eq(44)
end
it 'returns permissions for a guest' do
project.team << [user, :guest]
results = described_class.project_abilities(user, project)
expect(results.count).to eq(21)
end
end
end
describe '.project_abilities with RequestStore' do
it_behaves_like ".project_abilities", true
end
describe '.project_abilities without RequestStore' do
it_behaves_like ".project_abilities", false
end
describe '.issues_readable_by_user' do describe '.issues_readable_by_user' do
context 'with an admin user' do context 'with an admin user' do
it 'returns all given issues' do it 'returns all given issues' do
......
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