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.
This commit is contained in:
Matchu 2023-07-22 14:04:01 -07:00 committed by Matchu
parent ed59a874ff
commit a29e016555
14 changed files with 59 additions and 51 deletions

View file

@ -15,17 +15,18 @@ class ClosetHanger < ActiveRecord::Base
validate :list_belongs_to_user validate :list_belongs_to_user
scope :alphabetical_by_item_name, lambda { scope :alphabetical_by_item_name, -> {
joins(:item => :translations). joins(:item => :translations).
where(Item::Translation.arel_table[:locale].eq(I18n.locale)). where(Item::Translation.arel_table[:locale].eq(I18n.locale)).
order(Item::Translation.arel_table[:name]) order(Item::Translation.arel_table[:name])
} }
scope :newest, order(arel_table[:created_at].desc) scope :newest, -> { order(arel_table[:created_at].desc) }
scope :owned_before_wanted, order(arel_table[:owned].desc) scope :owned_before_wanted, -> { order(arel_table[:owned].desc) }
scope :unlisted, where(:list_id => nil) scope :unlisted, -> { where(:list_id => nil) }
{:owned => true, :wanted => false}.each do |name, owned| {:owned => true, :wanted => false}.each do |name, owned|
scope "#{name}_trading", joins(:user).includes(:list). scope "#{name}_trading", -> {
joins(:user).includes(:list).
where(:owned => owned). where(:owned => owned).
where(( where((
arel_table[:list_id].eq(nil).and( arel_table[:list_id].eq(nil).and(
@ -34,6 +35,7 @@ class ClosetHanger < ActiveRecord::Base
).or( ).or(
ClosetList.arel_table[:visibility].gteq(ClosetVisibility[:trading].id) ClosetList.arel_table[:visibility].gteq(ClosetVisibility[:trading].id)
)) ))
}
end end
before_validation :merge_quantities, :set_owned_by_list before_validation :merge_quantities, :set_owned_by_list

View file

@ -9,9 +9,9 @@ class ClosetList < ActiveRecord::Base
validates :user, :presence => true validates :user, :presence => true
validates :hangers_owned, :inclusion => {:in => [true, false], :message => "can't be blank"} validates :hangers_owned, :inclusion => {:in => [true, false], :message => "can't be blank"}
scope :alphabetical, order(:name) scope :alphabetical, -> { order(:name) }
scope :public, where(arel_table[:visibility].gteq(ClosetVisibility[:public].id)) scope :public, -> { where(arel_table[:visibility].gteq(ClosetVisibility[:public].id)) }
scope :visible_to, lambda { |user| scope :visible_to, ->(user) {
condition = arel_table[:visibility].gteq(ClosetVisibility[:public].id) condition = arel_table[:visibility].gteq(ClosetVisibility[:public].id)
condition = condition.or(arel_table[:user_id].eq(user.id)) if user condition = condition.or(arel_table[:user_id].eq(user.id)) if user
where(condition) where(condition)

View file

@ -1,11 +1,11 @@
class Color < ActiveRecord::Base class Color < ActiveRecord::Base
translates :name translates :name
scope :alphabetical, lambda { with_translations(I18n.locale).order(Color::Translation.arel_table[:name]) } scope :alphabetical, -> { with_translations(I18n.locale).order(Color::Translation.arel_table[:name]) }
scope :basic, where(:basic => true) scope :basic, -> { where(:basic => true) }
scope :standard, where(:standard => true) scope :standard, -> { where(:standard => true) }
scope :nonstandard, where(:standard => false) scope :nonstandard, -> { where(:standard => false) }
scope :funny, lambda { order(:prank) unless pranks_funny? } scope :funny, -> { order(:prank) unless pranks_funny? }
validates :name, presence: true validates :name, presence: true

View file

@ -9,7 +9,7 @@ class Contribution < ActiveRecord::Base
belongs_to :contributed, :polymorphic => true belongs_to :contributed, :polymorphic => true
belongs_to :user belongs_to :user
scope :recent, order('id DESC') scope :recent, -> { order('id DESC') }
cattr_reader :per_page cattr_reader :per_page
@@per_page = 30 @@per_page = 30

View file

@ -23,23 +23,27 @@ class Item < ActiveRecord::Base
cattr_reader :per_page cattr_reader :per_page
@@per_page = 30 @@per_page = 30
scope :alphabetize_by_translations, lambda { scope :alphabetize_by_translations, -> {
it = Item::Translation.arel_table it = Item::Translation.arel_table
order(it[:name]) 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 :sold_in_mall, -> { where(:sold_in_mall => true) }
scope :not_sold_in_mall, where(:sold_in_mall => false) 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? def closeted?
@owned || @wanted @owned || @wanted

View file

@ -13,7 +13,7 @@ class Outfit < ActiveRecord::Base
attr_accessible :name, :pet_state_id, :starred, :worn_and_unworn_item_ids 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. # 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 # The `image` method below simulates the previous API for the rest

View file

@ -14,7 +14,7 @@ class Pet < ActiveRecord::Base
attr_reader :items, :pet_state attr_reader :items, :pet_state
attr_accessor :contributor 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)) joins(:pet_type).where(PetType.arel_table[:id].in(color_ids))
} }

View file

@ -34,10 +34,12 @@ class PetState < ActiveRecord::Base
# though, this strikes a good balance of bringing default to the front for # though, this strikes a good balance of bringing default to the front for
# many pet types (the highest priority!) and otherwise doing decent sorting. # many pet types (the highest priority!) and otherwise doing decent sorting.
bio_effect_zone_id = 4 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}"). 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"). 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") 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={}) def as_json(options={})
{ {

View file

@ -15,12 +15,12 @@ class PetType < ActiveRecord::Base
# Returns all pet types of a single standard color. The caller shouldn't care # 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. # 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, scope :includes_child_translations,
lambda { includes({:color => :translations, :species => :translations}) } -> { includes({:color => :translations, :species => :translations}) }
def self.special_color_or_basic(special_color) def self.special_color_or_basic(special_color)
color_ids = special_color ? [special_color.id] : Color.basic.select([:id]).map(&:id) color_ids = special_color ? [special_color.id] : Color.basic.select([:id]).map(&:id)

View file

@ -1,7 +1,7 @@
class Species < ActiveRecord::Base class Species < ActiveRecord::Base
translates :name 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={}) def as_json(options={})
{:id => id, :name => human_name} {:id => id, :name => human_name}

View file

@ -30,7 +30,7 @@ class SwfAsset < ActiveRecord::Base
belongs_to :zone belongs_to :zone
scope :includes_depth, lambda { includes(:zone) } scope :includes_depth, -> { includes(:zone) }
def local_swf_path def local_swf_path
LOCAL_ASSET_DIR.join(local_path_within_outfit_swfs) 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] @body_ids_fitting_standard ||= PetType.standard_body_ids + [0]
end end
scope :fitting_body_id, lambda { |body_id| scope :fitting_body_id, ->(body_id) {
where(arel_table[:body_id].in([body_id, 0])) 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)) 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 = PetType.select(:body_id).where(:color_id => color.id).map(&:body_id)
body_ids << 0 body_ids << 0
where(arel_table[:body_id].in(body_ids)) where(arel_table[:body_id].in(body_ids))
} }
scope :biology_assets, where(:type => PetState::SwfAssetType) scope :biology_assets, -> { where(:type => PetState::SwfAssetType) }
scope :object_assets, where(:type => Item::SwfAssetType) scope :object_assets, -> { where(:type => Item::SwfAssetType) }
scope :for_item_ids, lambda { |item_ids| scope :for_item_ids, ->(item_ids) {
joins(:parent_swf_asset_relationships). joins(:parent_swf_asset_relationships).
where(ParentSwfAssetRelationship.arel_table[:parent_id].in(item_ids)) 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') select('swf_assets.*, parents_swf_assets.parent_id')
} }

View file

@ -13,7 +13,7 @@ class User < ActiveRecord::Base
belongs_to :contact_neopets_connection, class_name: 'NeopetsConnection' 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 devise :rememberable

View file

@ -1,5 +1,5 @@
class WardrobeTip < ActiveRecord::Base class WardrobeTip < ActiveRecord::Base
translates :body translates :body
scope :by_index, order('`index` ASC') scope :by_index, -> { order('`index` ASC') }
end end

View file

@ -5,17 +5,17 @@ class Zone < ActiveRecord::Base
# whether or not the zone is "sometimes" occupied. This is false by default. # whether or not the zone is "sometimes" occupied. This is false by default.
attr_writer :sometimes attr_writer :sometimes
scope :alphabetical, lambda { scope :alphabetical, -> {
with_translations(I18n.locale).order(Zone::Translation.arel_table[:label]) with_translations(I18n.locale).order(Zone::Translation.arel_table[:label])
} }
scope :includes_translations, lambda { includes(:translations) } scope :includes_translations, -> { includes(:translations) }
scope :with_plain_label, lambda { |label| scope :with_plain_label, ->(label) {
t = Zone::Translation.arel_table t = Zone::Translation.arel_table
includes(:translations) includes(:translations)
.where(t[:plain_label].eq(Zone.plainify_label(label))) .where(t[:plain_label].eq(Zone.plainify_label(label)))
.where(t[:locale].eq(I18n.locale)) .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 def uncertain_label
@sometimes ? "#{label} sometimes" : label @sometimes ? "#{label} sometimes" : label