Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/environments/components/environment_item.vue

@@ -111,12 +111,7 @@ export default {
      * @returns {Boolean|Undefined}
      */
     canShowDate() {
-      return (
-        this.model &&
-        this.model.last_deployment &&
-        this.model.last_deployment.deployable &&
-        this.model.last_deployment.deployable !== undefined
-      );
+      return this.model && this.model.last_deployment && this.model.last_deployment.deployed_at;
     },
 
     /**
@@ -124,14 +119,9 @@ export default {
      *
      * @returns {String}
      */
-    createdDate() {
-      if (
-        this.model &&
-        this.model.last_deployment &&
-        this.model.last_deployment.deployable &&
-        this.model.last_deployment.deployable.created_at
-      ) {
-        return timeagoInstance.format(this.model.last_deployment.deployable.created_at);
+    deployedDate() {
+      if (this.canShowDate) {
+        return timeagoInstance.format(this.model.last_deployment.deployed_at);
       }
       return '';
     },
@@ -547,7 +537,7 @@ export default {
     <div v-if="!model.isFolder" class="table-section section-10" role="gridcell">
       <div role="rowheader" class="table-mobile-header">{{ s__('Environments|Updated') }}</div>
       <span v-if="canShowDate" class="environment-created-date-timeago table-mobile-content">
-        {{ createdDate }}
+        {{ deployedDate }}
       </span>
     </div>
 

app/assets/stylesheets/framework/forms.scss

@@ -245,27 +245,3 @@ label {
 .input-group-text {
   max-height: $input-height;
 }
-
-.gl-form-checkbox {
-  align-items: baseline;
-  margin-right: 1rem;
-  margin-bottom: 0.25rem;
-
-  .form-check-input {
-    margin-right: 0;
-  }
-
-  .form-check-label {
-    padding-left: $gl-padding-8;
-  }
-
-  &.form-check-inline .form-check-input {
-    align-self: flex-start;
-    height: 1.5 * $gl-font-size;
-  }
-
-  .form-check-input:disabled,
-  .form-check-input:disabled ~ .form-check-label {
-    cursor: not-allowed;
-  }
-}

app/views/projects/deployments/_deployment.html.haml

@@ -22,7 +22,8 @@
 
   .table-section.section-15{ role: 'gridcell' }
     .table-mobile-header{ role: 'rowheader' }= _("Created")
-    %span.table-mobile-content= time_ago_with_tooltip(deployment.created_at)
+    - if deployment.deployed_at
+      %span.table-mobile-content= time_ago_with_tooltip(deployment.deployed_at)
 
   .table-section.section-20.table-button-footer{ role: 'gridcell' }
     .btn-group.table-action-buttons

app/views/projects/milestones/_form.html.haml

@@ -21,7 +21,7 @@
 
   .form-actions
     - if @milestone.new_record?
-      = f.submit _('Create milestone'), class: 'btn-create btn qa-milestone-create-button'
+      = f.submit _('Create milestone'), class: 'btn-success btn qa-milestone-create-button'
       = link_to _('Cancel'), project_milestones_path(@project), class: 'btn btn-cancel'
     - else
       = f.submit _('Save changes'), class: 'btn-success btn'

changelogs/unreleased/56130-operations-environments-shows-incorrect-deployment-date-for-manual-.yml

+---
+title: Update the timestamp in Operations > Environments to show correct deployment
+  date for manual deploy jobs
+merge_request: 32072
+author:
+type: fixed

changelogs/unreleased/ce-xanf-move-auto-merge-failed-to-jest.yml

+---
+title: Refactored Karma spec to Jest for mr_widget_auto_merge_failed
+merge_request: 32282
+author: Illya Klymov
+type: other

changelogs/unreleased/new-project-milestone-primary-button.yml

+---
+title: New project milestone primary button
+merge_request: 32355
+author: Lee Tickett
+type: fixed

lib/api/entities.rb

@@ -1705,6 +1705,18 @@ module API
     class ClusterGroup < Cluster
       expose :group, using: Entities::BasicGroupDetails
     end
+
+    module InternalPostReceive
+      class Message < Grape::Entity
+        expose :message
+        expose :type
+      end
+
+      class Response < Grape::Entity
+        expose :messages, using: Message
+        expose :reference_counter_decreased
+      end
+    end
   end
 end
 

lib/api/helpers/internal_helpers.rb

@@ -44,8 +44,6 @@ module API
       end
 
       def process_mr_push_options(push_options, project, user, changes)
-        output = {}
-
         Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/61359')
 
         service = ::MergeRequests::PushOptionsHandlerService.new(
@@ -56,15 +54,13 @@ module API
         ).execute
 
         if service.errors.present?
-          output[:warnings] = push_options_warning(service.errors.join("\n\n"))
+          push_options_warning(service.errors.join("\n\n"))
         end
-
-        output
       end
 
       def push_options_warning(warning)
         options = Array.wrap(params[:push_options]).map { |p| "'#{p}'" }.join(' ')
-        "Error encountered with push options #{options}: #{warning}"
+        "WARNINGS:\nError encountered with push options #{options}: #{warning}"
       end
 
       def redis_ping

lib/api/internal.rb

@@ -256,25 +256,26 @@ module API
       post '/post_receive' do
         status 200
 
-        output = {} # Messages to gitlab-shell
+        response = Gitlab::InternalPostReceive::Response.new
         user = identify(params[:identifier])
         project = Gitlab::GlRepository.parse(params[:gl_repository]).first
         push_options = Gitlab::PushOptions.new(params[:push_options])
 
+        response.reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease
+
         PostReceive.perform_async(params[:gl_repository], params[:identifier],
           params[:changes], push_options.as_json)
 
         mr_options = push_options.get(:merge_request)
-        output.merge!(process_mr_push_options(mr_options, project, user, params[:changes])) if mr_options.present?
+        if mr_options.present?
+          message = process_mr_push_options(mr_options, project, user, params[:changes])
+          response.add_alert_message(message)
+        end
 
         broadcast_message = BroadcastMessage.current&.last&.message
-        reference_counter_decreased = Gitlab::ReferenceCounter.new(params[:gl_repository]).decrease
+        response.add_alert_message(broadcast_message)
 
-        output.merge!(
-          broadcast_message: broadcast_message,
-          reference_counter_decreased: reference_counter_decreased,
-          merge_request_urls: merge_request_urls
-        )
+        response.add_merge_request_urls(merge_request_urls)
 
         # A user is not guaranteed to be returned; an orphaned write deploy
         # key could be used
@@ -282,11 +283,11 @@ module API
           redirect_message = Gitlab::Checks::ProjectMoved.fetch_message(user.id, project.id)
           project_created_message = Gitlab::Checks::ProjectCreated.fetch_message(user.id, project.id)
 
-          output[:redirected_message] = redirect_message if redirect_message
-          output[:project_created_message] = project_created_message if project_created_message
+          response.add_basic_message(redirect_message)
+          response.add_basic_message(project_created_message)
         end
 
-        output
+        present response, with: Entities::InternalPostReceive::Response
       end
     end
   end

lib/gitlab/internal_post_receive/response.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module InternalPostReceive
+    class Response
+      attr_accessor :reference_counter_decreased
+      attr_reader :messages
+
+      Message = Struct.new(:message, :type) do
+        def self.basic(text)
+          new(text, :basic)
+        end
+
+        def self.alert(text)
+          new(text, :alert)
+        end
+      end
+
+      def initialize
+        @messages = []
+        @reference_counter_decreased = false
+      end
+
+      def add_merge_request_urls(urls_data)
+        urls_data.each do |url_data|
+          add_merge_request_url(url_data)
+        end
+      end
+
+      def add_merge_request_url(url_data)
+        message = if url_data[:new_merge_request]
+                    "To create a merge request for #{url_data[:branch_name]}, visit:"
+                  else
+                    "View merge request for #{url_data[:branch_name]}:"
+                  end
+
+        message += "\n  #{url_data[:url]}"
+
+        add_basic_message(message)
+      end
+
+      def add_basic_message(text)
+        @messages << Message.basic(text) if text.present?
+      end
+
+      def add_alert_message(text)
+        @messages.unshift(Message.alert(text)) if text.present?
+      end
+    end
+  end
+end

spec/javascripts/vue_mr_widget/components/states/mr_widget_auto_merge_failed_spec.js → spec/frontend/vue_mr_widget/components/states/mr_widget_auto_merge_failed_spec.js

-import Vue from 'vue';
-import autoMergeFailedComponent from '~/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed.vue';
+import { shallowMount } from '@vue/test-utils';
+import { GlLoadingIcon } from '@gitlab/ui';
+import AutoMergeFailedComponent from '~/vue_merge_request_widget/components/states/mr_widget_auto_merge_failed.vue';
 import eventHub from '~/vue_merge_request_widget/event_hub';
-import mountComponent from 'spec/helpers/vue_mount_component_helper';
 
 describe('MRWidgetAutoMergeFailed', () => {
-  let vm;
+  let wrapper;
   const mergeError = 'This is the merge error';
+  const findButton = () => wrapper.find('button');
+
+  const createComponent = (props = {}) => {
+    wrapper = shallowMount(AutoMergeFailedComponent, {
+      sync: false,
+      propsData: { ...props },
+    });
+  };
 
   beforeEach(() => {
-    const Component = Vue.extend(autoMergeFailedComponent);
-    vm = mountComponent(Component, {
+    createComponent({
       mr: { mergeError },
     });
   });
 
   afterEach(() => {
-    vm.$destroy();
+    wrapper.destroy();
   });
 
   it('renders failed message', () => {
-    expect(vm.$el.textContent).toContain('This merge request failed to be merged automatically');
+    expect(wrapper.text()).toContain('This merge request failed to be merged automatically');
   });
 
   it('renders merge error provided', () => {
-    expect(vm.$el.innerText).toContain(mergeError);
+    expect(wrapper.text()).toContain(mergeError);
   });
 
   it('render refresh button', () => {
-    expect(vm.$el.querySelector('button').textContent.trim()).toEqual('Refresh');
+    expect(findButton().text()).toEqual('Refresh');
   });
 
-  it('emits event and shows loading icon when button is clicked', done => {
-    spyOn(eventHub, '$emit');
-    vm.$el.querySelector('button').click();
+  it('emits event and shows loading icon when button is clicked', () => {
+    jest.spyOn(eventHub, '$emit');
+    findButton().trigger('click');
 
-    expect(eventHub.$emit.calls.argsFor(0)[0]).toEqual('MRWidgetUpdateRequested');
+    expect(eventHub.$emit.mock.calls[0][0]).toBe('MRWidgetUpdateRequested');
 
-    Vue.nextTick(() => {
-      expect(vm.$el.querySelector('button').getAttribute('disabled')).toEqual('disabled');
-      expect(vm.$el.querySelector('button .loading-container span').classList).toContain(
-        'gl-spinner',
-      );
-      done();
+    return wrapper.vm.$nextTick(() => {
+      expect(findButton().attributes('disabled')).toEqual('disabled');
+      expect(
+        findButton()
+          .find(GlLoadingIcon)
+          .exists(),
+      ).toBe(true);
     });
   });
 });

spec/javascripts/environments/environment_item_spec.js

@@ -106,6 +106,7 @@ describe('Environment item', () => {
               play_path: '/play',
             },
           ],
+          deployed_at: '2016-11-29T18:11:58.430Z',
         },
         has_stop_action: true,
         environment_path: 'root/ci-folders/environments/31',
@@ -139,9 +140,7 @@ describe('Environment item', () => {
 
       it('should render last deployment date', () => {
         const timeagoInstance = new timeago(); // eslint-disable-line
-        const formatedDate = timeagoInstance.format(
-          environment.last_deployment.deployable.created_at,
-        );
+        const formatedDate = timeagoInstance.format(environment.last_deployment.deployed_at);
 
         expect(
           component.$el.querySelector('.environment-created-date-timeago').textContent,

spec/lib/gitlab/internal_post_receive/response_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::InternalPostReceive::Response do
+  subject { described_class.new }
+
+  describe '#add_merge_request_urls' do
+    context 'when there are urls_data' do
+      it 'adds a message for each merge request URL' do
+        urls_data = [
+          { new_merge_request: false, branch_name: 'foo', url: 'http://example.com/foo/bar/merge_requests/1' },
+          { new_merge_request: true, branch_name: 'bar', url: 'http://example.com/foo/bar/merge_requests/new?merge_request%5Bsource_branch%5D=bar' }
+        ]
+
+        subject.add_merge_request_urls(urls_data)
+
+        expected = [a_kind_of(described_class::Message), a_kind_of(described_class::Message)]
+        expect(subject.messages).to match(expected)
+      end
+    end
+  end
+
+  describe '#add_merge_request_url' do
+    context 'when :new_merge_request is false' do
+      it 'adds a basic message to view the existing merge request' do
+        url_data = { new_merge_request: false, branch_name: 'foo', url: 'http://example.com/foo/bar/merge_requests/1' }
+
+        subject.add_merge_request_url(url_data)
+
+        message = <<~MESSAGE.strip
+          View merge request for foo:
+            http://example.com/foo/bar/merge_requests/1
+        MESSAGE
+
+        expect(subject.messages.first.message).to eq(message)
+        expect(subject.messages.first.type).to eq(:basic)
+      end
+    end
+
+    context 'when :new_merge_request is true' do
+      it 'adds a basic message to create a new merge request' do
+        url_data = { new_merge_request: true, branch_name: 'bar', url: 'http://example.com/foo/bar/merge_requests/new?merge_request%5Bsource_branch%5D=bar' }
+
+        subject.add_merge_request_url(url_data)
+
+        message = <<~MESSAGE.strip
+          To create a merge request for bar, visit:
+            http://example.com/foo/bar/merge_requests/new?merge_request%5Bsource_branch%5D=bar
+        MESSAGE
+
+        expect(subject.messages.first.message).to eq(message)
+        expect(subject.messages.first.type).to eq(:basic)
+      end
+    end
+  end
+
+  describe '#add_basic_message' do
+    context 'when text is present' do
+      it 'adds a basic message' do
+        subject.add_basic_message('hello')
+
+        expect(subject.messages.first.message).to eq('hello')
+        expect(subject.messages.first.type).to eq(:basic)
+      end
+    end
+
+    context 'when text is blank' do
+      it 'does not add a message' do
+        subject.add_basic_message(' ')
+
+        expect(subject.messages).to be_blank
+      end
+    end
+  end
+
+  describe '#add_alert_message' do
+    context 'when text is present' do
+      it 'adds a alert message' do
+        subject.add_alert_message('hello')
+
+        expect(subject.messages.first.message).to eq('hello')
+        expect(subject.messages.first.type).to eq(:alert)
+      end
+    end
+
+    context 'when text is blank' do
+      it 'does not add a message' do
+        subject.add_alert_message(' ')
+
+        expect(subject.messages).to be_blank
+      end
+    end
+  end
+
+  describe '#reference_counter_decreased' do
+    context 'initially' do
+      it 'reference_counter_decreased is set to false' do
+        expect(subject.reference_counter_decreased).to eq(false)
+      end
+    end
+  end
+
+  describe '#reference_counter_decreased=' do
+    context 'when the argument is truthy' do
+      it 'reference_counter_decreased is truthy' do
+        subject.reference_counter_decreased = true
+
+        expect(subject.reference_counter_decreased).to be_truthy
+      end
+    end
+
+    context 'when the argument is falsey' do
+      it 'reference_counter_decreased is falsey' do
+        subject.reference_counter_decreased = false
+
+        expect(subject.reference_counter_decreased).to be_falsey
+      end
+    end
+  end
+end

spec/requests/api/internal_spec.rb

@@ -925,19 +925,20 @@ describe API::Internal do
     it 'returns link to create new merge request' do
       post api('/internal/post_receive'), params: valid_params
 
-      expect(json_response['merge_request_urls']).to match [{
-        "branch_name" => branch_name,
-        "url" => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{branch_name}",
-        "new_merge_request" => true
-      }]
+      message = <<~MESSAGE.strip
+        To create a merge request for #{branch_name}, visit:
+          http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/new?merge_request%5Bsource_branch%5D=#{branch_name}
+      MESSAGE
+
+      expect(json_response['messages']).to include(build_basic_message(message))
     end
 
-    it 'returns empty array if printing_merge_request_link_enabled is false' do
+    it 'returns no merge request messages if printing_merge_request_link_enabled is false' do
       project.update!(printing_merge_request_link_enabled: false)
 
       post api('/internal/post_receive'), params: valid_params
 
-      expect(json_response['merge_request_urls']).to eq([])
+      expect(json_response['messages']).to be_blank
     end
 
     it 'does not invoke MergeRequests::PushOptionsHandlerService' do
@@ -968,11 +969,12 @@ describe API::Internal do
       it 'links to the newly created merge request' do
         post api('/internal/post_receive'), params: valid_params
 
-        expect(json_response['merge_request_urls']).to match [{
-          'branch_name' => branch_name,
-          'url' => "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/1",
-          'new_merge_request' => false
-        }]
+        message = <<~MESSAGE.strip
+          View merge request for #{branch_name}:
+            http://#{Gitlab.config.gitlab.host}/#{project.full_path}/merge_requests/1
+        MESSAGE
+
+        expect(json_response['messages']).to include(build_basic_message(message))
       end
 
       it 'adds errors on the service instance to warnings' do
@@ -982,7 +984,8 @@ describe API::Internal do
 
         post api('/internal/post_receive'), params: valid_params
 
-        expect(json_response['warnings']).to eq('Error encountered with push options \'merge_request.create\': my error')
+        message = "WARNINGS:\nError encountered with push options 'merge_request.create': my error"
+        expect(json_response['messages']).to include(build_alert_message(message))
       end
 
       it 'adds ActiveRecord errors on invalid MergeRequest records to warnings' do
@@ -995,38 +998,39 @@ describe API::Internal do
 
         post api('/internal/post_receive'), params: valid_params
 
-        expect(json_response['warnings']).to eq('Error encountered with push options \'merge_request.create\': my error')
+        message = "WARNINGS:\nError encountered with push options 'merge_request.create': my error"
+        expect(json_response['messages']).to include(build_alert_message(message))
       end
     end
 
     context 'broadcast message exists' do
       let!(:broadcast_message) { create(:broadcast_message, starts_at: 1.day.ago, ends_at: 1.day.from_now ) }
 
-      it 'returns one broadcast message' do
+      it 'outputs a broadcast message' do
         post api('/internal/post_receive'), params: valid_params
 
         expect(response).to have_gitlab_http_status(200)
-        expect(json_response['broadcast_message']).to eq(broadcast_message.message)
+        expect(json_response['messages']).to include(build_alert_message(broadcast_message.message))
       end
     end
 
     context 'broadcast message does not exist' do
-      it 'returns empty string' do
+      it 'does not output a broadcast message' do
         post api('/internal/post_receive'), params: valid_params
 
         expect(response).to have_gitlab_http_status(200)
-        expect(json_response['broadcast_message']).to eq(nil)
+        expect(has_alert_messages?(json_response['messages'])).to be_falsey
       end
     end
 
     context 'nil broadcast message' do
-      it 'returns empty string' do
+      it 'does not output a broadcast message' do
         allow(BroadcastMessage).to receive(:current).and_return(nil)
 
         post api('/internal/post_receive'), params: valid_params
 
         expect(response).to have_gitlab_http_status(200)
-        expect(json_response['broadcast_message']).to eq(nil)
+        expect(has_alert_messages?(json_response['messages'])).to be_falsey
       end
     end
 
@@ -1038,8 +1042,7 @@ describe API::Internal do
         post api('/internal/post_receive'), params: valid_params
 
         expect(response).to have_gitlab_http_status(200)
-        expect(json_response["redirected_message"]).to be_present
-        expect(json_response["redirected_message"]).to eq(project_moved.message)
+        expect(json_response['messages']).to include(build_basic_message(project_moved.message))
       end
     end
 
@@ -1051,8 +1054,7 @@ describe API::Internal do
         post api('/internal/post_receive'), params: valid_params
 
         expect(response).to have_gitlab_http_status(200)
-        expect(json_response["project_created_message"]).to be_present
-        expect(json_response["project_created_message"]).to eq(project_created.message)
+        expect(json_response['messages']).to include(build_basic_message(project_created.message))
       end
     end
 
@@ -1172,4 +1174,18 @@ describe API::Internal do
       }
     )
   end
+
+  def build_alert_message(message)
+    { 'type' => 'alert', 'message' => message }
+  end
+
+  def build_basic_message(message)
+    { 'type' => 'basic', 'message' => message }
+  end
+
+  def has_alert_messages?(messages)
+    messages.any? do |message|
+      message['type'] == 'alert'
+    end
+  end
 end