Merge pull request #751 from jhawthorn/manual_rubocop_fixes

Manual rubocop fixes

.rubocop.yml

@@ -22,6 +22,9 @@ Style/ClassVars:
 Style/PredicateName:
  Enabled: false
 
+Style/AccessorMethodName:
+ Enabled: false
+
 # This has been used for customization
 Style/MutableConstant:
  Enabled: false
@@ -41,6 +44,20 @@ Style/ConditionalAssignment:
 Performance/Count:
  Enabled: false
 
+Style/RaiseArgs:
+ Enabled: false
+
+Style/OpMethod:
+ Enabled: false
+
+# We can use good judgement here
+Style/RegexpLiteral:
+ Enabled: false
+
+# Unicode comments are useful
+Style/AsciiComments:
+ Enabled: false
+
 Lint/EndAlignment:
  Enabled: false
 

Rakefile

@@ -4,7 +4,6 @@ begin
  require 'spree/testing_support/common_rake'
 rescue LoadError
  raise "Could not find spree/testing_support/common_rake. You need to run this command using Bundler."
- exit
 end
 
 task default: :test

api/app/controllers/spree/api/images_controller.rb

@@ -37,9 +37,9 @@ def image_params
 
  def scope
  if params[:product_id]
- scope = Spree::Product.friendly.find(params[:product_id])
+ Spree::Product.friendly.find(params[:product_id])
  elsif params[:variant_id]
- scope = Spree::Variant.find(params[:variant_id])
+ Spree::Variant.find(params[:variant_id])
  end
  end
  end

api/app/controllers/spree/api/taxons_controller.rb

@@ -4,12 +4,10 @@ class TaxonsController < Spree::Api::BaseController
  def index
  if taxonomy
  @taxons = taxonomy.root.children
+ elsif params[:ids]
+ @taxons = Spree::Taxon.accessible_by(current_ability, :read).where(id: params[:ids].split(','))
  else
- if params[:ids]
- @taxons = Spree::Taxon.accessible_by(current_ability, :read).where(id: params[:ids].split(','))
- else
- @taxons = Spree::Taxon.accessible_by(current_ability, :read).order(:taxonomy_id, :lft).ransack(params[:q]).result
- end
+ @taxons = Spree::Taxon.accessible_by(current_ability, :read).order(:taxonomy_id, :lft).ransack(params[:q]).result
  end
 
  @taxons = @taxons.page(params[:page]).per(params[:per_page])

api/app/helpers/spree/api/api_helpers.rb

@@ -37,7 +37,7 @@ module ApiHelpers
  :variant_property_attributes
  ]
 
- mattr_reader *ATTRIBUTES
+ mattr_reader(*ATTRIBUTES)
 
  def required_fields_for(model)
  required_fields = model._validators.select do |_field, validations|

api/lib/spree/api/engine.rb

@@ -28,7 +28,7 @@ def self.activate
  Rails.configuration.cache_classes ? require(c) : load(c)
  end
  end
- config.to_prepare &method(:activate).to_proc
+ config.to_prepare(&method(:activate).to_proc)
 
  def self.root
  @root ||= Pathname.new(File.expand_path('../../../../', __FILE__))

api/spec/controllers/spree/api/option_types_controller_spec.rb

@@ -35,7 +35,7 @@ def check_option_values(option_values)
 
  it "can retrieve a list of specific option types" do
  option_type_1 = create(:option_type)
- option_type_2 = create(:option_type)
+ create(:option_type)
  api_get :index, ids: "#{option_type.id},#{option_type_1.id}"
  expect(json_response.count).to eq(2)
 
@@ -90,7 +90,6 @@ def check_option_values(option_values)
  end
 
  it "can update an option type" do
- original_name = option_type.name
  api_put :update, id: option_type.id, option_type: {
  name: "Option Type"
  }

api/spec/controllers/spree/api/option_values_controller_spec.rb

@@ -49,7 +49,7 @@ def check_option_values(option_values)
 
  it "can retrieve a list of option types" do
  option_value_1 = create(:option_value, option_type: option_type)
- option_value_2 = create(:option_value, option_type: option_type)
+ create(:option_value, option_type: option_type)
  api_get :index, ids: [option_value.id, option_value_1.id]
  expect(json_response.count).to eq(2)
  end
@@ -101,7 +101,6 @@ def check_option_values(option_values)
  end
 
  it "can update an option value" do
- original_name = option_value.name
  api_put :update, id: option_value.id, option_value: {
  name: "Option Value"
  }

api/spec/controllers/spree/api/orders_controller_spec.rb

@@ -508,7 +508,6 @@ module Spree
  before { order.create_proposed_shipments }
 
  it "clears out all existing shipments on line item udpate" do
- previous_shipments = order.shipments
  api_put :update, id: order.to_param, order: {
  line_items: {
  0 => { id: line_item.id, quantity: 10 }
@@ -722,7 +721,6 @@ module Spree
  it "can create an order without any parameters" do
  api_post :create
  expect(response.status).to eq(201)
- order = Order.last
  expect(json_response["state"]).to eq("cart")
  end
 

api/spec/controllers/spree/api/products_controller_spec.rb

@@ -99,7 +99,7 @@ module Spree
 
  context "pagination" do
  it "can select the next page of products" do
- second_product = create(:product)
+ create(:product)
  api_get :index, page: 2, per_page: 1
  expect(json_response["products"].first).to have_attributes(show_attributes)
  expect(json_response["total_count"]).to eq(2)

api/spec/controllers/spree/api/stock_movements_controller_spec.rb

@@ -55,7 +55,7 @@ module Spree
  end
 
  it 'can query the results through a paramter' do
- expected_result = create(:stock_movement, :received, quantity: 10, stock_item: stock_item)
+ create(:stock_movement, :received, quantity: 10, stock_item: stock_item)
  api_get :index, stock_location_id: stock_location.to_param, q: { quantity_eq: '10' }
  expect(json_response['count']).to eq(1)
  end

api/spec/controllers/spree/api/taxonomies_controller_spec.rb

@@ -102,7 +102,6 @@ module Spree
  api_post :create, taxonomy: {}
  expect(response.status).to eq(422)
  expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.")
- errors = json_response["errors"]
  end
 
  it "can destroy" do

api/spec/controllers/spree/api/taxons_controller_spec.rb

@@ -150,7 +150,6 @@ module Spree
  api_post :create, taxonomy_id: taxonomy.id, taxon: {}
  expect(response.status).to eq(422)
  expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.")
- errors = json_response["errors"]
 
  expect(taxonomy.reload.root.children.count).to eq 1
  end

api/spec/controllers/spree/api/users_controller_spec.rb

@@ -40,7 +40,6 @@ module Spree
  api_post :create, user: {}, token: user.spree_api_key
  expect(response.status).to eq(422)
  expect(json_response["error"]).to eq("Invalid resource. Please fix errors and try again.")
- errors = json_response["errors"]
  end
 
  it "can update own details" do

api/spec/controllers/spree/api/variants_controller_spec.rb

@@ -115,7 +115,7 @@ module Spree
 
  context "pagination" do
  it "can select the next page of variants" do
- second_variant = create(:variant)
+ create(:variant)
  api_get :index, page: 2, per_page: 1
  expect(json_response["variants"].first).to have_attributes(show_attributes)
  expect(json_response["total_count"]).to eq(3)

api/spec/features/checkout_spec.rb

@@ -92,7 +92,7 @@ def assert_order_expectations
  expect(@order.completed_at).to be_a ActiveSupport::TimeWithZone
  expect(@order.item_total).to eq 600.00
  expect(@order.total).to eq 600.00
- expect(@order.adjustment_total).to eq -10.00
+ expect(@order.adjustment_total).to eq(-10.00)
  expect(@order.shipment_total).to eq 10.00
  expect(@order.user).to eq @user
  expect(@order.bill_address).to eq bill_address

api/spec/requests/ransackable_attributes_spec.rb

@@ -33,16 +33,16 @@
 
  get "/api/variants?q[product_name_or_sku_cont]=fritos", token: user.spree_api_key
 
- skus = JSON.parse(response.body)['variants'].map { |variant| variant['sku'] }
+ skus = JSON.parse(response.body)['variants'].map { |x| x['sku'] }
  expect(skus).to include variant.sku
  expect(skus).not_to include other_variant.sku
  end
  end
 
  context "filtering by attributes" do
  it "most attributes are not filterable by default" do
- product = create(:product, description: "special product")
- other_product = create(:product)
+ create(:product, description: "special product")
+ create(:product)
 
  get "/api/products?q[description_cont]=special", token: user.spree_api_key
 
@@ -56,7 +56,7 @@
 
  get "/api/products?q[id_eq]=#{product.id}", token: user.spree_api_key
 
- product_names = JSON.parse(response.body)['products'].map { |product| product['name'] }
+ product_names = JSON.parse(response.body)['products'].map { |x| x['name'] }
  expect(product_names).to include product.name
  expect(product_names).not_to include other_product.name
  end
@@ -69,7 +69,7 @@
 
  get "/api/products?q[name_cont]=fritos", token: user.spree_api_key
 
- product_names = JSON.parse(response.body)['products'].map { |product| product['name'] }
+ product_names = JSON.parse(response.body)['products'].map { |x| x['name'] }
  expect(product_names).to include product.name
  expect(product_names).not_to include other_product.name
  end

backend/app/controllers/spree/admin/base_controller.rb

@@ -57,7 +57,7 @@ def config_locale
 
  def lock_order
  OrderMutex.with_lock!(@order) { yield }
- rescue Spree::OrderMutex::LockFailed => e
+ rescue Spree::OrderMutex::LockFailed
  flash[:error] = Spree.t(:order_mutex_admin_error)
  redirect_to order_mutex_redirect_path
  end

backend/app/controllers/spree/admin/customer_returns_controller.rb

@@ -63,7 +63,7 @@ def build_return_items_from_params
  return_item = item_params[:id] ? Spree::ReturnItem.find(item_params[:id]) : Spree::ReturnItem.new
  return_item.assign_attributes(item_params)
  if item_params[:reception_status_event].blank?
- redirect_to(new_object_url, flash: { error: 'Reception status choice required' }) && return
+ return redirect_to(new_object_url, flash: { error: 'Reception status choice required' })
  end
  return_item
  end.compact

backend/app/controllers/spree/admin/orders/customer_details_controller.rb

@@ -22,7 +22,7 @@ def update
 
  if should_associate_user?
  requested_user = Spree.user_class.find(params[:user_id])
- @order.associate_user!(Spree.user_class.find(params[:user_id]), @order.email.blank?)
+ @order.associate_user!(requested_user, @order.email.blank?)
  end
 
  unless @order.completed?

backend/app/controllers/spree/admin/orders_controller.rb

@@ -98,11 +98,10 @@ def advance
 
  if @order.confirm?
  flash[:success] = Spree.t('order_ready_for_confirm')
- redirect_to confirm_admin_order_url(@order)
  else
  flash[:error] = @order.errors.full_messages
- redirect_to confirm_admin_order_url(@order)
  end
+ redirect_to confirm_admin_order_url(@order)
  end
  end
 

backend/app/controllers/spree/admin/stock_transfers_controller.rb

@@ -129,8 +129,11 @@ def ensure_access_to_stock_location
  end
 
  def source_location
- @source_location ||= params.key?(:transfer_receive_stock) ? nil :
+ @source_location ||= if params.key?(:transfer_receive_stock)
+ nil
+ else
  StockLocation.find(params[:transfer_source_location_id])
+ end
  end
 
  def destination_location

backend/app/helpers/spree/admin/products_helper.rb

@@ -2,18 +2,18 @@ module Spree
  module Admin
  module ProductsHelper
  def taxon_options_for(product)
- options = @taxons.map do |taxon|
+ @taxons.map do |taxon|
  selected = product.taxons.include?(taxon)
  content_tag(:option,
  value: taxon.id,
  selected: ('selected' if selected)) do
- (taxon.ancestors.pluck(:name) + [taxon.name]).join(" -> ")
- end
+  (taxon.ancestors.pluck(:name) + [taxon.name]).join(" -> ")
+  end
  end.join("").html_safe
  end
 
  def option_types_options_for(product)
- options = @option_types.map do |option_type|
+ @option_types.map do |option_type|
  selected = product.option_types.include?(option_type)
  content_tag(:option,
  value: option_type.id,

backend/spec/controllers/spree/admin/cancellations_controller_spec.rb

@@ -20,7 +20,7 @@
  describe "#cancel" do
  subject { spree_post :short_ship, order_id: order.number, inventory_unit_ids: inventory_units.map(&:id) }
 
- let(:order) { order = create(:order_ready_to_ship, number: "R100", state: "complete", line_items_count: 1) }
+ let(:order) { create(:order_ready_to_ship, number: "R100", state: "complete", line_items_count: 1) }
  let(:referer) { "order_admin_page" }
 
  context "no inventory unit ids are provided" do

backend/spec/controllers/spree/admin/customer_returns_controller_spec.rb

@@ -65,7 +65,7 @@ module Admin
  total_inventory_count = 4
  rma_return_items_count = 1
  customer_return_return_items_count = 1
- expect(assigns(:new_return_items).length).to eq (total_inventory_count - rma_return_items_count - customer_return_return_items_count)
+ expect(assigns(:new_return_items).length).to eq(total_inventory_count - rma_return_items_count - customer_return_return_items_count)
  end
 
  it "builds new return items" do

backend/spec/controllers/spree/admin/orders_controller_spec.rb

@@ -179,7 +179,7 @@
 
  it 'messages and redirects' do
  subject
- expect(flash[:error]) == order.errors.full_messages
+ expect(flash[:error]).to eq order.errors.full_messages
  expect(response).to redirect_to(spree.confirm_admin_order_path(order))
  end
  end

backend/spec/controllers/spree/admin/payments_controller_spec.rb

@@ -128,7 +128,6 @@ module Admin
  end
 
  describe '#fire' do
-
  describe 'authorization' do
  let(:payment) { create(:payment, state: 'checkout') }
  let(:order) { payment.order }

backend/spec/features/admin/configuration/stock_locations_spec.rb

@@ -4,7 +4,7 @@
  stub_authorization!
 
  before(:each) do
- country = create(:country)
+ create(:country)
  visit spree.admin_path
  click_link "Settings"
  click_link "Stock Locations"
@@ -21,7 +21,7 @@
  end
 
  it "can delete an existing stock location", js: true do
- location = create(:stock_location)
+ create(:stock_location)
  visit current_path
 
  expect(find('#listing_stock_locations')).to have_content("NY Warehouse")

backend/spec/features/admin/orders/adjustments_promotions_spec.rb

@@ -4,13 +4,13 @@
  stub_authorization!
 
  before(:each) do
- promotion = create(:promotion_with_item_adjustment,
-  name: "$10 off",
-  path: 'test',
-  code: "10_off",
-  starts_at: 1.day.ago,
-  expires_at: 1.day.from_now,
-  adjustment_rate: 10)
+ create(:promotion_with_item_adjustment,
+ name: "$10 off",
+ path: 'test',
+ code: "10_off",
+ starts_at: 1.day.ago,
+ expires_at: 1.day.from_now,
+ adjustment_rate: 10)
 
  order = create(:order_with_totals)
  line_item = order.line_items.first