Merge branch 'master' into fix/rename-mwbs-to-merge-when-pipeline-succeeds

* master: (110 commits)
  Rewrite an HTTP link to use HTTPS
  Edit /spec/features/profiles/preferences_spec.rb to match changes in 084d90ac
  Add blue back to sub nav active
  Remove JSX/React eslint plugins.
  Fix a transient spec failure
  Adds hoverstates for collapsed Issue/Merge Request sidebar
  Moved groups above projects
  Add StackProf to the Gemfile, along with a utility to get a profile for a spec
  Update Sidekiq-cron to fix compatibility issues with Sidekiq 4.2.1
  Add a CHANGELOG entry
  Alert user when logged in user email is not the same as the invitation
  Expose timestamp in build entity used by serializer
  Rename `MergeRequest#pipeline` to `head_pipeline`
  Remove unnecessary database indexes
  CE-specific changes gitlab-org/gitlab-ee#1137
  Fixing typo & Clarifying Key name
  fix started_at check
  fix blob controller spec failure - updated not to use file-path-
  fix blob controller spec failure
  Merge branch 'jej-use-issuable-finder-instead-of-access-check' into 'security'
  ...

Conflicts:
	app/controllers/projects/merge_requests_controller.rb
	lib/api/merge_requests.rb
	spec/requests/api/merge_requests_spec.rb

9 files changed, 69 insertions, 78 deletions

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 7a724487e02..fdb19558c1c 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -607,6 +607,7 @@ module API
       expose :user, with: Entities::UserBasic
       expose :created_at, :updated_at, :started_at, :finished_at, :committed_at
       expose :duration
+      expose :coverage
     end
 
     class EnvironmentBasic < Grape::Entity
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index fc39fdf4b67..5315c22e1e4 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -82,7 +82,7 @@ module API
                         :lfs_enabled, :request_access_enabled
       end
       put ':id' do
-        group = find_group(params[:id])
+        group = find_group!(params[:id])
         authorize! :admin_group, group
 
         if ::Groups::UpdateService.new(group, current_user, declared_params(include_missing: false)).execute
@@ -96,13 +96,13 @@ module API
         success Entities::GroupDetail
       end
       get ":id" do
-        group = find_group(params[:id])
+        group = find_group!(params[:id])
         present group, with: Entities::GroupDetail
       end
 
       desc 'Remove a group.'
       delete ":id" do
-        group = find_group(params[:id])
+        group = find_group!(params[:id])
         authorize! :admin_group, group
         DestroyGroupService.new(group, current_user).execute
       end
@@ -111,7 +111,7 @@ module API
         success Entities::Project
       end
       get ":id/projects" do
-        group = find_group(params[:id])
+        group = find_group!(params[:id])
         projects = GroupProjectsFinder.new(group).execute(current_user)
         projects = paginate projects
         present projects, with: Entities::Project, user: current_user
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 2c593dbb4ea..34d9c3c6932 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -68,7 +68,7 @@ module API
     end
 
     def user_project
-      @project ||= find_project(params[:id])
+      @project ||= find_project!(params[:id])
     end
 
     def available_labels
@@ -76,7 +76,15 @@ module API
     end
 
     def find_project(id)
-      project = Project.find_with_namespace(id) || Project.find_by(id: id)
+      if id =~ /^\d+$/
+        Project.find_by(id: id)
+      else
+        Project.find_with_namespace(id)
+      end
+    end
+
+    def find_project!(id)
+      project = find_project(id)
 
       if can?(current_user, :read_project, project)
         project
@@ -97,7 +105,15 @@ module API
     end
 
     def find_group(id)
-      group = Group.find_by(path: id) || Group.find_by(id: id)
+      if id =~ /^\d+$/
+        Group.find_by(id: id)
+      else
+        Group.find_by(path: id)
+      end
+    end
+
+    def find_group!(id)
+      group = find_group(id)
 
       if can?(current_user, :read_group, group)
         group
@@ -112,9 +128,7 @@ module API
     end
 
     def find_project_issue(id)
-      issue = user_project.issues.find(id)
-      not_found! unless can?(current_user, :read_issue, issue)
-      issue
+      IssuesFinder.new(current_user, project_id: user_project.id).find(id)
     end
 
     def paginate(relation)
@@ -182,20 +196,6 @@ module API
       ActionController::Parameters.new(attrs).permit!
     end
 
-    # Helper method for validating all labels against its names
-    def validate_label_params(params)
-      errors = {}
-
-      params[:labels].to_s.split(',').each do |label_name|
-        label = available_labels.find_or_initialize_by(title: label_name.strip)
-        next if label.valid?
-
-        errors[label.title] = label.errors
-      end
-
-      errors
-    end
-
     # Checks the occurrences of datetime attributes, each attribute if present in the params hash must be in ISO 8601
     # format (YYYY-MM-DDTHH:MM:SSZ) or a Bad Request error is invoked.
     #
diff --git a/lib/api/helpers/members_helpers.rb b/lib/api/helpers/members_helpers.rb
index 90114f6f667..d9cae1501f8 100644
--- a/lib/api/helpers/members_helpers.rb
+++ b/lib/api/helpers/members_helpers.rb
@@ -2,7 +2,7 @@ module API
   module Helpers
     module MembersHelpers
       def find_source(source_type, id)
-        public_send("find_#{source_type}", id)
+        public_send("find_#{source_type}!", id)
       end
 
       def authorize_admin_source!(source_type, source)
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index eea5b91d4f9..049b4fb214c 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -19,6 +19,15 @@ module API
       def filter_issues_milestone(issues, milestone)
         issues.includes(:milestone).where('milestones.title' => milestone)
       end
+
+      def issue_params
+        new_params = declared(params, include_parent_namespace: false, include_missing: false).to_h
+        new_params = new_params.with_indifferent_access
+        new_params.delete(:id)
+        new_params.delete(:issue_id)
+
+        new_params
+      end
     end
 
     resource :issues do
@@ -68,7 +77,7 @@ module API
       #   GET /groups/:id/issues?milestone=1.0.0
       #   GET /groups/:id/issues?milestone=1.0.0&state=closed
       get ":id/issues" do
-        group = find_group(params[:id])
+        group = find_group!(params[:id])
 
         params[:state] ||= 'opened'
         params[:group_id] = group.id
@@ -86,6 +95,10 @@ module API
       end
     end
 
+    params do
+      requires :id, type: String, desc: 'The ID of a project'
+    end
+
     resource :projects do
       # Get a list of project issues
       #
@@ -109,7 +122,7 @@ module API
       #   GET /projects/:id/issues?milestone=1.0.0&state=closed
       #   GET /issues?iid=42
       get ":id/issues" do
-        issues = user_project.issues.inc_notes_with_associations.visible_to_user(current_user)
+        issues = IssuesFinder.new(current_user, project_id: user_project.id).execute.inc_notes_with_associations
         issues = filter_issues_state(issues, params[:state]) unless params[:state].nil?
         issues = filter_issues_labels(issues, params[:labels]) unless params[:labels].nil?
         issues = filter_by_iid(issues, params[:iid]) unless params[:iid].nil?
@@ -152,17 +165,10 @@ module API
       post ':id/issues' do
         required_attributes! [:title]
 
-        keys = [:title, :description, :assignee_id, :milestone_id, :due_date, :confidential]
+        keys = [:title, :description, :assignee_id, :milestone_id, :due_date, :confidential, :labels]
         keys << :created_at if current_user.admin? || user_project.owner == current_user
         attrs = attributes_for_keys(keys)
 
-        # Validate label names in advance
-        if (errors = validate_label_params(params)).any?
-          render_api_error!({ labels: errors }, 400)
-        end
-
-        attrs[:labels] = params[:labels] if params[:labels]
-
         # Convert and filter out invalid confidential flags
         attrs['confidential'] = to_boolean(attrs['confidential'])
         attrs.delete('confidential') if attrs['confidential'].nil?
@@ -180,41 +186,35 @@ module API
         end
       end
 
-      # Update an existing issue
-      #
-      # Parameters:
-      #   id (required) - The ID of a project
-      #   issue_id (required) - The ID of a project issue
-      #   title (optional) - The title of an issue
-      #   description (optional) - The description of an issue
-      #   assignee_id (optional) - The ID of a user to assign issue
-      #   milestone_id (optional) - The ID of a milestone to assign issue
-      #   labels (optional) - The labels of an issue
-      #   state_event (optional) - The state event of an issue (close|reopen)
-      #   updated_at (optional) - Date time string, ISO 8601 formatted
-      #   due_date (optional)     - Date time string in the format YEAR-MONTH-DAY
-      #   confidential (optional) - Boolean parameter if the issue should be confidential
-      # Example Request:
-      #   PUT /projects/:id/issues/:issue_id
+      desc 'Update an existing issue' do
+        success Entities::Issue
+      end
+      params do
+        requires :id, type: String, desc: 'The ID of a project'
+        requires :issue_id, type: Integer, desc: "The ID of a project issue"
+        optional :title, type: String, desc: 'The new title of the issue'
+        optional :description, type: String, desc: 'The description of an issue'
+        optional :assignee_id, type: Integer, desc: 'The ID of a user to assign issue'
+        optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue'
+        optional :labels, type: String, desc: 'The labels of an issue'
+        optional :state_event, type: String, values: ['close', 'reopen'], desc: 'The state event of an issue'
+        # TODO 9.0, use the Grape DateTime type here
+        optional :updated_at, type: String, desc: 'Date time string, ISO 8601 formatted'
+        optional :due_date, type: String, desc: 'Date time string in the format YEAR-MONTH-DAY'
+        # TODO 9.0, use the Grape boolean type here
+        optional :confidential, type: String, desc: 'Boolean parameter if the issue should be confidential'
+      end
       put ':id/issues/:issue_id' do
         issue = user_project.issues.find(params[:issue_id])
         authorize! :update_issue, issue
-        keys = [:title, :description, :assignee_id, :milestone_id, :state_event, :due_date, :confidential]
-        keys << :updated_at if current_user.admin? || user_project.owner == current_user
-        attrs = attributes_for_keys(keys)
-
-        # Validate label names in advance
-        if (errors = validate_label_params(params)).any?
-          render_api_error!({ labels: errors }, 400)
-        end
-
-        attrs[:labels] = params[:labels] if params[:labels]
 
         # Convert and filter out invalid confidential flags
-        attrs['confidential'] = to_boolean(attrs['confidential'])
-        attrs.delete('confidential') if attrs['confidential'].nil?
+        params[:confidential] = to_boolean(params[:confidential])
+        params.delete(:confidential) if params[:confidential].nil?
+
+        params.delete(:updated_at) unless current_user.admin? || user_project.owner == current_user
 
-        issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue)
+        issue = ::Issues::UpdateService.new(user_project, current_user, issue_params).execute(issue)
 
         if issue.valid?
           present issue, with: Entities::Issue, current_user: current_user, project: user_project
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index c8cfdb6a90e..ad9b3d8039f 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -77,11 +77,6 @@ module API
 
         mr_params = declared_params
 
-        # Validate label names in advance
-        if (errors = validate_label_params(mr_params)).any?
-          render_api_error!({ labels: errors }, 400)
-        end
-
         merge_request = ::MergeRequests::CreateService.new(user_project, current_user, mr_params).execute
 
         if merge_request.valid?
@@ -157,11 +152,6 @@ module API
 
           mr_params = declared_params(include_missing: false)
 
-          # Validate label names in advance
-          if (errors = validate_label_params(mr_params)).any?
-            render_api_error!({ labels: errors }, 400)
-          end
-
           merge_request = ::MergeRequests::UpdateService.new(user_project, current_user, mr_params).execute(merge_request)
 
           if merge_request.valid?
@@ -202,7 +192,7 @@ module API
             should_remove_source_branch: params[:should_remove_source_branch]
           }
 
-          if params[:merge_when_build_succeeds] && merge_request.pipeline && merge_request.pipeline.active?
+          if params[:merge_when_build_succeeds] && merge_request.head_pipeline && merge_request.head_pipeline.active?
             ::MergeRequests::MergeWhenPipelineSucceedsService
               .new(merge_request.target_project, current_user, merge_params)
               .execute(merge_request)
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index ddfde178d30..2ea3c433ae2 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -379,7 +379,7 @@ module API
       #   POST /projects/:id/fork/:forked_from_id
       post ":id/fork/:forked_from_id" do
         authenticated_as_admin!
-        forked_from_project = find_project(params[:forked_from_id])
+        forked_from_project = find_project!(params[:forked_from_id])
         unless forked_from_project.nil?
           if user_project.forked_from_project.nil?
             user_project.create_forked_project_link(forked_to_project_id: user_project.id, forked_from_project_id: forked_from_project.id)
diff --git a/lib/api/services.rb b/lib/api/services.rb
index 4d23499aa39..bc427705777 100644
--- a/lib/api/services.rb
+++ b/lib/api/services.rb
@@ -65,7 +65,7 @@ module API
         detail 'Added in GitLab 8.13'
       end
       post ':id/services/:service_slug/trigger' do
-        project = Project.find_with_namespace(params[:id]) || Project.find_by(id: params[:id])
+        project = find_project(params[:id])
 
         # This is not accurate, but done to prevent leakage of the project names
         not_found!('Service') unless project
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index 569598fbd2c..bb4de39def1 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -13,7 +13,7 @@ module API
         optional :variables, type: Hash, desc: 'The list of variables to be injected into build'
       end
       post ":id/(ref/:ref/)trigger/builds" do
-        project = Project.find_with_namespace(params[:id]) || Project.find_by(id: params[:id])
+        project = find_project(params[:id])
         trigger = Ci::Trigger.find_by_token(params[:token].to_s)
         not_found! unless project && trigger
         unauthorized! unless trigger.project == project