Add latest changes from gitlab-org/gitlab@master

10 files changed, 627 insertions, 223 deletions

diff --git a/spec/features/issues/gfm_autocomplete_spec.rb b/spec/features/issues/gfm_autocomplete_spec.rb
index 42c5f0ec4a8..1149468f4a9 100644
--- a/spec/features/issues/gfm_autocomplete_spec.rb
+++ b/spec/features/issues/gfm_autocomplete_spec.rb
@@ -214,6 +214,13 @@ RSpec.describe 'GFM autocomplete', :js do
       expect(find_field('Description').value).to have_content "@#{user.username}"
     end
 
+    it 'does not show `@undefined` when pressing `@` then Enter' do
+      fill_in 'Description', with: "@\n"
+
+      expect(find_field('Description').value).to have_content "@"
+      expect(find_field('Description').value).not_to have_content "@undefined"
+    end
+
     it 'selects the first item for non-assignee dropdowns if a query is entered' do
       page.within '.timeline-content-form' do
         find('#note-body').native.send_keys(':1')
diff --git a/spec/features/projects/ci/lint_spec.rb b/spec/features/projects/ci/lint_spec.rb
index ccffe25f45e..353c8558185 100644
--- a/spec/features/projects/ci/lint_spec.rb
+++ b/spec/features/projects/ci/lint_spec.rb
@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'CI Lint', :js do
+RSpec.describe 'CI Lint', :js, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297782' do
   include Spec::Support::Helpers::Features::EditorLiteSpecHelpers
 
   let(:project) { create(:project, :repository) }
diff --git a/spec/lib/gitlab/etag_caching/router/graphql_spec.rb b/spec/lib/gitlab/etag_caching/router/graphql_spec.rb
new file mode 100644
index 00000000000..d151dcba413
--- /dev/null
+++ b/spec/lib/gitlab/etag_caching/router/graphql_spec.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::EtagCaching::Router::Graphql do
+  it 'matches pipelines endpoint' do
+    result = match_route('/api/graphql', 'pipelines/id/1')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'pipelines_graph'
+  end
+
+  it 'has a valid feature category for every route', :aggregate_failures do
+    feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).to_set
+
+    described_class::ROUTES.each do |route|
+      expect(feature_categories).to include(route.feature_category), "#{route.name} has a category of #{route.feature_category}, which is not valid"
+    end
+  end
+
+  def match_route(path, header)
+    described_class.match(
+      double(path_info: path,
+        headers: { 'X-GITLAB-GRAPHQL-RESOURCE-ETAG' => header }))
+  end
+
+  describe '.cache_key' do
+    let(:path) { '/api/graphql' }
+    let(:header_value) { 'pipelines/id/1' }
+    let(:headers) do
+      { 'X-GITLAB-GRAPHQL-RESOURCE-ETAG' => header_value }.compact
+    end
+
+    subject do
+      described_class.cache_key(double(path: path, headers: headers))
+    end
+
+    it 'uses request path and headers as cache key' do
+      is_expected.to eq '/api/graphql:pipelines/id/1'
+    end
+
+    context 'when the header is missing' do
+      let(:header_value) {}
+
+      it 'does not raise errors' do
+        is_expected.to eq '/api/graphql'
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/etag_caching/router/restful_spec.rb b/spec/lib/gitlab/etag_caching/router/restful_spec.rb
new file mode 100644
index 00000000000..877789b320f
--- /dev/null
+++ b/spec/lib/gitlab/etag_caching/router/restful_spec.rb
@@ -0,0 +1,124 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::EtagCaching::Router::Restful do
+  it 'matches issue notes endpoint' do
+    result = match_route('/my-group/and-subgroup/here-comes-the-project/noteable/issue/1/notes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'issue_notes'
+  end
+
+  it 'matches MR notes endpoint' do
+    result = match_route('/my-group/and-subgroup/here-comes-the-project/noteable/merge_request/1/notes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'merge_request_notes'
+  end
+
+  it 'matches issue title endpoint' do
+    result = match_route('/my-group/my-project/-/issues/123/realtime_changes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'issue_title'
+  end
+
+  it 'matches with a project name that includes a suffix of create' do
+    result = match_route('/group/test-create/-/issues/123/realtime_changes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'issue_title'
+  end
+
+  it 'matches with a project name that includes a prefix of create' do
+    result = match_route('/group/create-test/-/issues/123/realtime_changes')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'issue_title'
+  end
+
+  it 'matches project pipelines endpoint' do
+    result = match_route('/my-group/my-project/-/pipelines.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'project_pipelines'
+  end
+
+  it 'matches commit pipelines endpoint' do
+    result = match_route('/my-group/my-project/-/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'commit_pipelines'
+  end
+
+  it 'matches new merge request pipelines endpoint' do
+    result = match_route('/my-group/my-project/-/merge_requests/new.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'new_merge_request_pipelines'
+  end
+
+  it 'matches merge request pipelines endpoint' do
+    result = match_route('/my-group/my-project/-/merge_requests/234/pipelines.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'merge_request_pipelines'
+  end
+
+  it 'matches build endpoint' do
+    result = match_route('/my-group/my-project/builds/234.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'project_build'
+  end
+
+  it 'does not match blob with confusing name' do
+    result = match_route('/my-group/my-project/-/blob/master/pipelines.json')
+
+    expect(result).to be_blank
+  end
+
+  it 'matches the cluster environments path' do
+    result = match_route('/my-group/my-project/-/clusters/47/environments')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'cluster_environments'
+  end
+
+  it 'matches the environments path' do
+    result = match_route('/my-group/my-project/environments.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'environments'
+  end
+
+  it 'matches pipeline#show endpoint' do
+    result = match_route('/my-group/my-project/-/pipelines/2.json')
+
+    expect(result).to be_present
+    expect(result.name).to eq 'project_pipeline'
+  end
+
+  it 'has a valid feature category for every route', :aggregate_failures do
+    feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).to_set
+
+    described_class::ROUTES.each do |route|
+      expect(feature_categories).to include(route.feature_category), "#{route.name} has a category of #{route.feature_category}, which is not valid"
+    end
+  end
+
+  def match_route(path)
+    described_class.match(double(path_info: path))
+  end
+
+  describe '.cache_key' do
+    subject do
+      described_class.cache_key(double(path: '/my-group/my-project/builds/234.json'))
+    end
+
+    it 'uses request path as cache key' do
+      is_expected.to eq '/my-group/my-project/builds/234.json'
+    end
+  end
+end
diff --git a/spec/lib/gitlab/etag_caching/router_spec.rb b/spec/lib/gitlab/etag_caching/router_spec.rb
index dbd9cc230f1..c748ee00721 100644
--- a/spec/lib/gitlab/etag_caching/router_spec.rb
+++ b/spec/lib/gitlab/etag_caching/router_spec.rb
@@ -3,136 +3,33 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::EtagCaching::Router do
-  it 'matches issue notes endpoint' do
-    result = described_class.match(
-      '/my-group/and-subgroup/here-comes-the-project/noteable/issue/1/notes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'issue_notes'
-  end
-
-  it 'matches MR notes endpoint' do
-    result = described_class.match(
-      '/my-group/and-subgroup/here-comes-the-project/noteable/merge_request/1/notes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'merge_request_notes'
-  end
-
-  it 'matches issue title endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/issues/123/realtime_changes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'issue_title'
-  end
-
-  it 'matches with a project name that includes a suffix of create' do
-    result = described_class.match(
-      '/group/test-create/-/issues/123/realtime_changes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'issue_title'
-  end
-
-  it 'matches with a project name that includes a prefix of create' do
-    result = described_class.match(
-      '/group/create-test/-/issues/123/realtime_changes'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'issue_title'
-  end
-
-  it 'matches project pipelines endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/pipelines.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'project_pipelines'
-  end
-
-  it 'matches commit pipelines endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/commit/aa8260d253a53f73f6c26c734c72fdd600f6e6d4/pipelines.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'commit_pipelines'
-  end
-
-  it 'matches new merge request pipelines endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/merge_requests/new.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'new_merge_request_pipelines'
-  end
-
-  it 'matches merge request pipelines endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/merge_requests/234/pipelines.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'merge_request_pipelines'
-  end
-
-  it 'matches build endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/builds/234.json'
-    )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'project_build'
-  end
-
-  it 'does not match blob with confusing name' do
-    result = described_class.match(
-      '/my-group/my-project/-/blob/master/pipelines.json'
-    )
-
-    expect(result).to be_blank
-  end
+  describe '.match', :aggregate_failures do
+    context 'with RESTful routes' do
+      it 'matches project pipelines endpoint' do
+        result = match_route('/my-group/my-project/-/pipelines.json')
+
+        expect(result).to be_present
+        expect(result.name).to eq 'project_pipelines'
+        expect(result.router).to eq Gitlab::EtagCaching::Router::Restful
+      end
+    end
 
-  it 'matches the cluster environments path' do
-    result = described_class.match(
-      '/my-group/my-project/-/clusters/47/environments'
-    )
+    context 'with GraphQL routes' do
+      it 'matches pipelines endpoint' do
+        result = match_route('/api/graphql', 'pipelines/id/12')
 
-    expect(result).to be_present
-    expect(result.name).to eq 'cluster_environments'
+        expect(result).to be_present
+        expect(result.name).to eq 'pipelines_graph'
+        expect(result.router).to eq Gitlab::EtagCaching::Router::Graphql
+      end
+    end
   end
 
-  it 'matches the environments path' do
-    result = described_class.match(
-      '/my-group/my-project/environments.json'
-    )
+  def match_route(path, header = nil)
+    headers = { 'X-GITLAB-GRAPHQL-RESOURCE-ETAG' => header }.compact
 
-    expect(result).to be_present
-    expect(result.name).to eq 'environments'
-  end
-
-  it 'matches pipeline#show endpoint' do
-    result = described_class.match(
-      '/my-group/my-project/-/pipelines/2.json'
+    described_class.match(
+      double(path_info: path, headers: headers)
     )
-
-    expect(result).to be_present
-    expect(result.name).to eq 'project_pipeline'
-  end
-
-  it 'has a valid feature category for every route', :aggregate_failures do
-    feature_categories = YAML.load_file(Rails.root.join('config', 'feature_categories.yml')).to_set
-
-    described_class::ROUTES.each do |route|
-      expect(feature_categories).to include(route.feature_category), "#{route.name} has a category of #{route.feature_category}, which is not valid"
-    end
   end
 end
diff --git a/spec/lib/gitlab/etag_caching/store_spec.rb b/spec/lib/gitlab/etag_caching/store_spec.rb
new file mode 100644
index 00000000000..46195e64715
--- /dev/null
+++ b/spec/lib/gitlab/etag_caching/store_spec.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::EtagCaching::Store, :clean_gitlab_redis_shared_state do
+  let(:store) { described_class.new }
+
+  describe '#get' do
+    subject { store.get(key) }
+
+    context 'with invalid keys' do
+      let(:key) { 'a' }
+
+      it 'raises errors' do
+        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original
+
+        expect { subject }.to raise_error Gitlab::EtagCaching::Store::InvalidKeyError
+      end
+
+      it 'does not raise errors in production' do
+        expect(store).to receive(:skip_validation?).and_return true
+        expect(Gitlab::ErrorTracking).not_to receive(:track_and_raise_for_dev_exception)
+
+        subject
+      end
+    end
+
+    context 'with GraphQL keys' do
+      let(:key) { '/api/graphql:pipelines/id/5' }
+
+      it 'returns a stored value' do
+        etag = store.touch(key)
+
+        is_expected.to eq(etag)
+      end
+    end
+
+    context 'with RESTful keys' do
+      let(:key) { '/my-group/my-project/builds/234.json' }
+
+      it 'returns a stored value' do
+        etag = store.touch(key)
+
+        is_expected.to eq(etag)
+      end
+    end
+  end
+
+  describe '#touch' do
+    subject { store.touch(key) }
+
+    context 'with invalid keys' do
+      let(:key) { 'a' }
+
+      it 'raises errors' do
+        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).and_call_original
+
+        expect { subject }.to raise_error Gitlab::EtagCaching::Store::InvalidKeyError
+      end
+    end
+
+    context 'with GraphQL keys' do
+      let(:key) { '/api/graphql:pipelines/id/5' }
+
+      it 'stores and returns a value' do
+        etag = store.touch(key)
+
+        expect(etag).to be_present
+        expect(store.get(key)).to eq(etag)
+      end
+    end
+
+    context 'with RESTful keys' do
+      let(:key) { '/my-group/my-project/builds/234.json' }
+
+      it 'stores and returns a value' do
+        etag = store.touch(key)
+
+        expect(etag).to be_present
+        expect(store.get(key)).to eq(etag)
+      end
+    end
+  end
+end
diff --git a/spec/services/ci/expire_pipeline_cache_service_spec.rb b/spec/services/ci/expire_pipeline_cache_service_spec.rb
index 8df5d0bc159..54f5e4c5c07 100644
--- a/spec/services/ci/expire_pipeline_cache_service_spec.rb
+++ b/spec/services/ci/expire_pipeline_cache_service_spec.rb
@@ -13,10 +13,14 @@ RSpec.describe Ci::ExpirePipelineCacheService do
       pipelines_path = "/#{project.full_path}/-/pipelines.json"
       new_mr_pipelines_path = "/#{project.full_path}/-/merge_requests/new.json"
       pipeline_path = "/#{project.full_path}/-/pipelines/#{pipeline.id}.json"
+      graphql_pipeline_path = "/api/graphql:pipelines/id/#{pipeline.id}"
 
-      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipelines_path)
-      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(new_mr_pipelines_path)
-      expect_any_instance_of(Gitlab::EtagCaching::Store).to receive(:touch).with(pipeline_path)
+      expect_next_instance_of(Gitlab::EtagCaching::Store) do |store|
+        expect(store).to receive(:touch).with(pipelines_path)
+        expect(store).to receive(:touch).with(new_mr_pipelines_path)
+        expect(store).to receive(:touch).with(pipeline_path)
+        expect(store).to receive(:touch).with(graphql_pipeline_path)
+      end
 
       subject.execute(pipeline)
     end
diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb
index 90548cf9a99..2a8c3bd75fa 100644
--- a/spec/services/notes/build_service_spec.rb
+++ b/spec/services/notes/build_service_spec.rb
@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 RSpec.describe Notes::BuildService do
+  include AdminModeHelper
+
   let(:note) { create(:discussion_note_on_issue) }
   let(:project) { note.project }
   let(:author) { note.author }
@@ -147,6 +149,123 @@ RSpec.describe Notes::BuildService do
       end
     end
 
+    context 'confidential comments' do
+      before do
+        project.add_reporter(author)
+      end
+
+      context 'when replying to a confidential comment' do
+        let(:note) { create(:note_on_issue, confidential: true) }
+
+        context 'when the user can read confidential comments' do
+          subject do
+            described_class.new(
+              project,
+              author,
+              note: 'Test',
+              in_reply_to_discussion_id: note.discussion_id,
+              confidential: false
+            ).execute
+          end
+
+          it '`confidential` param is ignored and set to `true`' do
+            expect(subject.confidential).to be_truthy
+          end
+        end
+
+        context 'when the user cannot read confidential comments' do
+          let(:another_user) { create(:user) }
+
+          subject do
+            described_class.new(
+              project,
+              another_user,
+              note: 'Test',
+              in_reply_to_discussion_id: note.discussion_id,
+              confidential: false
+            ).execute
+          end
+
+          it 'returns `Discussion to reply to cannot be found` error' do
+            expect(subject.errors.first).to include("Discussion to reply to cannot be found")
+          end
+        end
+      end
+
+      context 'when replying to a public comment' do
+        let(:note) { create(:note_on_issue, confidential: false) }
+
+        subject do
+          described_class.new(
+            project,
+            author,
+            note: 'Test',
+            in_reply_to_discussion_id: note.discussion_id,
+            confidential: true
+          ).execute
+        end
+
+        it '`confidential` param is ignored and set to `false`' do
+          expect(subject.confidential).to be_falsey
+        end
+      end
+
+      context 'when creating a new comment' do
+        context 'when the `confidential` note flag is set to `true`' do
+          context 'when the user is allowed (reporter)' do
+            subject { described_class.new(project, author, note: 'Test', noteable: merge_request, confidential: true).execute }
+
+            it 'note `confidential` flag is set to `true`' do
+              expect(subject.confidential).to be_truthy
+            end
+          end
+
+          context 'when the user is allowed (issuable author)' do
+            let(:another_user) { create(:user) }
+            let(:issue) { create(:issue, author: another_user) }
+
+            subject { described_class.new(project, another_user, note: 'Test', noteable: issue, confidential: true).execute }
+
+            it 'note `confidential` flag is set to `true`' do
+              expect(subject.confidential).to be_truthy
+            end
+          end
+
+          context 'when the user is allowed (admin)' do
+            before do
+              enable_admin_mode!(another_user)
+            end
+
+            let(:another_user) { create(:admin) }
+
+            subject { described_class.new(project, another_user, note: 'Test', noteable: merge_request, confidential: true).execute }
+
+            it 'note `confidential` flag is set to `true`' do
+              expect(subject.confidential).to be_truthy
+            end
+          end
+
+          context 'when the user is not allowed' do
+            let(:another_user) { create(:user) }
+
+            subject { described_class.new(project, another_user, note: 'Test', noteable: merge_request, confidential: true).execute }
+
+            it 'note `confidential` flag is set to `false`' do
+              expect(subject.confidential).to be_falsey
+            end
+          end
+        end
+
+        context 'when the `confidential` note flag is set to `false`' do
+          subject { described_class.new(project, author, note: 'Test', noteable: merge_request, confidential: false).execute }
+
+          it 'note `confidential` flag is set to `false`' do
+            expect(subject.confidential).to be_falsey
+          end
+        end
+      end
+    end
+
     it 'builds a note without saving it' do
       new_note = described_class.new(project,
                                     author,
diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb
index c0eca67ce92..8d056b8a78e 100644
--- a/spec/tooling/danger/changelog_spec.rb
+++ b/spec/tooling/danger/changelog_spec.rb
@@ -2,50 +2,78 @@
 
 require_relative 'danger_spec_helper'
 
+require_relative '../../../tooling/danger/helper'
 require_relative '../../../tooling/danger/changelog'
 
 RSpec.describe Tooling::Danger::Changelog do
   include DangerSpecHelper
 
-  let(:added_files) { nil }
-  let(:fake_git) { double('fake-git', added_files: added_files) }
+  let(:change_class) { Tooling::Danger::Helper::Change }
+  let(:changes_class) { Tooling::Danger::Helper::Changes }
+  let(:changes) { changes_class.new([]) }
 
-  let(:mr_labels) { nil }
-  let(:mr_json) { nil }
-  let(:fake_gitlab) { double('fake-gitlab', mr_labels: mr_labels, mr_json: mr_json) }
+  let(:mr_labels) { [] }
+  let(:sanitize_mr_title) { 'Fake Title' }
 
-  let(:changes_by_category) { nil }
-  let(:sanitize_mr_title) { nil }
-  let(:ee?) { false }
-  let(:fake_helper) { double('fake-helper', changes_by_category: changes_by_category, sanitize_mr_title: sanitize_mr_title, ee?: ee?) }
+  let(:fake_helper) { double('fake-helper', changes: changes, mr_iid: 1234, mr_title: sanitize_mr_title, mr_labels: mr_labels) }
 
   let(:fake_danger) { new_fake_danger.include(described_class) }
 
-  subject(:changelog) { fake_danger.new(git: fake_git, gitlab: fake_gitlab, helper: fake_helper) }
+  subject(:changelog) { fake_danger.new(helper: fake_helper) }
+
+  describe '#required_reasons' do
+    subject { changelog.required_reasons }
+
+    context "added files contain a migration" do
+      let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) }
+
+      it { is_expected.to include(:db_changes) }
+    end
+
+    context "removed files contains a feature flag" do
+      let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) }
+
+      it { is_expected.to include(:feature_flag_removed) }
+    end
+
+    context "added files do not contain a migration" do
+      let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) }
+
+      it { is_expected.to be_empty }
+    end
+
+    context "removed files do not contain a feature flag" do
+      let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) }
+
+      it { is_expected.to be_empty }
+    end
+  end
 
   describe '#required?' do
     subject { changelog.required? }
 
     context 'added files contain a migration' do
-      [
-        'db/migrate/20200000000000_new_migration.rb',
-        'db/post_migrate/20200000000000_new_migration.rb'
-      ].each do |file_path|
-        let(:added_files) { [file_path] }
+      let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) }
 
-        it { is_expected.to be_truthy }
-      end
+      it { is_expected.to be_truthy }
+    end
+
+    context "removed files contains a feature flag" do
+      let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) }
+
+      it { is_expected.to be_truthy }
     end
 
     context 'added files do not contain a migration' do
-      [
-        'app/models/model.rb',
-        'app/assets/javascripts/file.js'
-      ].each do |file_path|
-        let(:added_files) { [file_path] }
+      let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) }
 
-        it { is_expected.to be_falsey }
-      end
+      it { is_expected.to be_falsey }
+    end
+
+    context "removed files do not contain a feature flag" do
+      let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) }
+
+      it { is_expected.to be_falsey }
     end
   end
 
@@ -58,8 +86,7 @@ RSpec.describe Tooling::Danger::Changelog do
     subject { changelog.optional? }
 
     context 'when MR contains only categories requiring no changelog' do
-      let(:changes_by_category) { { category_without_changelog => nil } }
-      let(:mr_labels) { [] }
+      let(:changes) { changes_class.new([change_class.new('foo', :modified, category_without_changelog)]) }
 
       it 'is falsey' do
         is_expected.to be_falsy
@@ -67,7 +94,7 @@ RSpec.describe Tooling::Danger::Changelog do
     end
 
     context 'when MR contains a label that require no changelog' do
-      let(:changes_by_category) { { category_with_changelog => nil } }
+      let(:changes) { changes_class.new([change_class.new('foo', :modified, category_with_changelog)]) }
       let(:mr_labels) { [label_with_changelog, label_without_changelog] }
 
       it 'is falsey' do
@@ -76,29 +103,28 @@ RSpec.describe Tooling::Danger::Changelog do
     end
 
     context 'when MR contains a category that require changelog and a category that require no changelog' do
-      let(:changes_by_category) { { category_with_changelog => nil, category_without_changelog => nil } }
-      let(:mr_labels) { [] }
+      let(:changes) { changes_class.new([change_class.new('foo', :modified, category_with_changelog), change_class.new('foo', :modified, category_without_changelog)]) }
 
-      it 'is truthy' do
-        is_expected.to be_truthy
+      context 'with no labels' do
+        it 'is truthy' do
+          is_expected.to be_truthy
+        end
       end
-    end
 
-    context 'when MR contains a category that require changelog and a category that require no changelog with changelog label' do
-      let(:changes_by_category) { { category_with_changelog => nil, category_without_changelog => nil } }
-      let(:mr_labels) { ['feature'] }
+      context 'with changelog label' do
+        let(:mr_labels) { ['feature'] }
 
-      it 'is truthy' do
-        is_expected.to be_truthy
+        it 'is truthy' do
+          is_expected.to be_truthy
+        end
       end
-    end
 
-    context 'when MR contains a category that require changelog and a category that require no changelog with no changelog label' do
-      let(:changes_by_category) { { category_with_changelog => nil, category_without_changelog => nil } }
-      let(:mr_labels) { ['tooling'] }
+      context 'with no changelog label' do
+        let(:mr_labels) { ['tooling'] }
 
-      it 'is truthy' do
-        is_expected.to be_falsey
+        it 'is truthy' do
+          is_expected.to be_falsey
+        end
       end
     end
   end
@@ -107,50 +133,35 @@ RSpec.describe Tooling::Danger::Changelog do
     subject { changelog.found }
 
     context 'added files contain a changelog' do
-      [
-        'changelogs/unreleased/entry.yml',
-        'ee/changelogs/unreleased/entry.yml'
-      ].each do |file_path|
-        let(:added_files) { [file_path] }
+      let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) }
 
-        it { is_expected.to be_truthy }
-      end
+      it { is_expected.to be_truthy }
     end
 
     context 'added files do not contain a changelog' do
-      [
-        'app/models/model.rb',
-        'app/assets/javascripts/file.js'
-      ].each do |file_path|
-        let(:added_files) { [file_path] }
-        it { is_expected.to eq(nil) }
-      end
+      let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) }
+
+      it { is_expected.to eq(nil) }
     end
   end
 
   describe '#ee_changelog?' do
     subject { changelog.ee_changelog? }
 
-    before do
-      allow(changelog).to receive(:found).and_return(file_path)
-    end
-
     context 'is ee changelog' do
-      let(:file_path) { 'ee/changelogs/unreleased/entry.yml' }
+      let(:changes) { changes_class.new([change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog)]) }
 
       it { is_expected.to be_truthy }
     end
 
     context 'is not ee changelog' do
-      let(:file_path) { 'changelogs/unreleased/entry.yml' }
+      let(:changes) { changes_class.new([change_class.new('changelogs/unreleased/entry.yml', :added, :changelog)]) }
 
       it { is_expected.to be_falsy }
     end
   end
 
   describe '#modified_text' do
-    let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } }
-
     subject { changelog.modified_text }
 
     context "when title is not changed from sanitization", :aggregate_failures do
@@ -174,35 +185,42 @@ RSpec.describe Tooling::Danger::Changelog do
     end
   end
 
-  describe '#required_text' do
-    let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } }
-
-    subject { changelog.required_text }
+  describe '#required_texts' do
+    let(:sanitize_mr_title) { 'Fake Title' }
 
-    context "when title is not changed from sanitization", :aggregate_failures do
-      let(:sanitize_mr_title) { 'Fake Title' }
+    subject { changelog.required_texts }
 
+    shared_examples 'changelog required text' do |key|
       specify do
-        expect(subject).to include('CHANGELOG missing')
-        expect(subject).to include('bin/changelog -m 1234 "Fake Title"')
-        expect(subject).not_to include('--ee')
+        expect(subject).to have_key(key)
+        expect(subject[key]).to include('CHANGELOG missing')
+        expect(subject[key]).to include('bin/changelog -m 1234 "Fake Title"')
+        expect(subject[key]).not_to include('--ee')
       end
     end
 
-    context "when title needs sanitization", :aggregate_failures do
-      let(:sanitize_mr_title) { 'DRAFT: Fake Title' }
+    context 'with a new migration file' do
+      let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) }
 
-      specify do
-        expect(subject).to include('CHANGELOG missing')
-        expect(subject).to include('bin/changelog -m 1234 "Fake Title"')
-        expect(subject).not_to include('--ee')
+      context "when title is not changed from sanitization", :aggregate_failures do
+        it_behaves_like 'changelog required text', :db_changes
+      end
+
+      context "when title needs sanitization", :aggregate_failures do
+        let(:sanitize_mr_title) { 'DRAFT: Fake Title' }
+
+        it_behaves_like 'changelog required text', :db_changes
       end
     end
+
+    context 'with a removed feature flag file' do
+      let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) }
+
+      it_behaves_like 'changelog required text', :feature_flag_removed
+    end
   end
 
   describe '#optional_text' do
-    let(:mr_json) { { "iid" => 1234, "title" => sanitize_mr_title } }
-
     subject { changelog.optional_text }
 
     context "when title is not changed from sanitization", :aggregate_failures do
diff --git a/spec/tooling/danger/helper_spec.rb b/spec/tooling/danger/helper_spec.rb
index 59af3ecd145..9783705dca0 100644
--- a/spec/tooling/danger/helper_spec.rb
+++ b/spec/tooling/danger/helper_spec.rb
@@ -10,13 +10,27 @@ RSpec.describe Tooling::Danger::Helper do
   using RSpec::Parameterized::TableSyntax
   include DangerSpecHelper
 
-  let(:fake_git) { double('fake-git') }
-
   let(:mr_author) { nil }
   let(:fake_gitlab) { double('fake-gitlab', mr_author: mr_author) }
 
   let(:fake_danger) { new_fake_danger.include(described_class) }
 
+  let(:added_files) { %w[added1] }
+  let(:modified_files) { %w[modified1] }
+  let(:deleted_files) { %w[deleted1] }
+  let(:renamed_before_file) { 'renamed_before' }
+  let(:renamed_after_file) { 'renamed_after' }
+  let(:renamed_files) { [{ before: renamed_before_file, after: renamed_after_file }] }
+
+  let(:fake_git) { double('fake-git') }
+
+  before do
+    allow(fake_git).to receive(:added_files) { added_files }
+    allow(fake_git).to receive(:modified_files) { modified_files }
+    allow(fake_git).to receive(:deleted_files) { deleted_files }
+    allow(fake_git).to receive(:renamed_files).at_least(:twice) { renamed_files }
+  end
+
   subject(:helper) { fake_danger.new(git: fake_git, gitlab: fake_gitlab) }
 
   describe '#gitlab_helper' do
@@ -191,15 +205,16 @@ RSpec.describe Tooling::Danger::Helper do
   end
 
   describe '#changes_by_category' do
-    it 'categorizes changed files' do
-      expect(fake_git).to receive(:added_files) { %w[foo foo.md foo.rb foo.js db/migrate/foo lib/gitlab/database/foo.rb qa/foo ee/changelogs/foo.yml] }
-      allow(fake_git).to receive(:modified_files) { [] }
-      allow(fake_git).to receive(:renamed_files) { [] }
+    let(:added_files) { %w[foo foo.md foo.rb foo.js] }
+    let(:modified_files) { %w[db/migrate/foo lib/gitlab/database/foo.rb] }
+    let(:renamed_files) { [{ before: '', after: 'qa/foo' }, { before: '', after: 'ee/changelogs/foo.yml' }] }
 
+    it 'categorizes changed files' do
       expect(helper.changes_by_category).to eq(
         backend: %w[foo.rb],
         database: %w[db/migrate/foo lib/gitlab/database/foo.rb],
         frontend: %w[foo.js],
+        migration: %w[db/migrate/foo],
         none: %w[ee/changelogs/foo.yml foo.md],
         qa: %w[qa/foo],
         unknown: %w[foo]
@@ -207,6 +222,62 @@ RSpec.describe Tooling::Danger::Helper do
     end
   end
 
+  describe 'Tooling::Danger::Helper::Changes', :aggregate_failures do
+    let(:added_files) { %w[db/migrate/foo ee/changelogs/unreleased/foo.yml] }
+
+    describe '#has_category?' do
+      it 'returns true when changes include given category, false otherwise' do
+        changes = helper.changes
+
+        expect(changes.has_category?(:migration)).to eq(true)
+        expect(changes.has_category?(:changelog)).to eq(true)
+        expect(changes.has_category?(:backend)).to eq(false)
+      end
+    end
+
+    describe '#by_category' do
+      it 'returns an array of Change objects' do
+        expect(helper.changes.by_category(:migration)).to all(be_an(described_class::Change))
+      end
+
+      it 'returns an array of Change objects with the given category' do
+        changes = helper.changes
+
+        expect(changes.by_category(:migration).files).to eq(['db/migrate/foo'])
+        expect(changes.by_category(:changelog).files).to eq(['ee/changelogs/unreleased/foo.yml'])
+        expect(changes.by_category(:backend)).to be_empty
+      end
+    end
+
+    describe '#categories' do
+      it 'returns an array of category symbols' do
+        expect(helper.changes.categories).to contain_exactly(:database, :migration, :changelog, :unknown)
+      end
+    end
+
+    describe '#files' do
+      it 'returns an array of files' do
+        expect(helper.changes.files).to include(*added_files)
+      end
+    end
+  end
+
+  describe '#changes' do
+    it 'returns an array of Change objects' do
+      expect(helper.changes).to all(be_an(described_class::Change))
+    end
+
+    it 'groups changes by change type' do
+      changes = helper.changes
+
+      expect(changes.added.files).to eq(added_files)
+      expect(changes.modified.files).to eq(modified_files)
+      expect(changes.deleted.files).to eq(deleted_files)
+      expect(changes.renamed_before.files).to eq([renamed_before_file])
+      expect(changes.renamed_after.files).to eq([renamed_after_file])
+    end
+  end
+
   describe '#categories_for_file' do
     before do
       allow(fake_git).to receive(:diff_for_file).with('usage_data.rb') { double(:diff, patch: "+ count(User.active)") }
@@ -304,12 +375,10 @@ RSpec.describe Tooling::Danger::Helper do
 
       'db/schema.rb'                                              | [:database]
       'db/structure.sql'                                          | [:database]
-      'db/migrate/foo'                                            | [:database]
-      'db/post_migrate/foo'                                       | [:database]
-      'ee/db/migrate/foo'                                         | [:database]
-      'ee/db/post_migrate/foo'                                    | [:database]
-      'ee/db/geo/migrate/foo'                                     | [:database]
-      'ee/db/geo/post_migrate/foo'                                | [:database]
+      'db/migrate/foo'                                            | [:database, :migration]
+      'db/post_migrate/foo'                                       | [:database, :migration]
+      'ee/db/geo/migrate/foo'                                     | [:database, :migration]
+      'ee/db/geo/post_migrate/foo'                                | [:database, :migration]
       'app/models/project_authorization.rb'                       | [:database]
       'app/services/users/refresh_authorized_projects_service.rb' | [:database]
       'lib/gitlab/background_migration.rb'                        | [:database]
@@ -400,6 +469,22 @@ RSpec.describe Tooling::Danger::Helper do
     end
   end
 
+  describe '#mr_iid' do
+    it 'returns "" when `gitlab_helper` is unavailable' do
+      expect(helper).to receive(:gitlab_helper).and_return(nil)
+
+      expect(helper.mr_iid).to eq('')
+    end
+
+    it 'returns the MR IID when `gitlab_helper` is available' do
+      mr_iid = '1234'
+      expect(fake_gitlab).to receive(:mr_json)
+        .and_return('iid' => mr_iid)
+
+      expect(helper.mr_iid).to eq(mr_iid)
+    end
+  end
+
   describe '#mr_title' do
     it 'returns "" when `gitlab_helper` is unavailable' do
       expect(helper).to receive(:gitlab_helper).and_return(nil)
@@ -432,6 +517,22 @@ RSpec.describe Tooling::Danger::Helper do
     end
   end
 
+  describe '#mr_labels' do
+    it 'returns "" when `gitlab_helper` is unavailable' do
+      expect(helper).to receive(:gitlab_helper).and_return(nil)
+
+      expect(helper.mr_labels).to eq([])
+    end
+
+    it 'returns the MR labels when `gitlab_helper` is available' do
+      mr_labels = %w[foo bar baz]
+      expect(fake_gitlab).to receive(:mr_labels)
+        .and_return(mr_labels)
+
+      expect(helper.mr_labels).to eq(mr_labels)
+    end
+  end
+
   describe '#mr_target_branch' do
     it 'returns "" when `gitlab_helper` is unavailable' do
       expect(helper).to receive(:gitlab_helper).and_return(nil)