Filter visible related issues for a given Issue

app/services/related_issues/list_service.rb

@@ -7,14 +7,7 @@ module RelatedIssues
     end
 
     def execute
-      related_issues.map do |related_issue|
-        referenced_issue =
-          if related_issue.related_issue == @issue
-            related_issue.issue
-          else
-            related_issue.related_issue
-          end
-
+      issues.map do |referenced_issue|
         {
           title: referenced_issue.title,
           state: referenced_issue.state,
@@ -26,11 +19,25 @@ module RelatedIssues
 
     private
 
-    def related_issues
-      RelatedIssue
-        .where("issue_id = #{@issue.id} OR related_issue_id = #{@issue.id}")
-        .preload(related_issue: :project, issue: :project)
-        .order(:created_at)
+    def issues
+      return @issues if defined?(@issues)
+
+      # TODO: Simplify query using AR
+      @issues = Issue.find_by_sql(
+        <<-SQL.strip_heredoc
+          SELECT issues.*, related_issues.id as related_issues_id FROM issues
+          INNER JOIN related_issues ON related_issues.related_issue_id = issues.id
+          WHERE related_issues.issue_id = #{@issue.id}
+          UNION ALL
+          SELECT issues.*, related_issues.id as related_issues_id FROM issues
+          INNER JOIN related_issues ON related_issues.issue_id = issues.id
+          WHERE related_issues.related_issue_id = #{@issue.id}
+          ORDER BY related_issues_id
+        SQL
+      )
+
+      # TODO: Try to use SQL instead Array#select
+      @issues = Ability.issues_readable_by_user(@issues, @current_user)
     end
   end
 end

spec/services/related_issues/list_service_spec.rb

 require 'spec_helper'
 
 describe RelatedIssues::ListService, service: true do
+  let(:user) { create :user }
+  let(:project) { create(:project_empty_repo) }
+  let(:issue) { create :issue, project: project }
+
+  before do
+    project.team << [user, :developer]
+  end
+
   describe '#execute' do
-    let(:user) { create :user }
-    let(:project) { create(:project_empty_repo) }
-    let(:issue) { create :issue, project: project }
-
-    let(:issue_b) { create :issue, project: project }
-    let(:issue_c) { create :issue, project: project }
-    let(:issue_d) { create :issue, project: project }
-
-    let!(:related_issue_c) do
-      create(:related_issue, issue: issue_d,
-                             related_issue: issue,
-                             created_at: Date.today)
-    end
+    subject { described_class.new(issue, user).execute }
 
-    let!(:related_issue_b) do
-      create(:related_issue, issue: issue,
-                             related_issue: issue_c,
-                             created_at: 1.day.ago)
-    end
+    context 'user can see all issues' do
+      let(:issue_b) { create :issue, project: project }
+      let(:issue_c) { create :issue, project: project }
+      let(:issue_d) { create :issue, project: project }
 
-    let!(:related_issue_a) do
-      create(:related_issue, issue: issue,
-                             related_issue: issue_b,
-                             created_at: 2.days.ago)
-    end
+      let!(:related_issue_c) do
+        create(:related_issue, id: 999,
+                               issue: issue_d,
+                               related_issue: issue,
+                               created_at: Date.today)
+      end
 
-    subject { described_class.new(issue, user).execute }
+      let!(:related_issue_b) do
+        create(:related_issue, id: 998,
+                               issue: issue,
+                               related_issue: issue_c,
+                               created_at: 1.day.ago)
+      end
 
-    it 'verifies number of queries' do
-      recorded = ActiveRecord::QueryRecorder.new { subject }
-      expect(recorded.count).to be_within(1).of(29)
+      let!(:related_issue_a) do
+        create(:related_issue, id: 997,
+                               issue: issue,
+                               related_issue: issue_b,
+                               created_at: 2.days.ago)
+      end
+
+      it 'verifies number of queries' do
+        recorded = ActiveRecord::QueryRecorder.new { subject }
+        expect(recorded.count).to be_within(1).of(25)
+      end
+
+      it 'returns related issues JSON' do
+        expect(subject.size).to eq(3)
+
+        expect(subject[0]).to eq(
+          {
+            title: issue_b.title,
+            state: issue_b.state,
+            reference: issue_b.to_reference(project),
+            path: "/#{project.full_path}/issues/#{issue_b.iid}"
+          }
+        )
+
+        expect(subject[1]).to eq(
+          {
+            title: issue_c.title,
+            state: issue_c.state,
+            reference: issue_c.to_reference(project),
+            path: "/#{project.full_path}/issues/#{issue_c.iid}"
+          }
+        )
+
+        expect(subject[2]).to eq(
+          {
+            title: issue_d.title,
+            state: issue_d.state,
+            reference: issue_d.to_reference(project),
+            path: "/#{project.full_path}/issues/#{issue_d.iid}"
+          }
+        )
+      end
     end
 
-    it 'returns related issues JSON' do
-      expect(subject.size).to eq(3)
-
-      expect(subject[0]).to eq(
-        {
-          title: issue_b.title,
-          state: issue_b.state,
-          reference: issue_b.to_reference(project),
-          path: "/#{project.full_path}/issues/#{issue_b.iid}"
-        }
-      )
-
-      expect(subject[1]).to eq(
-        {
-          title: issue_c.title,
-          state: issue_c.state,
-          reference: issue_c.to_reference(project),
-          path: "/#{project.full_path}/issues/#{issue_c.iid}"
-        }
-      )
-
-      expect(subject[2]).to eq(
-        {
-          title: issue_d.title,
-          state: issue_d.state,
-          reference: issue_d.to_reference(project),
-          path: "/#{project.full_path}/issues/#{issue_d.iid}"
-        }
-      )
+    context 'user cannot see relations' do
+      context 'when user cannot see the referenced issue' do
+        let!(:related_issue) do
+          create(:related_issue, issue: issue)
+        end
+
+        it 'returns an empty list' do
+          is_expected.to eq([])
+        end
+      end
+
+      context 'when user cannot see the issue that referenced' do
+        let!(:related_issue) do
+          create(:related_issue, related_issue: issue)
+        end
+
+        it 'returns an empty list' do
+          is_expected.to eq([])
+        end
+      end
     end
   end
 end