Fix attr reader to force the intended values for source and target shas

When importing a pull request from GitHub, the old and new branches may no longer actually exist by those names, but we need to recreate the merge request diff with the right source and target shas. We use these `target_branch_sha` and `source_branch_sha` attributes to force these to the intended values. But the reader methods were always looking up to the target/source branch head instead of check if these values was previously set.

Fix attr reader to force the intended values for source and target shas
5b4ceeed · Douglas Barbosa Alexandre · e299504b · 5b4ceeed · 5b4ceeed
Commit 5b4ceeed authored Jul 29, 2016 by Douglas Barbosa Alexandre
Hide whitespace changes
Inline Side-by-side

Showing with 19 additions and 3 deletions

merge_request.rb app/models/merge_request.rb +4 -0

merge_request_spec.rb spec/models/merge_request_spec.rb +15 -3

No files found.
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -238,10 +238,14 @@ class MergeRequest < ActiveRecord::Base
  end

  def target_branch_sha
+    return @target_branch_sha if defined?(@target_branch_sha)
+
    target_branch_head.try(:sha)
  end

  def source_branch_sha
+    return @source_branch_sha if defined?(@source_branch_sha)
+
    source_branch_head.try(:sha)
  end


--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -65,11 +65,11 @@ describe MergeRequest, models: true do
  end

  describe '#target_branch_sha' do
-    context 'when the target branch does not exist anymore' do
-      let(:project) { create(:project) }
+    let(:project) { create(:project) }

-      subject { create(:merge_request, source_project: project, target_project: project) }
+    subject { create(:merge_request, source_project: project, target_project: project) }

+    context 'when the target branch does not exist' do
      before do
        project.repository.raw_repository.delete_branch(subject.target_branch)
      end
@@ -78,6 +78,12 @@ describe MergeRequest, models: true do
        expect(subject.target_branch_sha).to be_nil
      end
    end
+
+    it 'returns memoized value' do
+      subject.target_branch_sha = '8ffb3c15a5475e59ae909384297fede4badcb4c7'
+
+      expect(subject.target_branch_sha).to eq '8ffb3c15a5475e59ae909384297fede4badcb4c7'
+    end
  end

  describe '#source_branch_sha' do
@@ -103,6 +109,12 @@ describe MergeRequest, models: true do
        expect(subject.source_branch_sha).to be_nil
      end
    end
+
+    it 'returns memoized value' do
+      subject.source_branch_sha = '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
+
+      expect(subject.source_branch_sha).to eq '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'
+    end
  end

  describe '#to_reference' do