From 0ba5647d5ae3ce9e0b055316be78d7f3d9486203 Mon Sep 17 00:00:00 2001
From: Rob Paskin <rjpaskin+git@gmail.com>
Date: Wed, 10 May 2023 21:59:56 +0100
Subject: [PATCH] Fix no-longer-supported parameterised scope

Rails now throws an error if we `includes(:head_commit_statuses)`. Since
we don't eager-load that scope anywhere, we just remove that part of the
scope, and add the additional `repo_id` foreign key part to the instance
method where it's used.

Since we don't have the switch between eager-loading and instance
method, we delete the tests for this as well since we no longer need
them.
---
 app/models/pull_request.rb       | 13 ++-----------
 spec/models/pull_request_spec.rb | 24 ------------------------
 2 files changed, 2 insertions(+), 35 deletions(-)

diff --git a/app/models/pull_request.rb b/app/models/pull_request.rb
index d54c8bdc..81979646 100644
--- a/app/models/pull_request.rb
+++ b/app/models/pull_request.rb
@@ -18,17 +18,7 @@ class PullRequest < ApplicationRecord
     foreign_key: :creator_remote_id, primary_key: :uid
   has_many :pull_request_connections, autosave: true
   has_many :tickets, through: :pull_request_connections
-  has_many :head_commit_statuses, -> (model = nil) {
-    if model
-      where(repo_id: model.repo_id)
-    else # eager-loading/join
-      joins(
-        table.join(PullRequest.arel_table).on(
-          PullRequest.arel_table[:head_sha].eq(table[:sha])
-        ).join_sources
-      ).where(PullRequest.arel_table[:repo_id].eq(table[:repo_id]))
-    end
-  }, class_name: 'CommitStatus', foreign_key: :sha, primary_key: :head_sha
+  has_many :head_commit_statuses, class_name: 'CommitStatus', foreign_key: :sha, primary_key: :head_sha
   has_many :reviews, class_name: 'PullRequestReview', foreign_key: :remote_pull_request_id, primary_key: :remote_id
 
   before_save :update_pull_request_connections
@@ -82,6 +72,7 @@ def html_url
 
   def latest_commit_statuses
     head_commit_statuses
+      .where(repo_id: repo_id)
       .group_by(&:context)
       .map {|_, records| records.max_by(&:remote_created_at) }
   end
diff --git a/spec/models/pull_request_spec.rb b/spec/models/pull_request_spec.rb
index c4088413..b084b03a 100644
--- a/spec/models/pull_request_spec.rb
+++ b/spec/models/pull_request_spec.rb
@@ -6,30 +6,6 @@
     it { is_expected.to have_many(:pull_request_connections) }
     it { is_expected.to have_many(:tickets).through(:pull_request_connections) }
     it { is_expected.to have_many(:reviews).with_primary_key(:remote_id) }
-
-    describe 'head_commit_statuses' do
-      let(:sha) { generate(:sha) }
-      let(:repo) { create(:repo) }
-      let(:other_repo) { create(:repo) }
-
-      let!(:status) { create(:commit_status, repo: repo, sha: sha) }
-      let!(:other_repo_status) { create(:commit_status, repo: other_repo, sha: sha) }
-
-      let(:pull_request) { create(:pull_request, repo: repo, head_sha: sha) }
-      it 'only loads commit statuses from the same repo' do
-        expect(pull_request.head_commit_statuses).to match_array([status])
-      end
-
-      it 'supports eager loading' do
-        matcher = an_instance_of(described_class).and(
-          an_object_having_attributes(id: pull_request.id, head_commit_statuses: [status])
-        )
-        aggregate_failures do
-          expect(described_class.includes(:head_commit_statuses)).to include(matcher)
-          expect(repo.pull_request_models.includes(:head_commit_statuses)).to include(matcher)
-        end
-      end
-    end
   end
 
   describe '.import' do