don't sort pinned items first in lists

Author: drusepth

app/helpers/application_helper.rb

@@ -73,26 +73,57 @@ def combine_and_sort_gallery_images(regular_images, basil_images)
       }
     end
 
-    # Sort with consistent criteria
+    # Sort with consistent criteria - using only position and creation date
+    # This allows users to order images freely, including pinned images
     combined_images.sort_by do |img|
-      # First by pinned status - pin always takes precedence 
-      pinned_sort = (img[:data].respond_to?(:pinned?) && img[:data].pinned?) ? 0 : 1
-      
-      # Then by position - using presence check for nil/blank values
+      # First by position - using presence check for nil/blank values
       position_value = if img[:data].respond_to?(:position) && img[:data].position.present?
                          img[:data].position 
                        else
                          999999
                        end
 
-      # Finally by created date as tertiary sort with fallback
+      # Then by created date as secondary sort with fallback
       created_at = img[:created_at] || Time.current
 
       # Add a unique identifier to ensure stable sorting
       unique_id = "#{img[:type]}-#{img[:id]}"
 
-      # Return sort keys array for stable sorting
-      [pinned_sort, position_value, created_at, unique_id]
+      # Return sort keys array for stable sorting - no longer sorting by pinned status
+      [position_value, created_at, unique_id]
+    end
+  end
+  
+  # Gets the best image to use for a preview (card/header) by prioritizing pinned images
+  # @param regular_images [Array<ImageUpload>] regular image uploads
+  # @param basil_images [Array<BasilCommission>] AI-generated images
+  # @return [Hash] The best image to use as a preview (pinned if available)
+  def get_preview_image(regular_images, basil_images)
+    # First look for pinned images
+    pinned_regular = regular_images.find { |img| img.respond_to?(:pinned?) && img.pinned? }
+    pinned_basil = basil_images.find { |img| img.respond_to?(:pinned?) && img.pinned? }
+    
+    # Use the first pinned image found (prioritize regular uploads if both exist)
+    if pinned_regular.present?
+      return {
+        id: pinned_regular.id,
+        type: 'image_upload',
+        data: pinned_regular,
+        created_at: pinned_regular.created_at
+      }
+    elsif pinned_basil.present?
+      return {
+        id: pinned_basil.id,
+        type: 'basil_commission',
+        data: pinned_basil,
+        created_at: pinned_basil.saved_at
+      }
     end
+    
+    # If no pinned images, get all images
+    combined = combine_and_sort_gallery_images(regular_images, basil_images)
+    
+    # Return the first sorted image, or nil if none available
+    combined.first
   end
 end

app/models/page_types/content_page.rb

@@ -9,19 +9,24 @@ class ContentPage < ApplicationRecord
   include Authority::Abilities
   self.authorizer_name = 'ContentPageAuthorizer'
 
+  # Returns a single image for use in previews/cards, prioritizing pinned images
+  # This method keeps the original behavior of prioritizing pinned images for thumbnails/previews
   def random_image_including_private(format: :small)
+    # Always prioritize pinned images first for preview cards
     pinned_image = ImageUpload.where(content_type: self.page_type, content_id: self.id, pinned: true).first
     return pinned_image.src(format) if pinned_image
 
     pinned_commission = BasilCommission.where(entity_type: self.page_type, entity_id: self.id, pinned: true).where.not(saved_at: nil).includes([:image_attachment]).first
     return pinned_commission.image if pinned_commission
 
+    # Fall back to random images if no pinned images exist
     random_image = ImageUpload.where(content_type: self.page_type, content_id: self.id).sample
     return random_image.src(format) if random_image
 
     random_commission = BasilCommission.where(entity_type: self.page_type, entity_id: self.id).where.not(saved_at: nil).includes([:image_attachment]).sample
     return random_commission.image if random_commission
 
+    # Use default image as last resort
     ActionController::Base.helpers.asset_path("card-headers/#{self.page_type.downcase.pluralize}.webp")
   end
 

app/views/browse/tag.html.erb

@@ -10,6 +10,18 @@
     </ol>
     <p class="grey-text" style="font-size: 14px;">Note: It may take a few minutes for new content to appear.</p>
   </div>
+  <div class="modal-content" style="border-top: 1px solid #e0e0e0; padding-top: 15px;">
+    <h4><i class="material-icons <%= @accent_color %>-text left" style="margin-right: 10px;">brush</i> What is Art Fight?</h4>
+    <p>Art Fight is an annual art trading event that runs during July where:</p>
+    <ul class="browser-default">
+      <li>Artists are divided into two opposing teams</li>
+      <li>Participants "attack" the opposing team by drawing their original characters (OCs)</li> 
+      <li>Each completed artwork earns points for your team</li>
+      <li>The team with the most points at the end of July wins</li>
+    </ul>
+    <p>It's a great way to practice your art skills, discover new characters to draw, and connect with other creatives. Adding the <span class="highlight">ArtFight2025</span> tag to your public pages helps other Notebook.ai users find your characters for Art Fight!</p>
+    <p class="grey-text" style="font-size: 14px;">Note: Notebook.ai is not officially affiliated with Art Fight. Visit <a href="https://artfight.net" target="_blank" class="<%= @accent_color %>-text">artfight.net</a> to learn more and participate.</p>
+  </div>
   <div class="modal-footer">
     <a href="#!" class="modal-close waves-effect waves-light <%= @accent_color %> btn-flat white-text">Got it</a>
   </div>

app/views/content/display/_image_card_header.html.erb

@@ -12,8 +12,9 @@
       end
       basil_images = @content.basil_commissions.where.not(saved_at: nil).ordered.to_a
 
-      # Use our unified helper to combine and sort all images
-      combined_images = combine_and_sort_gallery_images(regular_images, basil_images)
+      # For headers/card previews, use get_preview_image to prioritize pinned images
+      preview_image = get_preview_image(regular_images, basil_images)
+      combined_images = preview_image.present? ? [preview_image] : []
 
       # If we have no images, try to add a default image
       if combined_images.empty? && @content.respond_to?(:default_image)

app/views/content/form/gallery/_panel.html.erb

@@ -8,7 +8,8 @@
   # Calculate total images for display purposes
   total_images = regular_images.count + basil_images.count
 
-  # Use the unified helper to combine and sort images consistently
+  # Use the unified helper to combine and sort images consistently by position
+  # This no longer prioritizes pinned images in the sort order
   combined_images = combine_and_sort_gallery_images(regular_images, basil_images)
 %>
 

app/views/content/gallery.html.erb

@@ -73,14 +73,16 @@
   <% else %>
     <!-- Gallery grid -->
     <% 
-      # Use the unified helper to combine and sort images consistently
+      # Use the unified helper to combine and sort images consistently by position
+      # This no longer prioritizes pinned images in the sort order
       combined_images = combine_and_sort_gallery_images(@images, @basil_images)
     %>
 
     <div class="gallery-grid" id="galleryGrid">
       <% combined_images.each_with_index do |image_item, index| %>
         <div class="gallery-item">
           <div class="gallery-card" data-index="<%= index %>">
+            <%# image_item[:data].position %>
             <% if image_item[:type] == 'image_upload' %>
               <%= image_tag image_item[:data].src(:medium), class: "gallery-image", 
                   data: { 

test/helpers/application_helper_test.rb

@@ -3,8 +3,11 @@
 class ApplicationHelperTest < ActionView::TestCase
   include ApplicationHelper
 
+  # Skip loading fixtures to avoid database issues
+  self.use_transactional_tests = false
+  
   # Use standalone test without fixtures for the helper method
-  test "combine_and_sort_gallery_images should sort correctly" do
+  test "combine_and_sort_gallery_images should sort by position and creation date" do
     # Skip test if helper method doesn't exist (for CI runs)
     skip unless respond_to?(:combine_and_sort_gallery_images)
 
@@ -23,7 +26,7 @@ def regular_image1.respond_to?(method)
     def regular_image2.id; 2; end
     def regular_image2.created_at; 1.day.ago; end
     def regular_image2.position; 2; end
-    def regular_image2.pinned?; true; end
+    def regular_image2.pinned?; true; end # Should be ignored in sorting
     def regular_image2.respond_to?(method)
       [:id, :created_at, :position, :pinned?].include?(method)
     end
@@ -44,11 +47,39 @@ def basil_image.respond_to?(method)
     # Assertions
     assert_equal 3, result.size
 
-    # First image should be the pinned one
-    assert_equal 2, result[0][:data].id
-    
-    # Then by position
-    assert_equal 3, result[1][:data].id # basil with position 1
+    # First by position (not by pinned status)
+    assert_equal 3, result[0][:data].id # basil with position 1
+    assert_equal 2, result[1][:data].id # regular with position 2 (pinned but not first)
     assert_equal 1, result[2][:data].id # regular with position 3
   end
+  
+  test "get_preview_image should prioritize pinned images" do
+    # Skip test if helper method doesn't exist (for CI runs)
+    skip unless respond_to?(:get_preview_image)
+    
+    # Create test objects with one pinned image
+    regular_image1 = Object.new
+    def regular_image1.id; 1; end
+    def regular_image1.created_at; 3.days.ago; end
+    def regular_image1.position; 1; end
+    def regular_image1.pinned?; false; end
+    def regular_image1.respond_to?(method)
+      [:id, :created_at, :position, :pinned?].include?(method)
+    end
+    
+    regular_image2 = Object.new
+    def regular_image2.id; 2; end
+    def regular_image2.created_at; 1.day.ago; end
+    def regular_image2.position; 3; end # Bad position, but pinned
+    def regular_image2.pinned?; true; end
+    def regular_image2.respond_to?(method)
+      [:id, :created_at, :position, :pinned?].include?(method)
+    end
+    
+    # Test preview image selection
+    result = get_preview_image([regular_image1, regular_image2], [])
+    
+    # Pinned image should be chosen for preview
+    assert_equal 2, result[:data].id
+  end
 end

test/unit/gallery_sorting_test.rb

@@ -4,6 +4,9 @@ class GallerySortingTest < ActiveSupport::TestCase
   # Include the helper module directly
   include ApplicationHelper
 
+  # Skip loading fixtures to avoid database issues
+  self.use_transactional_tests = false
+  
   # Create test classes that mimic needed behavior
   class MockImageUpload
     attr_reader :id, :created_at, :position, :pinned
@@ -43,18 +46,20 @@ def respond_to?(method)
     end
   end
 
-  test "combine_and_sort_gallery_images should prioritize pinned images" do
-    # Create images with different creation dates but pin one
-    oldest = MockImageUpload.new(id: 1, created_at: 3.days.ago)
-    pinned = MockImageUpload.new(id: 2, created_at: 2.days.ago, pinned: true)
-    newest = MockImageUpload.new(id: 3, created_at: 1.day.ago)
+  test "combine_and_sort_gallery_images should sort by position, not pinned status" do
+    # Create images with different creation dates including a pinned one
+    oldest = MockImageUpload.new(id: 1, created_at: 3.days.ago, position: 3)
+    pinned = MockImageUpload.new(id: 2, created_at: 2.days.ago, pinned: true, position: 2)
+    newest = MockImageUpload.new(id: 3, created_at: 1.day.ago, position: 1)
 
     # Call helper method
     result = combine_and_sort_gallery_images([oldest, pinned, newest], [])
 
-    # Verify the pinned image comes first despite not being newest
+    # Verify sorted by position, not pinned status
     assert_equal 3, result.size
-    assert_equal 2, result[0][:data].id  # Pinned should be first
+    assert_equal 3, result[0][:data].id  # Position 1 should be first
+    assert_equal 2, result[1][:data].id  # Position 2 should be second, even though pinned
+    assert_equal 1, result[2][:data].id  # Position 3 should be third
   end
 
   test "combine_and_sort_gallery_images should respect position values" do
@@ -92,16 +97,30 @@ def respond_to?(method)
     assert_equal 1, result[2][:data].id  # position 3
   end
 
-  test "combine_and_sort_gallery_images should prioritize pinned over position" do
-    # One pinned but with higher position value
+  test "get_preview_image should prioritize pinned images" do
+    # Create images with one of them pinned 
     image1 = MockImageUpload.new(id: 1, created_at: 3.days.ago, position: 1)
     image2 = MockImageUpload.new(id: 2, created_at: 2.days.ago, position: 2, pinned: true)
+    image3 = MockImageUpload.new(id: 3, created_at: 1.day.ago, position: 3)
+    
+    # Call helper method
+    result = get_preview_image([image1, image2, image3], [])
+    
+    # Verify the pinned image is returned
+    assert_equal 2, result[:data].id  # Pinned should be selected
+    assert result[:data].pinned?
+  end
+  
+  test "get_preview_image should fall back to first sorted image when no pinned images exist" do
+    # Images with no pinned status
+    image1 = MockImageUpload.new(id: 1, created_at: 3.days.ago, position: 3)
+    image2 = MockImageUpload.new(id: 2, created_at: 2.days.ago, position: 1)
+    image3 = MockImageUpload.new(id: 3, created_at: 1.day.ago, position: 2)
 
     # Call helper method
-    result = combine_and_sort_gallery_images([image1, image2], [])
+    result = get_preview_image([image1, image2, image3], [])
 
-    # Verify the pinned image comes first despite higher position
-    assert_equal 2, result[0][:data].id  # Pinned should be first
-    assert_equal 1, result[1][:data].id  # Non-pinned second
+    # Verify returns the first image by position
+    assert_equal 2, result[:data].id  # Position 1 should be selected
   end
 end