Add some fixes after review

2 files changed, 15 insertions, 15 deletions

diff --git a/lib/api/builds.rb b/lib/api/builds.rb
index d3f4e33ebbf..1337e1bb452 100644
--- a/lib/api/builds.rb
+++ b/lib/api/builds.rb
@@ -35,6 +35,7 @@ module API
 
         builds = commit.builds.order('id DESC')
         builds = filter_builds(builds, params[:scope])
+
         present paginate(builds), with: Entities::Build,
                                   user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
       end
@@ -118,25 +119,26 @@ module API
 
     helpers do
       def get_build(id)
-        user_project.builds.where(id: id).first
+        user_project.builds.find_by(id: id.to_i)
       end
 
       def filter_builds(builds, scope)
-        available_scopes = Ci::Build.available_statuses
+        return builds if scope.nil? || scope.empty?
+
+        available_statuses = Ci::Build.available_statuses
         scope =
-          if scope.is_a?(String) || scope.is_a?(Symbol)
-            available_scopes & [scope.to_s]
-          elsif scope.is_a?(Array)
-            available_scopes & scope
-          elsif scope.respond_to?(:to_h)
-            available_scopes & scope.to_h.values
+          if scope.is_a?(String)
+            [scope]
+          elsif scope.is_a?(Hashie::Mash)
+            scope.values
           else
-            []
+            ['unknown']
           end
 
-        return builds if scope.empty?
+        unknown = scope - available_statuses
+        render_api_error!('Scope contains invalid value(s)', 400) unless unknown.empty?
 
-        builds.where(status: scope)
+        builds.where(status: available_statuses && scope)
       end
 
       def authorize_manage_builds!
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index e19f9f8d75a..f0816a46529 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -378,18 +378,16 @@ module API
       expose :id, :status, :stage, :name, :ref, :tag, :coverage
       expose :created_at, :started_at, :finished_at
       expose :user, with: User
+      # TODO: download_url in Ci:Build model is an GitLab Web Interface URL, not API URL. We should think on some API
+      #       for downloading of artifacts (see: https://gitlab.com/gitlab-org/gitlab-ce/issues/4255)
       expose :download_url do |repo_obj, options|
         if options[:user_can_download_artifacts]
           repo_obj.download_url
-        else
-          nil
         end
       end
       expose :commit, with: RepoCommit do |repo_obj, _options|
         if repo_obj.respond_to?(:commit)
           repo_obj.commit.commit_data
-        else
-          nil
         end
       end
       expose :runner, with: Runner