Fix all tests

This commit was about 6 commits before squashing, with the main goal to
make all tests green. Now, after pushing this commit we'll see what the
CI has to say about that.

5 files changed, 179 insertions, 256 deletions

diff --git a/lib/api/v3/builds.rb b/lib/api/v3/builds.rb
index 33f9cfa6927..c8feba13527 100644
--- a/lib/api/v3/builds.rb
+++ b/lib/api/v3/builds.rb
@@ -12,21 +12,21 @@ module API
         helpers do
           params :optional_scope do
             optional :scope, types: [String, Array[String]], desc: 'The scope of builds to show',
-              values:  ['pending', 'running', 'failed', 'success', 'canceled'],
-              coerce_with: ->(scope) {
-                if scope.is_a?(String)
-                  [scope]
-                elsif scope.is_a?(Hashie::Mash)
-                  scope.values
-                else
-                  ['unknown']
-                end
-              }
+                             values:  %w(pending running failed success canceled skipped),
+                             coerce_with: ->(scope) {
+                                            if scope.is_a?(String)
+                                              [scope]
+                                            elsif   scope.is_a?(Hashie::Mash)
+                                              scope.values
+                                            else
+                                              ['unknown']
+                                            end
+                                          }
           end
         end
 
         desc 'Get a project builds' do
-          success V3::Entities::Build
+          success ::API::V3::Entities::Build
         end
         params do
           use :optional_scope
@@ -36,12 +36,12 @@ module API
           builds = user_project.builds.order('id DESC')
           builds = filter_builds(builds, params[:scope])
 
-          present paginate(builds), with: Entities::Build,
-            user_can_download_artifacts: can?(current_user, :read_build, user_project)
+          present paginate(builds), with: ::API::V3::Entities::Build,
+                                    user_can_download_artifacts: can?(current_user, :read_build, user_project)
         end
 
         desc 'Get builds for a specific commit of a project' do
-          success Entities::Build
+          success ::API::V3::Entities::Build
         end
         params do
           requires :sha, type: String, desc: 'The SHA id of a commit'
@@ -57,12 +57,12 @@ module API
           builds = user_project.builds.where(pipeline: pipelines).order('id DESC')
           builds = filter_builds(builds, params[:scope])
 
-          present paginate(builds), with: Entities::Build,
-            user_can_download_artifacts: can?(current_user, :read_build, user_project)
+          present paginate(builds), with: ::API::V3::Entities::Build,
+                                    user_can_download_artifacts: can?(current_user, :read_build, user_project)
         end
 
         desc 'Get a specific build of a project' do
-          success Entities::Build
+          success ::API::V3::Entities::Build
         end
         params do
           requires :build_id, type: Integer, desc: 'The ID of a build'
@@ -72,8 +72,8 @@ module API
 
           build = get_build!(params[:build_id])
 
-          present build, with: Entities::Build,
-            user_can_download_artifacts: can?(current_user, :read_build, user_project)
+          present build, with: ::API::V3::Entities::Build,
+                         user_can_download_artifacts: can?(current_user, :read_build, user_project)
         end
 
         desc 'Download the artifacts file from build' do
@@ -128,7 +128,7 @@ module API
         end
 
         desc 'Cancel a specific build of a project' do
-          success Entities::Build
+          success ::API::V3::Entities::Build
         end
         params do
           requires :build_id, type: Integer, desc: 'The ID of a build'
@@ -140,12 +140,12 @@ module API
 
           build.cancel
 
-          present build, with: Entities::Build,
-            user_can_download_artifacts: can?(current_user, :read_build, user_project)
+          present build, with: ::API::V3::Entities::Build,
+                         user_can_download_artifacts: can?(current_user, :read_build, user_project)
         end
 
         desc 'Retry a specific build of a project' do
-          success Entities::Build
+          success ::API::V3::Entities::Build
         end
         params do
           requires :build_id, type: Integer, desc: 'The ID of a build'
@@ -158,12 +158,12 @@ module API
 
           build = Ci::Build.retry(build, current_user)
 
-          present build, with: Entities::Build,
-            user_can_download_artifacts: can?(current_user, :read_build, user_project)
+          present build, with: ::API::V3::Entities::Build,
+                         user_can_download_artifacts: can?(current_user, :read_build, user_project)
         end
 
         desc 'Erase build (remove artifacts and build trace)' do
-          success Entities::Build
+          success ::API::V3::Entities::Build
         end
         params do
           requires :build_id, type: Integer, desc: 'The ID of a build'
@@ -175,12 +175,12 @@ module API
           return forbidden!('Build is not erasable!') unless build.erasable?
 
           build.erase(erased_by: current_user)
-          present build, with: Entities::Build,
-            user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
+          present build, with: ::API::V3::Entities::Build,
+                         user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
         end
 
         desc 'Keep the artifacts to prevent them from being deleted' do
-          success Entities::Build
+          success ::API::V3::Entities::Build
         end
         params do
           requires :build_id, type: Integer, desc: 'The ID of a build'
@@ -194,12 +194,12 @@ module API
           build.keep_artifacts!
 
           status 200
-          present build, with: Entities::Build,
-            user_can_download_artifacts: can?(current_user, :read_build, user_project)
+          present build, with: ::API::V3::Entities::Build,
+                         user_can_download_artifacts: can?(current_user, :read_build, user_project)
         end
 
         desc 'Trigger a manual build' do
-          success Entities::Build
+          success ::API::V3::Entities::Build
           detail 'This feature was added in GitLab 8.11'
         end
         params do
@@ -215,8 +215,8 @@ module API
           build.play(current_user)
 
           status 200
-          present build, with: Entities::Build,
-            user_can_download_artifacts: can?(current_user, :read_build, user_project)
+          present build, with: ::API::V3::Entities::Build,
+                         user_can_download_artifacts: can?(current_user, :read_build, user_project)
         end
       end
 
diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb
index 7b81d159b06..832b4bdeb4f 100644
--- a/lib/api/v3/entities.rb
+++ b/lib/api/v3/entities.rb
@@ -81,7 +81,7 @@ module API
         expose :request_access_enabled
         expose :only_allow_merge_if_all_discussions_are_resolved
 
-        expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics
+        expose :statistics, using: '::API::V3::Entities::ProjectStatistics', if: :statistics
       end
 
       class ProjectWithAccess < Project
@@ -210,106 +210,19 @@ module API
         expose :filename, :size
       end
 
-      class EnvironmentBasic < Grape::Entity
-        expose :id, :name, :slug, :external_url
-      end
-
-      class Environment < EnvironmentBasic
-        expose :project, using: Entities::Project
-      end
-
       class Deployment < Grape::Entity
         expose :id, :iid, :ref, :sha, :created_at
-        expose :user,        using: Entities::UserBasic
-        expose :environment, using: Entities::EnvironmentBasic
+        expose :user,        using: ::API::Entities::UserBasic
+        expose :environment, using: ::API::Entities::EnvironmentBasic
         expose :deployable,  using: Entities::Build
       end
 
-      class Group < Grape::Entity
-        expose :id, :name, :path, :description, :visibility_level
-        expose :lfs_enabled?, as: :lfs_enabled
-        expose :avatar_url
-        expose :web_url
-        expose :request_access_enabled
-        expose :statistics, if: :statistics do
-          with_options format_with: -> (value) { value.to_i } do
-            expose :storage_size
-            expose :repository_size
-            expose :lfs_objects_size
-            expose :build_artifacts_size
-          end
-        end
-      end
-
-      class GroupDetail < Group
-        expose :projects, using: Entities::Project
-        expose :shared_projects, using: Entities::Project
-      end
-
-      class MergeRequest < ProjectEntity
-        expose :target_branch, :source_branch
-        expose :upvotes, :downvotes
-        expose :author, :assignee, using: Entities::UserBasic
-        expose :source_project_id, :target_project_id
-        expose :label_names, as: :labels
-        expose :work_in_progress?, as: :work_in_progress
-        expose :milestone, using: Entities::Milestone
-        expose :merge_when_build_succeeds
-        expose :merge_status
-        expose :diff_head_sha, as: :sha
-        expose :merge_commit_sha
-        expose :subscribed do |merge_request, options|
-          merge_request.subscribed?(options[:current_user], options[:project])
-        end
-        expose :user_notes_count
-        expose :should_remove_source_branch?, as: :should_remove_source_branch
-        expose :force_remove_source_branch?, as: :force_remove_source_branch
-        expose :web_url do |merge_request, options|
-          Gitlab::UrlBuilder.build(merge_request)
-        end
-      end
-
       class MergeRequestChanges < MergeRequest
-        expose :diffs, as: :changes, using: Entities::RepoDiff do |compare, _|
+        expose :diffs, as: :changes, using: ::API::Entities::RepoDiff do |compare, _|
           compare.raw_diffs(all_diffs: true).to_a
         end
       end
 
-      class Project < Grape::Entity
-        expose :id, :description, :default_branch, :tag_list
-        expose :public?, as: :public
-        expose :archived?, as: :archived
-        expose :visibility_level, :ssh_url_to_repo, :http_url_to_repo, :web_url
-        expose :owner, using: Entities::UserBasic, unless: ->(project, options) { project.group }
-        expose :name, :name_with_namespace
-        expose :path, :path_with_namespace
-        expose :container_registry_enabled
-        # Expose old field names with the new permissions methods to keep API compatible
-        expose(:issues_enabled) { |project, options| project.feature_available?(:issues, options[:current_user]) }
-        expose(:merge_requests_enabled) { |project, options| project.feature_available?(:merge_requests, options[:current_user]) }
-        expose(:wiki_enabled) { |project, options| project.feature_available?(:wiki, options[:current_user]) }
-        expose(:builds_enabled) { |project, options| project.feature_available?(:builds, options[:current_user]) }
-        expose(:snippets_enabled) { |project, options| project.feature_available?(:snippets, options[:current_user]) }
-        expose :created_at, :last_activity_at
-        expose :shared_runners_enabled
-        expose :lfs_enabled?, as: :lfs_enabled
-        expose :creator_id
-        expose :namespace, using: 'API::Entities::Namespace'
-        expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda{ |project, options| project.forked? }
-        expose :avatar_url
-        expose :star_count, :forks_count
-        expose :open_issues_count, if: lambda { |project, options| project.feature_available?(:issues, options[:current_user]) && project.default_issues_tracker? }
-        expose :runners_token, if: lambda { |_project, options| options[:user_can_admin_project] }
-        expose :public_builds
-        expose :shared_with_groups do |project, options|
-          SharedGroup.represent(project.project_group_links.all, options)
-        end
-        expose :only_allow_merge_if_build_succeeds
-        expose :request_access_enabled
-        expose :only_allow_merge_if_all_discussions_are_resolved
-        expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics
-      end
-
       class ProjectStatistics < Grape::Entity
         expose :commit_count
         expose :storage_size
@@ -331,23 +244,10 @@ module API
         end
       end
 
-      class ProjectHook < Hook
+      class ProjectHook < ::API::Entities::Hook
         expose :project_id, :issues_events, :merge_requests_events
         expose :note_events, :build_events, :pipeline_events, :wiki_page_events
       end
-
-      class ProjectWithAccess < Project
-        expose :permissions do
-          expose :project_access, using: Entities::ProjectAccess do |project, options|
-            project.project_members.find_by(user_id: options[:current_user].id)
-          end
-          expose :group_access, using: Entities::GroupAccess do |project, options|
-            if project.group
-              project.group.group_members.find_by(user_id: options[:current_user].id)
-            end
-          end
-        end
-      end
     end
   end
 end
diff --git a/lib/api/v3/merge_request_diffs.rb b/lib/api/v3/merge_request_diffs.rb
index bc3d69f6904..a462803e26c 100644
--- a/lib/api/v3/merge_request_diffs.rb
+++ b/lib/api/v3/merge_request_diffs.rb
@@ -1,40 +1,42 @@
 module API
-  # MergeRequestDiff API
-  class MergeRequestDiffs < Grape::API
-    before { authenticate! }
-
-    resource :projects do
-      desc 'Get a list of merge request diff versions' do
-        detail 'This feature was introduced in GitLab 8.12.'
-        success Entities::MergeRequestDiff
-      end
-
-      params do
-        requires :id, type: String, desc: 'The ID of a project'
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
-      end
-
-      get ":id/merge_requests/:merge_request_id/versions" do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
-
-        present merge_request.merge_request_diffs, with: Entities::MergeRequestDiff
-      end
-
-      desc 'Get a single merge request diff version' do
-        detail 'This feature was introduced in GitLab 8.12.'
-        success Entities::MergeRequestDiffFull
-      end
-
-      params do
-        requires :id, type: String, desc: 'The ID of a project'
-        requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
-        requires :version_id, type: Integer, desc: 'The ID of a merge request diff version'
-      end
-
-      get ":id/merge_requests/:merge_request_id/versions/:version_id" do
-        merge_request = find_merge_request_with_access(params[:merge_request_id])
-
-        present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull
+  module V3
+    # MergeRequestDiff API
+    class MergeRequestDiffs < Grape::API
+      before { authenticate! }
+
+      resource :projects do
+        desc 'Get a list of merge request diff versions' do
+          detail 'This feature was introduced in GitLab 8.12.'
+          success ::API::Entities::MergeRequestDiff
+        end
+
+        params do
+          requires :id, type: String, desc: 'The ID of a project'
+          requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+        end
+
+        get ":id/merge_requests/:merge_request_id/versions" do
+          merge_request = find_merge_request_with_access(params[:merge_request_id])
+
+          present merge_request.merge_request_diffs, with: ::API::Entities::MergeRequestDiff
+        end
+
+        desc 'Get a single merge request diff version' do
+          detail 'This feature was introduced in GitLab 8.12.'
+          success ::API::Entities::MergeRequestDiffFull
+        end
+
+        params do
+          requires :id, type: String, desc: 'The ID of a project'
+          requires :merge_request_id, type: Integer, desc: 'The ID of a merge request'
+          requires :version_id, type: Integer, desc: 'The ID of a merge request diff version'
+        end
+
+        get ":id/merge_requests/:merge_request_id/versions/:version_id" do
+          merge_request = find_merge_request_with_access(params[:merge_request_id])
+
+          present merge_request.merge_request_diffs.find(params[:version_id]), with: ::API::Entities::MergeRequestDiffFull
+        end
       end
     end
   end
diff --git a/lib/api/v3/project_hooks.rb b/lib/api/v3/project_hooks.rb
index cb679e6658a..861b991b8e1 100644
--- a/lib/api/v3/project_hooks.rb
+++ b/lib/api/v3/project_hooks.rb
@@ -1,102 +1,104 @@
 module API
-  class ProjectHooks < Grape::API
-    include PaginationParams
+  module V3
+    class ProjectHooks < Grape::API
+      include PaginationParams
 
-    before { authenticate! }
-    before { authorize_admin_project }
+      before { authenticate! }
+      before { authorize_admin_project }
 
-    helpers do
-      params :project_hook_properties do
-        requires :url, type: String, desc: "The URL to send the request to"
-        optional :push_events, type: Boolean, desc: "Trigger hook on push events"
-        optional :issues_events, type: Boolean, desc: "Trigger hook on issues events"
-        optional :merge_requests_events, type: Boolean, desc: "Trigger hook on merge request events"
-        optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events"
-        optional :note_events, type: Boolean, desc: "Trigger hook on note(comment) events"
-        optional :build_events, type: Boolean, desc: "Trigger hook on build events"
-        optional :pipeline_events, type: Boolean, desc: "Trigger hook on pipeline events"
-        optional :wiki_page_events, type: Boolean, desc: "Trigger hook on wiki events"
-        optional :enable_ssl_verification, type: Boolean, desc: "Do SSL verification when triggering the hook"
-        optional :token, type: String, desc: "Secret token to validate received payloads; this will not be returned in the response"
+      helpers do
+        params :project_hook_properties do
+          requires :url, type: String, desc: "The URL to send the request to"
+          optional :push_events, type: Boolean, desc: "Trigger hook on push events"
+          optional :issues_events, type: Boolean, desc: "Trigger hook on issues events"
+          optional :merge_requests_events, type: Boolean, desc: "Trigger hook on merge request events"
+          optional :tag_push_events, type: Boolean, desc: "Trigger hook on tag push events"
+          optional :note_events, type: Boolean, desc: "Trigger hook on note(comment) events"
+          optional :build_events, type: Boolean, desc: "Trigger hook on build events"
+          optional :pipeline_events, type: Boolean, desc: "Trigger hook on pipeline events"
+          optional :wiki_page_events, type: Boolean, desc: "Trigger hook on wiki events"
+          optional :enable_ssl_verification, type: Boolean, desc: "Do SSL verification when triggering the hook"
+          optional :token, type: String, desc: "Secret token to validate received payloads; this will not be returned in the response"
+        end
       end
-    end
 
-    params do
-      requires :id, type: String, desc: 'The ID of a project'
-    end
-    resource :projects do
-      desc 'Get project hooks' do
-        success Entities::ProjectHook
-      end
       params do
-        use :pagination
+        requires :id, type: String, desc: 'The ID of a project'
       end
-      get ":id/hooks" do
-        hooks = paginate user_project.hooks
+      resource :projects do
+        desc 'Get project hooks' do
+          success ::API::V3::Entities::ProjectHook
+        end
+        params do
+          use :pagination
+        end
+        get ":id/hooks" do
+          hooks = paginate user_project.hooks
 
-        present hooks, with: Entities::ProjectHook
-      end
+          present hooks, with: ::API::V3::Entities::ProjectHook
+        end
 
-      desc 'Get a project hook' do
-        success Entities::ProjectHook
-      end
-      params do
-        requires :hook_id, type: Integer, desc: 'The ID of a project hook'
-      end
-      get ":id/hooks/:hook_id" do
-        hook = user_project.hooks.find(params[:hook_id])
-        present hook, with: Entities::ProjectHook
-      end
+        desc 'Get a project hook' do
+          success ::API::V3::Entities::ProjectHook
+        end
+        params do
+          requires :hook_id, type: Integer, desc: 'The ID of a project hook'
+        end
+        get ":id/hooks/:hook_id" do
+          hook = user_project.hooks.find(params[:hook_id])
+          present hook, with: ::API::V3::Entities::ProjectHook
+        end
 
-      desc 'Add hook to project' do
-        success Entities::ProjectHook
-      end
-      params do
-        use :project_hook_properties
-      end
-      post ":id/hooks" do
-        hook = user_project.hooks.new(declared_params(include_missing: false))
+        desc 'Add hook to project' do
+          success ::API::V3::Entities::ProjectHook
+        end
+        params do
+          use :project_hook_properties
+        end
+        post ":id/hooks" do
+          hook = user_project.hooks.new(declared_params(include_missing: false))
 
-        if hook.save
-          present hook, with: Entities::ProjectHook
-        else
-          error!("Invalid url given", 422) if hook.errors[:url].present?
+          if hook.save
+            present hook, with: ::API::V3::Entities::ProjectHook
+          else
+            error!("Invalid url given", 422) if hook.errors[:url].present?
 
-          not_found!("Project hook #{hook.errors.messages}")
+            not_found!("Project hook #{hook.errors.messages}")
+          end
         end
-      end
 
-      desc 'Update an existing project hook' do
-        success Entities::ProjectHook
-      end
-      params do
-        requires :hook_id, type: Integer, desc: "The ID of the hook to update"
-        use :project_hook_properties
-      end
-      put ":id/hooks/:hook_id" do
-        hook = user_project.hooks.find(params.delete(:hook_id))
+        desc 'Update an existing project hook' do
+          success ::API::V3::Entities::ProjectHook
+        end
+        params do
+          requires :hook_id, type: Integer, desc: "The ID of the hook to update"
+          use :project_hook_properties
+        end
+        put ":id/hooks/:hook_id" do
+          hook = user_project.hooks.find(params.delete(:hook_id))
 
-        if hook.update_attributes(declared_params(include_missing: false))
-          present hook, with: Entities::ProjectHook
-        else
-          error!("Invalid url given", 422) if hook.errors[:url].present?
+          if hook.update_attributes(declared_params(include_missing: false))
+            present hook, with: ::API::V3::Entities::ProjectHook
+          else
+            error!("Invalid url given", 422) if hook.errors[:url].present?
 
-          not_found!("Project hook #{hook.errors.messages}")
+            not_found!("Project hook #{hook.errors.messages}")
+          end
         end
-      end
 
-      desc 'Deletes project hook' do
-        success Entities::ProjectHook
-      end
-      params do
-        requires :hook_id, type: Integer, desc: 'The ID of the hook to delete'
-      end
-      delete ":id/hooks/:hook_id" do
-        begin
-          present user_project.hooks.destroy(params[:hook_id]), with: Entities::ProjectHook
-        rescue
-          # ProjectHook can raise Error if hook_id not found
-          not_found!("Error deleting hook #{params[:hook_id]}")
+        desc 'Deletes project hook' do
+          success ::API::V3::Entities::ProjectHook
+        end
+        params do
+          requires :hook_id, type: Integer, desc: 'The ID of the hook to delete'
+        end
+        delete ":id/hooks/:hook_id" do
+          begin
+            present user_project.hooks.destroy(params[:hook_id]), with: ::API::V3::Entities::ProjectHook
+          rescue
+            # ProjectHook can raise Error if hook_id not found
+            not_found!("Error deleting hook #{params[:hook_id]}")
+          end
         end
       end
     end
diff --git a/lib/api/v3/services.rb b/lib/api/v3/services.rb
index de24e6418c7..d77185ffe5a 100644
--- a/lib/api/v3/services.rb
+++ b/lib/api/v3/services.rb
@@ -537,6 +537,23 @@ module API
         ]
       }
 
+      trigger_services = {
+        'mattermost-slash-commands' => [
+          {
+            name: :token,
+            type: String,
+            desc: 'The Mattermost token'
+          }
+        ],
+        'slack-slash-commands' => [
+          {
+            name: :token,
+            type: String,
+            desc: 'The Slack token'
+          }
+        ]
+      }.freeze
+
       resource :projects do
         before { authenticate! }
         before { authorize_admin_project }
@@ -561,6 +578,7 @@ module API
           end
 
           if service.update_attributes(attrs.merge(active: false))
+            status(200)
             true
           else
             render_api_error!('400 Bad Request', 400)
@@ -620,3 +638,4 @@ module API
       end
     end
   end
+end