Merge pull request #1852 from codidact/0valt/tags

Improve tag rename error handling & messages

Author: Oaphi

app/assets/javascripts/qpixel_api.js

@@ -430,19 +430,34 @@ window.QPixel = {
   },
 
   handleJSONResponse: (data, onSuccess, onFinally) => {
-    const is_modified = data.status === 'modified';
-    const is_success = data.status === 'success';
+    const isFailed = data.status === 'failed';
 
-    if (is_modified || is_success) {
-      onSuccess(/** @type {Parameters<typeof onSuccess>[0]} */(data));
+    if(isFailed) {
+      const { errors = [], message } = data;
+
+      if (message) {
+        const fullMessage =
+          errors.length > 1
+            ? `${message}:<ul>${errors.map((e) => `<li>${e.trim()}</li>`).join('')}</ul>`
+            : errors.length === 1
+              ? `${message} (${errors[0].toLowerCase().trim()})`
+              : message;
+
+          QPixel.createNotification('danger', fullMessage);
+      }
+      else {
+        for (const error of errors) {
+          QPixel.createNotification('danger', error);
+        }
+      }
     }
     else {
-      QPixel.createNotification('danger', data.message);
+      onSuccess(/** @type {Parameters<typeof onSuccess>[0]} */(data));
     }
 
     onFinally?.(data);
 
-    return is_success;
+    return !isFailed;
   },
 
   flag: async (flag) => {

app/assets/javascripts/tags.js

@@ -1,13 +1,4 @@
-/**
- * @typedef {{
- *  id: number | string
- *  text: string
- *  desc: string
- *  synonyms?: string | QPixelTagSynonym[]
- * }} ProcessedTag
- */
-
-$(() => {
+document.addEventListener('DOMContentLoaded', () => {
   const sum = (/** @type {number[]} */ ary) => ary.reduce((a, b) => a + b, 0);
 
   /**
@@ -35,11 +26,11 @@ $(() => {
     const tagSynonyms = !!tag.synonyms ? ` <i>(${tag.synonyms})</i>` : '';
     const tagSpan = `<span>${tag.text}${tagSynonyms}</span>`;
     let desc = !!tag.desc ? splitWordsMaxLength(tag.desc, 120) : '';
-    const descSpan = !!tag.desc ?
-      `<br/><span class="has-color-tertiary-900 has-font-size-caption">${desc[0]}${desc.length > 1 ? '...' : ''}</span>` :
-      '';
+    const descSpan = !!tag.desc
+      ? `<br/><span class="has-color-tertiary-900 has-font-size-caption">${desc[0]}${desc.length > 1 ? '...' : ''}</span>`
+      : '';
     return $(tagSpan + descSpan);
-  }
+  };
 
   $('.js-tag-select').each((_i, el) => {
     const $tgt = $(el);
@@ -49,10 +40,10 @@ $(() => {
       tags: $tgt.attr('data-create') !== 'false',
       /**
        * @param {Select2.IdTextPair[]} data
-       * @param {Select2.IdTextPair & { desc?: string }} tag 
+       * @param {Select2.IdTextPair & { desc?: string }} tag
        */
       insertTag: function (data, tag) {
-        tag.desc = "(Create new tag)"
+        tag.desc = '(Create new tag)';
         // Insert the tag at the end of the results
         data.push(tag);
       },
@@ -66,7 +57,7 @@ $(() => {
           }
           return Object.assign(params, { tag_set: $this.data('tag-set') });
         },
-        headers: { 'Accept': 'application/json' },
+        headers: { Accept: 'application/json' },
         delay: 100,
         /**
          * @param {QPixelTag[]} data
@@ -80,23 +71,23 @@ $(() => {
                 { id: 1, text: 'hot-red-firebreather', desc: 'Very cute dragon' },
                 { id: 2, text: 'training', desc: 'How to train a dragon' },
                 { id: 3, text: 'behavior', desc: 'How a dragon behaves' },
-                { id: 4, text: 'sapphire-blue-waterspouter', desc: 'Other cute dragon' }
-              ]
-            }
+                { id: 4, text: 'sapphire-blue-waterspouter', desc: 'Other cute dragon' },
+              ],
+            };
           }
           return {
             results: data.map((t) => ({
               id: useIds ? t.id : t.name,
               text: t.name.replace(/</g, '&#x3C;').replace(/>/g, '&#x3E;'),
               synonyms: processSynonyms($this, t.tag_synonyms),
-              desc: t.excerpt
-            }))
+              desc: t.excerpt,
+            })),
           };
         },
       },
       placeholder: '',
       templateResult: template,
-      allowClear: true
+      allowClear: true,
     });
   });
 
@@ -112,10 +103,13 @@ $(() => {
     if (synonyms.length > 3) {
       const searchValue = $search.data('select2').selection.$search.val().toLowerCase();
       displayedSynonyms = synonyms.filter((ts) => ts.name.includes(searchValue)).slice(0, 3);
-    } else {
+    }
+    else {
       displayedSynonyms = synonyms;
     }
-    let synonymsString = displayedSynonyms.map((ts) => `${ts.name.replace(/</g, '&#x3C;').replace(/>/g, '&#x3E;')}`).join(', ');
+    let synonymsString = displayedSynonyms
+      .map((ts) => `${ts.name.replace(/</g, '&#x3C;').replace(/>/g, '&#x3E;')}`)
+      .join(', ');
     if (synonyms.length > displayedSynonyms.length) {
       synonymsString += `, ${synonyms.length - displayedSynonyms.length} more synonyms`;
     }
@@ -128,10 +122,10 @@ $(() => {
     const newId = parseInt(lastId, 10) + 1;
 
     //Duplicate the first element at the end of the wrapper
-    const newField = $wrapper.find('.tag-synonym[data-id="0"]')[0]
-                             .outerHTML
-                             .replace(/data-id="0"/g, 'data-id="' + newId + '"')
-                             .replace(/(?<connector>attributes(\]\[)|(_))0/g, '$<connector>' + newId)
+    const newField = $wrapper
+      .find('.tag-synonym[data-id="0"]')[0]
+      .outerHTML.replace(/data-id="0"/g, 'data-id="' + newId + '"')
+      .replace(/(?<connector>attributes(\]\[)|(_))0/g, '$<connector>' + newId);
     $wrapper.append(newField);
 
     //Alter the newly added tag synonym
@@ -141,10 +135,10 @@ $(() => {
     $newTagSynonym.show();
 
     //Add handler for removing an element
-    $newTagSynonym.find(`.remove-tag-synonym`).click(removeTagSynonym);
+    $newTagSynonym.find(`.remove-tag-synonym`).on('click', removeTagSynonym);
   });
 
-  $('.remove-tag-synonym').click(removeTagSynonym);
+  $('.remove-tag-synonym').on('click', removeTagSynonym);
 
   function removeTagSynonym() {
     const synonym = $(this).closest('.tag-synonym');
@@ -174,7 +168,7 @@ $(() => {
     const tagId = $tgt.attr('data-tag');
     const tagName = $tgt.attr('data-name');
 
-    const renameTo = prompt(`Rename tag ${tagName} to:`);
+    const renameTo = prompt(`Rename tag "${tagName}" to:`);
 
     if (!renameTo) {
       return;

app/controllers/tags_controller.rb

@@ -94,10 +94,8 @@ def new
   def create
     @tag = Tag.new(tag_params.merge(tag_set_id: @category.tag_set.id))
     if @tag.save
-      flash[:danger] = nil
       redirect_to tag_path(id: @category.id, tag_id: @tag.id)
     else
-      flash[:danger] = @tag.errors.full_messages.join(', ')
       render :new, status: :bad_request
     end
   end
@@ -156,6 +154,7 @@ def rename
     else
       render json: { status: 'failed',
                      message: I18n.t('tags.errors.rename_generic'),
+                     errors: @tag.errors.full_messages,
                      tag: @tag },
              status: :bad_request
     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.validation.errors.no_blank_name')
+  }
+  validates :name, format: {
+    with: /\A[^\s]+\Z/, # https://regex101.com/r/7BxgIn/1
+    message: I18n.t('tags.validation.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.validation.errors.no_duplicate_names')
+  }
 
   # scopes
   scope :list_includes, -> { includes(:tag_synonyms) }

config/locales/strings/en.tags.yml

@@ -1,5 +1,13 @@
 en:
   tags:
+    validation:
+      errors:
+        no_blank_name: >
+          should contain at least 1 character
+        no_duplicate_names: >
+          cannot match an already existing tag
+        no_spaces_in_name: >
+          should not contain spaces
     errors:
       rename_generic: >
-        Failed to rename the tag.
+        Failed to rename the tag

global.d.ts

@@ -12,6 +12,13 @@ interface PostValidatorMessage {
 
 type PostValidator = (postText: string) => [boolean, PostValidatorMessage[]];
 
+ interface ProcessedTag {
+  id: number | string
+  text: string
+  desc: string
+  synonyms?: string | QPixelTagSynonym[]
+ }
+
 interface UserPreferences {
   community: Record<string, string | null>;
   global: Record<string, string | null>;