AO3-5556 Don't change collections on preview - Follow up PR

app/controllers/bookmarks_controller.rb

@@ -208,56 +208,9 @@ def create
   # PUT /bookmarks/1
   # PUT /bookmarks/1.xml
   def update
-    new_collections = []
-    unapproved_collections = []
-    errors = []
-    bookmark_params[:collection_names]&.split(",")&.map(&:strip)&.uniq&.each do |collection_name|
-      collection = Collection.find_by(name: collection_name)
-      if collection.nil?
-        errors << ts("%{name} does not exist.", name: collection_name)
-      else
-        if @bookmark.collections.include?(collection)
-          next
-        elsif collection.closed? && !collection.user_is_maintainer?(User.current_user)
-          errors << ts("%{title} is closed to new submissions.", title: collection.title)
-        elsif @bookmark.add_to_collection(collection) && @bookmark.save
-          if @bookmark.approved_collections.include?(collection)
-            new_collections << collection
-          else
-            unapproved_collections << collection
-          end
-        else
-          errors << ts("Something went wrong trying to add collection %{title}, sorry!", title: collection.title)
-        end
-      end
-    end
-
-    # messages to the user
-    unless errors.empty?
-      flash[:error] = ts("We couldn't add your submission to the following collections: ") + errors.join("<br />")
-    end
-
-    unless new_collections.empty?
-      flash[:notice] = ts("Added to collection(s): %{collections}.",
-                          collections: new_collections.collect(&:title).join(", "))
-    end
-    unless unapproved_collections.empty?
-      flash[:notice] = flash[:notice] ? flash[:notice] + " " : ""
-      flash[:notice] += if unapproved_collections.size > 1
-                          ts("You have submitted your bookmark to moderated collections (%{all_collections}). It will not become a part of those collections until it has been approved by a moderator.", all_collections: unapproved_collections.map(&:title).join(", "))
-                        else
-                          ts("You have submitted your bookmark to the moderated collection '%{collection}'. It will not become a part of the collection until it has been approved by a moderator.", collection: unapproved_collections.first.title)
-                        end
-    end
-
-    flash[:notice] = (flash[:notice]).html_safe unless flash[:notice].blank?
-    flash[:error] = (flash[:error]).html_safe unless flash[:error].blank?
-
-    if @bookmark.update(bookmark_params) && errors.empty?
-      flash[:notice] = flash[:notice] ? " " + flash[:notice] : ""
-      flash[:notice] = ts("Bookmark was successfully updated.").html_safe + flash[:notice]
-      flash[:notice] += t("bookmarks.create.warnings.private_bookmark_added_to_collection") if new_collections.any? || unapproved_collections.any?
-      flash[:notice] = flash[:notice].html_safe
+    if @bookmark.update(bookmark_params)
+      flash[:notice] = t("bookmarks.update.bookmark_updated")
+      new_moderated_collections_message
       redirect_to(@bookmark)
     else
       @bookmarkable = @bookmark.bookmarkable
@@ -351,6 +304,27 @@ def set_own_bookmarks
     end
   end
 
+  # Flash a message about the new moderated collections that the user has
+  # submitted their bookmark to.
+  def new_moderated_collections_message
+    new_moderated_collections = @bookmark.collection_items
+      .select(&:previously_new_record?)
+      .select(&:unreviewed_by_collection?)
+      .map(&:collection)
+
+    return if new_moderated_collections.blank?
+
+    links = new_moderated_collections.map do |collection|
+      view_context.link_to(collection.title, collection_path(collection))
+    end
+
+    message = t("bookmarks.new_moderated_collections_message_html",
+                count: new_moderated_collections.length,
+                collections: view_context.safe_join(links, ", "))
+
+    flash[:notice] = view_context.safe_join([flash[:notice], message].compact, " ")
+  end
+
   private
 
   def bookmark_params

app/controllers/collection_items_controller.rb

@@ -107,8 +107,8 @@ def create
         errors << ts("%{collection_title}, either you don't own this item or are not a moderator of the collection.", collection_title: collection.title)
       elsif @item.is_a?(Work) && @item.anonymous? && !current_user.is_author_of?(@item)
         errors << ts("%{collection_title}, because you don't own this item and the item is anonymous.", collection_title: collection.title)
-      # add the work to a collection, and try to save it
-      elsif @item.add_to_collection(collection) && @item.save(validate: false)
+      # add the work to a collection
+      elsif @item.add_to_collection(collection)
         # approved_by_user? and approved_by_collection? are both true.
         # This is will be true for archivists adding works to collections they maintain
         # or creators adding their works to a collection with auto-approval.

app/controllers/works_controller.rb

@@ -307,11 +307,11 @@ def create
       if @work.posted
         # We check here to see if we are attempting to post to moderated collection
         flash[:notice] = t(".posted_notice")
-        in_moderated_collection
+        show_moderated_collection_warning
         redirect_to work_path(@work)
       else
         flash[:notice] = t(".draft_notice_html", scheduled_for_deletion_bold: helpers.tag.strong(t(".scheduled_for_deletion")), deletion_date: view_context.date_in_zone(@work.created_at + 29.days))
-        in_moderated_collection
+        show_moderated_collection_warning
         redirect_to preview_work_path(@work)
       end
     end
@@ -362,7 +362,7 @@ def update
     elsif params[:preview_button]
       flash[:notice] = t(".unposted_notice") unless @work.posted?
 
-      in_moderated_collection
+      show_moderated_collection_warning(flash.now)
       @preview_mode = true
       render :preview
     else
@@ -375,7 +375,7 @@ def update
         if posted_changed
           flash[:notice] << ts(" It should appear in work listings within the next few minutes.")
         end
-        in_moderated_collection
+        show_moderated_collection_warning
         redirect_to work_path(@work)
       else
         @chapter.errors.full_messages.each { |err| @work.errors.add(:base, err) }
@@ -566,22 +566,23 @@ def send_external_invites(works)
   end
 
   # check to see if the work is being added / has been added to a moderated collection, then let user know that
-  def in_moderated_collection
-    moderated_collections = []
-    @work.collections.each do |collection|
-      next unless !collection.nil? && collection.moderated? && !collection.user_is_posting_participant?(current_user)
-      next unless @work.collection_items.present?
-      @work.collection_items.each do |collection_item|
-        next unless collection_item.collection == collection
-        if collection_item.approved_by_user? && collection_item.unreviewed_by_collection?
-          moderated_collections << collection
-        end
-      end
-    end
-    if moderated_collections.present?
-      flash[:notice] ||= ''
-      flash[:notice] += ts(" You have submitted your work to #{moderated_collections.size > 1 ? 'moderated collections (%{all_collections}). It will not become a part of those collections' : "the moderated collection '%{all_collections}'. It will not become a part of the collection"} until it has been approved by a moderator.", all_collections: moderated_collections.map(&:title).join(', '))
+  def show_moderated_collection_warning(flash = self.flash)
+    moderated_collections = @work.collection_items_after_saving
+      .select(&:approved_by_user?)
+      .select(&:unreviewed_by_collection?)
+      .map(&:collection)
+
+    return if moderated_collections.blank?
+
+    links = moderated_collections.map do |collection|
+      view_context.link_to(collection.title, collection_path(collection))
     end
+
+    message = t("works.moderated_collections_message_html",
+                count: moderated_collections.length,
+                collections: view_context.safe_join(links, ", "))
+
+    flash[:notice] = view_context.safe_join([flash[:notice], message].compact, " ")
   end
 
   public
@@ -786,7 +787,7 @@ def set_work_form_fields
     @serial_works = @work.serial_works
 
     if @collection.nil?
-      @collection = @work.approved_collections.first
+      @collection = @work.approved_collections_after_saving.first
     end
 
     if params[:claim_id]

app/helpers/works_helper.rb

@@ -30,6 +30,14 @@ def work_meta_list(work, chapter = nil)
     content_tag(:dl, list.to_s, class: 'stats').html_safe
   end
 
+  def collections_meta_tag(work)
+    collections_data = show_collections_data(work.approved_collections_after_saving)
+    return if collections_data.blank?
+
+    content_tag(:dt, "#{t('works_helper.collections_meta_tag.collections')} ", class: "collections") +
+      content_tag(:dd, collections_data, class: "collections")
+  end
+
   def work_page_title(work, title, options = {})
     fandoms = work.fandoms
     title_fandom = if fandoms.empty?

app/models/application_record.rb

@@ -15,4 +15,8 @@ def update_sanitizer_version
   def self.random_order
     order(Arel.sql("RAND()"))
   end
+
+  def unmark_for_destruction
+    @marked_for_destruction = false
+  end
 end

app/models/collection.rb

@@ -18,10 +18,10 @@ class Collection < ApplicationRecord
   has_many :children, class_name: "Collection", foreign_key: "parent_id", inverse_of: :parent
 
   has_one :collection_profile, dependent: :destroy
-  accepts_nested_attributes_for :collection_profile
+  accepts_nested_attributes_for :collection_profile, update_only: true
 
   has_one :collection_preference, dependent: :destroy
-  accepts_nested_attributes_for :collection_preference
+  accepts_nested_attributes_for :collection_preference, update_only: true
 
   before_validation :clear_icon
   before_validation :cleanup_url
@@ -297,13 +297,11 @@ def gift_notification
     self.collection_profile.gift_notification || (parent ? parent.collection_profile.gift_notification : "")
   end
 
-  def moderated?() = self.collection_preference.moderated
-
-  def closed?() = self.collection_preference.closed
-
-  def unrevealed?() = self.collection_preference.unrevealed
-
-  def anonymous?() = self.collection_preference.anonymous
+  delegate :moderated?,
+           :closed?,
+           :unrevealed?,
+           :anonymous?,
+           to: :collection_preference, allow_nil: true
 
   def challenge?() = !self.challenge.nil?