Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/api.js

@@ -12,6 +12,7 @@ const Api = {
   projectsPath: '/api/:version/projects.json',
   projectPath: '/api/:version/projects/:id',
   projectLabelsPath: '/:namespace_path/:project_path/labels',
+  projectMergeRequestsPath: '/api/:version/projects/:id/merge_requests',
   projectMergeRequestPath: '/api/:version/projects/:id/merge_requests/:mrid',
   projectMergeRequestChangesPath: '/api/:version/projects/:id/merge_requests/:mrid/changes',
   projectMergeRequestVersionsPath: '/api/:version/projects/:id/merge_requests/:mrid/versions',
@@ -111,6 +112,22 @@ const Api = {
     return axios.get(url);
   },
 
+  /**
+   * Get all Merge Requests for a project, eventually filtering based on
+   * supplied parameters
+   * @param projectPath
+   * @param params
+   * @returns {Promise}
+   */
+  projectMergeRequests(projectPath, params = {}) {
+    const url = Api.buildUrl(Api.projectMergeRequestsPath).replace(
+      ':id',
+      encodeURIComponent(projectPath),
+    );
+
+    return axios.get(url, { params });
+  },
+
   // Return Merge Request for project
   projectMergeRequest(projectPath, mergeRequestId, params = {}) {
     const url = Api.buildUrl(Api.projectMergeRequestPath)

app/assets/javascripts/ide/services/index.js

@@ -40,6 +40,9 @@ export default {
   getProjectData(namespace, project) {
     return Api.project(`${namespace}/${project}`);
   },
+  getProjectMergeRequests(projectId, params = {}) {
+    return Api.projectMergeRequests(projectId, params);
+  },
   getProjectMergeRequestData(projectId, mergeRequestId, params = {}) {
     return Api.projectMergeRequest(projectId, mergeRequestId, params);
   },

app/assets/javascripts/ide/stores/actions/merge_request.js

@@ -4,6 +4,38 @@ import service from '../../services';
 import * as types from '../mutation_types';
 import { activityBarViews } from '../../constants';
 
+export const getMergeRequestsForBranch = ({ commit }, { projectId, branchId } = {}) =>
+  service
+    .getProjectMergeRequests(`${projectId}`, {
+      source_branch: branchId,
+      order_by: 'created_at',
+      per_page: 1,
+    })
+    .then(({ data }) => {
+      if (data.length > 0) {
+        const currentMR = data[0];
+
+        commit(types.SET_MERGE_REQUEST, {
+          projectPath: projectId,
+          mergeRequestId: currentMR.iid,
+          mergeRequest: currentMR,
+        });
+
+        commit(types.SET_CURRENT_MERGE_REQUEST, `${currentMR.iid}`);
+      }
+    })
+    .catch(e => {
+      flash(
+        __(`Error fetching merge requests for ${branchId}`),
+        'alert',
+        document,
+        null,
+        false,
+        true,
+      );
+      throw e;
+    });
+
 export const getMergeRequestData = (
   { commit, dispatch, state },
   { projectId, mergeRequestId, targetProjectId = null, force = false } = {},

app/assets/javascripts/ide/stores/actions/project.js

@@ -136,17 +136,24 @@ export const openBranch = ({ dispatch, state }, { projectId, branchId, basePath
   return dispatch('getFiles', {
     projectId,
     branchId,
-  }).then(() => {
-    if (basePath) {
-      const path = basePath.slice(-1) === '/' ? basePath.slice(0, -1) : basePath;
-      const treeEntryKey = Object.keys(state.entries).find(
-        key => key === path && !state.entries[key].pending,
-      );
-      const treeEntry = state.entries[treeEntryKey];
+  })
+    .then(() => {
+      if (basePath) {
+        const path = basePath.slice(-1) === '/' ? basePath.slice(0, -1) : basePath;
+        const treeEntryKey = Object.keys(state.entries).find(
+          key => key === path && !state.entries[key].pending,
+        );
+        const treeEntry = state.entries[treeEntryKey];
 
-      if (treeEntry) {
-        dispatch('handleTreeEntryAction', treeEntry);
+        if (treeEntry) {
+          dispatch('handleTreeEntryAction', treeEntry);
+        }
       }
-    }
-  });
+    })
+    .then(() => {
+      dispatch('getMergeRequestsForBranch', {
+        projectId,
+        branchId,
+      });
+    });
 };

app/helpers/ci_status_helper.rb

@@ -100,17 +100,6 @@ module CiStatusHelper
     "pipeline-status/#{pipeline_status.sha}-#{pipeline_status.status}"
   end
 
-  def render_project_pipeline_status(pipeline_status, tooltip_placement: 'left')
-    project = pipeline_status.project
-    path = pipelines_project_commit_path(project, pipeline_status.sha, ref: pipeline_status.ref)
-
-    render_status_with_link(
-      'commit',
-      pipeline_status.status,
-      path,
-      tooltip_placement: tooltip_placement)
-  end
-
   def render_commit_status(commit, ref: nil, tooltip_placement: 'left')
     project = commit.project
     path = pipelines_project_commit_path(project, commit, ref: ref)
@@ -123,12 +112,6 @@ module CiStatusHelper
       icon_size: 24)
   end
 
-  def render_pipeline_status(pipeline, tooltip_placement: 'left')
-    project = pipeline.project
-    path = project_pipeline_path(project, pipeline)
-    render_status_with_link('pipeline', pipeline.status, path, tooltip_placement: tooltip_placement)
-  end
-
   def render_status_with_link(type, status, path = nil, tooltip_placement: 'left', cssclass: '', container: 'body', icon_size: 16)
     klass = "ci-status-link ci-status-icon-#{status.dasherize} #{cssclass}"
     title = "#{type.titleize}: #{ci_label_for_status(status)}"

app/views/ci/status/_icon.html.haml

+- status            = local_assigns.fetch(:status)
+- size              = local_assigns.fetch(:size, 16)
+- type              = local_assigns.fetch(:type, 'pipeline')
+- tooltip_placement = local_assigns.fetch(:tooltip_placement, "left")
+- path              = local_assigns.fetch(:path, status.has_details? ? status.details_path : nil)
+- css_classes       = "ci-status-link ci-status-icon ci-status-icon-#{status.group} has-tooltip"
+- title             = s_("PipelineStatusTooltip|Pipeline: %{ci_status}") % {ci_status: status.label}
+- if type == 'commit'
+  - title = s_("PipelineStatusTooltip|Commit: %{ci_status}") % {ci_status: status.label}
+
+- if path
+  = link_to path, class: css_classes, title: title, data: { placement: tooltip_placement } do
+    = sprite_icon(status.icon, size: size)
+- else
+  %span{ class: css_classes, title: title, data: { placement: tooltip_placement } }
+    = sprite_icon(status.icon, size: size)

app/views/projects/issues/_merge_requests.html.haml

@@ -27,7 +27,7 @@
                   = merge_request.to_reference
                 %span.mr-ci-status.flex-md-grow-1.justify-content-end.d-flex.ml-md-2
                   - if merge_request.can_read_pipeline?
-                    = render_pipeline_status(merge_request.head_pipeline, tooltip_placement: 'bottom')
+                    = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user), tooltip_placement: 'bottom'
                   - elsif has_any_head_pipeline
                     = icon('blank fw')
 

app/views/projects/issues/_related_branches.html.haml

@@ -8,7 +8,7 @@
         - pipeline = @project.pipeline_for(branch, target.sha) if target
         - if can?(current_user, :read_pipeline, pipeline)
           %span.related-branch-ci-status
-            = render_pipeline_status(pipeline)
+            = render 'ci/status/icon', status: pipeline.detailed_status(current_user)
         %span.related-branch-info
           %strong
             = link_to branch, project_compare_path(@project, from: @project.default_branch, to: branch), class: "ref-name"

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

@@ -48,7 +48,7 @@
             CLOSED
         - if can?(current_user, :read_pipeline, merge_request.head_pipeline)
           %li.issuable-pipeline-status.d-none.d-sm-inline-block
-            = render_pipeline_status(merge_request.head_pipeline)
+            = render 'ci/status/icon', status: merge_request.head_pipeline.detailed_status(current_user)
         - if merge_request.open? && merge_request.broken?
           %li.issuable-pipeline-broken.d-none.d-sm-inline-block
             = link_to merge_request_path(merge_request), class: "has-tooltip", title: _('Cannot be merged automatically') do

app/views/shared/projects/_project.html.haml

@@ -85,7 +85,8 @@
               = sprite_icon('issues', size: 14, css_class: 'append-right-4')
               = number_with_delimiter(project.open_issues_count)
           - if pipeline_status && can?(current_user, :read_cross_project) && project.pipeline_status.has_status? && can?(current_user, :read_build, project)
+            - pipeline_path = pipelines_project_commit_path(project.pipeline_status.project, project.pipeline_status.sha, ref: project.pipeline_status.ref)
             %span.icon-wrapper.pipeline-status
-              = render_project_pipeline_status(project.pipeline_status, tooltip_placement: 'top')
+              = render 'ci/status/icon', status: project.commit.last_pipeline.detailed_status(current_user), type: 'commit', tooltip_placement: 'top', path: pipeline_path
         .updated-note
           %span Updated #{updated_tooltip}

changelogs/unreleased/45305-ci-status-icon-mismatch-on-merge-requests-page-and-the-mr-itself.yml

+---
+title: Fix pipeline status icon mismatch
+merge_request: 25407
+author:
+type: fixed

changelogs/unreleased/49663-branch-to-mr-connection.yml

+---
+title: Link to most recent MR from a branch
+merge_request: 25689
+author:
+type: added

locale/gitlab.pot

@@ -7101,6 +7101,12 @@ msgstr ""
 msgid "PipelineSheduleIntervalPattern|Custom"
 msgstr ""
 
+msgid "PipelineStatusTooltip|Commit: %{ci_status}"
+msgstr ""
+
+msgid "PipelineStatusTooltip|Pipeline: %{ci_status}"
+msgstr ""
+
 msgid "Pipelines"
 msgstr ""
 

spec/javascripts/api_spec.js

@@ -139,6 +139,40 @@ describe('Api', () => {
     });
   });
 
+  describe('projectMergeRequests', () => {
+    const projectPath = 'abc';
+    const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${projectPath}/merge_requests`;
+
+    it('fetches all merge requests for a project', done => {
+      const mockData = [{ source_branch: 'foo' }, { source_branch: 'bar' }];
+      mock.onGet(expectedUrl).reply(200, mockData);
+      Api.projectMergeRequests(projectPath)
+        .then(({ data }) => {
+          expect(data.length).toEqual(2);
+          expect(data[0].source_branch).toBe('foo');
+          expect(data[1].source_branch).toBe('bar');
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+
+    it('fetches merge requests filtered with passed params', done => {
+      const params = {
+        source_branch: 'bar',
+      };
+      const mockData = [{ source_branch: 'bar' }];
+      mock.onGet(expectedUrl, { params }).reply(200, mockData);
+
+      Api.projectMergeRequests(projectPath, params)
+        .then(({ data }) => {
+          expect(data.length).toEqual(1);
+          expect(data[0].source_branch).toBe('bar');
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
   describe('projectMergeRequest', () => {
     it('fetches a merge request', done => {
       const projectPath = 'abc';

spec/javascripts/ide/stores/actions/merge_request_spec.js

@@ -2,6 +2,7 @@ import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
 import store from '~/ide/stores';
 import actions, {
+  getMergeRequestsForBranch,
   getMergeRequestData,
   getMergeRequestChanges,
   getMergeRequestVersions,
@@ -27,6 +28,98 @@ describe('IDE store merge request actions', () => {
     resetStore(store);
   });
 
+  describe('getMergeRequestsForBranch', () => {
+    describe('success', () => {
+      const mrData = { iid: 2, source_branch: 'bar' };
+      const mockData = [mrData];
+
+      describe('base case', () => {
+        beforeEach(() => {
+          spyOn(service, 'getProjectMergeRequests').and.callThrough();
+          mock.onGet(/api\/(.*)\/projects\/abcproject\/merge_requests/).reply(200, mockData);
+        });
+
+        it('calls getProjectMergeRequests service method', done => {
+          store
+            .dispatch('getMergeRequestsForBranch', { projectId: 'abcproject', branchId: 'bar' })
+            .then(() => {
+              expect(service.getProjectMergeRequests).toHaveBeenCalledWith('abcproject', {
+                source_branch: 'bar',
+                order_by: 'created_at',
+                per_page: 1,
+              });
+
+              done();
+            })
+            .catch(done.fail);
+        });
+
+        it('sets the "Merge Request" Object', done => {
+          store
+            .dispatch('getMergeRequestsForBranch', { projectId: 'abcproject', branchId: 'bar' })
+            .then(() => {
+              expect(Object.keys(store.state.projects.abcproject.mergeRequests).length).toEqual(1);
+              expect(Object.keys(store.state.projects.abcproject.mergeRequests)[0]).toEqual('2');
+              expect(store.state.projects.abcproject.mergeRequests[2]).toEqual(
+                jasmine.objectContaining(mrData),
+              );
+              done();
+            })
+            .catch(done.fail);
+        });
+
+        it('sets "Current Merge Request" object to the most recent MR', done => {
+          store
+            .dispatch('getMergeRequestsForBranch', { projectId: 'abcproject', branchId: 'bar' })
+            .then(() => {
+              expect(store.state.currentMergeRequestId).toEqual('2');
+              done();
+            })
+            .catch(done.fail);
+        });
+      });
+
+      describe('no merge requests for branch available case', () => {
+        beforeEach(() => {
+          spyOn(service, 'getProjectMergeRequests').and.callThrough();
+          mock.onGet(/api\/(.*)\/projects\/abcproject\/merge_requests/).reply(200, []);
+        });
+
+        it('does not fail if there are no merge requests for current branch', done => {
+          store
+            .dispatch('getMergeRequestsForBranch', { projectId: 'abcproject', branchId: 'foo' })
+            .then(() => {
+              expect(Object.keys(store.state.projects.abcproject.mergeRequests).length).toEqual(0);
+              expect(store.state.currentMergeRequestId).toEqual('');
+              done();
+            })
+            .catch(done.fail);
+        });
+      });
+    });
+
+    describe('error', () => {
+      beforeEach(() => {
+        mock.onGet(/api\/(.*)\/projects\/abcproject\/merge_requests/).networkError();
+      });
+
+      it('flashes message, if error', done => {
+        const flashSpy = spyOnDependency(actions, 'flash');
+
+        getMergeRequestsForBranch({ commit() {} }, { projectId: 'abcproject', branchId: 'bar' })
+          .then(() => {
+            fail('Expected getMergeRequestsForBranch to throw an error');
+          })
+          .catch(() => {
+            expect(flashSpy).toHaveBeenCalled();
+            expect(flashSpy.calls.argsFor(0)[0]).toEqual('Error fetching merge requests for bar');
+          })
+          .then(done)
+          .catch(done.fail);
+      });
+    });
+  });
+
   describe('getMergeRequestData', () => {
     describe('success', () => {
       beforeEach(() => {

spec/javascripts/ide/stores/actions/project_spec.js

@@ -249,6 +249,7 @@ describe('IDE store project actions', () => {
             ['setCurrentBranchId', branch.branchId],
             ['getBranchData', branch],
             ['getFiles', branch],
+            ['getMergeRequestsForBranch', branch],
           ]);
         })
         .then(done)

spec/views/ci/status/_icon.html.haml_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe 'ci/status/_icon' do
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :private) }
+  let(:pipeline) { create(:ci_pipeline, project: project) }
+
+  context 'when rendering status for build' do
+    let(:build) do
+      create(:ci_build, :success, pipeline: pipeline)
+    end
+
+    context 'when user has ability to see details' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'has link to build details page' do
+        details_path = project_job_path(project, build)
+
+        render_status(build)
+
+        expect(rendered).to have_link(href: details_path)
+      end
+    end
+
+    context 'when user do not have ability to see build details' do
+      before do
+        render_status(build)
+      end
+
+      it 'contains build status text' do
+        expect(rendered).to have_css('.ci-status-icon.ci-status-icon-success')
+      end
+
+      it 'does not contain links' do
+        expect(rendered).not_to have_link
+      end
+    end
+  end
+
+  context 'when rendering status for external job' do
+    context 'when user has ability to see commit status details' do
+      before do
+        project.add_developer(user)
+      end
+
+      context 'status has external target url' do
+        before do
+          external_job = create(:generic_commit_status,
+                                status: :running,
+                                pipeline: pipeline,
+                                target_url: 'http://gitlab.com')
+
+          render_status(external_job)
+        end
+
+        it 'contains valid commit status text' do
+          expect(rendered).to have_css('.ci-status-icon.ci-status-icon-running')
+        end
+
+        it 'has link to external status page' do
+          expect(rendered).to have_link(href: 'http://gitlab.com')
+        end
+      end
+
+      context 'status do not have external target url' do
+        before do
+          external_job = create(:generic_commit_status, status: :canceled)
+
+          render_status(external_job)
+        end
+
+        it 'contains valid commit status text' do
+          expect(rendered).to have_css('.ci-status-icon.ci-status-icon-canceled')
+        end
+
+        it 'has link to external status page' do
+          expect(rendered).not_to have_link
+        end
+      end
+    end
+  end
+
+  def render_status(resource)
+    render 'ci/status/icon', status: resource.detailed_status(user)
+  end
+end