Skip to content
This repository has been archived by the owner on Feb 28, 2024. It is now read-only.

Commit

Permalink
Fix no-longer-supported parameterised scope
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rjpaskin committed May 15, 2023
1 parent bd2c5c3 commit 0ba5647
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 35 deletions.
13 changes: 2 additions & 11 deletions app/models/pull_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
24 changes: 0 additions & 24 deletions spec/models/pull_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 0ba5647

Please sign in to comment.