Refactor issuable sidebar to have extras option

app/assets/javascripts/sidebar/stores/sidebar_store.js

@@ -71,7 +71,7 @@ export default class SidebarStore {
   }
 
   findAssignee(findAssignee) {
-    return this.assignees.filter(assignee => assignee.id === findAssignee.id)[0];
+    return this.assignees.find(assignee => assignee.id === findAssignee.id);
   }
 
   removeAssignee(removeAssignee) {

app/helpers/issuables_helper.rb

@@ -50,13 +50,13 @@ module IssuablesHelper
     end
   end
 
-  def issuable_json_path(issuable)
+  def issuable_json_path(issuable, url_params = {})
     project = issuable.project
 
     if issuable.is_a?(MergeRequest)
-      project_merge_request_path(project, issuable.iid, :json)
+      project_merge_request_path(project, issuable.iid, :json, url_params)
     else
-      project_issue_path(project, issuable.iid, :json)
+      project_issue_path(project, issuable.iid, :json, url_params)
     end
   end
 
@@ -420,7 +420,7 @@ module IssuablesHelper
 
   def issuable_sidebar_options(issuable, can_edit_issuable)
     {
-      endpoint: "#{issuable_json_path(issuable)}?serializer=sidebar",
+      endpoint: issuable_json_path(issuable, serializer: 'sidebar_extras'),
       toggleSubscriptionEndpoint: toggle_subscription_path(issuable),
       moveIssueEndpoint: move_namespace_project_issue_path(namespace_id: issuable.project.namespace.to_param, project_id: issuable.project, id: issuable),
       projectsAutocompleteEndpoint: autocomplete_projects_path(project_id: @project.id),

app/serializers/issuable_sidebar_entity.rb

 # frozen_string_literal: true
 
 class IssuableSidebarEntity < Grape::Entity
-  include TimeTrackableEntity
   include RequestAwareEntity
 
+  with_options if: { include_extras: true } do
+    include TimeTrackableEntity
+
     expose :participants, using: ::API::Entities::UserBasic do |issuable|
       issuable.participants(request.current_user)
     end
@@ -11,4 +13,5 @@ class IssuableSidebarEntity < Grape::Entity
     expose :subscribed do |issuable|
       issuable.subscribed?(request.current_user, issuable.project)
     end
+  end
 end

app/serializers/issue_board_entity.rb

@@ -37,7 +37,7 @@ class IssueBoardEntity < Grape::Entity
   end
 
   expose :issue_sidebar_endpoint, if: -> (issue) { issue.project } do |issue|
-    project_issue_path(issue.project, issue, format: :json, serializer: 'sidebar')
+    project_issue_path(issue.project, issue, format: :json, serializer: 'sidebar_extras')
   end
 
   expose :toggle_subscription_endpoint, if: -> (issue) { issue.project } do |issue|

app/serializers/issue_serializer.rb

@@ -2,12 +2,14 @@
 
 class IssueSerializer < BaseSerializer
   # This overrided method takes care of which entity should be used
-  # to serialize the `issue` based on `basic` key in `opts` param.
+  # to serialize the `issue` based on `serializer` key in `opts` param.
   # Hence, `entity` doesn't need to be declared on the class scope.
   def represent(issue, opts = {})
     entity =
       case opts[:serializer]
-      when 'sidebar'
+      when 'sidebar_extras'
+        opts[:include_basic] = false
+        opts[:include_extras] = true
         IssueSidebarEntity
       when 'board'
         IssueBoardEntity

app/serializers/issue_sidebar_entity.rb

 # frozen_string_literal: true
 
 class IssueSidebarEntity < IssuableSidebarEntity
+  with_options if: { include_extras: true } do
     expose :assignees, using: API::Entities::UserBasic
+  end
 end

app/serializers/merge_request_basic_entity.rb

 # frozen_string_literal: true
 
-class MergeRequestBasicEntity < IssuableSidebarEntity
+class MergeRequestBasicEntity < Grape::Entity
   expose :assignee_id
   expose :merge_status
   expose :merge_error

app/serializers/merge_request_basic_serializer.rb

-# frozen_string_literal: true
-
-class MergeRequestBasicSerializer < BaseSerializer
-  entity MergeRequestBasicEntity
-end

app/serializers/merge_request_serializer.rb

@@ -7,9 +7,14 @@ class MergeRequestSerializer < BaseSerializer
   def represent(merge_request, opts = {})
     entity =
       case opts[:serializer]
-      when 'basic', 'sidebar'
+      when 'sidebar_extras'
+        opts[:include_basic] = false
+        opts[:include_extras] = true
+        MergeRequestSidebarEntity
+      when 'basic'
         MergeRequestBasicEntity
-      else # It's 'widget'
+      else
+        # fallback to widget for old poll requests without `serializer` set
         MergeRequestWidgetEntity
       end
 

app/serializers/merge_request_sidebar_entity.rb

+# frozen_string_literal: true
+
+class MergeRequestSidebarEntity < IssuableSidebarEntity
+end

app/views/projects/merge_requests/conflicts.html.haml

-- page_title "Merge Conflicts", "#{@merge_request.title} (#{@merge_request.to_reference}", "Merge Requests"
-- content_for :page_specific_javascripts do
-  = page_specific_javascript_tag('lib/ace.js')
-= render "projects/merge_requests/mr_title"
-
-.merge-request-details.issuable-details
-  = render "projects/merge_requests/mr_box"
-
-= render 'shared/issuable/sidebar', issuable: @merge_request
-
-#conflicts{ "v-cloak" => "true", data: { conflicts_path: conflicts_project_merge_request_path(@merge_request.project, @merge_request, format: :json),
-    resolve_conflicts_path: resolve_conflicts_project_merge_request_path(@merge_request.project, @merge_request) } }
-  .loading{ "v-if" => "isLoading" }
-    %i.fa.fa-spinner.fa-spin
-
-  .nothing-here-block{ "v-if" => "hasError" }
-    {{conflictsData.errorMessage}}
-
-  = render partial: "projects/merge_requests/conflicts/commit_stats"
-
-  .files-wrapper{ "v-if" => "!isLoading && !hasError" }
-    .files
-      .diff-file.file-holder.conflict{ "v-for" => "file in conflictsData.files" }
-        .js-file-title.file-title
-          %i.fa.fa-fw{ ":class" => "file.iconClass" }
-          %strong {{file.filePath}}
-          = render partial: 'projects/merge_requests/conflicts/file_actions'
-        .diff-content.diff-wrap-lines
-          .diff-wrap-lines.code.file-content.js-syntax-highlight{ "v-show" => "!isParallel && file.resolveMode === 'interactive' && file.type === 'text'" }
-            = render partial: "projects/merge_requests/conflicts/components/inline_conflict_lines"
-          .diff-wrap-lines.code.file-content.js-syntax-highlight{ "v-show" => "isParallel && file.resolveMode === 'interactive' && file.type === 'text'" }
-            %parallel-conflict-lines{ ":file" => "file" }
-          %div{ "v-show" => "file.resolveMode === 'edit' ||  file.type === 'text-editor'" }
-            = render partial: "projects/merge_requests/conflicts/components/diff_file_editor"
-
-    = render partial: "projects/merge_requests/conflicts/submit_form"

spec/fixtures/api/schemas/entities/merge_request_basic.json

@@ -4,15 +4,9 @@
     "state": { "type": "string" },
     "merge_status": { "type": "string" },
     "source_branch_exists": { "type": "boolean" },
-    "time_estimate": { "type": "integer" },
-    "total_time_spent": { "type": "integer" },
-    "human_time_estimate": { "type": ["string", "null"] },
-    "human_total_time_spent": { "type": ["string", "null"] },
     "merge_error": { "type": ["string", "null"] },
     "rebase_in_progress": { "type": "boolean" },
     "assignee_id": { "type": ["integer", "null"] },
-    "subscribed": { "type": ["boolean", "null"] },
-    "participants": { "type": "array" },
     "allow_collaboration": { "type": "boolean"},
     "allow_maintainer_to_push": { "type": "boolean"},
     "assignee": {

spec/fixtures/api/schemas/entities/merge_request_sidebar.json

+{
+  "type": "object",
+  "properties" : {
+    "id": { "type": "integer" },
+    "iid": { "type": "integer" },
+    "subscribed": { "type": "boolean" },
+    "time_estimate": { "type": "integer" },
+    "total_time_spent": { "type": "integer" },
+    "human_time_estimate": { "type": ["integer", "null"] },
+    "human_total_time_spent": { "type": ["integer", "null"] },
+    "participants": {
+      "type": "array",
+      "items": { "$ref": "../public_api/v4/user/basic.json" }
+    },
+    "assignees": {
+      "type": "array",
+      "items": { "$ref": "../public_api/v4/user/basic.json" }
+    }
+  },
+  "additionalProperties": false
+}

spec/javascripts/sidebar/mock_data.js

@@ -66,7 +66,7 @@ const RESPONSE_MAP = {
       },
       labels: [],
     },
-    '/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar': {
+    '/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar_extras': {
       assignees: [
         {
           name: 'User 0',
@@ -181,7 +181,7 @@ const RESPONSE_MAP = {
 const mockData = {
   responseMap: RESPONSE_MAP,
   mediator: {
-    endpoint: '/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar',
+    endpoint: '/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar_extras',
     toggleSubscriptionEndpoint: '/gitlab-org/gitlab-shell/issues/5/toggle_subscription',
     moveIssueEndpoint: '/gitlab-org/gitlab-shell/issues/5/move',
     projectsAutocompleteEndpoint: '/autocomplete/projects?project_id=15',

spec/javascripts/sidebar/sidebar_mediator_spec.js

@@ -37,7 +37,7 @@ describe('Sidebar mediator', function() {
 
   it('fetches the data', done => {
     const mockData =
-      Mock.responseMap.GET['/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar'];
+      Mock.responseMap.GET['/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar_extras'];
     spyOn(this.mediator, 'processFetchedData').and.callThrough();
 
     this.mediator
@@ -51,7 +51,7 @@ describe('Sidebar mediator', function() {
 
   it('processes fetched data', () => {
     const mockData =
-      Mock.responseMap.GET['/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar'];
+      Mock.responseMap.GET['/gitlab-org/gitlab-shell/issues/5.json?serializer=sidebar_extras'];
     this.mediator.processFetchedData(mockData);
 
     expect(this.mediator.store.assignees).toEqual(mockData.assignees);

spec/serializers/issue_serializer_spec.rb

@@ -18,7 +18,7 @@ describe IssueSerializer do
   end
 
   context 'sidebar issue serialization' do
-    let(:serializer) { 'sidebar' }
+    let(:serializer) { 'sidebar_extras' }
 
     it 'matches sidebar issue json schema' do
       expect(json_entity).to match_schema('entities/issue_sidebar')

spec/serializers/merge_request_basic_serializer_spec.rb

-require 'spec_helper'
-
-describe MergeRequestBasicSerializer do
-  let(:resource) { create(:merge_request) }
-  let(:user)     { create(:user) }
-
-  let(:json_entity) do
-    described_class.new(current_user: user)
-      .represent(resource, serializer: 'basic')
-      .with_indifferent_access
-  end
-
-  it 'matches basic merge request json' do
-    expect(json_entity).to match_schema('entities/merge_request_basic')
-  end
-end

spec/serializers/merge_request_serializer_spec.rb

@@ -18,10 +18,10 @@ describe MergeRequestSerializer do
   end
 
   context 'sidebar merge request serialization' do
-    let(:serializer) { 'sidebar' }
+    let(:serializer) { 'sidebar_extras' }
 
     it 'matches basic merge request json schema' do
-      expect(json_entity).to match_schema('entities/merge_request_basic')
+      expect(json_entity).to match_schema('entities/merge_request_sidebar')
     end
   end