Merge branch 'ce-to-ee-2018-08-22' into 'master'

CE upstream - 2018-08-22 12:21 UTC

Closes gitlab-ce#50549 and gitlab-qa#317

See merge request gitlab-org/gitlab-ee!6968

CHANGELOG.md

@@ -8,11 +8,11 @@ entry.
 
 - Bump Gitaly to 0.117.1 for Rouge update. !21277
 - Fix symlink vulnerability in project import.
+- Bump rugged to 0.27.4 for security fixes.
+- Fixed XSS in branch name in Web IDE.
 - Adding CSRF protection to Hooks test action.
 - Don't expose project names in GitHub counters.
-- Bump rugged to 0.27.4 for security fixes.
 - Don't expose project names in various counters.
-- Fixed XSS in branch name in Web IDE.
 
 ### Removed (1 change)
 
@@ -72,9 +72,9 @@ entry.
 - Fix bug setting http headers in Files API. !20938
 - Rails5: fix flaky spec. !20953 (Jasper Maes)
 - Fixed list of projects not loading in group boards. !20955
-- Fix rendering of the context lines in MR diffs page. !20968
-- Fix navigation to First and Next discussion on MR Changes tab. !20968
 - Fix autosave and ESC confirmation issues for MR discussions. !20968
+- Fix navigation to First and Next discussion on MR Changes tab. !20968
+- Fix rendering of the context lines in MR diffs page. !20968
 - fix error caused when using the search bar while unauthenticated. !20970
 - Fix GPG status badge loading regressions. !20987
 - Ensure links in notifications footer are not escaped. !21000
@@ -88,18 +88,18 @@ entry.
 - Fix issue stopping Instance Statistics javascript to be executed. !21211
 - Fix broken JavaScript in IE11. !21214
 - Improve JUnit test reports in merge request widgets. !49966
-- Fix handling of annotated tags when Gitaly is not in use.
 - Properly handle colons in URL passwords.
 - Renders test reports for resolved failures and resets error state.
-- Sanitize git URL in import errors. (Jamie Schembri)
-- Fix updated_at if created_at is set for Note API.
-- Allow to toggle notifications for issues due soon.
+- Fix handling of annotated tags when Gitaly is not in use.
 - Fix serialization of LegacyDiffNote.
-- Update hamlit to fix ruby 2.5 incompatibilities, fixes #42045. (Matthew Dawson)
 - Escapes milestone and label's names on flash notice when promoting them.
+- Allow to toggle notifications for issues due soon.
+- Sanitize git URL in import errors. (Jamie Schembri)
 - Add missing predefined variable and fix docs.
 - Allow updating a project's avatar without other params. (Jamie Schembri)
 - Fix the UI for listing system-level labels.
+- Update hamlit to fix ruby 2.5 incompatibilities, fixes #42045. (Matthew Dawson)
+- Fix updated_at if created_at is set for Note API.
 - Fix search bar text input alignment.
 
 ### Changed (32 changes, 7 of them are from the community)
@@ -133,9 +133,9 @@ entry.
 - Update to Rouge 3.2.0, including Terraform and Crystal lexer and bug fixes. !20991
 - Update design of project templates. !21012
 - Update to Rouge 3.2.1, which includes a critical fix to the Perl Lexer. !21263
-- Redesign GCP offer banner.
 - Add a 10 ms bucket for SQL timings.
 - Show one digit after dot in commit_per_day value in charts page. (msdundar)
+- Redesign GCP offer banner.
 
 ### Performance (30 changes, 10 of them are from the community)
 
@@ -206,13 +206,13 @@ entry.
 - Clean orphaned files in object storage. !20918
 - Adds frontend support to render test reports on the MR widget. !20936
 - Trigger system hooks when project is archived/unarchived. !20995
+- Custom Wiki Sidebar Support Issue 14995. (Josh Sooter)
 - Emails on push recipients now accepts formats like John Doe <johndoe@example.com>. (George Thomas)
-- Added button to regenerate 2FA codes. (Luke Picciau)
-- Improve danger confirmation modals by focusing input field. (Jamie Schembri)
 - Add new model for tracking label events.
-- Custom Wiki Sidebar Support Issue 14995. (Josh Sooter)
+- Improve danger confirmation modals by focusing input field. (Jamie Schembri)
 - Clicking CI icon in Web IDE now opens up pipelines panel.
 - Enabled deletion of files in the Web IDE.
+- Added button to regenerate 2FA codes. (Luke Picciau)
 
 ### Other (26 changes, 7 of them are from the community)
 
@@ -238,9 +238,9 @@ entry.
 - Update git rerere link in docs. !21060 (gfyoung)
 - Add 'tabindex' attribute support on Icon component to show BS4 popover on trigger type 'focus'. !21066
 - Add a Gitlab::Profiler.print_by_total_time convenience method for profiling from a Rails console.
-- Disables toggle comments button if diff has no discussions.
 - Automatically expand runner's settings block when linking to the runner's settings page.
 - Increases title column on modal for reports.
+- Disables toggle comments button if diff has no discussions.
 - Moves help_popover component to a common location.
 
 

Gemfile

@@ -119,14 +119,14 @@ gem 'dropzonejs-rails', '~> 0.7.1'
 # for backups
 gem 'fog-aws', '~> 2.0.1'
 gem 'fog-core', '~> 1.44'
-gem 'fog-google', '~> 1.3.3'
+gem 'fog-google', '~> 1.7.1'
 gem 'fog-local', '~> 0.3'
 gem 'fog-openstack', '~> 0.1'
 gem 'fog-rackspace', '~> 0.1.1'
 gem 'fog-aliyun', '~> 0.2.0'
 
 # for Google storage
-gem 'google-api-client', '~> 0.19.8'
+gem 'google-api-client', '~> 0.23'
 
 # for aws storage
 gem 'unf', '~> 0.1.4'

Gemfile.lock

@@ -263,11 +263,11 @@ GEM
       builder
       excon (~> 0.58)
       formatador (~> 0.2)
-    fog-google (1.3.3)
+    fog-google (1.7.1)
       fog-core
       fog-json
       fog-xml
-      google-api-client (~> 0.19.1)
+      google-api-client (~> 0.23.0)
     fog-json (1.0.2)
       fog-core (~> 1.0)
       multi_json (~> 1.10)
@@ -354,7 +354,7 @@ GEM
       actionpack (>= 3.0)
       multi_json
       request_store (>= 1.0)
-    google-api-client (0.19.8)
+    google-api-client (0.23.4)
       addressable (~> 2.5, >= 2.5.1)
       googleauth (>= 0.5, < 0.7.0)
       httpclient (>= 2.8.1, < 3.0)
@@ -767,7 +767,7 @@ GEM
       http-cookie (>= 1.0.2, < 2.0)
       mime-types (>= 1.16, < 4.0)
       netrc (~> 0.8)
-    retriable (3.1.1)
+    retriable (3.1.2)
     rinku (2.0.0)
     rotp (2.1.2)
     rouge (3.2.1)
@@ -1065,7 +1065,7 @@ DEPENDENCIES
   fog-aliyun (~> 0.2.0)
   fog-aws (~> 2.0.1)
   fog-core (~> 1.44)
-  fog-google (~> 1.3.3)
+  fog-google (~> 1.7.1)
   fog-local (~> 0.3)
   fog-openstack (~> 0.1)
   fog-rackspace (~> 0.1.1)
@@ -1086,7 +1086,7 @@ DEPENDENCIES
   gitlab-styles (~> 2.4)
   gitlab_omniauth-ldap (~> 2.0.4)
   gon (~> 6.2)
-  google-api-client (~> 0.19.8)
+  google-api-client (~> 0.23)
   google-protobuf (= 3.5.1)
   gpgme
   grape (~> 1.0)

Gemfile.rails5.lock

@@ -266,11 +266,11 @@ GEM
       builder
       excon (~> 0.58)
       formatador (~> 0.2)
-    fog-google (1.3.3)
+    fog-google (1.7.1)
       fog-core
       fog-json
       fog-xml
-      google-api-client (~> 0.19.1)
+      google-api-client (~> 0.23.0)
     fog-json (1.0.2)
       fog-core (~> 1.0)
       multi_json (~> 1.10)
@@ -357,7 +357,7 @@ GEM
       actionpack (>= 3.0)
       multi_json
       request_store (>= 1.0)
-    google-api-client (0.19.8)
+    google-api-client (0.23.4)
       addressable (~> 2.5, >= 2.5.1)
       googleauth (>= 0.5, < 0.7.0)
       httpclient (>= 2.8.1, < 3.0)
@@ -777,7 +777,7 @@ GEM
       http-cookie (>= 1.0.2, < 2.0)
       mime-types (>= 1.16, < 4.0)
       netrc (~> 0.8)
-    retriable (3.1.1)
+    retriable (3.1.2)
     rinku (2.0.0)
     rotp (2.1.2)
     rouge (3.2.0)
@@ -1077,7 +1077,7 @@ DEPENDENCIES
   fog-aliyun (~> 0.2.0)
   fog-aws (~> 2.0.1)
   fog-core (~> 1.44)
-  fog-google (~> 1.3.3)
+  fog-google (~> 1.7.1)
   fog-local (~> 0.3)
   fog-openstack (~> 0.1)
   fog-rackspace (~> 0.1.1)
@@ -1098,7 +1098,7 @@ DEPENDENCIES
   gitlab-styles (~> 2.4)
   gitlab_omniauth-ldap (~> 2.0.4)
   gon (~> 6.2)
-  google-api-client (~> 0.19.8)
+  google-api-client (~> 0.23)
   google-protobuf (= 3.5.1)
   gpgme
   grape (~> 1.0)

VERSION

-11.2.0-pre
+11.3.0-pre

app/assets/stylesheets/framework/variables.scss

@@ -330,7 +330,6 @@ $line-select-yellow-dark: #f0e2bd;
 $dark-diff-match-bg: rgba(255, 255, 255, 0.3);
 $dark-diff-match-color: rgba(255, 255, 255, 0.1);
 $file-mode-changed: #777;
-$file-mode-changed: #777;
 $diff-image-info-color: gray;
 $diff-swipe-border: #999;
 $diff-view-modes-color: gray;

app/controllers/projects/commit_controller.rb

@@ -101,7 +101,7 @@ class Projects::CommitController < Projects::ApplicationController
 
     @branch_name = create_new_branch? ? @commit.cherry_pick_branch_name : @start_branch
 
-    create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully cherry-picked.",
+    create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title(current_user)} has been successfully cherry-picked into #{@branch_name}.",
                                               success_path: -> { successful_change_path }, failure_path: failed_change_path)
   end
 

app/models/commit.rb

@@ -193,6 +193,7 @@ class Commit
   # otherwise returns commit message without first line
   def description
     return safe_message if full_title.length >= 100
+    return no_commit_message if safe_message.blank?
 
     safe_message.split("\n", 2)[1].try(:chomp)
   end

app/models/member.rb

@@ -104,7 +104,7 @@ class Member < ActiveRecord::Base
     def filter_by_2fa(value)
       case value
       when 'enabled'
-        left_join_users.merge(User.with_two_factor_indistinct)
+        left_join_users.merge(User.with_two_factor)
       when 'disabled'
         left_join_users.merge(User.without_two_factor)
       else

app/models/user.rb

@@ -298,13 +298,16 @@ class User < ActiveRecord::Base
     end
   end
 
-  def self.with_two_factor_indistinct
-    joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id")
-      .where("u2f.id IS NOT NULL OR users.otp_required_for_login = ?", true)
-  end
-
   def self.with_two_factor
-    with_two_factor_indistinct.distinct(arel_table[:id])
+    with_u2f_registrations = <<-SQL
+      EXISTS (
+        SELECT *
+        FROM u2f_registrations AS u2f
+        WHERE u2f.user_id = users.id
+      ) OR users.otp_required_for_login = ?
+    SQL
+
+    where(with_u2f_registrations, true)
   end
 
   def self.without_two_factor

app/services/git_push_service.rb

@@ -146,7 +146,6 @@ class GitPushService < BaseService
     EventCreateService.new.push(project, current_user, build_push_data)
     Ci::CreatePipelineService.new(project, current_user, build_push_data).execute(:push, mirror_update: mirror_update)
 
-    SystemHookPushWorker.perform_async(build_push_data.dup, :push_hooks)
     project.execute_hooks(build_push_data.dup, :push_hooks)
     project.execute_services(build_push_data.dup, :push_hooks)
 

changelogs/unreleased/21326-avoid-nil-safe-message.yml

+---
+title: "Avoid nil safe message"
+merge_request: 21326
+author: Yi Siliang
+type: fixed

changelogs/unreleased/41441-add-target-branch-name-to-cherrypick-confirmation.yml

+---
+title: Add target branch name to cherrypick confirmation message
+merge_request: 20846
+author: George Andrinopoulos
+type: other

changelogs/unreleased/api-empty-commit-message.yml

+---
+title: 'API: Catch empty commit messages'
+merge_request: 21322
+author: Robert Schilling
+type: fixed

changelogs/unreleased/api-empty-project-snippets.yml

+---
+title: 'API: Catch empty code content for project snippets'
+merge_request: 21325
+author: Robert Schilling
+type: fixed

changelogs/unreleased/api-shared_group_expires-at.yml

+---
+title: 'API: Add expiration date for shared projects to the project entity'
+merge_request: 21104
+author: Robert Schilling
+type: added

changelogs/unreleased/dz-fix-sql-error-admin-users-2fa.yml

+---
+title: Fix SQL error when sorting 2FA-enabled users by name in admin area
+merge_request: 21324
+author:
+type: fixed

changelogs/unreleased/sh-bump-fog-google.yml

+---
+title: Bump fog-google to 1.7.0 and google-api-client to 0.23.0
+merge_request: 21295
+author:
+type: fixed

changelogs/unreleased/sh-conditional-system-hook-push.yml

+---
+title: Eliminate unnecessary and duplicate system hook fires
+merge_request: 21337
+author:
+type: performance

config/initializers/fog_google_https_private_urls.rb

@@ -7,7 +7,7 @@ module Fog
     class GoogleXML
       class File < Fog::Model
         module MonkeyPatch
-          def url(expires)
+          def url(expires, options = {})
             requires :key
             collection.get_https_url(key, expires)
           end

doc/api/groups.md

@@ -352,12 +352,14 @@ Example response:
         {
           "group_id": 4,
           "group_name": "Twitter",
-          "group_access_level": 30
+          "group_access_level": 30,
+          "expires_at": null
         },
         {
           "group_id": 3,
           "group_name": "Gitlab Org",
-          "group_access_level": 10
+          "group_access_level": 10,
+          "expires_at": "2018-08-14"
         }
       ]
     }

lib/api/entities.rb

@@ -91,6 +91,7 @@ module API
         group_link.group.name
       end
       expose :group_access, as: :group_access_level
+      expose :expires_at
     end
 
     class ProjectIdentity < Grape::Entity

lib/api/files.rb

@@ -59,7 +59,7 @@ module API
       params :simple_file_params do
         requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'
         requires :branch, type: String, desc: 'Name of the branch to commit into. To create a new branch, also provide `start_branch`.'
-        requires :commit_message, type: String, desc: 'Commit message'
+        requires :commit_message, type: String, allow_blank: false, desc: 'Commit message'
         optional :start_branch, type: String, desc: 'Name of the branch to start the new commit from'
         optional :author_email, type: String, desc: 'The email of the author'
         optional :author_name, type: String, desc: 'The name of the author'

lib/api/project_snippets.rb

@@ -49,7 +49,7 @@ module API
       params do
         requires :title, type: String, desc: 'The title of the snippet'
         requires :file_name, type: String, desc: 'The file name of the snippet'
-        requires :code, type: String, desc: 'The content of the snippet'
+        requires :code, type: String, allow_blank: false, desc: 'The content of the snippet'
         optional :description, type: String, desc: 'The description of a snippet'
         requires :visibility, type: String,
                               values: Gitlab::VisibilityLevel.string_values,
@@ -78,7 +78,7 @@ module API
         requires :snippet_id, type: Integer, desc: 'The ID of a project snippet'
         optional :title, type: String, desc: 'The title of the snippet'
         optional :file_name, type: String, desc: 'The file name of the snippet'
-        optional :code, type: String, desc: 'The content of the snippet'
+        optional :code, type: String, allow_blank: false, desc: 'The content of the snippet'
         optional :description, type: String, desc: 'The description of a snippet'
         optional :visibility, type: String,
                               values: Gitlab::VisibilityLevel.string_values,

qa/qa/scenario/test/integration/github.rb

@@ -2,7 +2,7 @@ module QA
   module Scenario
     module Test
       module Integration
-        class Github < Test::Instance
+        class Github < Test::Instance::All
           tags :github
 
           def perform(address, *rspec_options)

qa/qa/scenario/test/integration/kubernetes.rb

@@ -2,7 +2,7 @@ module QA
   module Scenario
     module Test
       module Integration
-        class Kubernetes < Test::Instance
+        class Kubernetes < Test::Instance::All
           tags :kubernetes
         end
       end

qa/qa/scenario/test/integration/ldap.rb

@@ -2,7 +2,7 @@ module QA
   module Scenario
     module Test
       module Integration
-        class LDAP < Test::Instance
+        class LDAP < Test::Instance::All
           tags :ldap
         end
       end

qa/qa/scenario/test/integration/mattermost.rb

@@ -6,7 +6,7 @@ module QA
         # Run test suite against any GitLab instance where mattermost is enabled,
         # including staging and on-premises installation.
         #
-        class Mattermost < Test::Instance
+        class Mattermost < Test::Instance::All
           tags :core, :mattermost
 
           def perform(address, mattermost, *rspec_options)

spec/controllers/projects/commit_controller_spec.rb

@@ -230,7 +230,7 @@ describe Projects::CommitController do
             id: master_pickable_commit.id)
 
         expect(response).to redirect_to project_commits_path(project, 'master')
-        expect(flash[:notice]).to eq('The commit has been successfully cherry-picked.')
+        expect(flash[:notice]).to eq('The commit has been successfully cherry-picked into master.')
       end
     end
 

spec/factories/project_group_links.rb

@@ -2,5 +2,6 @@ FactoryBot.define do
   factory :project_group_link do
     project
     group
+    expires_at nil
   end
 end

spec/features/projects/commit/cherry_pick_spec.rb

@@ -21,7 +21,7 @@ describe 'Cherry-pick Commits' do
         uncheck 'create_merge_request'
         click_button 'Cherry-pick'
       end
-      expect(page).to have_content('The commit has been successfully cherry-picked.')
+      expect(page).to have_content('The commit has been successfully cherry-picked into master.')
     end
   end
 
@@ -32,7 +32,7 @@ describe 'Cherry-pick Commits' do
         uncheck 'create_merge_request'
         click_button 'Cherry-pick'
       end
-      expect(page).to have_content('The commit has been successfully cherry-picked.')
+      expect(page).to have_content('The commit has been successfully cherry-picked into master.')
     end
   end
 
@@ -59,7 +59,7 @@ describe 'Cherry-pick Commits' do
       page.within('#modal-cherry-pick-commit') do
         click_button 'Cherry-pick'
       end
-      expect(page).to have_content('The commit has been successfully cherry-picked. You can now submit a merge request to get this change into the original branch.')
+      expect(page).to have_content("The commit has been successfully cherry-picked into cherry-pick-#{master_pickable_commit.short_id}. You can now submit a merge request to get this change into the original branch.")
       expect(page).to have_content("From cherry-pick-#{master_pickable_commit.short_id} into master")
     end
   end
@@ -86,7 +86,7 @@ describe 'Cherry-pick Commits' do
         click_button 'Cherry-pick'
       end
 
-      expect(page).to have_content('The commit has been successfully cherry-picked.')
+      expect(page).to have_content('The commit has been successfully cherry-picked into feature.')
     end
   end
 

spec/models/commit_spec.rb

@@ -225,6 +225,12 @@ eos
   end
 
   describe 'description' do
+    it 'returns no_commit_message when safe_message is blank' do
+      allow(commit).to receive(:safe_message).and_return(nil)
+
+      expect(commit.description).to eq('--no commit message')
+    end
+
     it 'returns description of commit message if title less than 100 characters' do
       message = <<eos
 Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec sodales id felis id blandit.

spec/models/user_spec.rb

@@ -343,6 +343,14 @@ describe User do
         expect(users_with_two_factor).to eq([user_with_2fa.id])
         expect(users_with_two_factor).not_to include(user_without_2fa.id)
       end
+
+      it 'works with ORDER BY' do
+        user_with_2fa = create(:user, :two_factor_via_otp, :two_factor_via_u2f)
+
+        expect(described_class
+          .with_two_factor
+          .reorder_by_name).to eq([user_with_2fa])
+      end
     end
 
     describe ".without_two_factor" do

spec/requests/api/files_spec.rb

@@ -313,7 +313,7 @@ describe API::Files do
 
   describe "POST /projects/:id/repository/files/:file_path" do
     let!(:file_path) { "new_subfolder%2Fnewfile%2Erb" }
-    let(:valid_params) do
+    let(:params) do
       {
         branch: "master",
         content: "puts 8",
@@ -322,7 +322,7 @@ describe API::Files do
     end
 
     it "creates a new file in project repo" do
-      post api(route(file_path), user), valid_params
+      post api(route(file_path), user), params
 
       expect(response).to have_gitlab_http_status(201)
       expect(json_response["file_path"]).to eq(CGI.unescape(file_path))
@@ -337,20 +337,28 @@ describe API::Files do
       expect(response).to have_gitlab_http_status(400)
     end
 
+    it 'returns a 400 bad request if the commit message is empty' do
+      params[:commit_message] = ''
+
+      post api(route(file_path), user), params
+
+      expect(response).to have_gitlab_http_status(400)
+    end
+
     it "returns a 400 if editor fails to create file" do
       allow_any_instance_of(Repository).to receive(:create_file)
         .and_raise(Gitlab::Git::CommitError, 'Cannot create file')
 
-      post api(route("any%2Etxt"), user), valid_params
+      post api(route("any%2Etxt"), user), params
 
       expect(response).to have_gitlab_http_status(400)
     end
 
     context "when specifying an author" do
       it "creates a new file with the specified author" do
-        valid_params.merge!(author_email: author_email, author_name: author_name)
+        params.merge!(author_email: author_email, author_name: author_name)
 
-        post api(route("new_file_with_author%2Etxt"), user), valid_params
+        post api(route("new_file_with_author%2Etxt"), user), params
 
         expect(response).to have_gitlab_http_status(201)
         expect(response.content_type).to eq('application/json')
@@ -364,7 +372,7 @@ describe API::Files do
       let!(:project) { create(:project_empty_repo, namespace: user.namespace ) }
 
       it "creates a new file in project repo" do
-        post api(route("newfile%2Erb"), user), valid_params
+        post api(route("newfile%2Erb"), user), params
 
         expect(response).to have_gitlab_http_status(201)
         expect(json_response['file_path']).to eq('newfile.rb')
@@ -376,7 +384,7 @@ describe API::Files do
   end
 
   describe "PUT /projects/:id/repository/files" do
-    let(:valid_params) do
+    let(:params) do
       {
         branch: 'master',
         content: 'puts 8',
@@ -385,7 +393,7 @@ describe API::Files do
     end
 
     it "updates existing file in project repo" do
-      put api(route(file_path), user), valid_params
+      put api(route(file_path), user), params
 
       expect(response).to have_gitlab_http_status(200)
       expect(json_response['file_path']).to eq(CGI.unescape(file_path))
@@ -394,8 +402,16 @@ describe API::Files do
       expect(last_commit.author_name).to eq(user.name)
     end
 
+    it 'returns a 400 bad request if the commit message is empty' do
+      params[:commit_message] = ''
+
+      put api(route(file_path), user), params
+
+      expect(response).to have_gitlab_http_status(400)
+    end
+
     it "returns a 400 bad request if update existing file with stale last commit id" do
-      params_with_stale_id = valid_params.merge(last_commit_id: 'stale')
+      params_with_stale_id = params.merge(last_commit_id: 'stale')
 
       put api(route(file_path), user), params_with_stale_id
 
@@ -406,7 +422,7 @@ describe API::Files do
     it "updates existing file in project repo with accepts correct last commit id" do
       last_commit = Gitlab::Git::Commit
                         .last_for_path(project.repository, 'master', URI.unescape(file_path))
-      params_with_correct_id = valid_params.merge(last_commit_id: last_commit.id)
+      params_with_correct_id = params.merge(last_commit_id: last_commit.id)
 
       put api(route(file_path), user), params_with_correct_id
 
@@ -421,9 +437,9 @@ describe API::Files do
 
     context "when specifying an author" do
       it "updates a file with the specified author" do
-        valid_params.merge!(author_email: author_email, author_name: author_name, content: "New content")
+        params.merge!(author_email: author_email, author_name: author_name, content: "New content")
 
-        put api(route(file_path), user), valid_params
+        put api(route(file_path), user), params
 
         expect(response).to have_gitlab_http_status(200)
         last_commit = project.repository.commit.raw
@@ -434,7 +450,7 @@ describe API::Files do
   end
 
   describe "DELETE /projects/:id/repository/files" do
-    let(:valid_params) do
+    let(:params) do
       {
         branch: 'master',
         commit_message: 'Changed file'
@@ -442,7 +458,7 @@ describe API::Files do
     end
 
     it "deletes existing file in project repo" do
-      delete api(route(file_path), user), valid_params
+      delete api(route(file_path), user), params
 
       expect(response).to have_gitlab_http_status(204)
     end
@@ -453,19 +469,27 @@ describe API::Files do
       expect(response).to have_gitlab_http_status(400)
     end
 
+    it 'returns a 400 bad request if the commit message is empty' do
+      params[:commit_message] = ''
+
+      delete api(route(file_path), user), params
+
+      expect(response).to have_gitlab_http_status(400)
+    end
+
     it "returns a 400 if fails to delete file" do
       allow_any_instance_of(Repository).to receive(:delete_file).and_raise(Gitlab::Git::CommitError, 'Cannot delete file')
 
-      delete api(route(file_path), user), valid_params
+      delete api(route(file_path), user), params
 
       expect(response).to have_gitlab_http_status(400)
     end
 
     context "when specifying an author" do
       it "removes a file with the specified author" do
-        valid_params.merge!(author_email: author_email, author_name: author_name)
+        params.merge!(author_email: author_email, author_name: author_name)
 
-        delete api(route(file_path), user), valid_params
+        delete api(route(file_path), user), params
 
         expect(response).to have_gitlab_http_status(204)
       end

spec/requests/api/project_snippets_spec.rb

@@ -116,6 +116,14 @@ describe API::ProjectSnippets do
       expect(response).to have_gitlab_http_status(400)
     end
 
+    it 'returns 400 for empty code field' do
+      params[:code] = ''
+
+      post api("/projects/#{project.id}/snippets/", admin), params
+
+      expect(response).to have_gitlab_http_status(400)
+    end
+
     context 'when the snippet is spam' do
       def create_snippet(project, snippet_params = {})
         project.add_developer(user)
@@ -180,6 +188,14 @@ describe API::ProjectSnippets do
       expect(response).to have_gitlab_http_status(400)
     end
 
+    it 'returns 400 for empty code field' do
+      new_content = ''
+
+      put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin), code: new_content
+
+      expect(response).to have_gitlab_http_status(400)
+    end
+
     context 'when the snippet is spam' do
       def update_snippet(snippet_params = {})
         put api("/projects/#{snippet.project.id}/snippets/#{snippet.id}", admin), snippet_params

spec/requests/api/projects_spec.rb

@@ -962,6 +962,7 @@ describe API::Projects do
         expect(json_response['shared_with_groups'][0]['group_id']).to eq(group.id)
         expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name)
         expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access)
+        expect(json_response['shared_with_groups'][0]['expires_at']).to be_nil
         expect(json_response['only_allow_merge_if_pipeline_succeeds']).to eq(project.only_allow_merge_if_pipeline_succeeds)
         expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved)
         expect(json_response['merge_method']).to eq(project.merge_method.to_s)
@@ -969,6 +970,21 @@ describe API::Projects do
         expect(json_response).not_to have_key('repository_storage')
       end
 
+      it 'returns a group link with expiration date' do
+        group = create(:group)
+        expires_at = 5.days.from_now.to_date
+        link = create(:project_group_link, project: project, group: group, expires_at: expires_at)
+
+        get api("/projects/#{project.id}", user)
+
+        expect(json_response['shared_with_groups']).to be_an Array
+        expect(json_response['shared_with_groups'].length).to eq(1)
+        expect(json_response['shared_with_groups'][0]['group_id']).to eq(group.id)
+        expect(json_response['shared_with_groups'][0]['group_name']).to eq(group.name)
+        expect(json_response['shared_with_groups'][0]['group_access_level']).to eq(link.group_access)
+        expect(json_response['shared_with_groups'][0]['expires_at']).to eq(expires_at.to_s)
+      end
+
       it 'returns a project by path name' do
         get api("/projects/#{project.id}", user)
         expect(response).to have_gitlab_http_status(200)

spec/services/git_push_service_spec.rb

@@ -245,9 +245,13 @@ describe GitPushService do
       end
     end
 
-    context "Sends System Push data" do
-      it "when pushing on a branch" do
-        expect(SystemHookPushWorker).to receive(:perform_async).with(push_data, :push_hooks)
+    describe 'system hooks' do
+      let(:system_hook_service) { double() }
+
+      it "sends a system hook after pushing a branch" do
+        expect(SystemHooksService).to receive(:new).and_return(system_hook_service)
+        expect(system_hook_service).to receive(:execute_hooks).with(push_data, :push_hooks)
+
         execute_service(project, user, oldrev, newrev, ref)
       end
     end