Merge branch 'master' of https://github.com/smclairecarden/otwarchive …

…into AO3-6608_deleteAltText

.github/workflows/reviewdog.yml

@@ -11,6 +11,8 @@ jobs:
   rubocop:
     name: Rubocop
     runs-on: ubuntu-latest
+    env:
+      BUNDLE_ONLY: linters
     steps:
       - name: Check out code
         uses: actions/checkout@v4
@@ -21,7 +23,7 @@ jobs:
           bundler-cache: true
 
       - name: rubocop
-        uses: reviewdog/action-rubocop@2f726ae5dd8df72b4faa9d93669cdab96aeb2153
+        uses: reviewdog/action-rubocop@2c8048e3169487eccc1eed812daaa6e5275a809f
         with:
           use_bundler: true
           reporter: github-pr-check
@@ -30,6 +32,8 @@ jobs:
   erb-lint:
     name: ERB Lint runner
     runs-on: ubuntu-latest
+    env:
+      BUNDLE_ONLY: linters
     steps:
       - name: Check out code
         uses: actions/checkout@v4

app/controllers/series_controller.rb

@@ -43,7 +43,7 @@ def index
   # GET /series/1
   # GET /series/1.xml
   def show
-    @works = @series.works_in_order.posted.select(&:visible?)
+    @works = @series.works_in_order.posted.select(&:visible?).paginate(page: params[:page])
 
     # sets the page title with the data for the series
     @page_title = @series.unrevealed? ? ts("Mystery Series") : get_page_title(@series.allfandoms.collect(&:name).join(', '), @series.anonymous? ? ts("Anonymous") : @series.allpseuds.collect(&:byline).join(', '), @series.title)

app/helpers/validation_helper.rb

@@ -42,9 +42,11 @@ def error_messages_for(object)
   end
 
   def error_messages_formatted(errors, intro = "")
-    return unless errors && !errors.empty?
-    error_messages = errors.map { |msg| content_tag(:li, msg.gsub(/^(.*)\^/, '').html_safe) }.join("\n").html_safe
-    content_tag(:div, intro.html_safe + content_tag(:ul, error_messages), id:"error", class:"error")    
+    return unless errors.present?
+
+    error_messages = errors.map { |msg| content_tag(:li, msg.gsub(/^(.*?)\^/, "").html_safe) }
+      .join("\n").html_safe
+    content_tag(:div, intro.html_safe + content_tag(:ul, error_messages), id: "error", class: "error")
   end
 
   # use to make sure we have consistent name throughout

app/models/comment.rb

@@ -495,7 +495,7 @@ def mark_unhidden!
   end
 
   def sanitized_content
-    sanitize_field self, :comment_content
+    sanitize_field(self, :comment_content, strip_images: ultimate_parent.is_a?(AdminPost))
   end
   include Responder
 end

app/models/feedback_reporters/abuse_reporter.rb

@@ -30,6 +30,8 @@ def subject
   end
 
   def ticket_description
-    description.present? ? description.html_safe : "No comment submitted."
+    return "No comment submitted." if description.blank?
+
+    strip_images(description.html_safe)
   end
 end

app/models/feedback_reporters/support_reporter.rb

@@ -31,6 +31,8 @@ def subject
   end
 
   def ticket_description
-    description.present? ? description.html_safe : "No description submitted."
+    return "No description submitted." if description.blank?
+
+    strip_images(description.html_safe)
   end
 end

app/models/work.rb

@@ -183,10 +183,34 @@ def new_recipients_allow_gifts
     self.new_gifts.each do |gift|
       next if gift.pseud.blank?
       next if gift.pseud&.user&.preference&.allow_gifts?
-      next if self.challenge_assignments.map(&:requesting_pseud).include?(gift.pseud)
-      next if self.challenge_claims.reject { |c| c.request_prompt.anonymous? }.map(&:requesting_pseud).include?(gift.pseud)
+      next if challenge_bypass(gift)
 
-      self.errors.add(:base, ts("%{byline} does not accept gifts.", byline: gift.pseud.byline))
+      self.errors.add(:base, :blocked_gifts, byline: gift.pseud.byline)
+    end
+  end
+
+  validate :new_recipients_have_not_blocked_gift_giver
+  def new_recipients_have_not_blocked_gift_giver
+    return if self.new_gifts.blank?
+
+    self.new_gifts.each do |gift|
+      # Already dealt with in #new_recipients_allow_gifts
+      next if gift.pseud&.user&.preference && !gift.pseud.user.preference.allow_gifts?
+
+      next if challenge_bypass(gift)
+
+      blocked_users = gift.pseud&.user&.blocked_users || []
+      next if blocked_users.empty?
+
+      pseuds_after_saving.each do |pseud|
+        next unless blocked_users.include?(pseud.user)
+
+        if User.current_user == pseud.user
+          self.errors.add(:base, :blocked_your_gifts, byline: gift.pseud.byline)
+        else
+          self.errors.add(:base, :blocked_gifts, byline: gift.pseud.byline)
+        end
+      end
     end
   end
 
@@ -1258,4 +1282,14 @@ def nonfiction
   def allow_collection_invitation?
     users.any? { |user| user.preference.allow_collection_invitation }
   end
+
+  private
+
+  def challenge_bypass(gift)
+    self.challenge_assignments.map(&:requesting_pseud).include?(gift.pseud) ||
+      self.challenge_claims
+        .reject { |c| c.request_prompt.anonymous? }
+        .map(&:requesting_pseud)
+        .include?(gift.pseud)
+  end
 end

app/views/admin/_header.html.erb

@@ -1,72 +1,73 @@
-<h3 class="landmark heading"><%= ts("Admin Navigation", key: "header") %></h3>
-<ul class="admin primary navigation actions" role="navigation">
-  <% if policy(User).can_manage_users? %>
+<nav aria-label="<%= t(".nav.label") %>">
+  <ul class="admin primary navigation actions">
+    <% if policy(User).can_manage_users? %>
+      <li class="dropdown">
+        <%= link_to t(".nav.users.manage"), admin_users_path %>
+        <ul class="menu">
+          <li><%= link_to t(".nav.users.search"), admin_users_path %></li>
+          <li><%= link_to t(".nav.users.email_search"), bulk_search_admin_users_path %></li>
+        </ul>
+      </li>
+    <% end %>
+
     <li class="dropdown">
-      <%= link_to ts("Manage Users", key: "header"), admin_users_path %>
-      <ul class="menu" role="menu">
-        <li><%= link_to ts("Find Users", key: "header"), admin_users_path %></li>
-        <li><%= link_to ts("Bulk Email Search", key: "header"), bulk_search_admin_users_path %></li>
+      <%= link_to t(".nav.invitations.invitations"), admin_invitations_path %>
+      <ul class="menu">
+        <li><%= link_to t(".nav.invitations.new"), admin_invitations_path %></li>
+        <li><%= link_to t(".nav.invitations.requests"), user_invite_requests_path %></li>
+        <% if policy(InviteRequest).can_manage? %>
+          <li><%= link_to t(".nav.invitations.queue"), manage_invite_requests_path %></li>
+        <% end %>
       </ul>
     </li>
-  <% end %>
-
-  <li class="dropdown">
-    <%= link_to ts("Invitations", key: "header"), admin_invitations_path %>
-    <ul class="menu" role="menu">
-      <li><%= link_to ts("Invite New Users", key: "header"), admin_invitations_path %></li>
-      <li><%= link_to ts("Manage Requests", key: "header"), user_invite_requests_path %></li>
-      <% if policy(InviteRequest).can_manage? %>
-        <li><%= link_to ts("Manage Queue", key: "header"), manage_invite_requests_path %></li>
-      <% end %>
-    </ul>
-  </li>
-  <li class="dropdown">
-    <%= link_to ts("Admin Posts", key: "header"), admin_posts_path %>
-    <ul class="menu" role="menu">
-      <li><%= link_to ts("AO3 News", key: "header"), admin_posts_path %></li>
-      <% if policy(AdminPost).can_post? %>
-        <li><%= link_to ts("Post AO3 News", key: "header"), new_admin_post_path %></li>
-      <% end %>
-      <li><%= link_to ts("Archive FAQ", key: "header"), archive_faqs_path %></li>
-      <li><%= link_to ts("Known Issues", key: "header"), known_issues_path %></li>
-      <li><%= link_to ts("Wrangling Guidelines", key: "header"), wrangling_guidelines_path %></li>
-    </ul>
-  </li>
-  <% if policy(AdminBlacklistedEmail).index? %>
-    <li><%= link_to ts("Banned Emails", key: "header"), admin_blacklisted_emails_path %></li>
-  <% end %>
+    <li class="dropdown">
+      <%= link_to t(".nav.posts.admin_posts"), admin_posts_path %>
+      <ul class="menu">
+        <li><%= link_to t(".nav.posts.news"), admin_posts_path %></li>
+        <% if policy(AdminPost).can_post? %>
+          <li><%= link_to t(".nav.posts.post_news"), new_admin_post_path %></li>
+        <% end %>
+        <li><%= link_to t(".nav.posts.faqs"), archive_faqs_path %></li>
+        <li><%= link_to t(".nav.posts.known_issues"), known_issues_path %></li>
+        <li><%= link_to t(".nav.posts.wrangling_guidelines"), wrangling_guidelines_path %></li>
+      </ul>
+    </li>
+    <% if policy(AdminBlacklistedEmail).index? %>
+      <li><%= link_to t(".nav.banned_emails"), admin_blacklisted_emails_path %></li>
+    <% end %>
 
-  <% if policy(ModeratedWork).index? %>
-    <li><%= link_to ts("Spam", key: "header"), admin_spam_index_path %></li>
-  <% end %>
+    <% if policy(ModeratedWork).index? %>
+      <li><%= link_to t(".nav.spam"), admin_spam_index_path %></li>
+    <% end %>
 
-  <% if policy(AdminSetting).can_view_settings? %>
-    <li><%= link_to ts("Settings", key: "header"), admin_settings_path %></li>
-  <% end %>
+    <% if policy(AdminSetting).can_view_settings? %>
+      <li><%= link_to t(".nav.settings"), admin_settings_path %></li>
+    <% end %>
 
-  <% if policy(AdminBanner).index? %>
-    <li><%= link_to ts("Banners", key: "header"), admin_banners_path %></li>
-  <% end %>
+    <% if policy(AdminBanner).index? %>
+      <li><%= link_to t(".nav.banners"), admin_banners_path %></li>
+    <% end %>
 
-  <% if policy(Skin).index? %>
-    <li class="dropdown">
-      <%= link_to ts("Skins", key: "header"), admin_skins_path %>
-      <ul class="menu" role="menu">
-        <li><%= link_to ts("Approval Queue", key: "header"), admin_skins_path %></li>
-        <li><%= link_to ts("Approved Skins", key: "header"), index_approved_admin_skins_path %></li>
-        <li><%= link_to ts("Rejected Skins", key: "header"), index_rejected_admin_skins_path %></li>
-      </ul>
-    </li>
-  <% end %>
-  <li><%= link_to ts("Tag Wrangling", key: "header"), tag_wranglings_path %></li>
+    <% if policy(Skin).index? %>
+      <li class="dropdown">
+        <%= link_to t(".nav.skins.skins"), admin_skins_path %>
+        <ul class="menu">
+          <li><%= link_to t(".nav.skins.queue"), admin_skins_path %></li>
+          <li><%= link_to t(".nav.skins.approved"), index_approved_admin_skins_path %></li>
+          <li><%= link_to t(".nav.skins.rejected"), index_rejected_admin_skins_path %></li>
+        </ul>
+      </li>
+    <% end %>
+    <li><%= link_to t(".nav.wrangling"), tag_wranglings_path %></li>
 
-  <% if policy(Locale).index? %>
-    <li><%= link_to ts("Locales", key: "header"), locales_path %></li>
-  <% end %>
+    <% if policy(Locale).index? %>
+      <li><%= link_to t(".nav.locales"), locales_path %></li>
+    <% end %>
 
-  <% if policy(AdminActivity).index? %>
-    <li><%= link_to ts("Activities", key: "header"), admin_activities_path %></li>
-  <% end %>
+    <% if policy(AdminActivity).index? %>
+      <li><%= link_to t(".nav.activities"), admin_activities_path %></li>
+    <% end %>
 
-  <li><%= link_to ts("Manage API Tokens", key: "header"), admin_api_index_path %></li>
-</ul>
+    <li><%= link_to t(".nav.api_tokens"), admin_api_index_path %></li>
+  </ul>
+</nav>

app/views/admin/banners/_banner.html.erb

@@ -2,6 +2,6 @@
 <% # don't forget to update layouts/banner! %>
 <div class="<%= admin_banner.banner_type %> announcement group">
   <blockquote class="userstuff">
-    <%=raw sanitize_field(admin_banner, :content) %>
+    <%= raw sanitize_field(admin_banner, :content, strip_images: true) %>
   </blockquote>
 </div>

app/views/blocked/users/confirm_block.html.erb

@@ -10,6 +10,7 @@
     <ul>
       <li><%= t(".will.commenting") %></li>
       <li><%= t(".will.replying") %></li>
+      <li><%= t(".will.gifting") %></li>
     </ul>
 
     <p><%= t(".will_not.intro") %></p>

app/views/blocked/users/confirm_unblock.html.erb

@@ -10,6 +10,7 @@
     <ul>
       <li><%= t(".resume.commenting") %></li>
       <li><%= t(".resume.replying") %></li>
+      <li><%= t(".resume.gifting") %></li>
     </ul>
   </div>
 

app/views/blocked/users/index.html.erb

@@ -13,6 +13,7 @@
   <ul>
     <li><%= t(".will.commenting") %></li>
     <li><%= t(".will.replying") %></li>
+    <li><%= t(".will.gifting") %></li>
   </ul>
 
   <p><%= t(".will_not.intro") %></p>

app/views/bookmarks/_bookmark_blurb_short.html.erb

@@ -28,10 +28,10 @@
     </ul>
   <% end %>
   <!--notes-->
-  <% unless bookmark.bookmarker_notes.blank? %>
+  <% if bookmark.bookmarker_notes.present? %>
     <h6 class="landmark heading"><%= ts("Bookmark Notes:") %></h6>
     <blockquote class="userstuff summary">
-      <%=raw sanitize_field(bookmark, :bookmarker_notes) %>
+      <%= raw sanitize_field(bookmark, :bookmarker_notes, strip_images: true) %>
     </blockquote>
   <% end %>
   <!--actions-->

app/views/bookmarks/_bookmark_user_module.html.erb

@@ -45,10 +45,10 @@
     <% end %>
 
     <!--notes-->
-    <% unless bookmark.bookmarker_notes.blank? %>
+    <% if bookmark.bookmarker_notes.present? %>
       <h6 class="landmark heading"><%= ts('Bookmarker\'s Notes') %></h6>
       <blockquote class="userstuff notes">
-        <%=raw sanitize_field(bookmark, :bookmarker_notes) %>
+        <%= raw sanitize_field(bookmark, :bookmarker_notes, strip_images: true) %>
       </blockquote>
     <% end %>
     <% # end of information added by the bookmark owner %>

app/views/collections/_collection_blurb.html.erb

@@ -72,8 +72,8 @@
       <% end %>
       <% if !collection.user_is_owner?(current_user) && collection.moderated? && !(collection.challenge && collection.challenge.signup_open) %>
         <li>
-          <% if (@participant ||= collection.get_participants_for_user(current_user).first) %>
-            <%= link_to ts("Leave"), collection_participant_path(collection, @participant),
+          <% if (participant = collection.get_participants_for_user(current_user).first) %>
+            <%= link_to ts("Leave"), collection_participant_path(collection, participant),
               data: {confirm: ts('Are you certain you want to leave this collection?')},
               :method => :delete %></li>
           <% else %>

app/views/comments/_single_comment.html.erb

@@ -55,7 +55,9 @@
         <% if single_comment.hidden_by_admin? %>
           <p class="notice"><%= ts("This comment has been hidden by an admin.") %></p>
         <% end %>
-        <blockquote class="userstuff"><%=raw sanitize_field(single_comment, :comment_content) %></blockquote>
+        <blockquote class="userstuff">
+          <%= raw sanitize_field(single_comment, :comment_content, strip_images: single_comment.ultimate_parent.is_a?(AdminPost)) %>
+        </blockquote>
       <% end %>
       <% if single_comment.edited_at.present? %>
         <p class="edited datetime">

app/views/inbox/_inbox_comment_contents.html.erb

@@ -26,7 +26,6 @@
   <% end %>
 </div>
 
-<% # This feedback_comment used to be inbox_comment... not sure why %>
 <blockquote class="userstuff">
-  <%= raw sanitize_field(feedback_comment, :comment_content) %>
+  <%= raw sanitize_field(feedback_comment, :comment_content, strip_images: feedback_comment.ultimate_parent.is_a?(AdminPost)) %>
 </blockquote>

app/views/layouts/_banner.html.erb

@@ -1,25 +1,20 @@
-<% # BACK END this seems giant and messy and confusing, pls can we review? 
-   # FRONT END yes let us rewrite this
-%>
-<% unless current_user && current_user.try(:preference).try(:banner_seen) %>
-<% if @admin_banner && @admin_banner.active? %>
-<% unless current_user.nil? && session[:hide_banner] %>
-  <div class="<%= @admin_banner.banner_type %> announcement group" id="admin-banner">
-    <blockquote class="userstuff">
-      <%=raw sanitize_field(@admin_banner, :content) %>
-    </blockquote>
-    <% if current_user.nil? %>
-      <p class="submit">
-        <%= link_to "&times;".html_safe, current_path_with(hide_banner: true), :class => 'showme action', :title => ts("hide banner") %>
-      </p>
-    <% else %>
-      <%= form_tag end_banner_user_path(current_user), :method => :post, :remote => true do %>
+<% if @admin_banner&.active? %>
+  <% unless session[:hide_banner] || current_user&.preference&.banner_seen %>
+    <div class="<%= @admin_banner.banner_type %> announcement group" id="admin-banner">
+      <blockquote class="userstuff">
+        <%= raw sanitize_field(@admin_banner, :content, strip_images: true) %>
+      </blockquote>
+      <% if current_user.nil? %>
         <p class="submit">
-          <%= submit_tag "&times;".html_safe, :title => ts("hide banner") %>
+          <%= link_to "&times;".html_safe, current_path_with(hide_banner: true), class: "showme action", title: ts("hide banner") %>
         </p>
+      <% else %>
+        <%= form_tag end_banner_user_path(current_user), method: :post, remote: true do %>
+          <p class="submit">
+            <%= submit_tag "&times;".html_safe, title: ts("hide banner") %>
+          </p>
+        <% end %>
       <% end %>
-    <% end %>
-  </div>
-<% end %>
-<% end %>
+    </div>
+  <% end %>
 <% end %>