From cd88fa8f80710ec977a85ab8701570073c94f017 Mon Sep 17 00:00:00 2001
From: "micael.bergeron" <micaelbergeron@gmail.com>
Date: Wed, 1 Nov 2017 10:31:35 -0400
Subject: [PATCH 1/7] removed the #ensure_ref_fetched from all controllers

also, I refactored the MergeRequest#fetch_ref method to express
the side-effect that this method has.

  MergeRequest#fetch_ref -> MergeRequest#fetch_ref!
  Repository#fetch_source_branch -> Repository#fetch_source_branch!
---
 .../merge_requests/application_controller.rb  |  7 ----
 .../merge_requests/creations_controller.rb    |  1 -
 .../projects/merge_requests_controller.rb     |  2 -
 app/models/merge_request.rb                   | 27 ++------------
 app/models/repository.rb                      |  4 +-
 config/initializers/8_metrics.rb              |  3 +-
 ..._remove_ref_fetched_from_merge_requests.rb | 14 +++++++
 db/schema.rb                                  |  3 +-
 lib/github/import.rb                          |  1 -
 lib/gitlab/git/repository.rb                  |  2 +-
 spec/lib/gitlab/git/repository_spec.rb        | 10 ++---
 spec/lib/gitlab/import_export/fork_spec.rb    |  2 +-
 spec/models/merge_request_spec.rb             | 37 +++----------------
 13 files changed, 33 insertions(+), 80 deletions(-)
 create mode 100644 db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb

diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb
index 0e71977a58a..1269759fc2b 100644
--- a/app/controllers/projects/merge_requests/application_controller.rb
+++ b/app/controllers/projects/merge_requests/application_controller.rb
@@ -2,7 +2,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
   before_action :check_merge_requests_available!
   before_action :merge_request
   before_action :authorize_read_merge_request!
-  before_action :ensure_ref_fetched
 
   private
 
@@ -10,12 +9,6 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
     @issuable = @merge_request ||= @project.merge_requests.find_by!(iid: params[:id])
   end
 
-  # Make sure merge requests created before 8.0
-  # have head file in refs/merge-requests/
-  def ensure_ref_fetched
-    @merge_request.ensure_ref_fetched if Gitlab::Database.read_write?
-  end
-
   def merge_request_params
     params.require(:merge_request).permit(merge_request_params_attributes)
   end
diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index 99dc3dda9e7..129682f64aa 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -4,7 +4,6 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
   include RendersCommits
 
   skip_before_action :merge_request
-  skip_before_action :ensure_ref_fetched
   before_action :authorize_create_merge_request!
   before_action :apply_diff_view_cookie!, only: [:diffs, :diff_for_path]
   before_action :build_merge_request, except: [:create]
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 17cac69e588..c86acae8fe4 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -7,7 +7,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   include IssuableCollections
 
   skip_before_action :merge_request, only: [:index, :bulk_update]
-  skip_before_action :ensure_ref_fetched, only: [:index, :bulk_update]
 
   before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort]
 
@@ -52,7 +51,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
 
   def show
     validates_merge_request
-    ensure_ref_fetched
     close_merge_request_without_source_project
     check_if_can_be_merged
 
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 3133dc9e7eb..ccad8e102aa 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -426,7 +426,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def create_merge_request_diff
-    fetch_ref
+    fetch_ref!
 
     # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37435
     Gitlab::GitalyClient.allow_n_plus_1_calls do
@@ -811,29 +811,14 @@ class MergeRequest < ActiveRecord::Base
     end
   end
 
-  def fetch_ref
-    write_ref
-    update_column(:ref_fetched, true)
+  def fetch_ref!
+    target_project.repository.fetch_source_branch!(source_project.repository, source_branch, ref_path)
   end
 
   def ref_path
     "refs/#{Repository::REF_MERGE_REQUEST}/#{iid}/head"
   end
 
-  def ref_fetched?
-    super ||
-      begin
-        computed_value = project.repository.ref_exists?(ref_path)
-        update_column(:ref_fetched, true) if computed_value
-
-        computed_value
-      end
-  end
-
-  def ensure_ref_fetched
-    fetch_ref unless ref_fetched?
-  end
-
   def in_locked_state
     begin
       lock_mr
@@ -975,10 +960,4 @@ class MergeRequest < ActiveRecord::Base
 
     project.merge_requests.merged.where(author_id: author_id).empty?
   end
-
-  private
-
-  def write_ref
-    target_project.repository.fetch_source_branch(source_project.repository, source_branch, ref_path)
-  end
 end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 44a1e9ce529..3b6285f786d 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -982,8 +982,8 @@ class Repository
     gitlab_shell.fetch_remote(raw_repository, remote, forced: forced, no_tags: no_tags)
   end
 
-  def fetch_source_branch(source_repository, source_branch, local_ref)
-    raw_repository.fetch_source_branch(source_repository.raw_repository, source_branch, local_ref)
+  def fetch_source_branch!(source_repository, source_branch, local_ref)
+    raw_repository.fetch_source_branch!(source_repository.raw_repository, source_branch, local_ref)
   end
 
   def compare_source_branch(target_branch_name, source_repository, source_branch_name, straight:)
diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb
index 2d8704622b6..4fa72a494d0 100644
--- a/config/initializers/8_metrics.rb
+++ b/config/initializers/8_metrics.rb
@@ -118,8 +118,7 @@ def instrument_classes(instrumentation)
   instrumentation.instrument_instance_method(MergeRequestDiff, :load_commits)
 
   # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/36061
-  instrumentation.instrument_instance_method(MergeRequest, :ensure_ref_fetched)
-  instrumentation.instrument_instance_method(MergeRequest, :fetch_ref)
+  instrumentation.instrument_instance_method(MergeRequest, :fetch_ref!)
 end
 # rubocop:enable Metrics/AbcSize
 
diff --git a/db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb b/db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
new file mode 100644
index 00000000000..acba8384da4
--- /dev/null
+++ b/db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
@@ -0,0 +1,14 @@
+class RemoveRefFetchedFromMergeRequests < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # We don't need to cache this anymore: the refs are now created
+  # upon save/update and there is no more use for this flag
+  #
+  # See https://gitlab.com/gitlab-org/gitlab-ce/issues/36061
+  def change
+    remove_column :merge_requests, :ref_fetched
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 80d8ff92d6e..7f1cfed4bb8 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171026082505) do
+ActiveRecord::Schema.define(version: 20171101134435) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -970,7 +970,6 @@ ActiveRecord::Schema.define(version: 20171026082505) do
     t.datetime "last_edited_at"
     t.integer "last_edited_by_id"
     t.integer "head_pipeline_id"
-    t.boolean "ref_fetched"
     t.string "merge_jid"
     t.boolean "discussion_locked"
     t.integer "latest_merge_request_diff_id"
diff --git a/lib/github/import.rb b/lib/github/import.rb
index 76612799412..13ecf58bd37 100644
--- a/lib/github/import.rb
+++ b/lib/github/import.rb
@@ -161,7 +161,6 @@ module Github
               iid: pull_request.iid,
               title: pull_request.title,
               description: description,
-              ref_fetched: true,
               source_project: pull_request.source_project,
               source_branch: pull_request.source_branch_name,
               source_branch_sha: pull_request.source_branch_sha,
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 4f9eac92d9a..00f19257994 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -1034,7 +1034,7 @@ module Gitlab
         delete_refs(tmp_ref) if tmp_ref
       end
 
-      def fetch_source_branch(source_repository, source_branch, local_ref)
+      def fetch_source_branch!(source_repository, source_branch, local_ref)
         with_repo_branch_commit(source_repository, source_branch) do |commit|
           if commit
             write_ref(local_ref, commit.sha)
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index bf6d199ebe2..816157d8c0f 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1482,7 +1482,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
   end
 
-  describe '#fetch_source_branch' do
+  describe '#fetch_source_branch!' do
     let(:local_ref) { 'refs/merge-requests/1/head' }
 
     context 'when the branch exists' do
@@ -1491,11 +1491,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
       it 'writes the ref' do
         expect(repository).to receive(:write_ref).with(local_ref, /\h{40}/)
 
-        repository.fetch_source_branch(repository, source_branch, local_ref)
+        repository.fetch_source_branch!(repository, source_branch, local_ref)
       end
 
       it 'returns true' do
-        expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(true)
+        expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(true)
       end
     end
 
@@ -1505,11 +1505,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
       it 'does not write the ref' do
         expect(repository).not_to receive(:write_ref)
 
-        repository.fetch_source_branch(repository, source_branch, local_ref)
+        repository.fetch_source_branch!(repository, source_branch, local_ref)
       end
 
       it 'returns false' do
-        expect(repository.fetch_source_branch(repository, source_branch, local_ref)).to eq(false)
+        expect(repository.fetch_source_branch!(repository, source_branch, local_ref)).to eq(false)
       end
     end
   end
diff --git a/spec/lib/gitlab/import_export/fork_spec.rb b/spec/lib/gitlab/import_export/fork_spec.rb
index dd0ce0dae41..cfb15ee7e8b 100644
--- a/spec/lib/gitlab/import_export/fork_spec.rb
+++ b/spec/lib/gitlab/import_export/fork_spec.rb
@@ -46,7 +46,7 @@ describe 'forked project import' do
   end
 
   it 'can access the MR' do
-    project.merge_requests.first.ensure_ref_fetched
+    project.merge_requests.first.fetch_ref!
 
     expect(project.repository.ref_exists?('refs/merge-requests/1/head')).to be_truthy
   end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 476a2697605..f719ee26767 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1755,39 +1755,12 @@ describe MergeRequest do
     end
   end
 
-  describe '#fetch_ref' do
-    it 'sets "ref_fetched" flag to true' do
-      subject.update!(ref_fetched: nil)
+  describe '#fetch_ref!' do
+    it 'fetch the ref correctly' do
+      expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error
 
-      subject.fetch_ref
-
-      expect(subject.reload.ref_fetched).to be_truthy
-    end
-  end
-
-  describe '#ref_fetched?' do
-    it 'does not perform git operation when value is cached' do
-      subject.ref_fetched = true
-
-      expect_any_instance_of(Repository).not_to receive(:ref_exists?)
-      expect(subject.ref_fetched?).to be_truthy
-    end
-
-    it 'caches the value when ref exists but value is not cached' do
-      subject.update!(ref_fetched: nil)
-      allow_any_instance_of(Repository).to receive(:ref_exists?)
-        .and_return(true)
-
-      expect(subject.ref_fetched?).to be_truthy
-      expect(subject.reload.ref_fetched).to be_truthy
-    end
-
-    it 'returns false when ref does not exist' do
-      subject.update!(ref_fetched: nil)
-      allow_any_instance_of(Repository).to receive(:ref_exists?)
-        .and_return(false)
-
-      expect(subject.ref_fetched?).to be_falsey
+      subject.fetch_ref!
+      expect(subject.target_project.repository.ref_exists?(subject.ref_path)).to be_truthy
     end
   end
 

From 5c367a2a36dec7a4b4a6f0b911db5551d7fd8411 Mon Sep 17 00:00:00 2001
From: "micael.bergeron" <micaelbergeron@gmail.com>
Date: Wed, 1 Nov 2017 11:20:06 -0400
Subject: [PATCH 2/7] add changelog and move migration to post_migrate

---
 .../remove-ensure-ref-fetched-from-controllers.yml           | 5 +++++
 .../20171101134435_remove_ref_fetched_from_merge_requests.rb | 0
 2 files changed, 5 insertions(+)
 create mode 100644 changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
 rename db/{migrate => post_migrate}/20171101134435_remove_ref_fetched_from_merge_requests.rb (100%)

diff --git a/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml b/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
new file mode 100644
index 00000000000..5044a4e8d4c
--- /dev/null
+++ b/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
@@ -0,0 +1,5 @@
+---
+title: Removed merge requests fetching their refs on every action.
+merge_request:
+author:
+type: removed
diff --git a/db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb b/db/post_migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
similarity index 100%
rename from db/migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
rename to db/post_migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb

From 84b9343230f73dfd2ee6de0b894160bb4b745780 Mon Sep 17 00:00:00 2001
From: "micael.bergeron" <micaelbergeron@gmail.com>
Date: Wed, 1 Nov 2017 12:30:16 -0400
Subject: [PATCH 3/7] make the migration reversible

---
 .../unreleased/remove-ensure-ref-fetched-from-controllers.yml   | 2 +-
 .../20171101134435_remove_ref_fetched_from_merge_requests.rb    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml b/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
index 5044a4e8d4c..24a4bf561d4 100644
--- a/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
+++ b/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
@@ -1,5 +1,5 @@
 ---
 title: Removed merge requests fetching their refs on every action.
-merge_request:
+merge_request: 15129
 author:
 type: removed
diff --git a/db/post_migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb b/db/post_migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
index acba8384da4..4e8f495d65d 100644
--- a/db/post_migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
+++ b/db/post_migrate/20171101134435_remove_ref_fetched_from_merge_requests.rb
@@ -9,6 +9,6 @@ class RemoveRefFetchedFromMergeRequests < ActiveRecord::Migration
   #
   # See https://gitlab.com/gitlab-org/gitlab-ce/issues/36061
   def change
-    remove_column :merge_requests, :ref_fetched
+    remove_column :merge_requests, :ref_fetched, :boolean
   end
 end

From 6c1c64d4bb36099fa4876db309b5f8873650e909 Mon Sep 17 00:00:00 2001
From: "micael.bergeron" <micaelbergeron@gmail.com>
Date: Fri, 3 Nov 2017 11:29:06 -0400
Subject: [PATCH 4/7] fix the failing spec

---
 lib/gitlab/hook_data/merge_request_builder.rb           | 1 -
 spec/lib/gitlab/hook_data/merge_request_builder_spec.rb | 1 -
 2 files changed, 2 deletions(-)

diff --git a/lib/gitlab/hook_data/merge_request_builder.rb b/lib/gitlab/hook_data/merge_request_builder.rb
index eaef19c9d04..503452c8ff3 100644
--- a/lib/gitlab/hook_data/merge_request_builder.rb
+++ b/lib/gitlab/hook_data/merge_request_builder.rb
@@ -19,7 +19,6 @@ module Gitlab
         merge_user_id
         merge_when_pipeline_succeeds
         milestone_id
-        ref_fetched
         source_branch
         source_project_id
         state
diff --git a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb
index 92bf87bbad4..78475403f9e 100644
--- a/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb
+++ b/spec/lib/gitlab/hook_data/merge_request_builder_spec.rb
@@ -26,7 +26,6 @@ describe Gitlab::HookData::MergeRequestBuilder do
         merge_user_id
         merge_when_pipeline_succeeds
         milestone_id
-        ref_fetched
         source_branch
         source_project_id
         state

From bb0543ef4782febe4dd0f26fc8a476b743fb86ca Mon Sep 17 00:00:00 2001
From: "micael.bergeron" <micaelbergeron@gmail.com>
Date: Mon, 6 Nov 2017 09:03:11 -0500
Subject: [PATCH 5/7] ignore the column before the migration

reword the changelog
remove dead code in the specs
---
 app/models/merge_request.rb                                   | 3 ++-
 .../unreleased/remove-ensure-ref-fetched-from-controllers.yml | 2 +-
 spec/factories/merge_requests.rb                              | 4 ++--
 spec/requests/api/merge_requests_spec.rb                      | 2 --
 spec/requests/api/v3/merge_requests_spec.rb                   | 2 --
 5 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index ccad8e102aa..f80601f3484 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -8,7 +8,8 @@ class MergeRequest < ActiveRecord::Base
   include CreatedAtFilterable
   include TimeTrackable
 
-  ignore_column :locked_at
+  ignore_column :locked_at,
+                :ref_fetched
 
   belongs_to :target_project, class_name: "Project"
   belongs_to :source_project, class_name: "Project"
diff --git a/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml b/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
index 24a4bf561d4..57f54bec1e6 100644
--- a/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
+++ b/changelogs/unreleased/remove-ensure-ref-fetched-from-controllers.yml
@@ -1,5 +1,5 @@
 ---
-title: Removed merge requests fetching their refs on every action.
+title: Stop merge requests from fetching their refs when the data is already available.
 merge_request: 15129
 author:
 type: removed
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 7c4a22c94c2..cc6cef63b47 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -83,10 +83,10 @@ FactoryGirl.define do
       target_project = merge_request.target_project
       source_project = merge_request.source_project
 
-      # Fake `write_ref` if we don't have repository
+      # Fake `fetch_ref!` if we don't have repository
       # We have too many existing tests replying on this behaviour
       unless [target_project, source_project].all?(&:repository_exists?)
-        allow(merge_request).to receive(:write_ref)
+        allow(merge_request).to receive(:fetch_ref!)
       end
     end
 
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 024cfe8b372..e16be3c46e1 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -623,8 +623,6 @@ describe API::MergeRequests do
 
       before do
         forked_project.add_reporter(user2)
-
-        allow_any_instance_of(MergeRequest).to receive(:write_ref)
       end
 
       it "returns merge_request" do
diff --git a/spec/requests/api/v3/merge_requests_spec.rb b/spec/requests/api/v3/merge_requests_spec.rb
index 26251b95680..91897e5ee01 100644
--- a/spec/requests/api/v3/merge_requests_spec.rb
+++ b/spec/requests/api/v3/merge_requests_spec.rb
@@ -319,8 +319,6 @@ describe API::MergeRequests do
 
       before do
         forked_project.add_reporter(user2)
-
-        allow_any_instance_of(MergeRequest).to receive(:write_ref)
       end
 
       it "returns merge_request" do

From 5ab3ed7a9ad16ad20fd3341a02b597697644ea3f Mon Sep 17 00:00:00 2001
From: "micael.bergeron" <micaelbergeron@gmail.com>
Date: Mon, 6 Nov 2017 09:23:41 -0500
Subject: [PATCH 6/7] align with the comments

---
 config/initializers/8_metrics.rb  | 3 ---
 spec/models/merge_request_spec.rb | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/config/initializers/8_metrics.rb b/config/initializers/8_metrics.rb
index 4fa72a494d0..efa4d870e58 100644
--- a/config/initializers/8_metrics.rb
+++ b/config/initializers/8_metrics.rb
@@ -116,9 +116,6 @@ def instrument_classes(instrumentation)
 
   # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/30224#note_32306159
   instrumentation.instrument_instance_method(MergeRequestDiff, :load_commits)
-
-  # Needed for https://gitlab.com/gitlab-org/gitlab-ce/issues/36061
-  instrumentation.instrument_instance_method(MergeRequest, :fetch_ref!)
 end
 # rubocop:enable Metrics/AbcSize
 
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index f719ee26767..d022dae3476 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1756,7 +1756,7 @@ describe MergeRequest do
   end
 
   describe '#fetch_ref!' do
-    it 'fetch the ref correctly' do
+    it 'fetches the ref correctly' do
       expect { subject.target_project.repository.delete_refs(subject.ref_path) }.not_to raise_error
 
       subject.fetch_ref!

From d934d6504a9f1a706dbfc0b4a28c4cf8dbe8c8eb Mon Sep 17 00:00:00 2001
From: "micael.bergeron" <micaelbergeron@gmail.com>
Date: Mon, 6 Nov 2017 10:20:20 -0500
Subject: [PATCH 7/7] updated the ignore_column concern to support multiple
 columns

This method is an ActiveRecord extension and it should behave
like one. I expected this to work.
---
 app/models/concerns/ignorable_column.rb       |  4 ++--
 spec/models/concerns/ignorable_column_spec.rb | 12 ++++++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/app/models/concerns/ignorable_column.rb b/app/models/concerns/ignorable_column.rb
index eb9f3423e48..03793e8bcbb 100644
--- a/app/models/concerns/ignorable_column.rb
+++ b/app/models/concerns/ignorable_column.rb
@@ -21,8 +21,8 @@ module IgnorableColumn
       @ignored_columns ||= Set.new
     end
 
-    def ignore_column(name)
-      ignored_columns << name.to_s
+    def ignore_column(*names)
+      ignored_columns.merge(names.map(&:to_s))
     end
   end
 end
diff --git a/spec/models/concerns/ignorable_column_spec.rb b/spec/models/concerns/ignorable_column_spec.rb
index dba9fe43327..b70f2331a0e 100644
--- a/spec/models/concerns/ignorable_column_spec.rb
+++ b/spec/models/concerns/ignorable_column_spec.rb
@@ -5,7 +5,11 @@ describe IgnorableColumn do
     Class.new do
       def self.columns
         # This method does not have access to "double"
-        [Struct.new(:name).new('id'), Struct.new(:name).new('title')]
+        [
+          Struct.new(:name).new('id'),
+          Struct.new(:name).new('title'),
+          Struct.new(:name).new('date')
+        ]
       end
     end
   end
@@ -18,7 +22,7 @@ describe IgnorableColumn do
 
   describe '.columns' do
     it 'returns the columns, excluding the ignored ones' do
-      model.ignore_column(:title)
+      model.ignore_column(:title, :date)
 
       expect(model.columns.map(&:name)).to eq(%w(id))
     end
@@ -30,9 +34,9 @@ describe IgnorableColumn do
     end
 
     it 'returns the names of the ignored columns' do
-      model.ignore_column(:title)
+      model.ignore_column(:title, :date)
 
-      expect(model.ignored_columns).to eq(Set.new(%w(title)))
+      expect(model.ignored_columns).to eq(Set.new(%w(title date)))
     end
   end
 end