Skip to content

Commit

Permalink
Replace ActiveRecord dynamic finders
Browse files Browse the repository at this point in the history
Replaced find_by_attribute(value) style methods with with
find_by(attribute: value) syntax.

As per ActiveRecord documentation in lib/active_record/base.rb such
dynamic finders are mildy deprecated. This change also improves some
extensions compatibility.
  • Loading branch information
spaghetticode committed May 15, 2017
1 parent d97a9e9 commit 2c775ab
Show file tree
Hide file tree
Showing 31 changed files with 88 additions and 88 deletions.
2 changes: 1 addition & 1 deletion api/app/controllers/spree/api/promotions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def requires_admin
end

def load_promotion
@promotion = Spree::Promotion.find_by_id(params[:id]) || Spree::Promotion.with_coupon_code(params[:id])
@promotion = Spree::Promotion.find_by(id: params[:id]) || Spree::Promotion.with_coupon_code(params[:id])
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion api/spec/controllers/spree/api/products_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ module Spree
api_post :create, product: hash
expect(response.status).to eq 201

shipping_id = ShippingCategory.find_by_name("Free Ships").id
shipping_id = ShippingCategory.find_by(name: "Free Ships").id
expect(json_response['shipping_category_id']).to eq shipping_id
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def whodunnit
end

def load_order
@order = Spree::Order.find_by_number!(params[:order_id])
@order = Spree::Order.find_by!(number: params[:order_id])
authorize! action, @order
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def order_params
end

def load_order
@order = Spree::Order.includes(:adjustments).find_by_number!(params[:order_id])
@order = Spree::Order.includes(:adjustments).find_by!(number: params[:order_id])
end

def model_class
Expand Down
4 changes: 2 additions & 2 deletions backend/app/controllers/spree/admin/orders_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def index
end

def new
user = Spree.user_class.find_by_id(params[:user_id]) if params[:user_id]
user = Spree.user_class.find_by(id: params[:user_id]) if params[:user_id]
@order = Spree::Core::Importer::Order.import(user, order_params)
redirect_to cart_admin_order_url(@order)
end
Expand Down Expand Up @@ -178,7 +178,7 @@ def order_params
end

def load_order
@order = Spree::Order.includes(:adjustments).find_by_number!(params[:id])
@order = Spree::Order.includes(:adjustments).find_by!(number: params[:id])
authorize! action, @order
end

Expand Down
4 changes: 2 additions & 2 deletions backend/app/controllers/spree/admin/payments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def new
def create
@payment = PaymentCreate.new(@order, object_params).build
if @payment.payment_method.source_required? && params[:card].present? && params[:card] != 'new'
@payment.source = @payment.payment_method.payment_source_class.find_by_id(params[:card])
@payment.source = @payment.payment_method.payment_source_class.find_by(id: params[:card])
end

begin
Expand Down Expand Up @@ -87,7 +87,7 @@ def load_data
end

def load_order
@order = Spree::Order.find_by_number!(params[:order_id])
@order = Spree::Order.find_by!(number: params[:order_id])
authorize! action, @order
@order
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

let(:order) { create(:order, number: "R123456789") }

before { allow(Spree::Order).to receive(:find_by_number!) { order } }
before { allow(Spree::Order).to receive_message_chain(:includes, :find_by!) { order } }

context "#update" do
it "updates + progresses the order" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
let(:adjustments) { double('adjustments') }

before do
allow(Spree::Order).to receive_messages(find_by_number!: order)
allow(Spree::Order).to receive_message_chain(:includes, find_by!: order)
allow(order).to receive_messages(contents: Spree::OrderContents.new(order))
end

Expand Down Expand Up @@ -96,7 +96,7 @@

context "when a user_id is passed as a parameter" do
let(:user) { mock_model(Spree.user_class) }
before { allow(Spree.user_class).to receive_messages find_by_id: user }
before { allow(Spree.user_class).to receive_messages find_by: user }

it "imports a new order and assigns the user to the order" do
expect(Spree::Core::Importer::Order).to receive(:import)
Expand Down
18 changes: 9 additions & 9 deletions backend/spec/features/admin/promotion_adjustments_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
within('.calculator-fields') { fill_in "Amount", with: 5 }
within('#actions_container') { click_button "Update" }

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")
expect(promotion.codes.first.value).to eq("order")

first_rule = promotion.rules.first
Expand Down Expand Up @@ -60,7 +60,7 @@
within('#action_fields') { fill_in "Amount", with: "5" }
within('#actions_container') { click_button "Update" }

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")
expect(promotion.usage_limit).to eq(1)
expect(promotion.codes.first.value).to eq("single_use")

Expand Down Expand Up @@ -90,7 +90,7 @@
within('.calculator-fields') { fill_in "Flat Percent", with: "10" }
within('#actions_container') { click_button "Update" }

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")
expect(promotion.codes.first).to be_nil

first_rule = promotion.rules.first
Expand Down Expand Up @@ -124,7 +124,7 @@
within('.calculator-fields') { fill_in "Percent", with: "10" }
within('#actions_container') { click_button "Update" }

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")
expect(promotion.codes.first).to be_nil

first_rule = promotion.rules.first
Expand Down Expand Up @@ -153,7 +153,7 @@
within('#action_fields') { click_button "Add" }
expect(page).to have_content('Makes all shipments for the order free')

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")
expect(promotion.codes).to be_empty
expect(promotion.rules.first).to be_a(Spree::Promotion::Rules::ItemTotal)
expect(promotion.actions.first).to be_a(Spree::Promotion::Actions::FreeShipping)
Expand All @@ -165,7 +165,7 @@
click_button "Create"
expect(page).to have_content("PromotionsPromotion")

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")
expect(promotion).to be_apply_automatically
expect(promotion.path).to be_nil
expect(promotion.codes).to be_empty
Expand All @@ -179,7 +179,7 @@
click_button "Create"
expect(page).to have_content("PromotionsPromotion")

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")
expect(promotion.path).to eq("content/cvv")
expect(promotion).not_to be_apply_automatically
expect(promotion.codes).to be_empty
Expand All @@ -194,7 +194,7 @@
click_button "Create"
expect(page).to have_content("PromotionsPromotion")

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")
expect(promotion.path).to be_nil
expect(promotion).not_to be_apply_automatically
expect(promotion.rules).to be_blank
Expand All @@ -220,7 +220,7 @@
within('.calculator-fields') { fill_in "Amount", with: "5" }
within('#actions_container') { click_button "Update" }

promotion = Spree::Promotion.find_by_name("Promotion")
promotion = Spree::Promotion.find_by(name: "Promotion")

first_rule = promotion.rules.first
expect(first_rule.class).to eq(Spree::Promotion::Rules::ItemTotal)
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/payment_method/store_credit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def handle_action_call(store_credit, action, action_name, auth_code = nil)

def handle_action(action, action_name, auth_code)
# Find first event with provided auth_code
store_credit = Spree::StoreCreditEvent.find_by_authorization_code(auth_code).try(:store_credit)
store_credit = Spree::StoreCreditEvent.find_by(authorization_code: auth_code).try(:store_credit)

if store_credit.nil?
ActiveMerchant::Billing::Response.new(false, Spree.t('store_credit.unable_to_find_for_action', auth_code: auth_code, action: action_name), {}, {})
Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/preferences/store.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def get(key)
# has been cleared from the cache

# does it exist in the database?
if preference = Spree::Preference.find_by_key(key)
if preference = Spree::Preference.find_by(key: key)
# it does exist
val = preference.value
else
Expand Down Expand Up @@ -74,7 +74,7 @@ def persist(cache_key, value)
def destroy(cache_key)
return unless should_persist?

preference = Spree::Preference.find_by_key(cache_key)
preference = Spree::Preference.find_by(key: cache_key)
preference.destroy if preference
end

Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/store_credit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ def validate_no_amount_used
def associate_credit_type
unless type_id
credit_type_name = category.try(:non_expiring?) ? Spree.t("store_credit.non_expiring") : Spree.t("store_credit.expiring")
self.credit_type = Spree::StoreCreditType.find_by_name(credit_type_name)
self.credit_type = Spree::StoreCreditType.find_by(name: credit_type_name)
end
end
end
2 changes: 1 addition & 1 deletion core/app/models/spree/store_credit_event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def display_action
end

def order
Spree::Payment.find_by_response_code(authorization_code).try(:order)
Spree::Payment.find_by(response_code: authorization_code).try(:order)
end
end
end
8 changes: 4 additions & 4 deletions core/lib/spree/core/importer/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def self.create_shipments_from_params(shipments_hash, order)
shipments_hash.each do |s|
shipment = Shipment.new
shipment.tracking = s[:tracking]
shipment.stock_location = Spree::StockLocation.find_by_admin_name(s[:stock_location]) || Spree::StockLocation.find_by_name!(s[:stock_location])
shipment.stock_location = Spree::StockLocation.find_by(admin_name: s[:stock_location]) || Spree::StockLocation.find_by!(name: s[:stock_location])

inventory_units = s[:inventory_units] || []
inventory_units.each do |iu|
Expand Down Expand Up @@ -87,7 +87,7 @@ def self.create_shipments_from_params(shipments_hash, order)
order.shipments << shipment
shipment.save!

shipping_method = Spree::ShippingMethod.find_by_name(s[:shipping_method]) || Spree::ShippingMethod.find_by_admin_name!(s[:shipping_method])
shipping_method = Spree::ShippingMethod.find_by(name: s[:shipping_method]) || Spree::ShippingMethod.find_by!(admin_name: s[:shipping_method])
rate = shipment.shipping_rates.create!(shipping_method: shipping_method,
cost: s[:cost])
shipment.selected_shipping_rate_id = rate.id
Expand Down Expand Up @@ -131,7 +131,7 @@ def self.create_payments_from_params(payments_hash, order)
# Order API should be using state as that's the normal payment field.
# spree_wombat serializes payment state as status so imported orders should fall back to status field.
payment.state = p[:state] || p[:status] || 'completed'
payment.payment_method = Spree::PaymentMethod.find_by_name!(p[:payment_method])
payment.payment_method = Spree::PaymentMethod.find_by!(name: p[:payment_method])
payment.source = create_source_payment_from_params(p[:source], payment) if p[:source]
payment.save!
end
Expand All @@ -154,7 +154,7 @@ def self.create_source_payment_from_params(source_hash, payment)
def self.ensure_variant_id_from_params(hash)
sku = hash.delete(:sku)
unless hash[:variant_id].present?
hash[:variant_id] = Spree::Variant.with_prices.find_by_sku!(sku).id
hash[:variant_id] = Spree::Variant.with_prices.find_by!(sku: sku).id
end
hash
end
Expand Down
4 changes: 2 additions & 2 deletions core/lib/tasks/migrations/migrate_user_addresses.rake
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ namespace 'spree:migrations:migrate_user_addresses' do

task up: :environment do
Spree.user_class.find_each(batch_size: 500) do |user|
ship_address = Spree::Address.find_by_id(user.ship_address_id)
bill_address = Spree::Address.find_by_id(user.bill_address_id)
ship_address = Spree::Address.find_by(id: user.ship_address_id)
bill_address = Spree::Address.find_by(id: user.bill_address_id)

current_addresses = [bill_address, ship_address].compact.uniq

Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/preference_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def round_trip_preference(key, value)
p.key = key
p.save

Spree::Preference.find_by_key(key)
Spree::Preference.find_by(key: key)
end

it ":boolean" do
Expand Down
4 changes: 2 additions & 2 deletions frontend/app/controllers/spree/orders_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class OrdersController < Spree::StoreController
skip_before_action :verify_authenticity_token, only: [:populate]

def show
@order = Spree::Order.find_by_number!(params[:id])
@order = Spree::Order.find_by!(number: params[:id])
end

def update
Expand Down Expand Up @@ -93,7 +93,7 @@ def accurate_title

def check_authorization
cookies.permanent.signed[:guest_token] = params[:token] if params[:token]
order = Spree::Order.find_by_number(params[:id]) || current_order
order = Spree::Order.find_by(number: params[:id]) || current_order

if order
authorize! :edit, order, cookies.signed[:guest_token]
Expand Down
2 changes: 1 addition & 1 deletion frontend/app/controllers/spree/taxons_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class TaxonsController < Spree::StoreController
respond_to :html

def show
@taxon = Spree::Taxon.find_by_permalink!(params[:id])
@taxon = Spree::Taxon.find_by!(permalink: params[:id])
return unless @taxon

@searcher = build_searcher(params.merge(taxon: @taxon.id, include_images: true))
Expand Down
2 changes: 1 addition & 1 deletion frontend/spec/controllers/spree/orders_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
post :populate
expect(cookies.signed[:guest_token]).not_to be_blank

order_by_token = Spree::Order.find_by_guest_token(cookies.signed[:guest_token])
order_by_token = Spree::Order.find_by(guest_token: cookies.signed[:guest_token])
assigned_order = assigns[:order]

expect(assigned_order).to eq order_by_token
Expand Down
10 changes: 5 additions & 5 deletions frontend/spec/features/products_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
end

describe 'meta tags and title' do
let(:jersey) { Spree::Product.find_by_name('Ruby on Rails Baseball Jersey') }
let(:jersey) { Spree::Product.find_by(name: 'Ruby on Rails Baseball Jersey') }
let(:metas) { { meta_description: 'Brand new Ruby on Rails Jersey', meta_title: 'Ruby on Rails Baseball Jersey Buy High Quality Geek Apparel', meta_keywords: 'ror, jersey, ruby' } }

it 'should return the correct title when displaying a single product' do
Expand Down Expand Up @@ -84,7 +84,7 @@
end

let!(:product) do
product = Spree::Product.find_by_name("Ruby on Rails Ringer T-Shirt")
product = Spree::Product.find_by(name: "Ruby on Rails Ringer T-Shirt")
product.price = 19.99
product.tap(&:save)
end
Expand Down Expand Up @@ -135,7 +135,7 @@
end

context "a product with variants" do
let(:product) { Spree::Product.find_by_name("Ruby on Rails Baseball Jersey") }
let(:product) { Spree::Product.find_by(name: "Ruby on Rails Baseball Jersey") }
let(:option_value) { create(:option_value) }
let!(:variant) { product.variants.create!(price: 5.59) }

Expand Down Expand Up @@ -168,7 +168,7 @@
end

context "a product with variants, images only for the variants" do
let(:product) { Spree::Product.find_by_name("Ruby on Rails Baseball Jersey") }
let(:product) { Spree::Product.find_by(name: "Ruby on Rails Baseball Jersey") }

before do
image = File.open(File.expand_path('../../fixtures/thinking-cat.jpg', __FILE__))
Expand Down Expand Up @@ -258,7 +258,7 @@
end

it "should return the correct title when displaying a single product" do
product = Spree::Product.find_by_name("Ruby on Rails Baseball Jersey")
product = Spree::Product.find_by(name: "Ruby on Rails Baseball Jersey")
click_link product.name

within("div#product-description") do
Expand Down
4 changes: 2 additions & 2 deletions sample/db/samples/addresses.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
united_states = Spree::Country.find_by_iso!("US")
new_york = Spree::State.find_by_name!("New York")
united_states = Spree::Country.find_by!(iso: "US")
new_york = Spree::State.find_by!(name: "New York")

# Billing address
Spree::Address.create!(
Expand Down
18 changes: 9 additions & 9 deletions sample/db/samples/assets.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
Spree::Sample.load_sample("variants")

products = {}
products[:ror_baseball_jersey] = Spree::Product.find_by_name!("Ruby on Rails Baseball Jersey")
products[:ror_tote] = Spree::Product.find_by_name!("Ruby on Rails Tote")
products[:ror_bag] = Spree::Product.find_by_name!("Ruby on Rails Bag")
products[:ror_jr_spaghetti] = Spree::Product.find_by_name!("Ruby on Rails Jr. Spaghetti")
products[:ror_mug] = Spree::Product.find_by_name!("Ruby on Rails Mug")
products[:ror_ringer] = Spree::Product.find_by_name!("Ruby on Rails Ringer T-Shirt")
products[:ror_stein] = Spree::Product.find_by_name!("Ruby on Rails Stein")
products[:ruby_baseball_jersey] = Spree::Product.find_by_name!("Ruby Baseball Jersey")
products[:apache_baseball_jersey] = Spree::Product.find_by_name!("Apache Baseball Jersey")
products[:ror_baseball_jersey] = Spree::Product.find_by!(name: "Ruby on Rails Baseball Jersey")
products[:ror_tote] = Spree::Product.find_by!(name: "Ruby on Rails Tote")
products[:ror_bag] = Spree::Product.find_by!(name: "Ruby on Rails Bag")
products[:ror_jr_spaghetti] = Spree::Product.find_by!(name: "Ruby on Rails Jr. Spaghetti")
products[:ror_mug] = Spree::Product.find_by!(name: "Ruby on Rails Mug")
products[:ror_ringer] = Spree::Product.find_by!(name: "Ruby on Rails Ringer T-Shirt")
products[:ror_stein] = Spree::Product.find_by!(name: "Ruby on Rails Stein")
products[:ruby_baseball_jersey] = Spree::Product.find_by!(name: "Ruby Baseball Jersey")
products[:apache_baseball_jersey] = Spree::Product.find_by!(name: "Apache Baseball Jersey")

def image(name, type = "jpeg")
images_path = Pathname.new(File.dirname(__FILE__)) + "images"
Expand Down
Loading

0 comments on commit 2c775ab

Please sign in to comment.