FIX: Validate category name on parent change (#10815)

FIX: Validate category name on parent change (#10815)

Previously, moving a category into another one, that already had a child category of that name (but with a non-conflicting slug) would cause a 500 error:

# PG::UniqueViolation:
#   ERROR:  duplicate key value violates unique constraint "unique_index_categories_on_name"
#   DETAIL:  Key (COALESCE(parent_category_id, '-1'::integer), name)=(5662, Amazing Category 0) already exists.

It now returns 422, and shows the same message as when you’re renaming a category: “Category Name has already been taken”.

diff --git a/app/models/category.rb b/app/models/category.rb
index 7c2a8ea..91daf8e 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -49,7 +49,7 @@ class Category < ActiveRecord::Base
 
   validates :user_id, presence: true
 
-  validates :name, if: Proc.new { |c| c.new_record? || c.will_save_change_to_name? },
+  validates :name, if: Proc.new { |c| c.new_record? || c.will_save_change_to_name? || c.will_save_change_to_parent_category_id? },
                    presence: true,
                    uniqueness: { scope: :parent_category_id, case_sensitive: false },
                    length: { in: 1..50 }
diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb
index 65cd0a7..6fce406 100644
--- a/spec/requests/categories_controller_spec.rb
+++ b/spec/requests/categories_controller_spec.rb
@@ -314,6 +314,17 @@ describe CategoriesController do
         expect(response.status).to eq(422)
       end
 
+      it "returns errors when there is a name conflict while moving a category into another" do
+        parent_category = Fabricate(:category, name: "Parent", user: admin)
+        other_category = Fabricate(:category, name: category.name, user: admin, parent_category: parent_category, slug: "a-different-slug")
+
+        put "/categories/#{category.id}.json", params: {
+          parent_category_id: parent_category.id,
+        }
+
+        expect(response.status).to eq(422)
+      end
+
       it "returns 422 if email_in address is already in use for other category" do
         _other_category = Fabricate(:category, name: "Other", email_in: "mail@examle.com")
 

GitHub sha: cf44cdb0

This commit appears in #10815 which was approved by ZogStriP. It was merged by CvX.