Skip to content

Commit 5729602

Browse files
drusepthdrusepth
committed
unify logic for content#show / content#edit / content#gallery image lists
1 parent fe63a25 commit 5729602

File tree

5 files changed

+177
-184
lines changed

5 files changed

+177
-184
lines changed

‎app/controllers/api/v1/gallery_images_controller.rb‎

Lines changed: 74 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,31 +29,56 @@ def sort
2929
# Process each image in the array
3030
success = true
3131

32+
# Add a validation check to ensure all images in params exist
33+
# This helps prevent race conditions where an image might have been deleted
34+
image_ids_by_type = {
35+
'image_upload' => [],
36+
'basil_commission' => []
37+
}
38+
39+
params[:images].each do |image_data|
40+
if ['image_upload', 'basil_commission'].include?(image_data[:type])
41+
image_ids_by_type[image_data[:type]] << image_data[:id].to_i
42+
else
43+
success = false
44+
return render json: { error: 'Invalid image type' }, status: :unprocessable_entity
45+
end
46+
end
47+
48+
# Verify all images exist and belong to this content
49+
image_upload_count = ImageUpload.where(
50+
id: image_ids_by_type['image_upload'],
51+
content_type: content_type,
52+
content_id: content_id
53+
).count
54+
55+
basil_commission_count = BasilCommission.where(
56+
id: image_ids_by_type['basil_commission'],
57+
entity_type: content_type,
58+
entity_id: content_id
59+
).count
60+
61+
if image_upload_count != image_ids_by_type['image_upload'].size ||
62+
basil_commission_count != image_ids_by_type['basil_commission'].size
63+
return render json: { error: 'Some images do not exist or do not belong to this content' },
64+
status: :unprocessable_entity
65+
end
66+
67+
# Use an advisory lock to prevent concurrent updates
68+
# This ensures only one request at a time can update positions for this content
69+
lock_key = "gallery_images_#{content_type}_#{content_id}"
70+
3271
# Use a transaction to ensure all positions are updated or none are
3372
ActiveRecord::Base.transaction do
34-
params[:images].each do |image_data|
35-
if image_data[:type] == 'image_upload'
36-
# Update ImageUpload position
37-
image = ImageUpload.find_by(id: image_data[:id])
38-
if image && image.content_type == content_type && image.content_id.to_s == content_id.to_s
39-
image.insert_at(image_data[:position].to_i)
40-
else
41-
success = false
42-
raise ActiveRecord::Rollback
43-
end
44-
elsif image_data[:type] == 'basil_commission'
45-
# Update BasilCommission position
46-
image = BasilCommission.find_by(id: image_data[:id])
47-
if image && image.entity_type == content_type && image.entity_id.to_s == content_id.to_s
48-
image.insert_at(image_data[:position].to_i)
49-
else
50-
success = false
51-
raise ActiveRecord::Rollback
52-
end
53-
else
54-
success = false
55-
raise ActiveRecord::Rollback
73+
# With_advisory_lock is a gem method, but we handle it with our own locking mechanism
74+
# if it's not available
75+
if ActiveRecord::Base.connection.respond_to?(:with_advisory_lock)
76+
ActiveRecord::Base.connection.with_advisory_lock(lock_key) do
77+
update_image_positions(params[:images], content_type, content_id)
5678
end
79+
else
80+
# Fallback to regular transaction if advisory locks aren't supported
81+
update_image_positions(params[:images], content_type, content_id)
5782
end
5883
end
5984

@@ -63,4 +88,31 @@ def sort
6388
render json: { error: 'Failed to update image positions' }, status: :unprocessable_entity
6489
end
6590
end
91+
92+
private
93+
94+
def update_image_positions(images, content_type, content_id)
95+
# Process each image in the array
96+
images.each do |image_data|
97+
if image_data[:type] == 'image_upload'
98+
# Update ImageUpload position
99+
image = ImageUpload.find_by(id: image_data[:id])
100+
if image && image.content_type == content_type && image.content_id.to_s == content_id.to_s
101+
image.insert_at(image_data[:position].to_i)
102+
else
103+
raise ActiveRecord::Rollback
104+
end
105+
elsif image_data[:type] == 'basil_commission'
106+
# Update BasilCommission position
107+
image = BasilCommission.find_by(id: image_data[:id])
108+
if image && image.entity_type == content_type && image.entity_id.to_s == content_id.to_s
109+
image.insert_at(image_data[:position].to_i)
110+
else
111+
raise ActiveRecord::Rollback
112+
end
113+
else
114+
raise ActiveRecord::Rollback
115+
end
116+
end
117+
end
66118
end

‎app/helpers/application_helper.rb‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,55 @@ def clean_links(html)
4444
def show_notice?(id: nil)
4545
user_signed_in? && current_user.notice_dismissals.where(notice_id: id).none?
4646
end
47+
48+
# Combines and sorts gallery images from both ImageUploads and BasilCommissions
49+
# for consistent display across all gallery views in the application.
50+
# @param regular_images [Array<ImageUpload>] regular image uploads
51+
# @param basil_images [Array<BasilCommission>] AI-generated images
52+
# @return [Array<Hash>] Combined and sorted array of image hashes
53+
def combine_and_sort_gallery_images(regular_images, basil_images)
54+
combined_images = []
55+
56+
# Add regular images with consistent structure
57+
regular_images.each do |image|
58+
combined_images << {
59+
id: image.id,
60+
type: 'image_upload', # Use consistent naming across all usages
61+
data: image,
62+
created_at: image.created_at
63+
}
64+
end
65+
66+
# Add basil images with consistent structure
67+
basil_images.each do |commission|
68+
combined_images << {
69+
id: commission.id,
70+
type: 'basil_commission', # Use consistent naming across all usages
71+
data: commission,
72+
created_at: commission.saved_at
73+
}
74+
end
75+
76+
# Sort with consistent criteria
77+
combined_images.sort_by do |img|
78+
# First by pinned status - pin always takes precedence
79+
pinned_sort = (img[:data].respond_to?(:pinned?) && img[:data].pinned?) ? 0 : 1
80+
81+
# Then by position - using presence check for nil/blank values
82+
position_value = if img[:data].respond_to?(:position) && img[:data].position.present?
83+
img[:data].position
84+
else
85+
999999
86+
end
87+
88+
# Finally by created date as tertiary sort with fallback
89+
created_at = img[:created_at] || Time.current
90+
91+
# Add a unique identifier to ensure stable sorting
92+
unique_id = "#{img[:type]}-#{img[:id]}"
93+
94+
# Return sort keys array for stable sorting
95+
[pinned_sort, position_value, created_at, unique_id]
96+
end
97+
end
4798
end

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

Lines changed: 39 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -3,39 +3,47 @@
33

44
<ul class="slides">
55
<%
6-
# First check for any pinned images (ImageUploads)
7-
pinned_upload = @content.image_uploads.pinned.where(privacy: 'public').first
8-
if pinned_upload.present?
6+
# Get all images with ordering - respect privacy in show page
7+
# In content#show we want to be consistent with edit, but only show public images
8+
regular_images = @content.image_uploads.ordered.to_a
9+
# Filter for public if the current user isn't the owner
10+
unless user_signed_in? && @content.user_id == current_user.id
11+
regular_images = regular_images.select { |img| img.privacy == 'public' }
12+
end
13+
basil_images = @content.basil_commissions.where.not(saved_at: nil).ordered.to_a
14+
15+
# Use our unified helper to combine and sort all images
16+
combined_images = combine_and_sort_gallery_images(regular_images, basil_images)
17+
18+
# If we have no images, try to add a default image
19+
if combined_images.empty? && @content.respond_to?(:default_image)
20+
default_image = @content.default_image
21+
if default_image.present?
22+
combined_images << {
23+
type: 'default',
24+
data: default_image
25+
}
26+
end
27+
end
28+
29+
# Now loop through all combined images (or at least the first few)
30+
display_limit = 1 # For the header, we just show the first image
31+
combined_images.take(display_limit).each do |image_item|
32+
image_type = image_item[:type]
33+
image_data = image_item[:data]
934
%>
1035
<li>
11-
<%= image_tag pinned_upload.src(:large) %>
12-
<div class="caption bordered-text center">
13-
<h3>
14-
<% if @content.persisted? %>
15-
<%= link_to @content do %>
16-
<%= simple_format ContentFormatterService.show(
17-
text: @content.name_field_value,
18-
viewing_user: current_user
19-
) %>
20-
<% end %>
21-
<small>
22-
<%= simple_format ContentFormatterService.show(
23-
text: @content.description,
24-
viewing_user: current_user
25-
) %>
26-
</small>
27-
<% else %>
28-
New <%= @content.class.name %>
29-
<% end %>
30-
</h3>
31-
</div>
32-
</li>
33-
<%
34-
# Then check for pinned BasilCommissions
35-
elsif (pinned_commission = @content.basil_commissions.pinned.where.not(saved_at: nil).first).present?
36-
%>
37-
<li>
38-
<%= image_tag pinned_commission.image %>
36+
<% if image_type == 'image_upload' %>
37+
<%= image_tag image_data.src(:large) %>
38+
<% elsif image_type == 'basil_commission' %>
39+
<% if image_data.image.attached? %>
40+
<%= image_tag rails_blob_path(image_data.image, disposition: "attachment") %>
41+
<% else %>
42+
<%= image_tag asset_path("missing-image.jpg") %>
43+
<% end %>
44+
<% elsif image_type == 'default' && image_data.is_a?(String) %>
45+
<img src="<%= image_data %>">
46+
<% end %>
3947
<div class="caption bordered-text center">
4048
<h3>
4149
<% if @content.persisted? %>
@@ -57,64 +65,6 @@
5765
</h3>
5866
</div>
5967
</li>
60-
<%
61-
# If no pinned images, show all public images as before
62-
else
63-
%>
64-
<% @content.public_image_uploads.each do |image_source| %>
65-
<li>
66-
<% if image_source.is_a?(String) %>
67-
<img src="<%= image_source %>">
68-
<% else %>
69-
<%= image_tag image_source.src(:large) %>
70-
<% end %>
71-
<div class="caption bordered-text center">
72-
<h3>
73-
<% if @content.persisted? %>
74-
<%= link_to @content do %>
75-
<%= simple_format ContentFormatterService.show(
76-
text: @content.name_field_value,
77-
viewing_user: current_user
78-
) %>
79-
<% end %>
80-
<small>
81-
<%= simple_format ContentFormatterService.show(
82-
text: @content.description,
83-
viewing_user: current_user
84-
) %>
85-
</small>
86-
<% else %>
87-
New <%= @content.class.name %>
88-
<% end %>
89-
</h3>
90-
</div>
91-
</li>
92-
<% end %>
93-
<% @content.basil_commissions.where.not(saved_at: nil).each do |commission| %>
94-
<li>
95-
<%= image_tag commission.image %>
96-
<div class="caption bordered-text center">
97-
<h3>
98-
<% if @content.persisted? %>
99-
<%= link_to @content do %>
100-
<%= simple_format ContentFormatterService.show(
101-
text: @content.name_field_value,
102-
viewing_user: current_user
103-
) %>
104-
<% end %>
105-
<small>
106-
<%= simple_format ContentFormatterService.show(
107-
text: @content.description,
108-
viewing_user: current_user
109-
) %>
110-
</small>
111-
<% else %>
112-
New <%= @content.class.name %>
113-
<% end %>
114-
</h3>
115-
</div>
116-
</li>
117-
<% end %>
11868
<% end %>
11969
</ul>
12070
</div>

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

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -2,49 +2,14 @@
22
raw_model = content.is_a?(ContentSerializer) ? content.raw_model : content
33

44
# Get both image types with ordering
5-
regular_images = raw_model.image_uploads.ordered.to_a
6-
basil_images = (@basil_images || []).sort_by(&:position)
5+
regular_images = ImageUpload.where(content_type: raw_model.class.name, content_id: raw_model.id).ordered.to_a
6+
basil_images = (@basil_images || []).ordered.to_a
77

88
# Calculate total images for display purposes
99
total_images = regular_images.count + basil_images.count
10-
11-
# Find the pinned image (if any) - there should only be one pinned image
12-
pinned_image = regular_images.find(&:pinned?) || basil_images.find(&:pinned?)
1310

14-
# Create a combined list of all images for sortable functionality
15-
# We'll maintain type information for each
16-
combined_images = []
17-
regular_images.each do |image|
18-
combined_images << {
19-
id: image.id,
20-
type: 'image_upload',
21-
data: image,
22-
pinned: image.pinned?,
23-
created_at: image.created_at,
24-
position: image.position || 999999
25-
}
26-
end
27-
28-
basil_images.each do |commission|
29-
combined_images << {
30-
id: commission.id,
31-
type: 'basil_commission',
32-
data: commission,
33-
pinned: commission.pinned?,
34-
created_at: commission.saved_at,
35-
position: commission.position || 999999
36-
}
37-
end
38-
39-
# Sort by position (using pinned status as priority)
40-
combined_images.sort_by! do |img|
41-
# The pinned flag is the primary sort key
42-
pinned_sort = img[:pinned] ? 0 : 1
43-
# Position is the secondary sort key
44-
position_sort = img[:position] || 999999
45-
# Created date is tertiary sort
46-
[pinned_sort, position_sort, img[:created_at] || Time.current]
47-
end
11+
# Use the unified helper to combine and sort images consistently
12+
combined_images = combine_and_sort_gallery_images(regular_images, basil_images)
4813
%>
4914

5015
<div class="row">
@@ -65,10 +30,10 @@
6530
<!-- Show images in their sorted order -->
6631
<% combined_images.each do |image_item| %>
6732
<%
68-
is_pinned = image_item[:pinned]
6933
image_data = image_item[:data]
7034
image_type = image_item[:type]
7135
image_id = image_item[:id]
36+
is_pinned = image_data.respond_to?(:pinned?) && image_data.pinned?
7237
%>
7338
<div class="col s12 m6 l6 gallery-sortable-item" data-image-id="<%= image_id %>" data-image-type="<%= image_type %>">
7439
<div class="image-card <%= is_pinned ? 'image-pinned' : '' %>">

0 commit comments

Comments
 (0)