don't sort pinned items first in lists

This commit is contained in:
Andrew Brown 2025-06-29 23:26:38 -07:00
parent 60c355a6f0
commit c42a55c9cd
8 changed files with 134 additions and 32 deletions

View File

@ -73,26 +73,57 @@ module ApplicationHelper
}
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

View File

@ -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

View File

@ -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>

View File

@ -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)

View File

@ -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)
%>

View File

@ -73,7 +73,8 @@
<% 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)
%>
@ -81,6 +82,7 @@
<% 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: {

View File

@ -3,8 +3,11 @@ require 'test_helper'
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 @@ class ApplicationHelperTest < ActionView::TestCase
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 @@ class ApplicationHelperTest < ActionView::TestCase
# 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

View File

@ -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 @@ class GallerySortingTest < ActiveSupport::TestCase
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 @@ class GallerySortingTest < ActiveSupport::TestCase
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 = 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 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 = get_preview_image([image1, image2, image3], [])
# Verify returns the first image by position
assert_equal 2, result[:data].id # Position 1 should be selected
end
end