mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-03-27 06:05:19 +00:00
Merge pull request #13341 from chitty/rubocop-style--fixes
Rubocop Style corrections for Style/NestedModifier and Style/ReturnNilInPredicateMethodDefinition
This commit is contained in:
@@ -283,21 +283,6 @@ Style/MissingRespondToMissing:
|
||||
- 'app/models/spree/gateway.rb'
|
||||
- 'app/models/spree/preferences/configuration.rb'
|
||||
|
||||
# Offense count: 22
|
||||
# This cop supports safe autocorrection (--autocorrect).
|
||||
Style/NestedModifier:
|
||||
Exclude:
|
||||
- 'spec/controllers/admin/subscriptions_controller_spec.rb'
|
||||
- 'spec/controllers/line_items_controller_spec.rb'
|
||||
- 'spec/controllers/spree/admin/orders_controller_spec.rb'
|
||||
- 'spec/controllers/spree/orders_controller_spec.rb'
|
||||
- 'spec/factories/order_factory.rb'
|
||||
- 'spec/models/proxy_order_spec.rb'
|
||||
- 'spec/models/spree/line_item_spec.rb'
|
||||
- 'spec/services/place_proxy_order_spec.rb'
|
||||
- 'spec/system/admin/payments_stripe_spec.rb'
|
||||
- 'spec/system/admin/reports_spec.rb'
|
||||
|
||||
# Offense count: 38
|
||||
Style/OpenStructUse:
|
||||
Exclude:
|
||||
@@ -326,12 +311,3 @@ Style/OptionalBooleanParameter:
|
||||
- 'engines/order_management/app/services/order_management/stock/estimator.rb'
|
||||
- 'lib/spree/core/controller_helpers/order.rb'
|
||||
- 'spec/support/request/web_helper.rb'
|
||||
|
||||
# Offense count: 19
|
||||
# This cop supports unsafe autocorrection (--autocorrect-all).
|
||||
# Configuration parameters: AllowedMethods, AllowedPatterns.
|
||||
Style/ReturnNilInPredicateMethodDefinition:
|
||||
Exclude:
|
||||
- 'app/models/order_cycle.rb'
|
||||
- 'app/serializers/api/admin/customer_serializer.rb'
|
||||
- 'engines/order_management/app/services/order_management/subscriptions/validator.rb'
|
||||
|
||||
@@ -345,8 +345,8 @@ class OrderCycle < ApplicationRecord
|
||||
end
|
||||
|
||||
def orders_close_at_after_orders_open_at?
|
||||
return if orders_open_at.blank? || orders_close_at.blank?
|
||||
return if orders_close_at > orders_open_at
|
||||
return false if orders_open_at.blank? || orders_close_at.blank?
|
||||
return false if orders_close_at > orders_open_at
|
||||
|
||||
errors.add(:orders_close_at, :after_orders_open_at)
|
||||
end
|
||||
|
||||
@@ -25,7 +25,7 @@ module Api
|
||||
end
|
||||
|
||||
def default_card_present?
|
||||
return unless object.user
|
||||
return false unless object.user
|
||||
|
||||
object.user.default_card.present?
|
||||
end
|
||||
|
||||
@@ -41,53 +41,53 @@ module OrderManagement
|
||||
private
|
||||
|
||||
def shipping_method_allowed?
|
||||
return unless shipping_method
|
||||
return if shipping_method.distributors.include?(shop)
|
||||
return false unless shipping_method
|
||||
return false if shipping_method.distributors.include?(shop)
|
||||
|
||||
errors.add(:shipping_method, :not_available_to_shop, shop: shop.name)
|
||||
end
|
||||
|
||||
def payment_method_allowed?
|
||||
return unless payment_method
|
||||
return if payment_method.distributors.include?(shop)
|
||||
return false unless payment_method
|
||||
return false if payment_method.distributors.include?(shop)
|
||||
|
||||
errors.add(:payment_method, :not_available_to_shop, shop: shop.name)
|
||||
end
|
||||
|
||||
def payment_method_type_allowed?
|
||||
return unless payment_method
|
||||
return if Subscription::ALLOWED_PAYMENT_METHOD_TYPES.include? payment_method.type
|
||||
return false unless payment_method
|
||||
return false if Subscription::ALLOWED_PAYMENT_METHOD_TYPES.include? payment_method.type
|
||||
|
||||
errors.add(:payment_method, :invalid_type)
|
||||
end
|
||||
|
||||
def ends_at_after_begins_at?
|
||||
# Only validates ends_at if it is present
|
||||
return if begins_at.blank? || ends_at.blank?
|
||||
return if ends_at > begins_at
|
||||
return false if begins_at.blank? || ends_at.blank?
|
||||
return false if ends_at > begins_at
|
||||
|
||||
errors.add(:ends_at, :after_begins_at)
|
||||
end
|
||||
|
||||
def customer_allowed?
|
||||
return unless customer
|
||||
return if customer.enterprise == shop
|
||||
return false unless customer
|
||||
return false if customer.enterprise == shop
|
||||
|
||||
errors.add(:customer, :does_not_belong_to_shop, shop: shop.name)
|
||||
end
|
||||
|
||||
def schedule_allowed?
|
||||
return unless schedule
|
||||
return if schedule.coordinators.include?(shop)
|
||||
return false unless schedule
|
||||
return false if schedule.coordinators.include?(shop)
|
||||
|
||||
errors.add(:schedule, :not_coordinated_by_shop, shop: shop.name)
|
||||
end
|
||||
|
||||
def credit_card_ok?
|
||||
return unless customer && payment_method
|
||||
return unless stripe_payment_method?(payment_method)
|
||||
return false unless customer && payment_method
|
||||
return false unless stripe_payment_method?(payment_method)
|
||||
return errors.add(:payment_method, :charges_not_allowed) unless customer.allow_charges
|
||||
return if customer.user&.default_card.present?
|
||||
return false if customer.user&.default_card.present?
|
||||
|
||||
errors.add(:payment_method, :no_default_card)
|
||||
end
|
||||
@@ -97,7 +97,7 @@ module OrderManagement
|
||||
end
|
||||
|
||||
def subscription_line_items_present?
|
||||
return if subscription_line_items.any? { |sli|
|
||||
return false if subscription_line_items.any? { |sli|
|
||||
sli.quantity > 0 && !sli.marked_for_destruction?
|
||||
}
|
||||
|
||||
|
||||
@@ -458,7 +458,7 @@ RSpec.describe Admin::SubscriptionsController do
|
||||
}
|
||||
let!(:order) { proxy_order.initialise_order! }
|
||||
|
||||
before { break unless order.next! while !order.completed? }
|
||||
before { Orders::WorkflowService.new(order).complete! }
|
||||
|
||||
context "when no 'open_orders' directive has been provided" do
|
||||
it "renders an error, asking what to do" do
|
||||
@@ -562,7 +562,7 @@ RSpec.describe Admin::SubscriptionsController do
|
||||
}
|
||||
let!(:order) { proxy_order.initialise_order! }
|
||||
|
||||
before { break unless order.next! while !order.completed? }
|
||||
before { Orders::WorkflowService.new(order).complete! }
|
||||
|
||||
context "when no 'open_orders' directive has been provided" do
|
||||
it "renders an error, asking what to do" do
|
||||
@@ -668,7 +668,7 @@ RSpec.describe Admin::SubscriptionsController do
|
||||
}
|
||||
let!(:order) { proxy_order.initialise_order! }
|
||||
|
||||
before { break unless order.next! while !order.completed? }
|
||||
before { Orders::WorkflowService.new(order).complete! }
|
||||
|
||||
context "when no associated orders are 'canceled'" do
|
||||
it 'renders the unpaused subscription as json, leaves the order untouched' do
|
||||
|
||||
@@ -11,7 +11,7 @@ RSpec.describe LineItemsController do
|
||||
let!(:completed_order) do
|
||||
order = create(:completed_order_with_totals, user:, distributor:,
|
||||
order_cycle:, line_items_count: 1)
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
order
|
||||
end
|
||||
|
||||
@@ -35,7 +35,7 @@ RSpec.describe LineItemsController do
|
||||
let(:item) do
|
||||
order = create(:completed_order_with_totals)
|
||||
item = create(:line_item, order:)
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
item
|
||||
end
|
||||
|
||||
@@ -177,7 +177,7 @@ RSpec.describe LineItemsController do
|
||||
order_cycle:, line_items_count: 2)
|
||||
order.reload.line_items.first.update(variant_id: variant1.id)
|
||||
order.line_items.last.update(variant_id: variant2.id)
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
order.recreate_all_fees!
|
||||
order
|
||||
end
|
||||
|
||||
@@ -85,7 +85,7 @@ RSpec.describe Spree::Admin::OrdersController do
|
||||
order_cycle:)
|
||||
order.reload.line_items.first.update(variant_id: variant1.id)
|
||||
order.line_items.last.update(variant_id: variant2.id)
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
order.recreate_all_fees!
|
||||
order
|
||||
end
|
||||
|
||||
@@ -293,7 +293,7 @@ RSpec.describe Spree::OrdersController do
|
||||
order_cycle:)
|
||||
order.reload.line_items.first.update(variant_id: variant1.id)
|
||||
order.reload.line_items.last.update(variant_id: variant2.id)
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
order.recreate_all_fees!
|
||||
order
|
||||
end
|
||||
|
||||
@@ -128,10 +128,12 @@ FactoryBot.define do
|
||||
payment_method: evaluator.payment_method)
|
||||
order.recreate_all_fees!
|
||||
order.ship_address = evaluator.ship_address
|
||||
break unless a = order.next! while !order.delivery?
|
||||
while !order.delivery?
|
||||
break unless a = order.next!
|
||||
end
|
||||
order.select_shipping_method(evaluator.shipping_method.id)
|
||||
|
||||
break unless a = order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -164,7 +166,7 @@ FactoryBot.define do
|
||||
create(:payment, state: "checkout", order:, amount: order.total,
|
||||
payment_method: evaluator.payment_method)
|
||||
order.ship_address = evaluator.ship_address
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
|
||||
order.update_columns(
|
||||
completed_at: evaluator.completed_at,
|
||||
@@ -260,7 +262,7 @@ FactoryBot.define do
|
||||
tax_category: evaluator.shipping_tax_category)
|
||||
|
||||
order.reload
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
order.reload
|
||||
end
|
||||
end
|
||||
|
||||
@@ -116,7 +116,7 @@ RSpec.describe ProxyOrder do
|
||||
allow(Spree::OrderMailer).to receive(:cancel_email) {
|
||||
double(:email, deliver_later: true)
|
||||
}
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
order.cancel
|
||||
order.reload
|
||||
end
|
||||
@@ -130,7 +130,7 @@ RSpec.describe ProxyOrder do
|
||||
end
|
||||
|
||||
context "and the order has not been cancelled" do
|
||||
before { break unless order.next! while !order.completed? }
|
||||
before { Orders::WorkflowService.new(order).complete! }
|
||||
|
||||
it "returns true and clears canceled_at" do
|
||||
expect(proxy_order.resume).to be true
|
||||
@@ -159,7 +159,7 @@ RSpec.describe ProxyOrder do
|
||||
allow(Spree::OrderMailer).to receive(:cancel_email) {
|
||||
double(:email, deliver_later: true)
|
||||
}
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
order.cancel
|
||||
end
|
||||
|
||||
@@ -172,7 +172,7 @@ RSpec.describe ProxyOrder do
|
||||
end
|
||||
|
||||
context "and the order has not been cancelled" do
|
||||
before { break unless order.next! while !order.completed? }
|
||||
before { Orders::WorkflowService.new(order).complete! }
|
||||
|
||||
it "returns false and does nothing" do
|
||||
expect(proxy_order.resume).to eq false
|
||||
|
||||
@@ -300,7 +300,7 @@ module Spree
|
||||
order.reload
|
||||
order.update_totals
|
||||
order.payments << create(:payment, amount: order.total)
|
||||
break unless order.next! until order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
order.payment_state = 'paid'
|
||||
order.select_shipping_method(shipping_method.id)
|
||||
order.shipment.update!(order)
|
||||
|
||||
@@ -52,7 +52,7 @@ RSpec.describe PlaceProxyOrder do
|
||||
|
||||
before do
|
||||
proxy_order.initialise_order!
|
||||
break unless order.next! while !order.completed?
|
||||
Orders::WorkflowService.new(order).complete!
|
||||
end
|
||||
|
||||
it "records an issue and ignores it" do
|
||||
|
||||
@@ -101,7 +101,7 @@ RSpec.describe '
|
||||
stub_payment_intents_post_request order:, stripe_account_header: true
|
||||
stub_successful_capture_request(order:)
|
||||
|
||||
break unless order.next! while !order.payment?
|
||||
Orders::WorkflowService.new(order).advance_to_payment
|
||||
end
|
||||
|
||||
it "adds a payment with state complete" do
|
||||
|
||||
@@ -261,15 +261,18 @@ RSpec.describe '
|
||||
|
||||
before do
|
||||
order1.reload
|
||||
break unless order1.next! until order1.delivery?
|
||||
while !order1.delivery?
|
||||
break unless order1.next!
|
||||
end
|
||||
|
||||
order1.select_shipping_method(shipping_method.id)
|
||||
order1.recreate_all_fees!
|
||||
break unless order1.next! until order1.payment?
|
||||
order_workflow = Orders::WorkflowService.new(order1)
|
||||
order_workflow.advance_to_payment
|
||||
|
||||
create(:payment, state: "checkout", order: order1, amount: order1.reload.total,
|
||||
payment_method: create(:payment_method, distributors: [distributor1]))
|
||||
break unless order1.next! until order1.complete?
|
||||
order_workflow.complete!
|
||||
|
||||
login_as_admin
|
||||
visit admin_reports_path
|
||||
|
||||
Reference in New Issue
Block a user