Improve error reporting on users API

* users (#6878, #3526, #4209): Validation error messages are now exposed through 400 responses, 409 response are sent in case of duplicate email or username
* MRs (#5335): 409 responses are sent in case of duplicate merge request (source/target branches), 422 responses are sent when submiting MR fo/from unrelated forks
* issues
* labels
* projects

8 files changed, 76 insertions, 49 deletions

diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb
index 7f5a125038c..06eb7756841 100644
--- a/lib/api/deploy_keys.rb
+++ b/lib/api/deploy_keys.rb
@@ -58,7 +58,7 @@ module API
         if key.valid? && user_project.deploy_keys << key
           present key, with: Entities::SSHKey
         else
-          not_found!
+          render_validation_error!(key)
         end
       end
 
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 6af0f6d1b25..3a619169eca 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -155,7 +155,17 @@ module API
     end
 
     def not_allowed!
-      render_api_error!('Method Not Allowed', 405)
+      render_api_error!('405 Method Not Allowed', 405)
+    end
+
+    def conflict!(message = nil)
+      render_api_error!(message || '409 Conflict', 409)
+    end
+
+    def render_validation_error!(model)
+      unless model.valid?
+        render_api_error!(model.errors.messages || '400 Bad Request', 400)
+      end
     end
 
     def render_api_error!(message, status)
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 5369149cdfc..30170c657ba 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -109,7 +109,7 @@ module API
 
           present issue, with: Entities::Issue
         else
-          not_found!
+          render_validation_error!(issue)
         end
       end
 
@@ -149,7 +149,7 @@ module API
 
           present issue, with: Entities::Issue
         else
-          not_found!
+          render_validation_error!(issue)
         end
       end
 
diff --git a/lib/api/labels.rb b/lib/api/labels.rb
index 2fdf53ffec2..78ca58ad0d1 100644
--- a/lib/api/labels.rb
+++ b/lib/api/labels.rb
@@ -30,16 +30,14 @@ module API
         attrs = attributes_for_keys [:name, :color]
         label = user_project.find_label(attrs[:name])
 
-        if label
-          return render_api_error!('Label already exists', 409)
-        end
+        conflict!('Label already exists') if label
 
         label = user_project.labels.create(attrs)
 
         if label.valid?
           present label, with: Entities::Label
         else
-          render_api_error!(label.errors.full_messages.join(', '), 400)
+          render_validation_error!(label)
         end
       end
 
@@ -56,9 +54,7 @@ module API
         required_attributes! [:name]
 
         label = user_project.find_label(params[:name])
-        if !label
-          return render_api_error!('Label not found', 404)
-        end
+        not_found!('Label') unless label
 
         label.destroy
       end
@@ -66,10 +62,11 @@ module API
       # Updates an existing label. At least one optional parameter is required.
       #
       # Parameters:
-      #   id    (required) - The ID of a project
-      #   name  (optional) - The name of the label to be deleted
-      #   color (optional) - Color of the label given in 6-digit hex
-      #                      notation with leading '#' sign (e.g. #FFAABB)
+      #   id        (required) - The ID of a project
+      #   name      (required) - The name of the label to be deleted
+      #   new_name  (optional) - The new name of the label
+      #   color     (optional) - Color of the label given in 6-digit hex
+      #                          notation with leading '#' sign (e.g. #FFAABB)
       # Example Request:
       #   PUT /projects/:id/labels
       put ':id/labels' do
@@ -77,16 +74,14 @@ module API
         required_attributes! [:name]
 
         label = user_project.find_label(params[:name])
-        if !label
-          return render_api_error!('Label not found', 404)
-        end
+        not_found!('Label not found') unless label
 
         attrs = attributes_for_keys [:new_name, :color]
 
         if attrs.empty?
-          return render_api_error!('Required parameters "name" or "color" ' \
-                                   'missing',
-                                   400)
+          render_api_error!('Required parameters "new_name" or "color" ' \
+                            'missing',
+                            400)
         end
 
         # Rename new name to the actual label attribute name
@@ -95,7 +90,7 @@ module API
         if label.update(attrs)
           present label, with: Entities::Label
         else
-          render_api_error!(label.errors.full_messages.join(', '), 400)
+          render_validation_error!(label)
         end
       end
     end
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 8726379bf3c..40111014032 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -10,8 +10,13 @@ module API
             error!(errors[:project_access], 422)
           elsif errors[:branch_conflict].any?
             error!(errors[:branch_conflict], 422)
+          elsif errors[:validate_fork].any?
+            error!(errors[:validate_fork], 422)
+          elsif errors[:validate_branches].any?
+            conflict!(errors[:validate_branches])
           end
-          not_found!
+
+          render_api_error!(errors, 400)
         end
       end
 
@@ -214,7 +219,7 @@ module API
         if note.save
           present note, with: Entities::MRNote
         else
-          not_found!
+          render_validation_error!(note)
         end
       end
     end
diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb
index 8e09fff6843..0c2d282f785 100644
--- a/lib/api/project_snippets.rb
+++ b/lib/api/project_snippets.rb
@@ -56,7 +56,7 @@ module API
         if @snippet.save
           present @snippet, with: Entities::ProjectSnippet
         else
-          not_found!
+          render_validation_error!(@snippet)
         end
       end
 
@@ -80,7 +80,7 @@ module API
         if @snippet.update_attributes attrs
           present @snippet, with: Entities::ProjectSnippet
         else
-          not_found!
+          render_validation_error!(@snippet)
         end
       end
 
@@ -97,6 +97,7 @@ module API
           authorize! :modify_project_snippet, @snippet
           @snippet.destroy
         rescue
+          not_found!('Snippet')
         end
       end
 
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 55f7975bbf7..f555819df1b 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -111,7 +111,7 @@ module API
           if @project.errors[:limit_reached].present?
             error!(@project.errors[:limit_reached], 403)
           end
-          not_found!
+          render_validation_error!(@project)
         end
       end
 
@@ -149,7 +149,7 @@ module API
         if @project.saved?
           present @project, with: Entities::Project
         else
-          not_found!
+          render_validation_error!(@project)
         end
       end
 
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 69553f16397..d07815a8a97 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -42,7 +42,8 @@ module API
       # Parameters:
       #   email (required)                  - Email
       #   password (required)               - Password
-      #   name                              - Name
+      #   name (required)                   - Name
+      #   username (required)               - Name
       #   skype                             - Skype ID
       #   linkedin                          - Linkedin
       #   twitter                           - Twitter account
@@ -65,7 +66,15 @@ module API
         if user.save
           present user, with: Entities::UserFull
         else
-          not_found!
+          conflict!('Email has already been taken') if User.
+              where(email: user.email).
+              count > 0
+
+          conflict!('Username has already been taken') if User.
+              where(username: user.username).
+              count > 0
+
+          render_validation_error!(user)
         end
       end
 
@@ -92,14 +101,23 @@ module API
 
         attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :website_url, :projects_limit, :username, :extern_uid, :provider, :bio, :can_create_group, :admin]
         user = User.find(params[:id])
-        not_found!("User not found") unless user
+        not_found!('User') unless user
 
         admin = attrs.delete(:admin)
         user.admin = admin unless admin.nil?
+
+        conflict!('Email has already been taken') if attrs[:email] &&
+            User.where(email: attrs[:email]).
+                where.not(id: user.id).count > 0
+
+        conflict!('Username has already been taken') if attrs[:username] &&
+            User.where(username: attrs[:username]).
+                where.not(id: user.id).count > 0
+
         if user.update_attributes(attrs)
           present user, with: Entities::UserFull
         else
-          not_found!
+          render_validation_error!(user)
         end
       end
 
@@ -113,13 +131,15 @@ module API
       # POST /users/:id/keys
       post ":id/keys" do
         authenticated_as_admin!
+        required_attributes! [:title, :key]
+
         user = User.find(params[:id])
         attrs = attributes_for_keys [:title, :key]
         key = user.keys.new attrs
         if key.save
           present key, with: Entities::SSHKey
         else
-          not_found!
+          render_validation_error!(key)
         end
       end
 
@@ -132,11 +152,9 @@ module API
       get ':uid/keys' do
         authenticated_as_admin!
         user = User.find_by(id: params[:uid])
-        if user
-          present user.keys, with: Entities::SSHKey
-        else
-          not_found!
-        end
+        not_found!('User') unless user
+
+        present user.keys, with: Entities::SSHKey
       end
 
       # Delete existing ssh key of a specified user. Only available to admin
@@ -150,15 +168,13 @@ module API
       delete ':uid/keys/:id' do
         authenticated_as_admin!
         user = User.find_by(id: params[:uid])
-        if user
-          begin
-            key = user.keys.find params[:id]
-            key.destroy
-          rescue ActiveRecord::RecordNotFound
-            not_found!
-          end
-        else
-          not_found!
+        not_found!('User') unless user
+
+        begin
+          key = user.keys.find params[:id]
+          key.destroy
+        rescue ActiveRecord::RecordNotFound
+          not_found!('Key')
         end
       end
 
@@ -173,7 +189,7 @@ module API
         if user
           user.destroy
         else
-          not_found!
+          not_found!('User')
         end
       end
     end
@@ -219,7 +235,7 @@ module API
         if key.save
           present key, with: Entities::SSHKey
         else
-          not_found!
+          render_validation_error!(key)
         end
       end