Merge request widget redesign

4 files changed, 270 insertions, 158 deletions

diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb
index 8f915d9d210..f285e5333d6 100644
--- a/spec/controllers/projects/branches_controller_spec.rb
+++ b/spec/controllers/projects/branches_controller_spec.rb
@@ -213,33 +213,98 @@ describe Projects::BranchesController do
       sign_in(user)
 
       post :destroy,
-           format: :js,
-           id: branch,
-           namespace_id: project.namespace,
-           project_id: project
+        format: format,
+        id: branch,
+        namespace_id: project.namespace,
+        project_id: project
     end
 
-    context "valid branch name, valid source" do
+    context 'as JS' do
       let(:branch) { "feature" }
+      let(:format) { :js }
 
-      it { expect(response).to have_http_status(200) }
-    end
+      context "valid branch name, valid source" do
+        let(:branch) { "feature" }
+
+        it { expect(response).to have_http_status(200) }
+        it { expect(response.body).to be_blank }
+      end
+
+      context "valid branch name with unencoded slashes" do
+        let(:branch) { "improve/awesome" }
+
+        it { expect(response).to have_http_status(200) }
+        it { expect(response.body).to be_blank }
+      end
+
+      context "valid branch name with encoded slashes" do
+        let(:branch) { "improve%2Fawesome" }
 
-    context "valid branch name with unencoded slashes" do
-      let(:branch) { "improve/awesome" }
+        it { expect(response).to have_http_status(200) }
+        it { expect(response.body).to be_blank }
+      end
 
-      it { expect(response).to have_http_status(200) }
+      context "invalid branch name, valid ref" do
+        let(:branch) { "no-branch" }
+
+        it { expect(response).to have_http_status(404) }
+        it { expect(response.body).to be_blank }
+      end
     end
 
-    context "valid branch name with encoded slashes" do
-      let(:branch) { "improve%2Fawesome" }
+    context 'as JSON' do
+      let(:branch) { "feature" }
+      let(:format) { :json }
+
+      context 'valid branch name, valid source' do
+        let(:branch) { "feature" }
 
-      it { expect(response).to have_http_status(200) }
+        it 'returns JSON response with message' do
+          expect(json_response).to eql("message" => 'Branch was removed')
+        end
+
+        it { expect(response).to have_http_status(200) }
+      end
+
+      context 'valid branch name with unencoded slashes' do
+        let(:branch) { "improve/awesome" }
+
+        it 'returns JSON response with message' do
+          expect(json_response).to eql('message' => 'Branch was removed')
+        end
+
+        it { expect(response).to have_http_status(200) }
+      end
+
+      context "valid branch name with encoded slashes" do
+        let(:branch) { 'improve%2Fawesome' }
+
+        it 'returns JSON response with message' do
+          expect(json_response).to eql('message' => 'Branch was removed')
+        end
+
+        it { expect(response).to have_http_status(200) }
+      end
+
+      context 'invalid branch name, valid ref' do
+        let(:branch) { 'no-branch' }
+
+        it 'returns JSON response with message' do
+          expect(json_response).to eql('message' => 'No such branch')
+        end
+
+        it { expect(response).to have_http_status(404) }
+      end
     end
-    context "invalid branch name, valid ref" do
-      let(:branch) { "no-branch" }
 
-      it { expect(response).to have_http_status(404) }
+    context 'as HTML' do
+      let(:branch) { "feature" }
+      let(:format) { :html }
+
+      it 'redirects to branches path' do
+        expect(response)
+          .to redirect_to(namespace_project_branches_path(project.namespace, project))
+      end
     end
   end
 
diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb
index 89692b601b2..3de38bb4dac 100644
--- a/spec/controllers/projects/deployments_controller_spec.rb
+++ b/spec/controllers/projects/deployments_controller_spec.rb
@@ -8,7 +8,7 @@ describe Projects::DeploymentsController do
   let(:environment) { create(:environment, name: 'production', project: project) }
 
   before do
-    project.add_master(user)
+    project.team << [user, :master]
 
     sign_in(user)
   end
@@ -19,7 +19,7 @@ describe Projects::DeploymentsController do
       create(:deployment, environment: environment, created_at: 7.hours.ago)
       create(:deployment, environment: environment)
 
-      get :index, environment_params(after: 8.hours.ago)
+      get :index, deployment_params(after: 8.hours.ago)
 
       expect(response).to be_ok
 
@@ -29,14 +29,59 @@ describe Projects::DeploymentsController do
     it 'returns a list with deployments information' do
       create(:deployment, environment: environment)
 
-      get :index, environment_params
+      get :index, deployment_params
 
       expect(response).to be_ok
       expect(response).to match_response_schema('deployments')
     end
   end
 
-  def environment_params(opts = {})
-    opts.reverse_merge(namespace_id: project.namespace, project_id: project, environment_id: environment.id)
+  describe 'GET #metrics' do
+    let(:deployment) { create(:deployment, project: project, environment: environment) }
+
+    before do
+      allow(controller).to receive(:deployment).and_return(deployment)
+    end
+
+    context 'when environment has no metrics' do
+      before do
+        expect(deployment).to receive(:metrics).and_return(nil)
+      end
+
+      it 'returns a empty response 204 resposne' do
+        get :metrics, deployment_params(id: deployment.id)
+        expect(response).to have_http_status(204)
+        expect(response.body).to eq('')
+      end
+    end
+
+    context 'when environment has some metrics' do
+      let(:empty_metrics) do
+        {
+          success: true,
+          metrics: {},
+          last_update: 42
+        }
+      end
+
+      before do
+        expect(deployment).to receive(:metrics).and_return(empty_metrics)
+      end
+
+      it 'returns a metrics JSON document' do
+        get :metrics, deployment_params(id: deployment.id)
+
+        expect(response).to be_ok
+        expect(json_response['success']).to be(true)
+        expect(json_response['metrics']).to eq({})
+        expect(json_response['last_update']).to eq(42)
+      end
+    end
+  end
+
+  def deployment_params(opts = {})
+    opts.reverse_merge(namespace_id: project.namespace,
+                       project_id: project,
+                       environment_id: environment.id)
   end
 end
diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb
index 5c478534ff3..c0f8c36a018 100644
--- a/spec/controllers/projects/environments_controller_spec.rb
+++ b/spec/controllers/projects/environments_controller_spec.rb
@@ -149,6 +149,48 @@ describe Projects::EnvironmentsController do
     end
   end
 
+  describe 'PATCH #stop' do
+    context 'when env not available' do
+      it 'returns 404' do
+        allow_any_instance_of(Environment).to receive(:available?) { false }
+
+        patch :stop, environment_params(format: :json)
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    context 'when stop action' do
+      it 'returns action url' do
+        action = create(:ci_build, :manual)
+
+        allow_any_instance_of(Environment)
+          .to receive_messages(available?: true, stop_with_action!: action)
+
+        patch :stop, environment_params(format: :json)
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to eq(
+          { 'redirect_url' =>
+              "http://test.host/#{project.path_with_namespace}/builds/#{action.id}" })
+      end
+    end
+
+    context 'when no stop action' do
+      it 'returns env url' do
+        allow_any_instance_of(Environment)
+          .to receive_messages(available?: true, stop_with_action!: nil)
+
+        patch :stop, environment_params(format: :json)
+
+        expect(response).to have_http_status(200)
+        expect(json_response).to eq(
+          { 'redirect_url' =>
+              "http://test.host/#{project.path_with_namespace}/environments/#{environment.id}" })
+      end
+    end
+  end
+
   describe 'GET #terminal' do
     context 'with valid id' do
       it 'responds with a status code 200' do
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 0483c6b7879..964246b580c 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -59,6 +59,18 @@ describe Projects::MergeRequestsController do
     end
   end
 
+  describe 'GET commit_change_content' do
+    it 'renders commit_change_content template' do
+      get :commit_change_content,
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: merge_request.iid,
+        format: 'html'
+
+      expect(response).to render_template('_commit_change_content')
+    end
+  end
+
   shared_examples "loads labels" do |action|
     it "loads labels into the @labels variable" do
       get action,
@@ -71,63 +83,47 @@ describe Projects::MergeRequestsController do
   end
 
   describe "GET show" do
-    shared_examples "export merge as" do |format|
-      it "does generally work" do
-        get(:show,
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            id: merge_request.iid,
-            format: format)
+    def go(extra_params = {})
+      params = {
+        namespace_id: project.namespace.to_param,
+        project_id: project,
+        id: merge_request.iid
+      }
 
-        expect(response).to be_success
-      end
+      get :show, params.merge(extra_params)
+    end
 
-      it_behaves_like "loads labels", :show
+    it_behaves_like "loads labels", :show
 
-      it "generates it" do
-        expect_any_instance_of(MergeRequest).to receive(:"to_#{format}")
+    describe 'as html' do
+      it "renders merge request page" do
+        go(format: :html)
 
-        get(:show,
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            id: merge_request.iid,
-            format: format)
+        expect(response).to be_success
       end
+    end
 
-      it "renders it" do
-        get(:show,
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            id: merge_request.iid,
-            format: format)
+    describe 'as json' do
+      context 'with basic param' do
+        it 'renders basic MR entity as json' do
+          go(basic: true, format: :json)
 
-        expect(response.body).to eq(merge_request.send(:"to_#{format}").to_s)
+          expect(response).to match_response_schema('entities/merge_request_basic')
+        end
       end
 
-      it "does not escape Html" do
-        allow_any_instance_of(MergeRequest).to receive(:"to_#{format}").
-          and_return('HTML entities &<>" ')
+      context 'without basic param' do
+        it 'renders the merge request in the json format' do
+          go(format: :json)
 
-        get(:show,
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            id: merge_request.iid,
-            format: format)
-
-        expect(response.body).not_to include('&amp;')
-        expect(response.body).not_to include('&gt;')
-        expect(response.body).not_to include('&lt;')
-        expect(response.body).not_to include('&quot;')
+          expect(response).to match_response_schema('entities/merge_request')
+        end
       end
     end
 
     describe "as diff" do
       it "triggers workhorse to serve the request" do
-        get(:show,
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            id: merge_request.iid,
-            format: :diff)
+        go(format: :diff)
 
         expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-diff:")
       end
@@ -135,11 +131,7 @@ describe Projects::MergeRequestsController do
 
     describe "as patch" do
       it 'triggers workhorse to serve the request' do
-        get(:show,
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            id: merge_request.iid,
-            format: :patch)
+        go(format: :patch)
 
         expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-format-patch:")
       end
@@ -295,19 +287,18 @@ describe Projects::MergeRequestsController do
         namespace_id: project.namespace,
         project_id: project,
         id: merge_request.iid,
-        format: 'raw'
+        format: 'json'
       }
     end
 
-    context 'when the user does not have access' do
+    context 'when user cannot access' do
       before do
-        project.team.truncate
-        project.team << [user, :reporter]
-        post :merge, base_params
+        project.add_reporter(user)
+        xhr :post, :merge, base_params
       end
 
-      it 'returns not found' do
-        expect(response).to be_not_found
+      it 'returns 404' do
+        expect(response).to have_http_status(404)
       end
     end
 
@@ -319,7 +310,7 @@ describe Projects::MergeRequestsController do
       end
 
       it 'returns :failed' do
-        expect(assigns(:status)).to eq(:failed)
+        expect(json_response).to eq('status' => 'failed')
       end
     end
 
@@ -327,7 +318,7 @@ describe Projects::MergeRequestsController do
       before { post :merge, base_params.merge(sha: 'foo') }
 
       it 'returns :sha_mismatch' do
-        expect(assigns(:status)).to eq(:sha_mismatch)
+        expect(json_response).to eq('status' => 'sha_mismatch')
       end
     end
 
@@ -339,7 +330,7 @@ describe Projects::MergeRequestsController do
       it 'returns :success' do
         merge_with_sha
 
-        expect(assigns(:status)).to eq(:success)
+        expect(json_response).to eq('status' => 'success')
       end
 
       it 'starts the merge immediately' do
@@ -360,7 +351,7 @@ describe Projects::MergeRequestsController do
         it 'returns :merge_when_pipeline_succeeds' do
           merge_when_pipeline_succeeds
 
-          expect(assigns(:status)).to eq(:merge_when_pipeline_succeeds)
+          expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds')
         end
 
         it 'sets the MR to merge when the pipeline succeeds' do
@@ -382,7 +373,7 @@ describe Projects::MergeRequestsController do
           it 'returns :merge_when_pipeline_succeeds' do
             merge_when_pipeline_succeeds
 
-            expect(assigns(:status)).to eq(:merge_when_pipeline_succeeds)
+            expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds')
           end
         end
       end
@@ -403,7 +394,7 @@ describe Projects::MergeRequestsController do
             it 'returns :failed' do
               merge_with_sha
 
-              expect(assigns(:status)).to eq(:failed)
+              expect(json_response).to eq('status' => 'failed')
             end
           end
 
@@ -416,7 +407,7 @@ describe Projects::MergeRequestsController do
             it 'returns :success' do
               merge_with_sha
 
-              expect(assigns(:status)).to eq(:success)
+              expect(json_response).to eq('status' => 'success')
             end
           end
         end
@@ -434,7 +425,7 @@ describe Projects::MergeRequestsController do
             it 'returns :success' do
               merge_with_sha
 
-              expect(assigns(:status)).to eq(:success)
+              expect(json_response).to eq('status' => 'success')
             end
           end
 
@@ -447,7 +438,7 @@ describe Projects::MergeRequestsController do
             it 'returns :success' do
               merge_with_sha
 
-              expect(assigns(:status)).to eq(:success)
+              expect(json_response).to eq('status' => 'success')
             end
           end
         end
@@ -831,18 +822,55 @@ describe Projects::MergeRequestsController do
     end
   end
 
-  context 'POST remove_wip' do
-    it 'removes the wip status' do
+  describe 'POST remove_wip' do
+    before do
       merge_request.title = merge_request.wip_title
       merge_request.save
 
-      post :remove_wip,
-           namespace_id: merge_request.project.namespace.to_param,
-           project_id: merge_request.project,
-           id: merge_request.iid
+      xhr :post, :remove_wip,
+        namespace_id: merge_request.project.namespace.to_param,
+        project_id: merge_request.project,
+        id: merge_request.iid,
+        format: :json
+    end
 
+    it 'removes the wip status' do
       expect(merge_request.reload.title).to eq(merge_request.wipless_title)
     end
+
+    it 'renders MergeRequest as JSON' do
+      expect(json_response.keys).to include('id', 'iid', 'description')
+    end
+  end
+
+  describe 'POST cancel_merge_when_pipeline_succeeds' do
+    subject do
+      xhr :post, :cancel_merge_when_pipeline_succeeds,
+        namespace_id: merge_request.project.namespace.to_param,
+        project_id: merge_request.project,
+        id: merge_request.iid,
+        format: :json
+    end
+
+    it 'calls MergeRequests::MergeWhenPipelineSucceedsService' do
+      mwps_service = double
+
+      allow(MergeRequests::MergeWhenPipelineSucceedsService)
+        .to receive(:new)
+        .and_return(mwps_service)
+
+      expect(mwps_service).to receive(:cancel).with(merge_request)
+
+      subject
+    end
+
+    it { is_expected.to have_http_status(:success) }
+
+    it 'renders MergeRequest as JSON' do
+      subject
+
+      expect(json_response.keys).to include('id', 'iid', 'description')
+    end
   end
 
   describe 'GET conflict_for_path' do
@@ -1121,74 +1149,6 @@ describe Projects::MergeRequestsController do
     end
   end
 
-  describe 'GET merge_widget_refresh' do
-    let(:params) do
-      {
-        namespace_id: project.namespace,
-        project_id: project,
-        id: merge_request.iid,
-        format: :raw
-      }
-    end
-
-    before do
-      project.team << [user, :developer]
-      xhr :get, :merge_widget_refresh, params
-    end
-
-    context 'when merge in progress' do
-      let(:merge_request) { create(:merge_request, source_project: project, in_progress_merge_commit_sha: 'sha') }
-
-      it 'returns an OK response' do
-        expect(response).to have_http_status(:ok)
-      end
-
-      it 'sets status to :success' do
-        expect(assigns(:status)).to eq(:success)
-        expect(response).to render_template('merge')
-      end
-    end
-
-    context 'when merge request was merged already' do
-      let(:merge_request) { create(:merge_request, source_project: project, state: :merged) }
-
-      it 'returns an OK response' do
-        expect(response).to have_http_status(:ok)
-      end
-
-      it 'sets status to :success' do
-        expect(assigns(:status)).to eq(:success)
-        expect(response).to render_template('merge')
-      end
-    end
-
-    context 'when waiting for build' do
-      let(:merge_request) { create(:merge_request, source_project: project, merge_when_pipeline_succeeds: true, merge_user: user) }
-
-      it 'returns an OK response' do
-        expect(response).to have_http_status(:ok)
-      end
-
-      it 'sets status to :merge_when_pipeline_succeeds' do
-        expect(assigns(:status)).to eq(:merge_when_pipeline_succeeds)
-        expect(response).to render_template('merge')
-      end
-    end
-
-    context 'when MR does not have special state' do
-      let(:merge_request) { create(:merge_request, source_project: project) }
-
-      it 'returns an OK response' do
-        expect(response).to have_http_status(:ok)
-      end
-
-      it 'sets status to success' do
-        expect(assigns(:status)).to eq(:success)
-        expect(response).to render_template('merge')
-      end
-    end
-  end
-
   describe 'GET pipeline_status.json' do
     context 'when head_pipeline exists' do
       let!(:pipeline) do