Merge branch 'osw-isolate-mr-widget-exposed-attributes' into 'master'

Stop sending milestone and labels for Projects::MergeRequestsController#show.json requests

See merge request gitlab-org/gitlab-ce!15847

5 files changed, 40 insertions, 73 deletions

diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 51d5d6a52b3..d03ecefe47f 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -91,11 +91,11 @@ describe Projects::MergeRequestsController do
         end
       end
 
-      context 'without basic serializer param' do
-        it 'renders the merge request in the json format' do
-          go(format: :json)
+      context 'with widget serializer param' do
+        it 'renders widget MR entity as json' do
+          go(serializer: 'widget', format: :json)
 
-          expect(response).to match_response_schema('entities/merge_request')
+          expect(response).to match_response_schema('entities/merge_request_widget')
         end
       end
     end
diff --git a/spec/features/merge_requests/mini_pipeline_graph_spec.rb b/spec/features/merge_requests/mini_pipeline_graph_spec.rb
index 93c5e945453..a7e7c0eeff6 100644
--- a/spec/features/merge_requests/mini_pipeline_graph_spec.rb
+++ b/spec/features/merge_requests/mini_pipeline_graph_spec.rb
@@ -15,8 +15,8 @@ feature 'Mini Pipeline Graph', :js do
     visit_merge_request
   end
 
-  def visit_merge_request(format = :html)
-    visit project_merge_request_path(project, merge_request, format: format)
+  def visit_merge_request(format: :html, serializer: nil)
+    visit project_merge_request_path(project, merge_request, format: format, serializer: serializer)
   end
 
   it 'should display a mini pipeline graph' do
@@ -33,12 +33,12 @@ feature 'Mini Pipeline Graph', :js do
     end
 
     it 'avoids repeated database queries' do
-      before = ActiveRecord::QueryRecorder.new { visit_merge_request(:json) }
+      before = ActiveRecord::QueryRecorder.new { visit_merge_request(format: :json, serializer: 'widget') }
 
       create(:ci_build, pipeline: pipeline, legacy_artifacts_file: artifacts_file2)
       create(:ci_build, pipeline: pipeline, when: 'manual')
 
-      after = ActiveRecord::QueryRecorder.new { visit_merge_request(:json) }
+      after = ActiveRecord::QueryRecorder.new { visit_merge_request(format: :json, serializer: 'widget') }
 
       expect(before.count).to eq(after.count)
       expect(before.cached_count).to eq(after.cached_count)
diff --git a/spec/fixtures/api/schemas/entities/merge_request.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json
index ba094ba1657..342890c3dee 100644
--- a/spec/fixtures/api/schemas/entities/merge_request.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json
@@ -80,15 +80,15 @@
     "target_branch_tree_path": { "type": "string" },
     "source_branch_path": { "type": "string" },
     "conflict_resolution_path": { "type": ["string", "null"] },
-    "cancel_merge_when_pipeline_succeeds_path": { "type": "string" },
-    "create_issue_to_resolve_discussions_path": { "type": "string" },
-    "merge_path": { "type": "string" },
+    "cancel_merge_when_pipeline_succeeds_path": { "type": ["string", "null"] },
+    "create_issue_to_resolve_discussions_path": { "type": ["string", "null"] },
+    "merge_path": { "type": ["string", "null"] },
     "cherry_pick_in_fork_path": { "type": ["string", "null"] },
     "revert_in_fork_path": { "type": ["string", "null"] },
     "email_patches_path": { "type": "string" },
     "plain_diff_path": { "type": "string" },
     "status_path": { "type": "string" },
-    "new_blob_path": { "type": "string" },
+    "new_blob_path": { "type": ["string", "null"] },
     "merge_check_path": { "type": "string" },
     "ci_environments_status_path": { "type": "string" },
     "merge_commit_message_with_description": { "type": "string" },
diff --git a/spec/serializers/merge_request_serializer_spec.rb b/spec/serializers/merge_request_serializer_spec.rb
index e3abefa6d63..1ad974c774b 100644
--- a/spec/serializers/merge_request_serializer_spec.rb
+++ b/spec/serializers/merge_request_serializer_spec.rb
@@ -1,37 +1,43 @@
 require 'spec_helper'
 
 describe MergeRequestSerializer do
-  let(:user) { build_stubbed(:user) }
-  let(:merge_request) { build_stubbed(:merge_request) }
-
-  let(:serializer) do
+  let(:user) { create(:user) }
+  let(:resource) { create(:merge_request) }
+  let(:json_entity) do
     described_class.new(current_user: user)
+      .represent(resource, serializer: serializer)
+      .with_indifferent_access
   end
 
-  describe '#represent' do
-    let(:opts) { { serializer: serializer_entity } }
-    subject { serializer.represent(merge_request, serializer: serializer_entity) }
+  context 'widget merge request serialization' do
+    let(:serializer) { 'widget' }
 
-    context 'when passing basic serializer param' do
-      let(:serializer_entity) { 'basic' }
+    it 'matches issue json schema' do
+      expect(json_entity).to match_schema('entities/merge_request_widget')
+    end
+  end
 
-      it 'calls super class #represent with correct params' do
-        expect_any_instance_of(BaseSerializer).to receive(:represent)
-          .with(merge_request, opts, MergeRequestBasicEntity)
+  context 'sidebar merge request serialization' do
+    let(:serializer) { 'sidebar' }
 
-        subject
-      end
+    it 'matches basic merge request json schema' do
+      expect(json_entity).to match_schema('entities/merge_request_basic')
     end
+  end
 
-    context 'when serializer param is falsy' do
-      let(:serializer_entity) { nil }
+  context 'basic merge request serialization' do
+    let(:serializer) { 'basic' }
+
+    it 'matches basic merge request json schema' do
+      expect(json_entity).to match_schema('entities/merge_request_basic')
+    end
+  end
 
-      it 'calls super class #represent with correct params' do
-        expect_any_instance_of(BaseSerializer).to receive(:represent)
-          .with(merge_request, opts, MergeRequestEntity)
+  context 'no serializer' do
+    let(:serializer) { nil }
 
-        subject
-      end
+    it 'raises an error' do
+      expect { json_entity }.to raise_error(NoMethodError)
     end
   end
 end
diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index 1ad672fd355..a5924a8589c 100644
--- a/spec/serializers/merge_request_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe MergeRequestEntity do
+describe MergeRequestWidgetEntity do
   let(:project)  { create :project, :repository }
   let(:resource) { create(:merge_request, source_project: project, target_project: project) }
   let(:user)     { create(:user) }
@@ -35,33 +35,6 @@ describe MergeRequestEntity do
     end
   end
 
-  it 'includes issues_links' do
-    issues_links = subject[:issues_links]
-
-    expect(issues_links).to include(:closing, :mentioned_but_not_closing,
-                                    :assign_to_closing)
-  end
-
-  it 'has Issuable attributes' do
-    expect(subject).to include(:id, :iid, :author_id, :description, :lock_version, :milestone_id,
-                               :title, :updated_by_id, :created_at, :updated_at, :milestone, :labels)
-  end
-
-  it 'has time estimation attributes' do
-    expect(subject).to include(:time_estimate, :total_time_spent, :human_time_estimate, :human_total_time_spent)
-  end
-
-  it 'has important MergeRequest attributes' do
-    expect(subject).to include(:state, :deleted_at, :diff_head_sha, :merge_commit_message,
-                               :has_conflicts, :has_ci, :merge_path,
-                               :conflict_resolution_path,
-                               :cancel_merge_when_pipeline_succeeds_path,
-                               :create_issue_to_resolve_discussions_path,
-                               :source_branch_path, :target_branch_commits_path,
-                               :target_branch_tree_path, :commits_count, :merge_ongoing,
-                               :ff_only_enabled)
-  end
-
   it 'has email_patches_path' do
     expect(subject[:email_patches_path])
       .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.patch")
@@ -116,18 +89,6 @@ describe MergeRequestEntity do
     end
   end
 
-  it 'includes merge_event' do
-    create(:event, :merged, author: user, project: resource.project, target: resource)
-
-    expect(subject[:merge_event]).to include(:author, :updated_at)
-  end
-
-  it 'includes closed_event' do
-    create(:event, :closed, author: user, project: resource.project, target: resource)
-
-    expect(subject[:closed_event]).to include(:author, :updated_at)
-  end
-
   describe 'diverged_commits_count' do
     context 'when MR open and its diverging' do
       it 'returns diverged commits count' do