Merge branch 'master' into feature/multi-level-container-registry-images

* master: (230 commits)
  Fix N+1 query in loading pipelines in merge requests
  Fix Spinach and Capybara dependencies
  Prevent users from disconnecting gitlab account from CAS
  30276 Move issue, mr, todos next to profile dropdown in top nav
  Refactor SearchController#show
  Properly eagerly-load the Capybara server for JS feature specs only
  Updating documentation to include a missing step in the update procedure
  Eager-load the Capybara server to prevent timeouts
  Increase Capybara's timeout
  Add metrics button to Environment Overview page
  Fix link to Jira service documentation
  Handle parsing OpenBSD ps output properly to display sidekiq infos on ...
  Eliminate unnecessary queries that add ~500 ms of load time for a large issue
  20914 Limits line length for project home page
  Allow users to import GitHub projects to subgroups
  Update dpl CI example
  Fix the docs:check:links job
  Don't clean up the gitlab-test-fork_bare repo
  Make GitLab use Gitaly for commit_is_ancestor
  Remove unnecessary ORDER BY clause from `forked_to_project_id` subquery
  ...

7 files changed, 24 insertions, 49 deletions

diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index bd22b82476b..61527c1e20b 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -90,6 +90,11 @@ module API
       MergeRequestsFinder.new(current_user, project_id: user_project.id).find_by!(iid: iid)
     end
 
+    def find_project_snippet(id)
+      finder_params = { filter: :by_project, project: user_project }
+      SnippetsFinder.new.execute(current_user, finder_params).find(id)
+    end
+
     def find_merge_request_with_access(iid, access_level = :read_merge_request)
       merge_request = user_project.merge_requests.find_by!(iid: iid)
       authorize! access_level, merge_request
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 7eed93aba00..523f38d129e 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -139,7 +139,7 @@ module API
         return unless Gitlab::GitalyClient.enabled?
 
         begin
-          Gitlab::GitalyClient::Notifications.new.post_receive(params[:repo_path])
+          Gitlab::GitalyClient::Notifications.new(params[:repo_path]).post_receive
         rescue GRPC::Unavailable => e
           render_api_error(e, 500)
         end
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index fd2674910d2..4dce5dd130a 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -63,14 +63,14 @@ module API
         success Entities::IssueBasic
       end
       params do
-        optional :state, type: String, values: %w[opened closed all], default: 'opened',
+        optional :state, type: String, values: %w[opened closed all], default: 'all',
                          desc: 'Return opened, closed, or all issues'
         use :issues_params
       end
       get ":id/issues" do
         group = find_group!(params[:id])
 
-        issues = find_issues(group_id: group.id, state: params[:state] || 'opened')
+        issues = find_issues(group_id: group.id)
 
         present paginate(issues), with: Entities::IssueBasic, current_user: current_user
       end
diff --git a/lib/api/labels.rb b/lib/api/labels.rb
index d9a3cb7bb6b..20b25529d0c 100644
--- a/lib/api/labels.rb
+++ b/lib/api/labels.rb
@@ -23,7 +23,7 @@ module API
       end
       params do
         requires :name, type: String, desc: 'The name of the label to be created'
-        requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)"
+        requires :color, type: String, desc: "The color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the allowed CSS color names"
         optional :description, type: String, desc: 'The description of label to be created'
         optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true
       end
@@ -34,7 +34,7 @@ module API
         conflict!('Label already exists') if label
 
         priority = params.delete(:priority)
-        label = user_project.labels.create(declared_params(include_missing: false))
+        label = ::Labels::CreateService.new(declared_params(include_missing: false)).execute(project: user_project)
 
         if label.valid?
           label.prioritize!(user_project, priority) if priority
@@ -65,7 +65,7 @@ module API
       params do
         requires :name,  type: String, desc: 'The name of the label to be updated'
         optional :new_name, type: String, desc: 'The new name of the label'
-        optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB)"
+        optional :color, type: String, desc: "The new color of the label given in 6-digit hex notation with leading '#' sign (e.g. #FFAABB) or one of the allowed CSS color names"
         optional :description, type: String, desc: 'The new description of label'
         optional :priority, type: Integer, desc: 'The priority of the label', allow_blank: true
         at_least_one_of :new_name, :color, :description, :priority
@@ -82,7 +82,8 @@ module API
         # Rename new name to the actual label attribute name
         label_params[:name] = label_params.delete(:new_name) if label_params.key?(:new_name)
 
-        render_validation_error!(label) unless label.update(label_params)
+        label = ::Labels::UpdateService.new(label_params).execute(label)
+        render_validation_error!(label) unless label.valid?
 
         if update_priority
           if priority.nil?
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 5cc807d5bff..c8033664133 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -226,41 +226,6 @@ module API
           .cancel(merge_request)
       end
 
-      desc 'Get the comments of a merge request' do
-        success Entities::MRNote
-      end
-      params do
-        use :pagination
-      end
-      get ':id/merge_requests/:merge_request_iid/comments' do
-        merge_request = find_merge_request_with_access(params[:merge_request_iid])
-        present paginate(merge_request.notes.fresh), with: Entities::MRNote
-      end
-
-      desc 'Post a comment to a merge request' do
-        success Entities::MRNote
-      end
-      params do
-        requires :note, type: String, desc: 'The text of the comment'
-      end
-      post ':id/merge_requests/:merge_request_iid/comments' do
-        merge_request = find_merge_request_with_access(params[:merge_request_iid], :create_note)
-
-        opts = {
-          note: params[:note],
-          noteable_type: 'MergeRequest',
-          noteable_id: merge_request.id
-        }
-
-        note = ::Notes::CreateService.new(user_project, current_user, opts).execute
-
-        if note.save
-          present note, with: Entities::MRNote
-        else
-          render_api_error!("Failed to save note #{note.errors.messages}", 400)
-        end
-      end
-
       desc 'List issues that will be closed on merge' do
         success Entities::MRNote
       end
diff --git a/lib/api/notes.rb b/lib/api/notes.rb
index 29ceffdbd2d..de39e579ac3 100644
--- a/lib/api/notes.rb
+++ b/lib/api/notes.rb
@@ -21,7 +21,7 @@ module API
           use :pagination
         end
         get ":id/#{noteables_str}/:noteable_id/notes" do
-          noteable = user_project.send(noteables_str.to_sym).find(params[:noteable_id])
+          noteable = find_project_noteable(noteables_str, params[:noteable_id])
 
           if can?(current_user, noteable_read_ability_name(noteable), noteable)
             # We exclude notes that are cross-references and that cannot be viewed
@@ -49,7 +49,7 @@ module API
           requires :noteable_id, type: Integer, desc: 'The ID of the noteable'
         end
         get ":id/#{noteables_str}/:noteable_id/notes/:note_id" do
-          noteable = user_project.send(noteables_str.to_sym).find(params[:noteable_id])
+          noteable = find_project_noteable(noteables_str, params[:noteable_id])
           note = noteable.notes.find(params[:note_id])
           can_read_note = can?(current_user, noteable_read_ability_name(noteable), noteable) && !note.cross_reference_not_visible_for?(current_user)
 
@@ -69,14 +69,14 @@ module API
           optional :created_at, type: String, desc: 'The creation date of the note'
         end
         post ":id/#{noteables_str}/:noteable_id/notes" do
+          noteable = find_project_noteable(noteables_str, params[:noteable_id])
+
           opts = {
             note: params[:body],
             noteable_type: noteables_str.classify,
-            noteable_id: params[:noteable_id]
+            noteable_id: noteable.id
           }
 
-          noteable = user_project.send(noteables_str.to_sym).find(params[:noteable_id])
-
           if can?(current_user, noteable_read_ability_name(noteable), noteable)
             if params[:created_at] && (current_user.is_admin? || user_project.owner == current_user)
               opts[:created_at] = params[:created_at]
@@ -137,6 +137,10 @@ module API
     end
 
     helpers do
+      def find_project_noteable(noteables_str, noteable_id)
+        public_send("find_project_#{noteables_str.singularize}", noteable_id)
+      end
+
       def noteable_read_ability_name(noteable)
         "read_#{noteable.class.to_s.underscore}".to_sym
       end
diff --git a/lib/api/v3/issues.rb b/lib/api/v3/issues.rb
index 54c6a8060b8..715083fc4f8 100644
--- a/lib/api/v3/issues.rb
+++ b/lib/api/v3/issues.rb
@@ -73,14 +73,14 @@ module API
           success ::API::Entities::Issue
         end
         params do
-          optional :state, type: String, values: %w[opened closed all], default: 'opened',
+          optional :state, type: String, values: %w[opened closed all], default: 'all',
                            desc: 'Return opened, closed, or all issues'
           use :issues_params
         end
         get ":id/issues" do
           group = find_group!(params[:id])
 
-          issues = find_issues(group_id: group.id, state: params[:state] || 'opened', match_all_labels: true)
+          issues = find_issues(group_id: group.id, match_all_labels: true)
 
           present paginate(issues), with: ::API::Entities::Issue, current_user: current_user
         end