Method for returning issues readable by a user

The method Ability.issues_readable_by_user takes a list of users and an
optional user and returns an Array of issues readable by said user. This
method in turn is used by
Banzai::ReferenceParser::IssueParser#nodes_visible_to_user so this
method no longer needs to get all the available abilities just to check
if a user has the "read_issue" ability.

To test this I benchmarked an issue with 222 comments on my development
environment. Using these changes the time spent in nodes_visible_to_user
was reduced from around 120 ms to around 40 ms.

4 files changed, 312 insertions, 7 deletions

diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb
index 514c752546d..85cfe728b6a 100644
--- a/spec/lib/banzai/reference_parser/issue_parser_spec.rb
+++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb
@@ -16,17 +16,17 @@ describe Banzai::ReferenceParser::IssueParser, lib: true do
       end
 
       it 'returns the nodes when the user can read the issue' do
-        expect(Ability.abilities).to receive(:allowed?).
-          with(user, :read_issue, issue).
-          and_return(true)
+        expect(Ability).to receive(:issues_readable_by_user).
+          with([issue], user).
+          and_return([issue])
 
         expect(subject.nodes_visible_to_user(user, [link])).to eq([link])
       end
 
       it 'returns an empty Array when the user can not read the issue' do
-        expect(Ability.abilities).to receive(:allowed?).
-          with(user, :read_issue, issue).
-          and_return(false)
+        expect(Ability).to receive(:issues_readable_by_user).
+          with([issue], user).
+          and_return([])
 
         expect(subject.nodes_visible_to_user(user, [link])).to eq([])
       end
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index cd5f40fe3d2..853f6943cef 100644
--- a/spec/models/ability_spec.rb
+++ b/spec/models/ability_spec.rb
@@ -170,4 +170,52 @@ describe Ability, lib: true do
       end
     end
   end
+
+  describe '.issues_readable_by_user' do
+    context 'with an admin user' do
+      it 'returns all given issues' do
+        user = build(:user, admin: true)
+        issue = build(:issue)
+
+        expect(described_class.issues_readable_by_user([issue], user)).
+          to eq([issue])
+      end
+    end
+
+    context 'with a regular user' do
+      it 'returns the issues readable by the user' do
+        user = build(:user)
+        issue = build(:issue)
+
+        expect(issue).to receive(:readable_by?).with(user).and_return(true)
+
+        expect(described_class.issues_readable_by_user([issue], user)).
+          to eq([issue])
+      end
+
+      it 'returns an empty Array when no issues are readable' do
+        user = build(:user)
+        issue = build(:issue)
+
+        expect(issue).to receive(:readable_by?).with(user).and_return(false)
+
+        expect(described_class.issues_readable_by_user([issue], user)).to eq([])
+      end
+    end
+
+    context 'without a regular user' do
+      it 'returns issues that are publicly visible' do
+        hidden_issue = build(:issue)
+        visible_issue = build(:issue)
+
+        expect(hidden_issue).to receive(:publicly_visible?).and_return(false)
+        expect(visible_issue).to receive(:publicly_visible?).and_return(true)
+
+        issues = described_class.
+          issues_readable_by_user([hidden_issue, visible_issue])
+
+        expect(issues).to eq([visible_issue])
+      end
+    end
+  end
 end
diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb
index 5e652660e2c..549b0042038 100644
--- a/spec/models/concerns/mentionable_spec.rb
+++ b/spec/models/concerns/mentionable_spec.rb
@@ -68,7 +68,7 @@ describe Issue, "Mentionable" do
 
   describe '#create_cross_references!' do
     let(:project) { create(:project) }
-    let(:author)  { double('author') }
+    let(:author)  { build(:user) }
     let(:commit)  { project.commit }
     let(:commit2) { project.commit }
 
@@ -88,6 +88,10 @@ describe Issue, "Mentionable" do
     let(:author)  { create(:author) }
     let(:issues)  { create_list(:issue, 2, project: project, author: author) }
 
+    before do
+      project.team << [author, Gitlab::Access::DEVELOPER]
+    end
+
     context 'before changes are persisted' do
       it 'ignores pre-existing references' do
         issue = create_issue(description: issues[0].to_reference)
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 6a897c96690..3259f795296 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -306,4 +306,257 @@ describe Issue, models: true do
       expect(user2.assigned_open_issues_count).to eq(1)
     end
   end
+
+  describe '#visible_to_user?' do
+    context 'with a user' do
+      let(:user) { build(:user) }
+      let(:issue) { build(:issue) }
+
+      it 'returns true when the issue is readable' do
+        expect(issue).to receive(:readable_by?).with(user).and_return(true)
+
+        expect(issue.visible_to_user?(user)).to eq(true)
+      end
+
+      it 'returns false when the issue is not readable' do
+        expect(issue).to receive(:readable_by?).with(user).and_return(false)
+
+        expect(issue.visible_to_user?(user)).to eq(false)
+      end
+    end
+
+    context 'without a user' do
+      let(:issue) { build(:issue) }
+
+      it 'returns true when the issue is publicly visible' do
+        expect(issue).to receive(:publicly_visible?).and_return(true)
+
+        expect(issue.visible_to_user?).to eq(true)
+      end
+
+      it 'returns false when the issue is not publicly visible' do
+        expect(issue).to receive(:publicly_visible?).and_return(false)
+
+        expect(issue.visible_to_user?).to eq(false)
+      end
+    end
+  end
+
+  describe '#readable_by?' do
+    describe 'with a regular user that is not a team member' do
+      let(:user) { create(:user) }
+
+      context 'using a public project' do
+        let(:project) { create(:empty_project, :public) }
+
+        it 'returns true for a regular issue' do
+          issue = build(:issue, project: project)
+
+          expect(issue).to be_readable_by(user)
+        end
+
+        it 'returns false for a confidential issue' do
+          issue = build(:issue, project: project, confidential: true)
+
+          expect(issue).not_to be_readable_by(user)
+        end
+      end
+
+      context 'using an internal project' do
+        let(:project) { create(:empty_project, :internal) }
+
+        context 'using an internal user' do
+          it 'returns true for a regular issue' do
+            issue = build(:issue, project: project)
+
+            expect(issue).to be_readable_by(user)
+          end
+
+          it 'returns false for a confidential issue' do
+            issue = build(:issue, :confidential, project: project)
+
+            expect(issue).not_to be_readable_by(user)
+          end
+        end
+
+        context 'using an external user' do
+          before do
+            allow(user).to receive(:external?).and_return(true)
+          end
+
+          it 'returns false for a regular issue' do
+            issue = build(:issue, project: project)
+
+            expect(issue).not_to be_readable_by(user)
+          end
+
+          it 'returns false for a confidential issue' do
+            issue = build(:issue, :confidential, project: project)
+
+            expect(issue).not_to be_readable_by(user)
+          end
+        end
+      end
+
+      context 'using a private project' do
+        let(:project) { create(:empty_project, :private) }
+
+        it 'returns false for a regular issue' do
+          issue = build(:issue, project: project)
+
+          expect(issue).not_to be_readable_by(user)
+        end
+
+        it 'returns false for a confidential issue' do
+          issue = build(:issue, :confidential, project: project)
+
+          expect(issue).not_to be_readable_by(user)
+        end
+
+        context 'when the user is the project owner' do
+          it 'returns true for a regular issue' do
+            issue = build(:issue, project: project)
+
+            expect(issue).not_to be_readable_by(user)
+          end
+
+          it 'returns true for a confidential issue' do
+            issue = build(:issue, :confidential, project: project)
+
+            expect(issue).not_to be_readable_by(user)
+          end
+        end
+      end
+    end
+
+    context 'with a regular user that is a team member' do
+      let(:user) { create(:user) }
+      let(:project) { create(:empty_project, :public) }
+
+      context 'using a public project' do
+        before do
+          project.team << [user, Gitlab::Access::DEVELOPER]
+        end
+
+        it 'returns true for a regular issue' do
+          issue = build(:issue, project: project)
+
+          expect(issue).to be_readable_by(user)
+        end
+
+        it 'returns true for a confidential issue' do
+          issue = build(:issue, :confidential, project: project)
+
+          expect(issue).to be_readable_by(user)
+        end
+      end
+
+      context 'using an internal project' do
+        let(:project) { create(:empty_project, :internal) }
+
+        before do
+          project.team << [user, Gitlab::Access::DEVELOPER]
+        end
+
+        it 'returns true for a regular issue' do
+          issue = build(:issue, project: project)
+
+          expect(issue).to be_readable_by(user)
+        end
+
+        it 'returns true for a confidential issue' do
+          issue = build(:issue, :confidential, project: project)
+
+          expect(issue).to be_readable_by(user)
+        end
+      end
+
+      context 'using a private project' do
+        let(:project) { create(:empty_project, :private) }
+
+        before do
+          project.team << [user, Gitlab::Access::DEVELOPER]
+        end
+
+        it 'returns true for a regular issue' do
+          issue = build(:issue, project: project)
+
+          expect(issue).to be_readable_by(user)
+        end
+
+        it 'returns true for a confidential issue' do
+          issue = build(:issue, :confidential, project: project)
+
+          expect(issue).to be_readable_by(user)
+        end
+      end
+    end
+
+    context 'with an admin user' do
+      let(:project) { create(:empty_project) }
+      let(:user) { create(:user, admin: true) }
+
+      it 'returns true for a regular issue' do
+        issue = build(:issue, project: project)
+
+        expect(issue).to be_readable_by(user)
+      end
+
+      it 'returns true for a confidential issue' do
+        issue = build(:issue, :confidential, project: project)
+
+        expect(issue).to be_readable_by(user)
+      end
+    end
+  end
+
+  describe '#publicly_visible?' do
+    context 'using a public project' do
+      let(:project) { create(:empty_project, :public) }
+
+      it 'returns true for a regular issue' do
+        issue = build(:issue, project: project)
+
+        expect(issue).to be_publicly_visible
+      end
+
+      it 'returns false for a confidential issue' do
+        issue = build(:issue, :confidential, project: project)
+
+        expect(issue).not_to be_publicly_visible
+      end
+    end
+
+    context 'using an internal project' do
+      let(:project) { create(:empty_project, :internal) }
+
+      it 'returns false for a regular issue' do
+        issue = build(:issue, project: project)
+
+        expect(issue).not_to be_publicly_visible
+      end
+
+      it 'returns false for a confidential issue' do
+        issue = build(:issue, :confidential, project: project)
+
+        expect(issue).not_to be_publicly_visible
+      end
+    end
+
+    context 'using a private project' do
+      let(:project) { create(:empty_project, :private) }
+
+      it 'returns false for a regular issue' do
+        issue = build(:issue, project: project)
+
+        expect(issue).not_to be_publicly_visible
+      end
+
+      it 'returns false for a confidential issue' do
+        issue = build(:issue, :confidential, project: project)
+
+        expect(issue).not_to be_publicly_visible
+      end
+    end
+  end
 end