From 263670a01357a6e5398d482218382d49c285bf1d Mon Sep 17 00:00:00 2001 From: Matchu Date: Wed, 2 Aug 2023 11:34:27 -0700 Subject: [PATCH] Simplify item_link rendering In the interest of clearing out Resque, I'm just gonna remove a lot of our more complex caching stuff, and we can do a perf pass for things like big item list pages once everything's upgraded. (I'm hopeful that the upgrades themselves improve perf; and if not, that some improved sensibilities 10 years later can find simpler approaches.) --- app/controllers/closet_hangers_controller.rb | 7 ------ app/helpers/items_helper.rb | 22 +------------------ app/models/closet_hanger.rb | 2 -- app/models/item/proxy_array.rb | 3 --- app/models/item/update_task.rb | 1 - .../closet_hangers/_closet_hanger.html.haml | 2 +- 6 files changed, 2 insertions(+), 35 deletions(-) diff --git a/app/controllers/closet_hangers_controller.rb b/app/controllers/closet_hangers_controller.rb index b46e6afd..18e495aa 100644 --- a/app/controllers/closet_hangers_controller.rb +++ b/app/controllers/closet_hangers_controller.rb @@ -44,14 +44,11 @@ class ClosetHangersController < ApplicationController visible_groups = @user.closet_hangers_groups_visible_to(@perspective_user) @unlisted_closet_hangers_by_owned = find_unlisted_closet_hangers_by_owned(visible_groups) - item_proxies = Item::ProxyArray.new items = [] @closet_lists_by_owned.each do |owned, lists| lists.each do |list| list.hangers.each do |hanger| - hanger.item_proxy = Item::Proxy.new(hanger.item) - item_proxies << hanger.item_proxy items << hanger.item end end @@ -59,14 +56,10 @@ class ClosetHangersController < ApplicationController @unlisted_closet_hangers_by_owned.each do |owned, hangers| hangers.each do |hanger| - hanger.item_proxy = Item::Proxy.new(hanger.item) - item_proxies << hanger.item_proxy items << hanger.item end end - item_proxies.prepare_partial(:item_link_partial) - if @public_perspective && user_signed_in? current_user.assign_closeted_to_items!(items) end diff --git a/app/helpers/items_helper.rb b/app/helpers/items_helper.rb index 8a7a2577..71c19c58 100644 --- a/app/helpers/items_helper.rb +++ b/app/helpers/items_helper.rb @@ -116,27 +116,7 @@ module ItemsHelper end def render_item_link(item) - # I've discovered that checking the cache *before* attempting to render the - # partial is significantly faster: moving the cache line out here instead - # of having it wrap the partial's content speeds up closet_hangers#index - # rendering time by about a factor of 2. It's uglier, but this call happens - # a lot, so the performance gain is definitely worth it. I'd be interested - # in a more legit partial caching abstraction, but, for now, this will do. - # Because this is a returned-string helper, but uses a buffer-output - # helper, we have to do some indirection. Fake that the render is in a - # template, then capture the resulting buffer output. - capture do - # Try to read from the prepared proxy's known partial output, if it's - # even a proxy at all. - if item.respond_to?(:known_partial_output) - prepared_output = item.known_partial_output(:item_link_partial).try(:html_safe) - else - prepared_output = nil - end - prepared_output || localized_cache("items/#{item.id}#item_link_partial") do - safe_concat render(partial: 'items/item_link', locals: {item: item}) - end - end + render(partial: 'items/item_link', locals: {item: item}) end private diff --git a/app/models/closet_hanger.rb b/app/models/closet_hanger.rb index 40736801..37b6ee88 100644 --- a/app/models/closet_hanger.rb +++ b/app/models/closet_hanger.rb @@ -3,8 +3,6 @@ class ClosetHanger < ActiveRecord::Base belongs_to :list, :class_name => 'ClosetList' belongs_to :user - attr_accessor :item_proxy - delegate :name, to: :item, prefix: true validates :item_id, :uniqueness => {:scope => [:user_id, :owned, :list_id]} diff --git a/app/models/item/proxy_array.rb b/app/models/item/proxy_array.rb index 55dde892..4bd41cee 100644 --- a/app/models/item/proxy_array.rb +++ b/app/models/item/proxy_array.rb @@ -6,9 +6,6 @@ class Item SCOPES = HashWithIndifferentAccess.new({ method: { as_json: Item.includes(:translations), - }, - partial: { - item_link_partial: Item.includes(:translations) } }) diff --git a/app/models/item/update_task.rb b/app/models/item/update_task.rb index 1e7bc0e4..e53f869d 100644 --- a/app/models/item/update_task.rb +++ b/app/models/item/update_task.rb @@ -16,7 +16,6 @@ class Item private def self.expire_cache_for(item) - expire_fragment_in_all_locales("items/#{item.id}#item_link_partial") expire_fragment_in_all_locales("items/#{item.id} header") expire_fragment_in_all_locales("items/#{item.id} info") expire_key_in_all_locales("items/#{item.id}#as_json") diff --git a/app/views/closet_hangers/_closet_hanger.html.haml b/app/views/closet_hangers/_closet_hanger.html.haml index 4c456db9..ced215d2 100644 --- a/app/views/closet_hangers/_closet_hanger.html.haml +++ b/app/views/closet_hangers/_closet_hanger.html.haml @@ -1,4 +1,4 @@ %div{'class' => closet_hanger_partial_class(closet_hanger), 'data-item-id' => closet_hanger.item_id, 'data-quantity' => closet_hanger.quantity, 'data-id' => closet_hanger.id} - = render_item_link(closet_hanger.item_proxy || closet_hanger.item) + = render_item_link(closet_hanger.item) .quantity{:class => "quantity-#{closet_hanger.quantity}"} %span= closet_hanger.quantity