AO3-4820 AO3-3810 Validate skins with "archive" in title (otwcode#4771)

* Validate skins with "archive" in title

* AO3-4820-AO3-3810 Fix

* Normalize

* AO3-4820-AO3-3810 Remove support link

app/controllers/skins_controller.rb

@@ -1,7 +1,6 @@
 class SkinsController < ApplicationController
   before_action :users_only, only: [:new, :create, :destroy]
   before_action :load_skin, except: [:index, :new, :create, :unset]
-  before_action :check_title, only: [:create, :update]
   before_action :check_ownership_or_admin, only: [:edit, :update]
   before_action :check_ownership, only: [:confirm_delete, :destroy]
   before_action :check_visibility, only: [:show]
@@ -198,13 +197,6 @@ def check_editability
     end
   end
 
-  def check_title
-    if params[:skin][:title].match(/archive/i)
-      flash[:error] = ts("You can't use the word 'archive' in your skin title, sorry! (We have to reserve it for official skins.)")
-      render @skin ? :edit : :new
-    end
-  end
-
   # if we've been asked to load the archive parents, we do so and add them to params
   def load_archive_parents
     if params[:add_site_parents]

app/models/skin.rb

@@ -94,7 +94,11 @@ def clean_media
   validate :valid_media
   def valid_media
     if media && media.is_a?(Array) && media.any? {|m| !MEDIA.include?(m)}
-      errors.add(:base, ts("We don't currently support the media type %{media}, sorry! If we should, please let Support know.", media: media.join(', ')))
+      errors.add(
+        :base,
+        :invalid_media,
+        media: media.join(", ")
+      )
     end
   end
 
@@ -104,12 +108,24 @@ def valid_media
   validate :valid_public_preview
   def valid_public_preview
     return true if self.official? || !self.public? || self.icon.attached?
-
-    errors.add(:base, ts("You need to upload a screencap if you want to share your skin."))
+    errors.add(:base, :no_public_preview)
   end
 
-  validates_presence_of :title
-  validates :title, uniqueness: { message: ts("must be unique"), case_sensitive: true }
+  validates :title, presence: true, uniqueness: { case_sensitive: true }
+  validate :allowed_title
+  def allowed_title
+    return true unless self.title.match(/archive/i)
+
+    authorized_roles = if self.is_a?(WorkSkin)
+                         %w[superadmin support]
+                       else
+                         %w[superadmin]
+                       end
+
+    return true if (User.current_user.roles & authorized_roles).present?
+
+    errors.add(:base, :archive_in_title)
+  end
 
   validates_numericality_of :margin, :base_em, allow_nil: true
   validate :valid_font
@@ -479,7 +495,7 @@ def self.load_site_css
           skin.unusable = true
           skin.official = true
           skin.icon.attach(io: File.open("#{version_dir}preview.png", "rb"), content_type: "image/png", filename: "preview.png")
-          skin.save!
+          skin.save!(validate: false)
           skins << skin
         end
 
@@ -494,7 +510,7 @@ def self.load_site_css
         end
         top_skin.icon.attach(io: File.open("#{version_dir}preview.png", "rb"), content_type: "image/png", filename: "preview.png")
         top_skin.official = true
-        top_skin.save!
+        top_skin.save!(validate: false)
         skins.each_with_index do |skin, index|
           skin_parent = top_skin.skin_parents.build(child_skin: top_skin, parent_skin: skin, position: index+1)
           skin_parent.save!

config/locales/models/en.yml

@@ -136,6 +136,13 @@ en:
               not_work: Only a link to a work can be listed as an inspiration.
               protected: You can't use the related works function to cite works by the protected user %{login}.
           format: "%{message}"
+        skin:
+          archive_in_title: Sorry, titles including the word 'Archive' are reserved for official skins.
+          attributes:
+            title:
+              taken: must be unique
+          invalid_media: We don't currently support the media type %{media}, sorry! If we should, please let Support know.
+          no_public_preview: You need to upload a screencap if you want to share your skin.
         skin/skin_parents:
           attributes:
             base:

db/migrate/20240416174046_add_unique_index_to_skins_title.rb

@@ -0,0 +1,8 @@
+class AddUniqueIndexToSkinsTitle < ActiveRecord::Migration[6.1]
+  uses_departure! if Rails.env.staging? || Rails.env.production?
+
+  def change
+    remove_index :skins, :title
+    add_index :skins, :title, unique: true
+  end
+end

features/admins/admin_skins.feature

@@ -137,3 +137,15 @@ Feature: Admin manage skins
   # A user created before changing the default skin will still have the same skin
   When I am logged in as "KnownUser"
   Then I should not see "{ text-decoration: blink; }" in the page style
+
+  Scenario: Admin can edit a skin with the word "archive" in the title
+  Given the approved public skin "official archive skin" has reserved words in the title
+    And I am logged in as a "superadmin" admin
+  When I go to "official archive skin" skin page
+    And I follow "Edit"
+    And I fill in "CSS" with "#greeting.logged-in { text-decoration: blink;}"
+    And I fill in "Description" with "all your skin are belong to us"
+    And I submit
+  Then I should see an update confirmation message
+    And I should see "all your skin are belong to us"
+    And I should see "#greeting.logged-in"

features/other_b/skin.feature

@@ -182,18 +182,14 @@ Feature: Non-public site and work skins
   Scenario: The cache should be flushed with a parent and not when unrelated
   Given I have loaded site skins
     And I am logged in as "skinner"
-  When I set up the skin "Complex"
-    And I select "replace archive skin entirely" from "What it does:"
-    And I check "Load Archive Skin Components"
-    And I submit
-  Then I should see a create confirmation message
+    And I have a skin "Child" with a parent "Parent"
    When I am on the new skin page
-    And I fill in "Title" with "my blinking skin"
+    And I fill in "Title" with "Unrelated"
     And I fill in "CSS" with "#title { text-decoration: blink;}"
     And I submit
   Then I should see "Skin was successfully created"
-    And the cache of the skin on "my blinking skin" should not expire after I save "Complex"
-    And the cache of the skin on "Complex" should expire after I save a parent skin
+    And the cache of the skin on "Unrelated" should not expire after I save "Child"
+    And the cache of the skin on "Child" should expire after I save a parent skin
 
   Scenario: Users should be able to create skins using @media queries
   Given I am logged in as "skinner"
@@ -231,9 +227,8 @@ Feature: Non-public site and work skins
   Scenario: A user can't make a skin with "Archive" in the title
   Given I am logged in as "skinner"
     And I set up the skin "My Archive Skin" with some css
-  When "AO3-4820" is fixed
-    # And I press "Submit"
-  # Then I should see "You can't use the word 'archive' in your skin title, sorry! (We have to reserve it for official skins.)"
+    And I press "Submit"
+  Then I should see "Sorry, titles including the word 'Archive' are reserved for official skins."
 
   Scenario: A user can't look at another user's skins
   Given the user "scully" exists and is activated

features/step_definitions/skin_steps.rb

@@ -80,17 +80,24 @@
   Skin.load_site_css
 end
 
-Given /^the approved public skin "([^"]*)" with css "([^"]*)"$/ do |skin_name, css|
-  step "the unapproved public skin \"#{skin_name}\" with css \"#{css}\""
-  step "I am logged in as an admin"
-  step "I approve the skin \"#{skin_name}\""
+Given "the approved public skin {string} with css {string}" do |skin_name, css|
+  step %{the unapproved public skin "#{skin_name}" with css "#{css}"}
+  step %{I approve the skin "#{skin_name}"}
   step "I am logged out"
 end
 
 Given /^the approved public skin "([^"]*)"$/ do |skin_name|
   step "the approved public skin \"#{skin_name}\" with css #{DEFAULT_CSS}"
 end
 
+Given "the approved public skin {string} has reserved words in the title" do |skin_name|
+  # Admins can't create skins, so we have to create it this way.
+  FactoryBot.build(:skin, title: skin_name, public: true).save!(validate: false)
+
+  step %{I approve the skin "#{skin_name}"}
+  step "I am logged out"
+end
+
 Given /^"([^"]*)" is using the approved public skin "([^"]*)" with css "([^"]*)"$/ do |login, skin_name, css|
   step "the approved public skin \"public skin\" with css \"#{css}\""
   step "I am logged in as \"#{login}\""