Merge branch '18586-user-authorized_projects-is-slow' into 'master'

Refactor user authorization check for a single project to avoid querying all user projects

See merge request !5102

2 files changed, 69 insertions, 0 deletions

diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index b87d68283e6..6a897c96690 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -22,6 +22,26 @@ describe Issue, models: true do
     it { is_expected.to have_db_index(:deleted_at) }
   end
 
+  describe 'visible_to_user' do
+    let(:user) { create(:user) }
+    let(:authorized_user) { create(:user) }
+    let(:project) { create(:project, namespace: authorized_user.namespace) }
+    let!(:public_issue) { create(:issue, project: project) }
+    let!(:confidential_issue) { create(:issue, project: project, confidential: true) }
+
+    it 'returns non confidential issues for nil user' do
+      expect(Issue.visible_to_user(nil).count).to be(1)
+    end
+
+    it 'returns non confidential issues for user not authorized for the issues projects' do
+      expect(Issue.visible_to_user(user).count).to be(1)
+    end
+
+    it 'returns all issues for user authorized for the issues projects' do
+      expect(Issue.visible_to_user(authorized_user).count).to be(2)
+    end
+  end
+
   describe '#to_reference' do
     it 'returns a String reference to the object' do
       expect(subject.to_reference).to eq "##{subject.iid}"
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index e3e7319beb2..a2b089c51e2 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1187,4 +1187,53 @@ describe Project, models: true do
       end
     end
   end
+
+  describe 'authorized_for_user' do
+    let(:group) { create(:group) }
+    let(:developer) { create(:user) }
+    let(:master) { create(:user) }
+    let(:personal_project) { create(:project, namespace: developer.namespace) }
+    let(:group_project) { create(:project, namespace: group) }
+    let(:members_project) { create(:project) }
+    let(:shared_project) { create(:project) }
+
+    before do
+      group.add_master(master)
+      group.add_developer(developer)
+
+      members_project.team << [developer, :developer]
+      members_project.team << [master, :master]
+
+      create(:project_group_link, project: shared_project, group: group)
+    end
+
+    it 'returns false for no user' do
+      expect(personal_project.authorized_for_user?(nil)).to be(false)
+    end
+
+    it 'returns true for personal projects of the user' do
+      expect(personal_project.authorized_for_user?(developer)).to be(true)
+    end
+
+    it 'returns true for projects of groups the user is a member of' do
+      expect(group_project.authorized_for_user?(developer)).to be(true)
+    end
+
+    it 'returns true for projects for which the user is a member of' do
+      expect(members_project.authorized_for_user?(developer)).to be(true)
+    end
+
+    it 'returns true for projects shared on a group the user is a member of' do
+      expect(shared_project.authorized_for_user?(developer)).to be(true)
+    end
+
+    it 'checks for the correct minimum level access' do
+      expect(group_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false)
+      expect(group_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true)
+      expect(members_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false)
+      expect(members_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true)
+      expect(shared_project.authorized_for_user?(developer, Gitlab::Access::MASTER)).to be(false)
+      expect(shared_project.authorized_for_user?(master, Gitlab::Access::MASTER)).to be(true)
+    end
+  end
 end