fixed tag name validation & switched to displaying all error messages

Author: Oaphi

app/controllers/tags_controller.rb

@@ -155,7 +155,7 @@ def rename
       render json: { status: 'success', tag: @tag }
     else
       render json: { status: 'failed',
-                     message: helpers.tag_rename_error_msg(@tag),
+                     errors: @tag.errors.messages.values.flatten,
                      tag: @tag },
              status: :bad_request
     end

app/helpers/tags_helper.rb

@@ -38,15 +38,4 @@ def post_ids_for_tags(tag_ids)
     sql = "SELECT post_id FROM posts_tags WHERE tag_id IN #{ApplicationRecord.sanitize_sql_in(tag_ids)}"
     ActiveRecord::Base.connection.execute(sql).to_a.flatten
   end
-
-  # Gets a standard tag rename error message for a tag
-  # @param post [Tag] target tag
-  # @return [String] error message
-  def tag_rename_error_msg(tag)
-    if tag.errors.where(:name, :taken)
-      I18n.t('tags.errors.rename_taken')
-    else
-      I18n.t('tags.errors.rename_generic')
-    end
-  end
 end

app/models/tag.rb

@@ -12,11 +12,21 @@ class Tag < ApplicationRecord
 
   validates :excerpt, length: { maximum: 600 }, allow_blank: true
   validates :wiki_markdown, length: { maximum: 30_000 }, allow_blank: true
-  validates :name, presence: true, format: { with: /[^ \t]+/, message: 'Tag names may not include spaces' }
+  validates :name, presence: {
+    message: I18n.t('tags.errors.no_blank_name')
+  }
+  validates :name, format: {
+    with: /\A[^\s]+\Z/, # https://regex101.com/r/7BxgIn/1
+    message: I18n.t('tags.errors.no_spaces_in_name')
+  }
   validate :parent_not_self
   validate :parent_not_own_child
   validate :synonym_unique
-  validates :name, uniqueness: { scope: [:tag_set_id], case_sensitive: false }
+  validates :name, uniqueness: {
+    scope: [:tag_set_id],
+    case_sensitive: false,
+    message: I18n.t('tags.errors.no_duplicate_names')
+  }
 
   # scopes
   scope :list_includes, -> { includes(:tag_synonyms) }

config/locales/strings/en.tags.yml

@@ -1,7 +1,11 @@
 en:
   tags:
     errors:
-      rename_taken: >
+      no_blank_name: >
+        Tag name should contain at least 1 character.
+      no_duplicate_names: >
         A tag with that name already exists.
+      no_spaces_in_name: >
+        Tag names may not contain spaces.
       rename_generic: >
         Failed to rename the tag.

test/controllers/tags_controller_test.rb

@@ -211,15 +211,23 @@ class TagsControllerTest < ActionController::TestCase
     sign_in users(:moderator)
 
     tag = tags(:base)
-    old_tag_name = tag.name
-
-    try_rename_tag(categories(:main), tag, '')
-
-    assert_json_failure(:bad_request)
-    res_body = JSON.parse(response.body)
-    assert_equal @controller.helpers.tag_rename_error_msg(tag), res_body['message']
-    tag.reload
-    assert_equal tag.name, old_tag_name
+    old_name = tag.name
+
+    [
+      ['', I18n.t('tags.errors.no_blank_name')],
+      ['name with spaces', I18n.t('tags.errors.no_spaces_in_name')],
+      [tags(:discussion).name, I18n.t('tags.errors.no_duplicate_names')]
+    ].each do |test_case|
+      new_name, error_message = test_case
+
+      try_rename_tag(categories(:main), tag, new_name)
+
+      assert_json_failure(:bad_request)
+      res_body = JSON.parse(response.body)
+      assert res_body['errors'].include?(error_message)
+      tag.reload
+      assert_equal tag.name, old_name
+    end
   end
 
   private