Merge pull request #1136 from eip-ewi/301-edit-suggestions-with-too-m…

…any-tags

app/controllers/posts_controller.rb

@@ -232,7 +232,7 @@ def update
           do_draft_delete(URI(request.referer || '').path)
           redirect_to post_path(@post)
         else
-          @post.errors = edit.errors
+          @post.errors.copy!(edit.errors)
           render :edit, status: :bad_request
         end
       end

app/models/concerns/post_validations.rb

@@ -0,0 +1,77 @@
+# Validations for posts which are shared between posts and suggested edits.
+module PostValidations
+  extend ActiveSupport::Concern
+
+  included do
+    validate :tags_in_tag_set, if: -> { post_type.has_tags }
+    validate :maximum_tags, if: -> { post_type.has_tags }
+    validate :maximum_tag_length, if: -> { post_type.has_tags }
+    validate :no_spaces_in_tags, if: -> { post_type.has_tags }
+    validate :stripped_minimum_body, if: -> { !body_markdown.nil? }
+    validate :stripped_minimum_title, if: -> { !title.nil? }
+    validate :maximum_title_length, if: -> { !title.nil? }
+    validate :required_tags?, if: -> { post_type.has_tags && post_type.has_category }
+  end
+
+  def maximum_tags
+    if tags_cache.length > 5
+      errors.add(:base, "Post can't have more than 5 tags.")
+    elsif tags_cache.empty?
+      errors.add(:base, 'Post must have at least one tag.')
+    end
+  end
+
+  def maximum_tag_length
+    tags_cache.each do |tag|
+      max_len = SiteSetting['MaxTagLength']
+      if tag.length > max_len
+        errors.add(:tags, "can't be more than #{max_len} characters long each")
+      end
+    end
+  end
+
+  def no_spaces_in_tags
+    tags_cache.each do |tag|
+      if tag.include?(' ') || tag.include?('_')
+        errors.add(:tags, 'may not include spaces or underscores - use hyphens for multiple-word tags')
+      end
+    end
+  end
+
+  def stripped_minimum_body
+    min_body = category.nil? ? 30 : category.min_body_length
+    if (body_markdown&.gsub(/(?:^[\s\t\u2000-\u200F]+|[\s\t\u2000-\u200F]+$)/, '')&.length || 0) < min_body
+      errors.add(:body, 'must be more than 30 non-whitespace characters long')
+    end
+  end
+
+  def stripped_minimum_title
+    min_title = category.nil? ? 15 : category.min_title_length
+    if (title&.gsub(/(?:^[\s\t\u2000-\u200F]+|[\s\t\u2000-\u200F]+$)/, '')&.length || 0) < min_title
+      errors.add(:title, 'must be more than 15 non-whitespace characters long')
+    end
+  end
+
+  def maximum_title_length
+    max_title_len = SiteSetting['MaxTitleLength']
+    if title.length > [(max_title_len || 255), 255].min
+      errors.add(:title, "can't be more than #{max_title_len} characters")
+    end
+  end
+
+  def tags_in_tag_set
+    tag_set = category.tag_set
+    unless tags.all? { |t| t.tag_set_id == tag_set.id }
+      errors.add(:base, "Not all of this question's tags are in the correct tag set.")
+    end
+  end
+
+  def required_tags?
+    required = category&.required_tag_ids
+    return unless required.present? && !required.empty?
+
+    unless tag_ids.any? { |t| required.include? t }
+      errors.add(:tags, "must contain at least one required tag (#{category.required_tags.pluck(:name).join(', ')})")
+    end
+  end
+end

app/models/post.rb

@@ -1,5 +1,6 @@
 class Post < ApplicationRecord
   include CommunityRelated
+  include PostValidations
 
   belongs_to :user, optional: true
   belongs_to :post_type
@@ -29,17 +30,14 @@ class Post < ApplicationRecord
 
   validates :body, presence: true, length: { minimum: 30, maximum: 30_000 }
   validates :doc_slug, uniqueness: { scope: [:community_id], case_sensitive: false }, if: -> { doc_slug.present? }
-  validates :title, :body, :tags_cache, presence: true, if: -> { post_type.has_tags }
-  validate :tags_in_tag_set, if: -> { post_type.has_tags }
-  validate :maximum_tags, if: -> { post_type.has_tags }
-  validate :maximum_tag_length, if: -> { post_type.has_tags }
-  validate :no_spaces_in_tags, if: -> { post_type.has_tags }
-  validate :stripped_minimum, if: -> { post_type.has_tags }
-  validate :maximum_title_length, if: -> { post_type.has_tags }
+  validates :title, presence: true
+  validates :tags_cache, presence: true, if: -> { post_type.has_tags }
+
   validate :category_allows_post_type, if: -> { category_id.present? }
   validate :license_valid, if: -> { post_type.has_license }
-  validate :required_tags?, if: -> { post_type.has_tags && post_type.has_category }
-  validate :moderator_tags, if: -> { post_type.has_tags && post_type.has_category }
+  validate :moderator_tags, if: -> { post_type.has_tags && post_type.has_category && tags_cache_changed? }
+
+  # Other validations (shared with suggested edits) are in concerns/PostValidations
 
   scope :undeleted, -> { where(deleted: false) }
   scope :deleted, -> { where(deleted: true) }
@@ -263,65 +261,6 @@ def license_valid
     end
   end
 
-  def maximum_tags
-    if tags_cache.length > 5
-      errors.add(:base, "Post can't have more than 5 tags.")
-    elsif tags_cache.empty?
-      errors.add(:base, 'Post must have at least one tag.')
-    end
-  end
-
-  def maximum_tag_length
-    tags_cache.each do |tag|
-      max_len = SiteSetting['MaxTagLength']
-      if tag.length > max_len
-        errors.add(:tags, "can't be more than #{max_len} characters long each")
-      end
-    end
-  end
-
-  def no_spaces_in_tags
-    tags_cache.each do |tag|
-      if tag.include?(' ') || tag.include?('_')
-        errors.add(:tags, 'may not include spaces or underscores - use hyphens for multiple-word tags')
-      end
-    end
-  end
-
-  def stripped_minimum
-    min_title = category.nil? ? 15 : category.min_title_length
-    min_body = category.nil? ? 30 : category.min_body_length
-    if (body&.gsub(/(?:^[\s\t\u2000-\u200F]+|[\s\t\u2000-\u200F]+$)/, '')&.length || 0) < min_body
-      errors.add(:body, 'must be more than 30 non-whitespace characters long')
-    end
-    if (title&.gsub(/(?:^[\s\t\u2000-\u200F]+|[\s\t\u2000-\u200F]+$)/, '')&.length || 0) < min_title
-      errors.add(:title, 'must be more than 15 non-whitespace characters long')
-    end
-  end
-
-  def maximum_title_length
-    max_title_len = SiteSetting['MaxTitleLength']
-    if title.length > [(max_title_len || 255), 255].min
-      errors.add(:title, "can't be more than #{max_title_len} characters")
-    end
-  end
-
-  def tags_in_tag_set
-    tag_set = category.tag_set
-    unless tags.all? { |t| t.tag_set_id == tag_set.id }
-      errors.add(:base, "Not all of this question's tags are in the correct tag set.")
-    end
-  end
-
-  def required_tags?
-    required = category&.required_tag_ids
-    return true unless required.present? && !required.empty?
-
-    unless tag_ids.any? { |t| required.include? t }
-      errors.add(:tags, "must contain at least one required tag (#{category.required_tags.pluck(:name).join(', ')})")
-    end
-  end
-
   def moderator_tags
     mod_tags = category&.moderator_tags&.map(&:name)
     return unless mod_tags.present? && !mod_tags.empty?

app/models/suggested_edit.rb

@@ -1,5 +1,6 @@
 class SuggestedEdit < ApplicationRecord
   include PostRelated
+  include PostValidations
 
   belongs_to :user
 
@@ -10,6 +11,9 @@ class SuggestedEdit < ApplicationRecord
   has_and_belongs_to_many :tags
   has_and_belongs_to_many :before_tags, class_name: 'Tag', join_table: 'suggested_edits_before_tags'
 
+  has_one :post_type, through: :post
+  has_one :category, through: :post
+
   after_save :clear_pending_cache, if: proc { saved_change_to_attribute?(:active) }
 
   def clear_pending_cache