Merge branch '32536-mr-widget-performance-improvements-ee' into 'master'

Load improvements related to MR widget [EE]

See merge request !1935

app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js

@@ -176,7 +176,6 @@ export default {
       });
     },
     handleMounted() {
-      this.checkStatus();
       this.setFavicon();
       this.initDeploymentsPolling();
     },

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.mergeCheckPath);
+    this.mergeCheckResource = Vue.resource(endpoints.statusPath);
     this.cancelAutoMergeResource = Vue.resource(endpoints.cancelAutoMergePath);
     this.removeWIPResource = Vue.resource(endpoints.removeWIPPath);
     this.removeSourceBranchResource = Vue.resource(endpoints.sourceBranchPath);

app/controllers/projects/merge_requests_controller.rb

@@ -9,17 +9,18 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   before_action :module_enabled
   before_action :merge_request, only: [
-    :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :pipelines, :merge, :merge_check,
+    :edit, :update, :show, :diffs, :commits, :conflicts, :conflict_for_path, :pipelines, :merge,
     :pipeline_status, :ci_environments_status, :toggle_subscription, :cancel_merge_when_pipeline_succeeds,
     :remove_wip, :resolve_conflicts, :assign_related_issues, :commit_change_content,
     # EE
     :approve, :approvals, :unapprove, :rebase
   ]
   before_action :validates_merge_request, only: [:show, :diffs, :commits, :pipelines]
-  before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines]
+  before_action :define_show_vars, only: [:diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines]
   before_action :define_commit_vars, only: [:diffs]
   before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :conflict_for_path, :pipelines]
   before_action :close_merge_request_without_source_project, only: [:show, :diffs, :commits, :builds, :pipelines]
+  before_action :check_if_can_be_merged, only: :show
   before_action :apply_diff_view_cookie!, only: [:new_diffs]
   before_action :build_merge_request, only: [:new, :new_diffs]
   before_action :set_suggested_approvers, only: [:new, :new_diffs, :edit]
@@ -79,9 +80,12 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     respond_to do |format|
       format.html do
         define_discussion_vars
+        define_show_vars
       end
 
       format.json do
+        Gitlab::PollingInterval.set_header(response, interval: 10_000)
+
         render json: serializer.represent(@merge_request, basic: params[:basic])
       end
 
@@ -319,12 +323,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     render json: serializer.represent(@merge_request)
   end
 
-  def merge_check
-    @merge_request.check_if_can_be_merged
-
-    render json: serializer.represent(@merge_request)
-  end
-
   def commit_change_content
     render partial: 'projects/merge_requests/widget/commit_change_content', layout: false
   end
@@ -727,6 +725,10 @@ class Projects::MergeRequestsController < Projects::ApplicationController
 
   private
 
+  def check_if_can_be_merged
+    @merge_request.check_if_can_be_merged
+  end
+
   def merge!
     # Disable the CI check if merge_when_pipeline_succeeds is enabled since we have
     # to wait until CI completes to know

app/serializers/merge_request_basic_entity.rb

 class MergeRequestBasicEntity < Grape::Entity
+  expose :assignee_id
   expose :merge_status
   expose :merge_error
   expose :state

app/serializers/merge_request_entity.rb

 class MergeRequestEntity < IssuableEntity
-  expose :assignee_id
   include RequestAwareEntity
 
   expose :in_progress_merge_commit_sha
@@ -165,12 +164,6 @@ class MergeRequestEntity < IssuableEntity
                                          format: :json)
   end
 
-  expose :merge_check_path do |merge_request|
-    merge_check_namespace_project_merge_request_path(merge_request.project.namespace,
-                                                     merge_request.project,
-                                                     merge_request)
-  end
-
   expose :ci_environments_status_path do |merge_request|
     ci_environments_status_namespace_project_merge_request_path(merge_request.project.namespace,
                                                                 merge_request.project,

app/views/shared/issuable/_sidebar.html.haml

@@ -167,7 +167,7 @@
 
     :javascript
       gl.sidebarOptions = {
-        endpoint: "#{issuable_json_path(issuable)}",
+        endpoint: "#{issuable_json_path(issuable)}?basic=true",
         editable: #{can_edit_issuable ? true : false},
         currentUser: #{current_user.to_json(only: [:username, :id, :name], methods: :avatar_url)},
         rootPath: "#{root_path}"

config/routes/project.rb

@@ -74,7 +74,6 @@ constraints(ProjectUrlConstrainer.new) do
           get :conflicts
           get :conflict_for_path
           get :pipelines
-          get :merge_check
           get :commit_change_content
           post :merge
           post :cancel_merge_when_pipeline_succeeds

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -282,6 +282,18 @@ describe Projects::MergeRequestsController do
           expect(response).to match_response_schema('entities/merge_request')
         end
       end
+
+      context 'number of queries' do
+        it 'verifies number of queries' do
+          # pre-create objects
+          merge_request
+
+          recorded = ActiveRecord::QueryRecorder.new { go(format: :json) }
+
+          expect(recorded.count).to be_within(1).of(100)
+          expect(recorded.cached_count).to eq(0)
+        end
+      end
     end
 
     describe "as diff" do

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

@@ -3,7 +3,6 @@
   "properties" : {
     "id": { "type": "integer" },
     "iid": { "type": "integer" },
-    "assignee_id": { "type": ["integer", "null"] },
     "author_id": { "type": "integer" },
     "description": { "type": ["string", "null"] },
     "lock_version": { "type": ["string", "null"] },

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

@@ -9,7 +9,8 @@
     "human_time_estimate": { "type": ["string", "null"] },
     "human_total_time_spent": { "type": ["string", "null"] },
     "merge_error": { "type": ["string", "null"] },
-    "rebase_in_progress": { "type": "boolean" }
+    "rebase_in_progress": { "type": "boolean" },
+    "assignee_id": { "type": ["integer", "null"] }
   },
   "additionalProperties": false
 }

spec/javascripts/vue_mr_widget/mr_widget_options_spec.js

@@ -227,13 +227,11 @@ describe('mrWidgetOptions', () => {
 
     describe('handleMounted', () => {
       it('should call required methods to do the initial kick-off', () => {
-        spyOn(vm, 'checkStatus');
         spyOn(vm, 'initDeploymentsPolling');
         spyOn(vm, 'setFavicon');
 
         vm.handleMounted();
 
-        expect(vm.checkStatus).toHaveBeenCalled();
         expect(vm.setFavicon).toHaveBeenCalled();
         expect(vm.initDeploymentsPolling).toHaveBeenCalled();
       });

spec/routing/project_routing_spec.rb

@@ -243,7 +243,6 @@ describe 'project routing' do
   #               diffs_namespace_project_merge_request GET      /:namespace_id/:project_id/merge_requests/:id/diffs(.:format)               projects/merge_requests#diffs
   #             commits_namespace_project_merge_request GET      /:namespace_id/:project_id/merge_requests/:id/commits(.:format)             projects/merge_requests#commits
   #           merge_namespace_project_merge_request POST     /:namespace_id/:project_id/merge_requests/:id/merge(.:format)           projects/merge_requests#merge
-  #     merge_check_namespace_project_merge_request GET      /:namespace_id/:project_id/merge_requests/:id/merge_check(.:format)     projects/merge_requests#merge_check
   #           ci_status_namespace_project_merge_request GET      /:namespace_id/:project_id/merge_requests/:id/ci_status(.:format)           projects/merge_requests#ci_status
   # toggle_subscription_namespace_project_merge_request POST     /:namespace_id/:project_id/merge_requests/:id/toggle_subscription(.:format) projects/merge_requests#toggle_subscription
   #        branch_from_namespace_project_merge_requests GET      /:namespace_id/:project_id/merge_requests/branch_from(.:format)             projects/merge_requests#branch_from
@@ -272,10 +271,6 @@ describe 'project routing' do
       )
     end
 
-    it 'to #merge_check' do
-      expect(get('/gitlab/gitlabhq/merge_requests/1/merge_check')).to route_to('projects/merge_requests#merge_check', namespace_id: 'gitlab', project_id: 'gitlabhq', id: '1')
-    end
-
     it 'to #branch_from' do
       expect(get('/gitlab/gitlabhq/merge_requests/branch_from')).to route_to('projects/merge_requests#branch_from', namespace_id: 'gitlab', project_id: 'gitlabhq')
     end