Improve performance of Owls values in Item Getting Guide

Now we preload them all concurrently, instead of in sequence when the
template gets around to asking for them!

app/assets/stylesheets/_layout.sass

@@ -16,7 +16,6 @@ caption, tbody, tfoot, thead, tr, th, td
   margin: 0
   padding: 0
   border: 0
-  outline: 0
   font-size: 100%
   vertical-align: baseline
   background: transparent

app/assets/stylesheets/items/sources.sass

@@ -61,6 +61,17 @@
 		text-wrap: balance
 		font-style: italic
 
+	.subtitle
+		font-size: 85%
+		opacity: .85
+
+		a[title]
+			text-decoration-line: underline
+			text-decoration-style: dotted
+
+			&:hover, &:focus
+				text-decoration-style: solid
+
 	/* For wearable items that belong to a specific set that all come together,
 	 * like a Paint Brush. */
 	&[data-group-type="bundle"]

app/controllers/items_controller.rb

@@ -128,6 +128,9 @@ class ItemsController < ApplicationController
     @np_items = @items.select(&:np?)
     @pb_items = @items.select(&:pb?)
 
+    # Start loading the NC trade values for the non-Mall NC items.
+    trade_values_task = Async { Item.preload_nc_trade_values(@other_nc_items) }
+
     # Also, PB items have some special handling: we group them by color, then
     # load example pet types for the colors that don't have paint brushes.
     @pb_items_by_color = @pb_items.group_by(&:pb_color).
@@ -144,6 +147,9 @@ class ItemsController < ApplicationController
       [color, color.example_pet_type(preferred_species: species)]
     end.to_h
 
+    # Finish loading the NC trade values.
+    trade_values_task.wait
+
     render layout: "application"
   end
 

app/helpers/items_helper.rb

@@ -156,6 +156,17 @@ module ItemsHelper
     "Last updated: #{date_str} (#{time_ago_str} ago)"
   end
 
+  def nc_trade_value_subtitle_for(item)
+    value = item.nc_trade_value
+    return nil if value.nil?
+
+    link_to "Owls listing: #{item.nc_trade_value.value_text}",
+      "https://www.neopets.com/~owls",
+      title: 'Owls keeps track of approximate "capsule" values of NC items ' +
+        "for trading. Items with similar values can often be traded for one " +
+        "another. This is an estimate, not a rule!"
+  end
+
   private
 
   def build_on_pet_types(species, special_color=nil, &block)

app/models/item.rb

@@ -1,3 +1,6 @@
+require "async"
+require "async/barrier"
+
 class Item < ApplicationRecord
   include PrettyParam
   
@@ -114,15 +117,25 @@ class Item < ApplicationRecord
 
   def nc_trade_value
     return nil unless nc?
-    begin
+
+    # Load the trade value, if we haven't already. Note that, because the trade
+    # value may be nil, we also save an explicit boolean for whether we've
+    # already looked it up, rather than checking if the saved value is empty.
+    return @nc_trade_value if @nc_trade_value_loaded
+
+    @nc_trade_value = begin
+      Rails.logger.debug "Item #{id} (#{name}) <lookup>"
       OwlsValueGuide.find_by_name(name)
     rescue OwlsValueGuide::NotFound => error
       Rails.logger.debug("No NC trade value listed for #{name} (#{id})")
-      return nil
+      nil
     rescue OwlsValueGuide::NetworkError => error
       Rails.logger.error("Couldn't load nc_trade_value: #{error.full_message}")
-      return nil
+      nil
     end
+
+    @nc_trade_value_loaded = true
+    @nc_trade_value
   end
   
   # Return an OrderedHash mapping users to the number of times they
@@ -599,6 +612,27 @@ class Item < ApplicationRecord
     end
   end
 
+  def self.preload_nc_trade_values(items)
+    # Only allow 10 trade values to be loaded at a time.
+    barrier = Async::Barrier.new
+    semaphore = Async::Semaphore.new(10, parent: barrier)
+
+    Sync do
+      # Load all the trade values in concurrent async tasks. (The
+      # `nc_trade_value` caches the value in the Item object.)
+      items.each do |item|
+        semaphore.async { item.nc_trade_value }
+      end
+
+      # Wait until all tasks are done.
+      barrier.wait
+    ensure
+      barrier.stop # If something goes wrong, clean up all tasks.
+    end
+
+    items
+  end
+
   def self.collection_from_pet_type_and_registries(pet_type, info_registry, asset_registry, scope=Item.all)
     # bear in mind that registries are arrays with many nil elements,
     # due to how the parser works

app/views/items/_item_header.html.haml

@@ -28,7 +28,7 @@
       = link_to t('items.show.resources.jn_items'), jn_items_url_for(item)
       = link_to t('items.show.resources.impress_2020'), impress_2020_url_for(item)
       - if item.nc_trade_value
-        = link_to t('items.show.resources.owls', value: item.nc_trade_value.value_text),
+        = link_to t('items.show.resources.owls', value: item.nc_trade_value_text),
           "https://www.neopets.com/~owls",
           title: nc_trade_value_updated_at_text(item.nc_trade_value)
       - unless item.nc?

app/views/items/_item_list_row.html.haml

@@ -2,5 +2,8 @@
 	%td.thumbnail-cell
 		= link_to item_thumbnail_for(item), item, target: "_blank",
 		  tabindex: "-1"
-	%td.name-cell= link_to item.name, item, target: "_blank"
+	%td.name-cell
+		= link_to item.name, item, target: "_blank"
+		- if subtitle.present?
+			.subtitle= subtitle
 	%td.actions-cell= yield

app/views/items/sources.html.haml

@@ -99,7 +99,17 @@
 		purchased at all anymore, and can only be obtained via gifts or trades.
 
 		[mall]: https://ncmall.neopets.com/
-	= render @other_nc_items
+	%table.item-list
+		%thead
+			%td
+			%th{colspan: 2}
+				Total: #{pluralize @other_nc_items.size, "item"}
+		%tbody
+			- @other_nc_items.each do |item|
+				= render "item_list_row", item:, subtitle: nc_trade_value_subtitle_for(item) do
+					= button_link_to "NC Trades",
+						item_trades_path(item, type: "offering"),
+						target: "_blank", icon: search_icon
 
 - content_for :stylesheets do
 	= page_stylesheet_link_tag "items/sources"