From cba40a1f551e4c1b46bfa49a709f59feb59782bd Mon Sep 17 00:00:00 2001
From: Oswaldo Ferreira <oswaldo@gitlab.com>
Date: Thu, 7 Dec 2017 19:56:59 -0200
Subject: Stop sending milestone and labels data over the wire for MR widget
 requests

---
 .../services/mr_widget_service.js                  |   2 +-
 .../projects/merge_requests_controller.rb          |   8 +-
 app/helpers/issuables_helper.rb                    |   4 +-
 app/serializers/issuable_entity.rb                 |   8 -
 app/serializers/issuable_sidebar_entity.rb         |   6 +-
 app/serializers/issue_entity.rb                    |   8 +
 app/serializers/merge_request_entity.rb            | 184 ---------------------
 app/serializers/merge_request_serializer.rb        |   6 +-
 app/serializers/merge_request_widget_entity.rb     | 181 ++++++++++++++++++++
 app/views/projects/merge_requests/show.html.haml   |   2 +-
 .../osw-isolate-mr-widget-exposed-attributes.yml   |   5 +
 .../projects/merge_requests_controller_spec.rb     |   8 +-
 .../merge_requests/mini_pipeline_graph_spec.rb     |   8 +-
 .../api/schemas/entities/merge_request.json        | 105 ------------
 .../api/schemas/entities/merge_request_widget.json | 105 ++++++++++++
 spec/serializers/merge_request_entity_spec.rb      | 157 ------------------
 spec/serializers/merge_request_serializer_spec.rb  |  48 +++---
 .../merge_request_widget_entity_spec.rb            | 118 +++++++++++++
 18 files changed, 466 insertions(+), 497 deletions(-)
 delete mode 100644 app/serializers/merge_request_entity.rb
 create mode 100644 app/serializers/merge_request_widget_entity.rb
 create mode 100644 changelogs/unreleased/osw-isolate-mr-widget-exposed-attributes.yml
 delete mode 100644 spec/fixtures/api/schemas/entities/merge_request.json
 create mode 100644 spec/fixtures/api/schemas/entities/merge_request_widget.json
 delete mode 100644 spec/serializers/merge_request_entity_spec.rb
 create mode 100644 spec/serializers/merge_request_widget_entity_spec.rb

diff --git a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
index 99f5c305df5..5fa838baba3 100644
--- a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
+++ b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
@@ -6,7 +6,7 @@ Vue.use(VueResource);
 export default class MRWidgetService {
   constructor(endpoints) {
     this.mergeResource = Vue.resource(endpoints.mergePath);
-    this.mergeCheckResource = Vue.resource(endpoints.statusPath);
+    this.mergeCheckResource = Vue.resource(`${endpoints.statusPath}?serializer=widget`);
     this.cancelAutoMergeResource = Vue.resource(endpoints.cancelAutoMergePath);
     this.removeWIPResource = Vue.resource(endpoints.removeWIPPath);
     this.removeSourceBranchResource = Vue.resource(endpoints.sourceBranchPath);
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index e7b3b73024b..6b59c8461a3 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -131,7 +131,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
       .new(project, current_user, wip_event: 'unwip')
       .execute(@merge_request)
 
-    render json: serializer.represent(@merge_request)
+    render json: serialize_widget(@merge_request)
   end
 
   def commit_change_content
@@ -147,7 +147,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
       .new(@project, current_user)
       .cancel(@merge_request)
 
-    render json: serializer.represent(@merge_request)
+    render json: serialize_widget(@merge_request)
   end
 
   def merge
@@ -304,6 +304,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     end
   end
 
+  def serialize_widget(merge_request)
+    serializer.represent(merge_request, serializer: 'widget')
+  end
+
   def serializer
     MergeRequestSerializer.new(current_user: current_user, project: merge_request.project)
   end
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 4c60f4b0cd0..b4ca0e68e0b 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -32,7 +32,7 @@ module IssuablesHelper
     end
   end
 
-  def serialize_issuable(issuable)
+  def serialize_issuable(issuable, serializer: nil)
     serializer_klass = case issuable
                        when Issue
                          IssueSerializer
@@ -42,7 +42,7 @@ module IssuablesHelper
 
     serializer_klass
       .new(current_user: current_user, project: issuable.project)
-      .represent(issuable)
+      .represent(issuable, serializer: serializer)
       .to_json
   end
 
diff --git a/app/serializers/issuable_entity.rb b/app/serializers/issuable_entity.rb
index 3b5a4fd4f79..6f31fbd6b7c 100644
--- a/app/serializers/issuable_entity.rb
+++ b/app/serializers/issuable_entity.rb
@@ -3,14 +3,6 @@ class IssuableEntity < Grape::Entity
 
   expose :id
   expose :iid
-  expose :author_id
   expose :description
-  expose :lock_version
-  expose :milestone_id
   expose :title
-  expose :updated_by_id
-  expose :created_at
-  expose :updated_at
-  expose :milestone, using: API::Entities::Milestone
-  expose :labels, using: LabelEntity
 end
diff --git a/app/serializers/issuable_sidebar_entity.rb b/app/serializers/issuable_sidebar_entity.rb
index ff23d8bf0c7..29138c803df 100644
--- a/app/serializers/issuable_sidebar_entity.rb
+++ b/app/serializers/issuable_sidebar_entity.rb
@@ -1,4 +1,5 @@
 class IssuableSidebarEntity < Grape::Entity
+  include TimeTrackableEntity
   include RequestAwareEntity
 
   expose :participants, using: ::API::Entities::UserBasic do |issuable|
@@ -8,9 +9,4 @@ class IssuableSidebarEntity < Grape::Entity
   expose :subscribed do |issuable|
     issuable.subscribed?(request.current_user, issuable.project)
   end
-
-  expose :time_estimate
-  expose :total_time_spent
-  expose :human_time_estimate
-  expose :human_total_time_spent
 end
diff --git a/app/serializers/issue_entity.rb b/app/serializers/issue_entity.rb
index 9d52b8d9752..0bdd4d7a272 100644
--- a/app/serializers/issue_entity.rb
+++ b/app/serializers/issue_entity.rb
@@ -2,7 +2,15 @@ class IssueEntity < IssuableEntity
   include TimeTrackableEntity
 
   expose :state
+  expose :milestone_id
+  expose :updated_by_id
+  expose :created_at
+  expose :updated_at
   expose :deleted_at
+  expose :milestone, using: API::Entities::Milestone
+  expose :labels, using: LabelEntity
+  expose :lock_version
+  expose :author_id
   expose :confidential
   expose :discussion_locked
   expose :assignees, using: API::Entities::UserBasic
diff --git a/app/serializers/merge_request_entity.rb b/app/serializers/merge_request_entity.rb
deleted file mode 100644
index eece9445dca..00000000000
--- a/app/serializers/merge_request_entity.rb
+++ /dev/null
@@ -1,184 +0,0 @@
-class MergeRequestEntity < IssuableEntity
-  include TimeTrackableEntity
-
-  expose :state
-  expose :deleted_at
-  expose :in_progress_merge_commit_sha
-  expose :merge_commit_sha
-  expose :merge_error
-  expose :merge_params
-  expose :merge_status
-  expose :merge_user_id
-  expose :merge_when_pipeline_succeeds
-  expose :source_branch
-  expose :source_project_id
-  expose :target_branch
-  expose :target_project_id
-
-  expose :should_be_rebased?, as: :should_be_rebased
-  expose :ff_only_enabled do |merge_request|
-    merge_request.project.merge_requests_ff_only_enabled
-  end
-
-  # Events
-  expose :merge_event, using: EventEntity
-  expose :closed_event, using: EventEntity
-
-  # User entities
-  expose :merge_user, using: UserEntity
-
-  # Diff sha's
-  expose :diff_head_sha do |merge_request|
-    merge_request.diff_head_sha if merge_request.diff_head_commit
-  end
-
-  expose :merge_commit_message
-  expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline
-
-  # Booleans
-  expose :merge_ongoing?, as: :merge_ongoing
-  expose :work_in_progress?, as: :work_in_progress
-  expose :source_branch_exists?, as: :source_branch_exists
-  expose :mergeable_discussions_state?, as: :mergeable_discussions_state
-  expose :branch_missing?, as: :branch_missing
-  expose :commits_count
-  expose :cannot_be_merged?, as: :has_conflicts
-  expose :can_be_merged?, as: :can_be_merged
-  expose :mergeable?, as: :mergeable
-  expose :remove_source_branch?, as: :remove_source_branch
-
-  expose :project_archived do |merge_request|
-    merge_request.project.archived?
-  end
-
-  expose :only_allow_merge_if_pipeline_succeeds do |merge_request|
-    merge_request.project.only_allow_merge_if_pipeline_succeeds?
-  end
-
-  # CI related
-  expose :has_ci?, as: :has_ci
-  expose :ci_status do |merge_request|
-    presenter(merge_request).ci_status
-  end
-
-  expose :issues_links do
-    expose :assign_to_closing do |merge_request|
-      presenter(merge_request).assign_to_closing_issues_link
-    end
-
-    expose :closing do |merge_request|
-      presenter(merge_request).closing_issues_links
-    end
-
-    expose :mentioned_but_not_closing do |merge_request|
-      presenter(merge_request).mentioned_issues_links
-    end
-  end
-
-  expose :source_branch_with_namespace_link do |merge_request|
-    presenter(merge_request).source_branch_with_namespace_link
-  end
-
-  expose :source_branch_path do |merge_request|
-    presenter(merge_request).source_branch_path
-  end
-
-  expose :current_user do
-    expose :can_remove_source_branch do |merge_request|
-      merge_request.source_branch_exists? && merge_request.can_remove_source_branch?(current_user)
-    end
-
-    expose :can_revert_on_current_merge_request do |merge_request|
-      presenter(merge_request).can_revert_on_current_merge_request?
-    end
-
-    expose :can_cherry_pick_on_current_merge_request do |merge_request|
-      presenter(merge_request).can_cherry_pick_on_current_merge_request?
-    end
-  end
-
-  # Paths
-  #
-  expose :target_branch_commits_path do |merge_request|
-    presenter(merge_request).target_branch_commits_path
-  end
-
-  expose :target_branch_tree_path do |merge_request|
-    presenter(merge_request).target_branch_tree_path
-  end
-
-  expose :new_blob_path do |merge_request|
-    if can?(current_user, :push_code, merge_request.project)
-      project_new_blob_path(merge_request.project, merge_request.source_branch)
-    end
-  end
-
-  expose :conflict_resolution_path do |merge_request|
-    presenter(merge_request).conflict_resolution_path
-  end
-
-  expose :remove_wip_path do |merge_request|
-    presenter(merge_request).remove_wip_path
-  end
-
-  expose :cancel_merge_when_pipeline_succeeds_path do |merge_request|
-    presenter(merge_request).cancel_merge_when_pipeline_succeeds_path
-  end
-
-  expose :create_issue_to_resolve_discussions_path do |merge_request|
-    presenter(merge_request).create_issue_to_resolve_discussions_path
-  end
-
-  expose :merge_path do |merge_request|
-    presenter(merge_request).merge_path
-  end
-
-  expose :cherry_pick_in_fork_path do |merge_request|
-    presenter(merge_request).cherry_pick_in_fork_path
-  end
-
-  expose :revert_in_fork_path do |merge_request|
-    presenter(merge_request).revert_in_fork_path
-  end
-
-  expose :email_patches_path do |merge_request|
-    project_merge_request_path(merge_request.project, merge_request, format: :patch)
-  end
-
-  expose :plain_diff_path do |merge_request|
-    project_merge_request_path(merge_request.project, merge_request, format: :diff)
-  end
-
-  expose :status_path do |merge_request|
-    project_merge_request_path(merge_request.target_project, merge_request, format: :json)
-  end
-
-  expose :ci_environments_status_path do |merge_request|
-    ci_environments_status_project_merge_request_path(merge_request.project, merge_request)
-  end
-
-  expose :merge_commit_message_with_description do |merge_request|
-    merge_request.merge_commit_message(include_description: true)
-  end
-
-  expose :diverged_commits_count do |merge_request|
-    if merge_request.open? && merge_request.diverged_from_target_branch?
-      merge_request.diverged_commits_count
-    else
-      0
-    end
-  end
-
-  expose :commit_change_content_path do |merge_request|
-    commit_change_content_project_merge_request_path(merge_request.project, merge_request)
-  end
-
-  private
-
-  delegate :current_user, to: :request
-
-  def presenter(merge_request)
-    @presenters ||= {}
-    @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user)
-  end
-end
diff --git a/app/serializers/merge_request_serializer.rb b/app/serializers/merge_request_serializer.rb
index e9d98d8baca..52eb30d688a 100644
--- a/app/serializers/merge_request_serializer.rb
+++ b/app/serializers/merge_request_serializer.rb
@@ -1,14 +1,14 @@
 class MergeRequestSerializer < BaseSerializer
   # This overrided method takes care of which entity should be used
-  # to serialize the `merge_request` based on `basic` key in `opts` param.
+  # to serialize the `merge_request` based on `serializer` key in `opts` param.
   # Hence, `entity` doesn't need to be declared on the class scope.
   def represent(merge_request, opts = {})
     entity =
       case opts[:serializer]
       when 'basic', 'sidebar'
         MergeRequestBasicEntity
-      else
-        MergeRequestEntity
+      when 'widget'
+        MergeRequestWidgetEntity
       end
 
     super(merge_request, opts, entity)
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
new file mode 100644
index 00000000000..f8e59b2ffd7
--- /dev/null
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -0,0 +1,181 @@
+class MergeRequestWidgetEntity < IssuableEntity
+  expose :state
+  expose :in_progress_merge_commit_sha
+  expose :merge_commit_sha
+  expose :merge_error
+  expose :merge_params
+  expose :merge_status
+  expose :merge_user_id
+  expose :merge_when_pipeline_succeeds
+  expose :source_branch
+  expose :source_project_id
+  expose :target_branch
+  expose :target_project_id
+
+  expose :should_be_rebased?, as: :should_be_rebased
+  expose :ff_only_enabled do |merge_request|
+    merge_request.project.merge_requests_ff_only_enabled
+  end
+
+  # Events
+  expose :merge_event, using: EventEntity
+  expose :closed_event, using: EventEntity
+
+  # User entities
+  expose :merge_user, using: UserEntity
+
+  # Diff sha's
+  expose :diff_head_sha do |merge_request|
+    merge_request.diff_head_sha if merge_request.diff_head_commit
+  end
+
+  expose :merge_commit_message
+  expose :actual_head_pipeline, with: PipelineDetailsEntity, as: :pipeline
+
+  # Booleans
+  expose :merge_ongoing?, as: :merge_ongoing
+  expose :work_in_progress?, as: :work_in_progress
+  expose :source_branch_exists?, as: :source_branch_exists
+  expose :mergeable_discussions_state?, as: :mergeable_discussions_state
+  expose :branch_missing?, as: :branch_missing
+  expose :commits_count
+  expose :cannot_be_merged?, as: :has_conflicts
+  expose :can_be_merged?, as: :can_be_merged
+  expose :mergeable?, as: :mergeable
+  expose :remove_source_branch?, as: :remove_source_branch
+
+  expose :project_archived do |merge_request|
+    merge_request.project.archived?
+  end
+
+  expose :only_allow_merge_if_pipeline_succeeds do |merge_request|
+    merge_request.project.only_allow_merge_if_pipeline_succeeds?
+  end
+
+  # CI related
+  expose :has_ci?, as: :has_ci
+  expose :ci_status do |merge_request|
+    presenter(merge_request).ci_status
+  end
+
+  expose :issues_links do
+    expose :assign_to_closing do |merge_request|
+      presenter(merge_request).assign_to_closing_issues_link
+    end
+
+    expose :closing do |merge_request|
+      presenter(merge_request).closing_issues_links
+    end
+
+    expose :mentioned_but_not_closing do |merge_request|
+      presenter(merge_request).mentioned_issues_links
+    end
+  end
+
+  expose :source_branch_with_namespace_link do |merge_request|
+    presenter(merge_request).source_branch_with_namespace_link
+  end
+
+  expose :source_branch_path do |merge_request|
+    presenter(merge_request).source_branch_path
+  end
+
+  expose :current_user do
+    expose :can_remove_source_branch do |merge_request|
+      merge_request.source_branch_exists? && merge_request.can_remove_source_branch?(current_user)
+    end
+
+    expose :can_revert_on_current_merge_request do |merge_request|
+      presenter(merge_request).can_revert_on_current_merge_request?
+    end
+
+    expose :can_cherry_pick_on_current_merge_request do |merge_request|
+      presenter(merge_request).can_cherry_pick_on_current_merge_request?
+    end
+  end
+
+  # Paths
+  #
+  expose :target_branch_commits_path do |merge_request|
+    presenter(merge_request).target_branch_commits_path
+  end
+
+  expose :target_branch_tree_path do |merge_request|
+    presenter(merge_request).target_branch_tree_path
+  end
+
+  expose :new_blob_path do |merge_request|
+    if can?(current_user, :push_code, merge_request.project)
+      project_new_blob_path(merge_request.project, merge_request.source_branch)
+    end
+  end
+
+  expose :conflict_resolution_path do |merge_request|
+    presenter(merge_request).conflict_resolution_path
+  end
+
+  expose :remove_wip_path do |merge_request|
+    presenter(merge_request).remove_wip_path
+  end
+
+  expose :cancel_merge_when_pipeline_succeeds_path do |merge_request|
+    presenter(merge_request).cancel_merge_when_pipeline_succeeds_path
+  end
+
+  expose :create_issue_to_resolve_discussions_path do |merge_request|
+    presenter(merge_request).create_issue_to_resolve_discussions_path
+  end
+
+  expose :merge_path do |merge_request|
+    presenter(merge_request).merge_path
+  end
+
+  expose :cherry_pick_in_fork_path do |merge_request|
+    presenter(merge_request).cherry_pick_in_fork_path
+  end
+
+  expose :revert_in_fork_path do |merge_request|
+    presenter(merge_request).revert_in_fork_path
+  end
+
+  expose :email_patches_path do |merge_request|
+    project_merge_request_path(merge_request.project, merge_request, format: :patch)
+  end
+
+  expose :plain_diff_path do |merge_request|
+    project_merge_request_path(merge_request.project, merge_request, format: :diff)
+  end
+
+  expose :status_path do |merge_request|
+    project_merge_request_path(merge_request.target_project, merge_request, format: :json)
+  end
+
+  expose :ci_environments_status_path do |merge_request|
+    ci_environments_status_project_merge_request_path(merge_request.project, merge_request)
+  end
+
+  expose :merge_commit_message_with_description do |merge_request|
+    merge_request.merge_commit_message(include_description: true)
+  end
+
+  expose :diverged_commits_count do |merge_request|
+    if merge_request.open? && merge_request.diverged_from_target_branch?
+      merge_request.diverged_commits_count
+    else
+      0
+    end
+  end
+
+  expose :commit_change_content_path do |merge_request|
+    commit_change_content_project_merge_request_path(merge_request.project, merge_request)
+  end
+
+  private
+
+  delegate :current_user, to: :request
+
+  def presenter(merge_request)
+    @presenters ||= {}
+    @presenters[merge_request] ||= MergeRequestPresenter.new(merge_request, current_user: current_user)
+  end
+end
diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml
index abff702fd9d..8740c6895df 100644
--- a/app/views/projects/merge_requests/show.html.haml
+++ b/app/views/projects/merge_requests/show.html.haml
@@ -20,7 +20,7 @@
     -# haml-lint:disable InlineJavaScript
     :javascript
       window.gl = window.gl || {};
-      window.gl.mrWidgetData = #{serialize_issuable(@merge_request)}
+      window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget')}
 
     #js-vue-mr-widget.mr-widget
 
diff --git a/changelogs/unreleased/osw-isolate-mr-widget-exposed-attributes.yml b/changelogs/unreleased/osw-isolate-mr-widget-exposed-attributes.yml
new file mode 100644
index 00000000000..6b05713d1a1
--- /dev/null
+++ b/changelogs/unreleased/osw-isolate-mr-widget-exposed-attributes.yml
@@ -0,0 +1,5 @@
+---
+title: Stop sending milestone and labels data over the wire for MR widget requests
+merge_request:
+author:
+type: performance
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.json
deleted file mode 100644
index ba094ba1657..00000000000
--- a/spec/fixtures/api/schemas/entities/merge_request.json
+++ /dev/null
@@ -1,105 +0,0 @@
-{
-  "type": "object",
-  "properties" : {
-    "id": { "type": "integer" },
-    "iid": { "type": "integer" },
-    "author_id": { "type": "integer" },
-    "description": { "type": ["string", "null"] },
-    "lock_version": { "type": ["string", "null"] },
-    "milestone_id": { "type": ["string", "null"] },
-    "position": { "type": "integer" },
-    "state": { "type": "string" },
-    "title": { "type": "string" },
-    "updated_by_id": { "type": ["string", "null"] },
-    "created_at": { "type": "string" },
-    "updated_at": { "type": "string" },
-    "deleted_at": { "type": ["string", "null"] },
-    "time_estimate": { "type": "integer" },
-    "total_time_spent": { "type": "integer" },
-    "human_time_estimate": { "type": ["integer", "null"] },
-    "human_total_time_spent": { "type": ["integer", "null"] },
-    "milestone": { "type": ["object", "null"] },
-    "labels": { "type": ["array", "null"] },
-    "in_progress_merge_commit_sha": { "type": ["string", "null"] },
-    "merge_error": { "type": ["string", "null"] },
-    "merge_commit_sha": { "type": ["string", "null"] },
-    "merge_params": { "type": ["object", "null"] },
-    "merge_status": { "type": "string" },
-    "merge_user_id": { "type": ["integer", "null"] },
-    "merge_when_pipeline_succeeds": { "type": "boolean" },
-    "source_branch": { "type": "string" },
-    "source_project_id": { "type": "integer" },
-    "target_branch": { "type": "string" },
-    "target_project_id": { "type": "integer" },
-    "merge_event": { "type": ["object", "null"] },
-    "closed_event": { "type": ["object", "null"] },
-    "author": { "type": ["object", "null"] },
-    "merge_user": { "type": ["object", "null"] },
-    "diff_head_sha": { "type": ["string", "null"] },
-    "diff_head_commit_short_id": { "type": ["string", "null"] },
-    "merge_commit_message": { "type": ["string", "null"] },
-    "pipeline": { "type": ["object", "null"] },
-    "work_in_progress": { "type": "boolean" },
-    "source_branch_exists": { "type": "boolean" },
-    "mergeable_discussions_state": { "type": "boolean" },
-    "conflicts_can_be_resolved_in_ui": { "type": "boolean" },
-    "branch_missing": { "type": "boolean" },
-    "has_conflicts": { "type": "boolean" },
-    "can_be_merged": { "type": "boolean" },
-    "mergeable": { "type": "boolean" },
-    "project_archived": { "type": "boolean" },
-    "only_allow_merge_if_pipeline_succeeds": { "type": "boolean" },
-    "has_ci": { "type": "boolean" },
-    "ci_status": { "type": ["string", "null"] },
-    "issues_links": {
-      "type": "object",
-      "required": ["closing", "mentioned_but_not_closing", "assign_to_closing"],
-      "properties" : {
-        "closing": { "type": "string" },
-        "mentioned_but_not_closing": { "type": "string" },
-        "assign_to_closing": { "type": ["string", "null"] }
-      },
-      "additionalProperties": false
-    },
-    "source_branch_with_namespace_link": { "type": "string" },
-    "current_user": {
-      "type": "object",
-      "required": [
-        "can_remove_source_branch",
-        "can_revert_on_current_merge_request",
-        "can_cherry_pick_on_current_merge_request"
-      ],
-      "properties": {
-        "can_remove_source_branch": { "type": "boolean" },
-        "can_revert_on_current_merge_request": { "type": ["boolean", "null"] },
-        "can_cherry_pick_on_current_merge_request": { "type": ["boolean", "null"] }
-      },
-      "additionalProperties": false
-    },
-    "target_branch_commits_path": { "type": "string" },
-    "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" },
-    "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" },
-    "merge_check_path": { "type": "string" },
-    "ci_environments_status_path": { "type": "string" },
-    "merge_commit_message_with_description": { "type": "string" },
-    "diverged_commits_count": { "type": "integer" },
-    "commit_change_content_path": { "type": "string" },
-    "remove_wip_path": { "type": ["string", "null"] },
-    "commits_count": { "type": "integer" },
-    "remove_source_branch": { "type": ["boolean", "null"] },
-    "merge_ongoing": { "type": "boolean" },
-    "ff_only_enabled": { "type": ["boolean", false] },
-    "should_be_rebased": { "type": "boolean" }
-  },
-  "additionalProperties": false
-}
diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json
new file mode 100644
index 00000000000..342890c3dee
--- /dev/null
+++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json
@@ -0,0 +1,105 @@
+{
+  "type": "object",
+  "properties" : {
+    "id": { "type": "integer" },
+    "iid": { "type": "integer" },
+    "author_id": { "type": "integer" },
+    "description": { "type": ["string", "null"] },
+    "lock_version": { "type": ["string", "null"] },
+    "milestone_id": { "type": ["string", "null"] },
+    "position": { "type": "integer" },
+    "state": { "type": "string" },
+    "title": { "type": "string" },
+    "updated_by_id": { "type": ["string", "null"] },
+    "created_at": { "type": "string" },
+    "updated_at": { "type": "string" },
+    "deleted_at": { "type": ["string", "null"] },
+    "time_estimate": { "type": "integer" },
+    "total_time_spent": { "type": "integer" },
+    "human_time_estimate": { "type": ["integer", "null"] },
+    "human_total_time_spent": { "type": ["integer", "null"] },
+    "milestone": { "type": ["object", "null"] },
+    "labels": { "type": ["array", "null"] },
+    "in_progress_merge_commit_sha": { "type": ["string", "null"] },
+    "merge_error": { "type": ["string", "null"] },
+    "merge_commit_sha": { "type": ["string", "null"] },
+    "merge_params": { "type": ["object", "null"] },
+    "merge_status": { "type": "string" },
+    "merge_user_id": { "type": ["integer", "null"] },
+    "merge_when_pipeline_succeeds": { "type": "boolean" },
+    "source_branch": { "type": "string" },
+    "source_project_id": { "type": "integer" },
+    "target_branch": { "type": "string" },
+    "target_project_id": { "type": "integer" },
+    "merge_event": { "type": ["object", "null"] },
+    "closed_event": { "type": ["object", "null"] },
+    "author": { "type": ["object", "null"] },
+    "merge_user": { "type": ["object", "null"] },
+    "diff_head_sha": { "type": ["string", "null"] },
+    "diff_head_commit_short_id": { "type": ["string", "null"] },
+    "merge_commit_message": { "type": ["string", "null"] },
+    "pipeline": { "type": ["object", "null"] },
+    "work_in_progress": { "type": "boolean" },
+    "source_branch_exists": { "type": "boolean" },
+    "mergeable_discussions_state": { "type": "boolean" },
+    "conflicts_can_be_resolved_in_ui": { "type": "boolean" },
+    "branch_missing": { "type": "boolean" },
+    "has_conflicts": { "type": "boolean" },
+    "can_be_merged": { "type": "boolean" },
+    "mergeable": { "type": "boolean" },
+    "project_archived": { "type": "boolean" },
+    "only_allow_merge_if_pipeline_succeeds": { "type": "boolean" },
+    "has_ci": { "type": "boolean" },
+    "ci_status": { "type": ["string", "null"] },
+    "issues_links": {
+      "type": "object",
+      "required": ["closing", "mentioned_but_not_closing", "assign_to_closing"],
+      "properties" : {
+        "closing": { "type": "string" },
+        "mentioned_but_not_closing": { "type": "string" },
+        "assign_to_closing": { "type": ["string", "null"] }
+      },
+      "additionalProperties": false
+    },
+    "source_branch_with_namespace_link": { "type": "string" },
+    "current_user": {
+      "type": "object",
+      "required": [
+        "can_remove_source_branch",
+        "can_revert_on_current_merge_request",
+        "can_cherry_pick_on_current_merge_request"
+      ],
+      "properties": {
+        "can_remove_source_branch": { "type": "boolean" },
+        "can_revert_on_current_merge_request": { "type": ["boolean", "null"] },
+        "can_cherry_pick_on_current_merge_request": { "type": ["boolean", "null"] }
+      },
+      "additionalProperties": false
+    },
+    "target_branch_commits_path": { "type": "string" },
+    "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", "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", "null"] },
+    "merge_check_path": { "type": "string" },
+    "ci_environments_status_path": { "type": "string" },
+    "merge_commit_message_with_description": { "type": "string" },
+    "diverged_commits_count": { "type": "integer" },
+    "commit_change_content_path": { "type": "string" },
+    "remove_wip_path": { "type": ["string", "null"] },
+    "commits_count": { "type": "integer" },
+    "remove_source_branch": { "type": ["boolean", "null"] },
+    "merge_ongoing": { "type": "boolean" },
+    "ff_only_enabled": { "type": ["boolean", false] },
+    "should_be_rebased": { "type": "boolean" }
+  },
+  "additionalProperties": false
+}
diff --git a/spec/serializers/merge_request_entity_spec.rb b/spec/serializers/merge_request_entity_spec.rb
deleted file mode 100644
index 1ad672fd355..00000000000
--- a/spec/serializers/merge_request_entity_spec.rb
+++ /dev/null
@@ -1,157 +0,0 @@
-require 'spec_helper'
-
-describe MergeRequestEntity do
-  let(:project)  { create :project, :repository }
-  let(:resource) { create(:merge_request, source_project: project, target_project: project) }
-  let(:user)     { create(:user) }
-
-  let(:request) { double('request', current_user: user, project: project) }
-
-  subject do
-    described_class.new(resource, request: request).as_json
-  end
-
-  describe 'pipeline' do
-    let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
-
-    context 'when is up to date' do
-      let(:req) { double('request', current_user: user, project: project) }
-
-      it 'returns pipeline' do
-        pipeline_payload = PipelineDetailsEntity
-          .represent(pipeline, request: req)
-          .as_json
-
-        expect(subject[:pipeline]).to eq(pipeline_payload)
-      end
-    end
-
-    context 'when is not up to date' do
-      it 'returns nil' do
-        pipeline.update(sha: "not up to date")
-
-        expect(subject[:pipeline]).to be_nil
-      end
-    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")
-  end
-
-  it 'has plain_diff_path' do
-    expect(subject[:plain_diff_path])
-      .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.diff")
-  end
-
-  it 'has merge_commit_message_with_description' do
-    expect(subject[:merge_commit_message_with_description])
-      .to eq(resource.merge_commit_message(include_description: true))
-  end
-
-  describe 'new_blob_path' do
-    context 'when user can push to project' do
-      it 'returns path' do
-        project.add_developer(user)
-
-        expect(subject[:new_blob_path])
-          .to eq("/#{resource.project.full_path}/new/#{resource.source_branch}")
-      end
-    end
-
-    context 'when user cannot push to project' do
-      it 'returns nil' do
-        expect(subject[:new_blob_path]).to be_nil
-      end
-    end
-  end
-
-  describe 'diff_head_sha' do
-    before do
-      allow(resource).to receive(:diff_head_sha) { 'sha' }
-    end
-
-    context 'when no diff head commit' do
-      it 'returns nil' do
-        allow(resource).to receive(:diff_head_commit) { nil }
-
-        expect(subject[:diff_head_sha]).to be_nil
-      end
-    end
-
-    context 'when diff head commit present' do
-      it 'returns diff head commit short id' do
-        allow(resource).to receive(:diff_head_commit) { double }
-
-        expect(subject[:diff_head_sha]).to eq('sha')
-      end
-    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
-        allow(resource).to receive_messages(open?: true, diverged_from_target_branch?: true,
-                                            diverged_commits_count: 10)
-
-        expect(subject[:diverged_commits_count]).to eq(10)
-      end
-    end
-
-    context 'when MR is not open' do
-      it 'returns 0' do
-        allow(resource).to receive_messages(open?: false)
-
-        expect(subject[:diverged_commits_count]).to be_zero
-      end
-    end
-
-    context 'when MR is not diverging' do
-      it 'returns 0' do
-        allow(resource).to receive_messages(open?: true, diverged_from_target_branch?: false)
-
-        expect(subject[:diverged_commits_count]).to be_zero
-      end
-    end
-  end
-end
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_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
new file mode 100644
index 00000000000..a5924a8589c
--- /dev/null
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -0,0 +1,118 @@
+require 'spec_helper'
+
+describe MergeRequestWidgetEntity do
+  let(:project)  { create :project, :repository }
+  let(:resource) { create(:merge_request, source_project: project, target_project: project) }
+  let(:user)     { create(:user) }
+
+  let(:request) { double('request', current_user: user, project: project) }
+
+  subject do
+    described_class.new(resource, request: request).as_json
+  end
+
+  describe 'pipeline' do
+    let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }
+
+    context 'when is up to date' do
+      let(:req) { double('request', current_user: user, project: project) }
+
+      it 'returns pipeline' do
+        pipeline_payload = PipelineDetailsEntity
+          .represent(pipeline, request: req)
+          .as_json
+
+        expect(subject[:pipeline]).to eq(pipeline_payload)
+      end
+    end
+
+    context 'when is not up to date' do
+      it 'returns nil' do
+        pipeline.update(sha: "not up to date")
+
+        expect(subject[:pipeline]).to be_nil
+      end
+    end
+  end
+
+  it 'has email_patches_path' do
+    expect(subject[:email_patches_path])
+      .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.patch")
+  end
+
+  it 'has plain_diff_path' do
+    expect(subject[:plain_diff_path])
+      .to eq("/#{resource.project.full_path}/merge_requests/#{resource.iid}.diff")
+  end
+
+  it 'has merge_commit_message_with_description' do
+    expect(subject[:merge_commit_message_with_description])
+      .to eq(resource.merge_commit_message(include_description: true))
+  end
+
+  describe 'new_blob_path' do
+    context 'when user can push to project' do
+      it 'returns path' do
+        project.add_developer(user)
+
+        expect(subject[:new_blob_path])
+          .to eq("/#{resource.project.full_path}/new/#{resource.source_branch}")
+      end
+    end
+
+    context 'when user cannot push to project' do
+      it 'returns nil' do
+        expect(subject[:new_blob_path]).to be_nil
+      end
+    end
+  end
+
+  describe 'diff_head_sha' do
+    before do
+      allow(resource).to receive(:diff_head_sha) { 'sha' }
+    end
+
+    context 'when no diff head commit' do
+      it 'returns nil' do
+        allow(resource).to receive(:diff_head_commit) { nil }
+
+        expect(subject[:diff_head_sha]).to be_nil
+      end
+    end
+
+    context 'when diff head commit present' do
+      it 'returns diff head commit short id' do
+        allow(resource).to receive(:diff_head_commit) { double }
+
+        expect(subject[:diff_head_sha]).to eq('sha')
+      end
+    end
+  end
+
+  describe 'diverged_commits_count' do
+    context 'when MR open and its diverging' do
+      it 'returns diverged commits count' do
+        allow(resource).to receive_messages(open?: true, diverged_from_target_branch?: true,
+                                            diverged_commits_count: 10)
+
+        expect(subject[:diverged_commits_count]).to eq(10)
+      end
+    end
+
+    context 'when MR is not open' do
+      it 'returns 0' do
+        allow(resource).to receive_messages(open?: false)
+
+        expect(subject[:diverged_commits_count]).to be_zero
+      end
+    end
+
+    context 'when MR is not diverging' do
+      it 'returns 0' do
+        allow(resource).to receive_messages(open?: true, diverged_from_target_branch?: false)
+
+        expect(subject[:diverged_commits_count]).to be_zero
+      end
+    end
+  end
+end
-- 
cgit v1.2.1