From f997513e87a743bd3dd1c744ff4ada3a08bf6f25 Mon Sep 17 00:00:00 2001
From: Matchu <matchu1993@@gmail.com>
Date: Sat, 22 Jul 2023 14:04:01 -0700
Subject: [PATCH] Update to new `scope` syntax

Ohh ok, without this change all of our `scope`s were just immediately evaluating the argument and fetching _all_ such matching records immediately, instead of waiting to actually be called. This led to bugs like `pet_type.as_json` returning ALL pet states in the whole db, because the `PetState.emotion_order` scope was being treated as a single predefined query, rather than a query fragment to merge into the current context.

This also explains what happened in 724ed83: that's why things before the scope in the query were being ignored.
---
 app/models/closet_hanger.rb | 28 +++++++++++++++-------------
 app/models/closet_list.rb   |  6 +++---
 app/models/color.rb         | 10 +++++-----
 app/models/contribution.rb  |  2 +-
 app/models/item.rb          | 20 ++++++++++++--------
 app/models/outfit.rb        |  2 +-
 app/models/pet.rb           |  2 +-
 app/models/pet_state.rb     |  4 +++-
 app/models/pet_type.rb      |  6 +++---
 app/models/species.rb       |  2 +-
 app/models/swf_asset.rb     | 16 ++++++++--------
 app/models/user.rb          |  2 +-
 app/models/wardrobe_tip.rb  |  2 +-
 app/models/zone.rb          |  8 ++++----
 14 files changed, 59 insertions(+), 51 deletions(-)

diff --git a/app/models/closet_hanger.rb b/app/models/closet_hanger.rb
index dce8a432..5ba52760 100644
--- a/app/models/closet_hanger.rb
+++ b/app/models/closet_hanger.rb
@@ -15,25 +15,27 @@ class ClosetHanger < ActiveRecord::Base
 
   validate :list_belongs_to_user
 
-  scope :alphabetical_by_item_name, lambda {
+  scope :alphabetical_by_item_name, -> {
     joins(:item => :translations).
       where(Item::Translation.arel_table[:locale].eq(I18n.locale)).
       order(Item::Translation.arel_table[:name])
   }
-  scope :newest, order(arel_table[:created_at].desc)
-  scope :owned_before_wanted, order(arel_table[:owned].desc)
-  scope :unlisted, where(:list_id => nil)
+  scope :newest, -> { order(arel_table[:created_at].desc) }
+  scope :owned_before_wanted, -> { order(arel_table[:owned].desc) }
+  scope :unlisted, -> { where(:list_id => nil) }
 
   {:owned => true, :wanted => false}.each do |name, owned|
-    scope "#{name}_trading", joins(:user).includes(:list).
-      where(:owned => owned).
-      where((
-        arel_table[:list_id].eq(nil).and(
-          User.arel_table["#{name}_closet_hangers_visibility"].gteq(ClosetVisibility[:trading].id)
-        )
-        ).or(
-        ClosetList.arel_table[:visibility].gteq(ClosetVisibility[:trading].id)
-      ))
+    scope "#{name}_trading", -> {
+      joins(:user).includes(:list).
+        where(:owned => owned).
+        where((
+          arel_table[:list_id].eq(nil).and(
+            User.arel_table["#{name}_closet_hangers_visibility"].gteq(ClosetVisibility[:trading].id)
+          )
+          ).or(
+          ClosetList.arel_table[:visibility].gteq(ClosetVisibility[:trading].id)
+        ))
+      }
   end
 
   before_validation :merge_quantities, :set_owned_by_list
diff --git a/app/models/closet_list.rb b/app/models/closet_list.rb
index 621653ff..8ca55e80 100644
--- a/app/models/closet_list.rb
+++ b/app/models/closet_list.rb
@@ -9,9 +9,9 @@ class ClosetList < ActiveRecord::Base
   validates :user, :presence => true
   validates :hangers_owned, :inclusion => {:in => [true, false], :message => "can't be blank"}
 
-  scope :alphabetical, order(:name)
-  scope :public, where(arel_table[:visibility].gteq(ClosetVisibility[:public].id))
-  scope :visible_to, lambda { |user|
+  scope :alphabetical, -> { order(:name) }
+  scope :public, -> { where(arel_table[:visibility].gteq(ClosetVisibility[:public].id)) }
+  scope :visible_to, ->(user) {
     condition = arel_table[:visibility].gteq(ClosetVisibility[:public].id)
     condition = condition.or(arel_table[:user_id].eq(user.id)) if user
     where(condition)
diff --git a/app/models/color.rb b/app/models/color.rb
index d512d324..4fe6aab4 100644
--- a/app/models/color.rb
+++ b/app/models/color.rb
@@ -1,11 +1,11 @@
 class Color < ActiveRecord::Base
   translates :name
   
-  scope :alphabetical, lambda { with_translations(I18n.locale).order(Color::Translation.arel_table[:name]) }
-  scope :basic, where(:basic => true)
-  scope :standard, where(:standard => true)
-  scope :nonstandard, where(:standard => false)
-  scope :funny, lambda { order(:prank) unless pranks_funny? }
+  scope :alphabetical, -> { with_translations(I18n.locale).order(Color::Translation.arel_table[:name]) }
+  scope :basic, -> { where(:basic => true) }
+  scope :standard, -> { where(:standard => true) }
+  scope :nonstandard, -> { where(:standard => false) }
+  scope :funny, -> { order(:prank) unless pranks_funny? }
 
   validates :name, presence: true
   
diff --git a/app/models/contribution.rb b/app/models/contribution.rb
index ffa79d02..bda74a40 100644
--- a/app/models/contribution.rb
+++ b/app/models/contribution.rb
@@ -9,7 +9,7 @@ class Contribution < ActiveRecord::Base
   belongs_to :contributed, :polymorphic => true
   belongs_to :user
   
-  scope :recent, order('id DESC')
+  scope :recent, -> { order('id DESC') }
   
   cattr_reader :per_page
   @@per_page = 30
diff --git a/app/models/item.rb b/app/models/item.rb
index 1828fc72..266d46ad 100644
--- a/app/models/item.rb
+++ b/app/models/item.rb
@@ -23,23 +23,27 @@ class Item < ActiveRecord::Base
   cattr_reader :per_page
   @@per_page = 30
 
-  scope :alphabetize_by_translations, lambda {
+  scope :alphabetize_by_translations, -> {
     it = Item::Translation.arel_table
     order(it[:name])
   }
 
-  scope :join_swf_assets, joins(:swf_assets).group(arel_table[:id])
+  scope :join_swf_assets, -> { joins(:swf_assets).group(arel_table[:id]) }
 
-  scope :newest, order(arel_table[:created_at].desc) if arel_table[:created_at]
+  scope :newest, -> {
+    order(arel_table[:created_at].desc) if arel_table[:created_at]
+  }
 
-  scope :spidered_longest_ago, order(["(last_spidered IS NULL) DESC", "last_spidered DESC"])
+  scope :spidered_longest_ago, -> {
+    order(["(last_spidered IS NULL) DESC", "last_spidered DESC"])
+  }
 
-  scope :sold_in_mall, where(:sold_in_mall => true)
-  scope :not_sold_in_mall, where(:sold_in_mall => false)
+  scope :sold_in_mall, -> { where(:sold_in_mall => true) }
+  scope :not_sold_in_mall, -> { where(:sold_in_mall => false) }
 
-  scope :sitemap, order([:id]).limit(49999)
+  scope :sitemap, -> { order([:id]).limit(49999) }
 
-  scope :with_closet_hangers, joins(:closet_hangers)
+  scope :with_closet_hangers, -> { joins(:closet_hangers) }
 
   def closeted?
     @owned || @wanted
diff --git a/app/models/outfit.rb b/app/models/outfit.rb
index a5d0e77a..4d52e65f 100644
--- a/app/models/outfit.rb
+++ b/app/models/outfit.rb
@@ -13,7 +13,7 @@ class Outfit < ActiveRecord::Base
 
   attr_accessible :name, :pet_state_id, :starred, :worn_and_unworn_item_ids
 
-  scope :wardrobe_order, order('starred DESC', :name)
+  scope :wardrobe_order, -> { order('starred DESC', :name) }
   
   # NOTE: We no longer save images, but we've left the code here for now.
   #       The `image` method below simulates the previous API for the rest
diff --git a/app/models/pet.rb b/app/models/pet.rb
index ead93f95..509c44a2 100644
--- a/app/models/pet.rb
+++ b/app/models/pet.rb
@@ -14,7 +14,7 @@ class Pet < ActiveRecord::Base
   attr_reader :items, :pet_state
   attr_accessor :contributor
 
-  scope :with_pet_type_color_ids, lambda { |color_ids|
+  scope :with_pet_type_color_ids, ->(color_ids) {
     joins(:pet_type).where(PetType.arel_table[:id].in(color_ids))
   }
 
diff --git a/app/models/pet_state.rb b/app/models/pet_state.rb
index beeea303..f7d41fc4 100644
--- a/app/models/pet_state.rb
+++ b/app/models/pet_state.rb
@@ -34,10 +34,12 @@ class PetState < ActiveRecord::Base
   # though, this strikes a good balance of bringing default to the front for
   # many pet types (the highest priority!) and otherwise doing decent sorting.
   bio_effect_zone_id = 4
-  scope :emotion_order, joins(:parent_swf_asset_relationships).
+  scope :emotion_order, -> {
+    joins(:parent_swf_asset_relationships).
     joins("LEFT JOIN swf_assets effect_assets ON effect_assets.id = parents_swf_assets.swf_asset_id AND effect_assets.zone_id = #{bio_effect_zone_id}").
     group("pet_states.id").
     order("glitched ASC, (mood_id = 1) DESC, COUNT(effect_assets.remote_id) ASC, COUNT(parents_swf_assets.swf_asset_id) DESC, female ASC, SUM(parents_swf_assets.swf_asset_id) ASC")
+  }
 
   def as_json(options={})
     {
diff --git a/app/models/pet_type.rb b/app/models/pet_type.rb
index 29241c2e..8eaf530f 100644
--- a/app/models/pet_type.rb
+++ b/app/models/pet_type.rb
@@ -15,12 +15,12 @@ class PetType < ActiveRecord::Base
 
   # Returns all pet types of a single standard color. The caller shouldn't care
   # which, though, in this implemention, it's always Blue. Don't depend on that.
-  scope :single_standard_color, lambda { where(:color_id => Color.basic.first) }
+  scope :single_standard_color, -> { where(:color_id => Color.basic.first) }
 
-  scope :nonstandard_colors, lambda { where(:color_id => Color.nonstandard) }
+  scope :nonstandard_colors, -> { where(:color_id => Color.nonstandard) }
   
   scope :includes_child_translations,
-    lambda { includes({:color => :translations, :species => :translations}) }
+    -> { includes({:color => :translations, :species => :translations}) }
 
   def self.special_color_or_basic(special_color)
     color_ids = special_color ? [special_color.id] : Color.basic.select([:id]).map(&:id)
diff --git a/app/models/species.rb b/app/models/species.rb
index 79f6d994..4b827c44 100644
--- a/app/models/species.rb
+++ b/app/models/species.rb
@@ -1,7 +1,7 @@
 class Species < ActiveRecord::Base
   translates :name
   
-  scope :alphabetical, lambda { with_translations(I18n.locale).order(Species::Translation.arel_table[:name]) }
+  scope :alphabetical, -> { with_translations(I18n.locale).order(Species::Translation.arel_table[:name]) }
   
   def as_json(options={})
     {:id => id, :name => human_name}
diff --git a/app/models/swf_asset.rb b/app/models/swf_asset.rb
index 6cad40a3..e8b4ca78 100644
--- a/app/models/swf_asset.rb
+++ b/app/models/swf_asset.rb
@@ -30,7 +30,7 @@ class SwfAsset < ActiveRecord::Base
   
   belongs_to :zone
   
-  scope :includes_depth, lambda { includes(:zone) }
+  scope :includes_depth, -> { includes(:zone) }
 
   def local_swf_path
     LOCAL_ASSET_DIR.join(local_path_within_outfit_swfs)
@@ -156,27 +156,27 @@ class SwfAsset < ActiveRecord::Base
     @body_ids_fitting_standard ||= PetType.standard_body_ids + [0]
   end
 
-  scope :fitting_body_id, lambda { |body_id|
+  scope :fitting_body_id, ->(body_id) {
     where(arel_table[:body_id].in([body_id, 0]))
   }
 
-  scope :fitting_standard_body_ids, lambda {
+  scope :fitting_standard_body_ids, -> {
     where(arel_table[:body_id].in(body_ids_fitting_standard))
   }
 
-  scope :fitting_color, lambda { |color|
+  scope :fitting_color, ->(color) {
     body_ids = PetType.select(:body_id).where(:color_id => color.id).map(&:body_id)
     body_ids << 0
     where(arel_table[:body_id].in(body_ids))
   }
 
-  scope :biology_assets, where(:type => PetState::SwfAssetType)
-  scope :object_assets, where(:type => Item::SwfAssetType)
-  scope :for_item_ids, lambda { |item_ids|
+  scope :biology_assets, -> { where(:type => PetState::SwfAssetType) }
+  scope :object_assets, -> { where(:type => Item::SwfAssetType) }
+  scope :for_item_ids, ->(item_ids) {
     joins(:parent_swf_asset_relationships).
       where(ParentSwfAssetRelationship.arel_table[:parent_id].in(item_ids))
   }
-  scope :with_parent_ids, lambda {
+  scope :with_parent_ids, -> {
     select('swf_assets.*, parents_swf_assets.parent_id')
   }
 
diff --git a/app/models/user.rb b/app/models/user.rb
index c709dc6a..f74407e0 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -13,7 +13,7 @@ class User < ActiveRecord::Base
 
   belongs_to :contact_neopets_connection, class_name: 'NeopetsConnection'
 
-  scope :top_contributors, order('points DESC').where('points > 0')
+  scope :top_contributors, -> { order('points DESC').where('points > 0') }
 
   devise :rememberable
 
diff --git a/app/models/wardrobe_tip.rb b/app/models/wardrobe_tip.rb
index 85278b4f..ae0c3178 100644
--- a/app/models/wardrobe_tip.rb
+++ b/app/models/wardrobe_tip.rb
@@ -1,5 +1,5 @@
 class WardrobeTip < ActiveRecord::Base
   translates :body
 
-  scope :by_index, order('`index` ASC')
+  scope :by_index, -> { order('`index` ASC') }
 end
diff --git a/app/models/zone.rb b/app/models/zone.rb
index 2b280907..b29312ab 100644
--- a/app/models/zone.rb
+++ b/app/models/zone.rb
@@ -5,17 +5,17 @@ class Zone < ActiveRecord::Base
   # whether or not the zone is "sometimes" occupied. This is false by default.
   attr_writer :sometimes
   
-  scope :alphabetical, lambda {
+  scope :alphabetical, -> {
     with_translations(I18n.locale).order(Zone::Translation.arel_table[:label])
   }
-  scope :includes_translations, lambda { includes(:translations) }
-  scope :with_plain_label, lambda { |label|
+  scope :includes_translations, -> { includes(:translations) }
+  scope :with_plain_label, ->(label) {
     t = Zone::Translation.arel_table
     includes(:translations)
       .where(t[:plain_label].eq(Zone.plainify_label(label)))
       .where(t[:locale].eq(I18n.locale))
   }
-  scope :for_items, lambda { where(arel_table[:type_id].gt(1)) }
+  scope :for_items, -> { where(arel_table[:type_id].gt(1)) }
 
   def uncertain_label
     @sometimes ? "#{label} sometimes" : label