diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 38fa7f623d..a856678c9a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -23,9 +23,7 @@ Layout/AlignArray: - 'lib/open_food_network/order_and_distributor_report.rb' - 'lib/open_food_network/orders_and_fulfillments_report.rb' - 'lib/open_food_network/packing_report.rb' - - 'spec/controllers/cart_controller_spec.rb' - 'spec/lib/open_food_network/order_grouper_spec.rb' - - 'spec/services/cart_service_spec.rb' # Offense count: 122 # Cop supports --auto-correct. @@ -246,7 +244,6 @@ Layout/EmptyLines: - 'spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb' - 'spec/serializers/admin/for_order_cycle/supplied_product_serializer_spec.rb' - 'spec/serializers/credit_card_serializer_spec.rb' - - 'spec/services/cart_service_spec.rb' - 'spec/support/delayed_job_helper.rb' - 'spec/support/matchers/table_matchers.rb' @@ -646,7 +643,6 @@ Layout/SpaceAroundOperators: - 'lib/open_food_network/xero_invoices_report.rb' - 'lib/spree/product_filters.rb' - 'spec/controllers/admin/enterprises_controller_spec.rb' - - 'spec/controllers/cart_controller_spec.rb' - 'spec/features/admin/bulk_order_management_spec.rb' - 'spec/features/admin/bulk_product_update_spec.rb' - 'spec/features/consumer/shopping/checkout_spec.rb' @@ -811,7 +807,6 @@ Layout/SpaceInsideHashLiteralBraces: - 'spec/controllers/admin/order_cycles_controller_spec.rb' - 'spec/controllers/admin/subscriptions_controller_spec.rb' - 'spec/controllers/api/statuses_controller_spec.rb' - - 'spec/controllers/cart_controller_spec.rb' - 'spec/controllers/checkout_controller_spec.rb' - 'spec/controllers/enterprises_controller_spec.rb' - 'spec/controllers/shop_controller_spec.rb' @@ -862,7 +857,6 @@ Layout/SpaceInsideHashLiteralBraces: - 'spec/requests/checkout/failed_checkout_spec.rb' - 'spec/requests/checkout/stripe_connect_spec.rb' - 'spec/serializers/enterprise_serializer_spec.rb' - - 'spec/services/cart_service_spec.rb' - 'spec/services/order_syncer_spec.rb' - 'spec/services/subscription_form_spec.rb' - 'spec/spec_helper.rb' @@ -900,14 +894,6 @@ Layout/Tab: - 'spec/lib/spree/product_filters_spec.rb' - 'spec/models/spree/line_item_spec.rb' -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: final_newline, final_blank_line -Layout/TrailingBlankLines: - Exclude: - - 'spec/controllers/cart_controller_spec.rb' - # Offense count: 57 # Cop supports --auto-correct. # Configuration parameters: AllowInHeredoc. @@ -1390,7 +1376,6 @@ Rails/HttpStatus: - 'app/controllers/api/enterprise_fees_controller.rb' - 'app/controllers/api/enterprises_controller.rb' - 'app/controllers/application_controller.rb' - - 'app/controllers/cart_controller.rb' - 'app/controllers/checkout_controller.rb' - 'app/controllers/enterprises_controller.rb' - 'app/controllers/line_items_controller.rb' @@ -1564,7 +1549,6 @@ Style/BracesAroundHashParameters: - 'spec/controllers/admin/manager_invitations_controller_spec.rb' - 'spec/controllers/admin/order_cycles_controller_spec.rb' - 'spec/controllers/api/order_cycles_controller_spec.rb' - - 'spec/controllers/cart_controller_spec.rb' - 'spec/controllers/enterprises_controller_spec.rb' - 'spec/controllers/line_items_controller_spec.rb' - 'spec/controllers/spree/admin/adjustments_controller_spec.rb' @@ -1597,7 +1581,6 @@ Style/BracesAroundHashParameters: - 'spec/models/spree/product_spec.rb' - 'spec/models/spree/taxon_spec.rb' - 'spec/serializers/admin/customer_serializer_spec.rb' - - 'spec/services/cart_service_spec.rb' - 'spec/spec_helper.rb' - 'spec/support/cancan_helper.rb' - 'spec/support/request/authentication_workflow.rb' @@ -1919,7 +1902,6 @@ Style/HashSyntax: - 'spec/controllers/admin/stripe_connect_settings_controller_spec.rb' - 'spec/controllers/api/order_cycles_controller_spec.rb' - 'spec/controllers/base_controller_spec.rb' - - 'spec/controllers/cart_controller_spec.rb' - 'spec/controllers/spree/admin/payment_methods_controller_spec.rb' - 'spec/controllers/spree/admin/payments_controller_spec.rb' - 'spec/controllers/spree/api/products_controller_spec.rb' @@ -2007,7 +1989,6 @@ Style/MethodCallWithoutArgsParentheses: Exclude: - 'app/controllers/spree/admin/payment_methods_controller_decorator.rb' - 'app/views/json/_groups.rabl' - - 'spec/controllers/cart_controller_spec.rb' - 'spec/features/consumer/registration_spec.rb' - 'spec/models/spree/payment_method_spec.rb' - 'spec/support/request/ui_component_helper.rb' diff --git a/app/controllers/cart_controller.rb b/app/controllers/cart_controller.rb index 5213078c57..d452228299 100644 --- a/app/controllers/cart_controller.rb +++ b/app/controllers/cart_controller.rb @@ -20,11 +20,11 @@ class CartController < BaseController variant_ids = variant_ids_in(cart_service.variants_h) - render json: { error: false, stock_levels: VariantsStockLevels.new().call(current_order, variant_ids) }, - status: 200 - + render json: { error: false, + stock_levels: VariantsStockLevels.new.call(current_order, variant_ids) }, + status: :ok else - render json: { error: true }, status: 412 + render json: { error: true }, status: :precondition_failed end end populate_variant_attributes diff --git a/app/services/variants_stock_levels.rb b/app/services/variants_stock_levels.rb index f9e8fc9ead..751edfe67e 100644 --- a/app/services/variants_stock_levels.rb +++ b/app/services/variants_stock_levels.rb @@ -6,7 +6,7 @@ class VariantsStockLevels variant_stock_levels = variant_stock_levels(order.line_items) # Potentially, the following lines are dead code, they are never reached - # Additionally, the variants are not scoped here and so the stock levels reported would be incorrect + # Additionally, variants are not scoped here and so the stock levels reported would be incorrect # See cart_controller_spec for more details and #3222 li_variant_ids = variant_stock_levels.keys (requested_variant_ids - li_variant_ids).each do |variant_id| @@ -23,9 +23,9 @@ class VariantsStockLevels Hash[ line_items.map do |line_item| [line_item.variant.id, - { quantity: line_item.quantity, - max_quantity: line_item.max_quantity, - on_hand: wrap_json_infinity(line_item.variant.on_hand) }] + { quantity: line_item.quantity, + max_quantity: line_item.max_quantity, + on_hand: wrap_json_infinity(line_item.variant.on_hand) }] end ] end @@ -33,6 +33,6 @@ class VariantsStockLevels # Rails to_json encodes Float::INFINITY as Infinity, which is not valid JSON # Return it as a large integer (max 32 bit signed int) def wrap_json_infinity(number) - number == Float::INFINITY ? 2147483647 : number + number == Float::INFINITY ? 2_147_483_647 : number end end diff --git a/spec/controllers/cart_controller_spec.rb b/spec/controllers/cart_controller_spec.rb index d9c3a29942..eabb671593 100644 --- a/spec/controllers/cart_controller_spec.rb +++ b/spec/controllers/cart_controller_spec.rb @@ -8,9 +8,9 @@ describe CartController, type: :controller do it "returns stock levels as JSON" do allow(controller).to receive(:variant_ids_in) { [123] } - allow(VariantsStockLevels).to receive(:new).and_return(variant_stock_levels = double()) + allow(VariantsStockLevels).to receive(:new).and_return(variant_stock_levels = double) allow(variant_stock_levels).to receive(:call) { 'my_stock_levels' } - allow(CartService).to receive(:new).and_return(cart_service = double()) + allow(CartService).to receive(:new).and_return(cart_service = double) allow(cart_service).to receive(:populate) { true } allow(cart_service).to receive(:variants_h) { {} } @@ -21,8 +21,8 @@ describe CartController, type: :controller do end it "extracts variant ids from the cart service" do - variants_h = [{:variant_id=>"900", :quantity=>2, :max_quantity=>nil}, - {:variant_id=>"940", :quantity=>3, :max_quantity=>3}] + variants_h = [{ variant_id: "900", quantity: 2, max_quantity: nil }, + { variant_id: "940", quantity: 3, max_quantity: 3 }] expect(controller.variant_ids_in(variants_h)).to eq([900, 940]) end @@ -71,20 +71,20 @@ describe CartController, type: :controller do context "adding a group buy product to the cart" do it "sets a variant attribute for the max quantity" do distributor_product = create(:distributor_enterprise) - p = create(:product, :distributors => [distributor_product], :group_buy => true) + p = create(:product, distributors: [distributor_product], group_buy: true) order = subject.current_order(true) allow(order).to receive(:distributor) { distributor_product } - expect(order).to receive(:set_variant_attributes).with(p.master, {'max_quantity' => '3'}) + expect(order).to receive(:set_variant_attributes).with(p.master, max_quantity: '3') allow(controller).to receive(:current_order).and_return(order) expect do - spree_post :populate, variants: { p.master.id => 1 }, variant_attributes: { p.master.id => {max_quantity: 3 } } + spree_post :populate, variants: { p.master.id => 1 }, variant_attributes: { p.master.id => { max_quantity: 3 } } end.to change(Spree::LineItem, :count).by(1) end it "returns HTTP success when successful" do - allow(CartService).to receive(:new).and_return(cart_service = double()) + allow(CartService).to receive(:new).and_return(cart_service = double) allow(cart_service).to receive(:populate) { true } allow(cart_service).to receive(:variants_h) { {} } xhr :post, :populate, use_route: :spree, format: :json @@ -92,14 +92,14 @@ describe CartController, type: :controller do end it "returns failure when unsuccessful" do - allow(CartService).to receive(:new).and_return(cart_service = double()) + allow(CartService).to receive(:new).and_return(cart_service = double) allow(cart_service).to receive(:populate).and_return false xhr :post, :populate, use_route: :spree, format: :json expect(response.status).to eq(412) end it "tells cart_service to overwrite" do - allow(CartService).to receive(:new).and_return(cart_service = double()) + allow(CartService).to receive(:new).and_return(cart_service = double) expect(cart_service).to receive(:populate).with({}, true) xhr :post, :populate, use_route: :spree, format: :json end diff --git a/spec/services/cart_service_spec.rb b/spec/services/cart_service_spec.rb index d3657bc809..5c5bec131a 100644 --- a/spec/services/cart_service_spec.rb +++ b/spec/services/cart_service_spec.rb @@ -21,7 +21,7 @@ describe CartService do describe "populate" do it "adds a variant" do - cart_service.populate({variants: {v.id.to_s => {quantity: '1', max_quantity: '2'}}}, true) + cart_service.populate({ variants: { v.id.to_s => { quantity: '1', max_quantity: '2' } } }, true) li = order.find_line_item_by_variant(v) expect(li).to be expect(li.quantity).to eq(1) @@ -32,7 +32,7 @@ describe CartService do it "updates a variant's quantity, max quantity and final_weight_volume" do order.add_variant v, 1, 2 - cart_service.populate({variants: {v.id.to_s => {quantity: '2', max_quantity: '3'}}}, true) + cart_service.populate({ variants: { v.id.to_s => { quantity: '2', max_quantity: '3' } } }, true) li = order.find_line_item_by_variant(v) expect(li).to be expect(li.quantity).to eq(2) @@ -43,7 +43,7 @@ describe CartService do it "removes a variant" do order.add_variant v, 1, 2 - cart_service.populate({variants: {}}, true) + cart_service.populate({ variants: {} }, true) order.line_items(:reload) li = order.find_line_item_by_variant(v) expect(li).not_to be @@ -73,7 +73,7 @@ describe CartService do it "attempts cart add with max_quantity" do allow(cart_service).to receive(:distribution_can_supply_products_in_cart).and_return true - params = {variants: {"1" => {quantity: 1, max_quantity: 2}}} + params = { variants: { "1" => { quantity: 1, max_quantity: 2 } } } allow(order).to receive(:with_lock).and_yield allow(cart_service).to receive(:varies_from_cart) { true } allow(cart_service).to receive(:variants_removed) { [] } @@ -87,45 +87,45 @@ describe CartService do it "returns true when item is not in cart and a quantity is specified" do expect(cart_service).to receive(:line_item_for_variant_id).with(variant.id).and_return(nil) - expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'})).to be true + expect(cart_service.send(:varies_from_cart, variant_id: variant.id, quantity: '2')).to be true end it "returns true when item is not in cart and a max_quantity is specified" do expect(cart_service).to receive(:line_item_for_variant_id).with(variant.id).and_return(nil) - expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '0', max_quantity: '2'})).to be true + expect(cart_service.send(:varies_from_cart, variant_id: variant.id, quantity: '0', max_quantity: '2')).to be true end it "returns false when item is not in cart and no quantity or max_quantity are specified" do expect(cart_service).to receive(:line_item_for_variant_id).with(variant.id).and_return(nil) - expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '0'})).to be false + expect(cart_service.send(:varies_from_cart, variant_id: variant.id, quantity: '0')).to be false end it "returns true when quantity varies" do li = double(:line_item, quantity: 1, max_quantity: nil) allow(cart_service).to receive(:line_item_for_variant_id) { li } - expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '2'})).to be true + expect(cart_service.send(:varies_from_cart, variant_id: variant.id, quantity: '2')).to be true end it "returns true when max_quantity varies" do li = double(:line_item, quantity: 1, max_quantity: nil) allow(cart_service).to receive(:line_item_for_variant_id) { li } - expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '3'})).to be true + expect(cart_service.send(:varies_from_cart, variant_id: variant.id, quantity: '1', max_quantity: '3')).to be true end it "returns false when max_quantity varies only in nil vs 0" do li = double(:line_item, quantity: 1, max_quantity: nil) allow(cart_service).to receive(:line_item_for_variant_id) { li } - expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1'})).to be false + expect(cart_service.send(:varies_from_cart, variant_id: variant.id, quantity: '1')).to be false end it "returns false when both are specified and neither varies" do li = double(:line_item, quantity: 1, max_quantity: 2) allow(cart_service).to receive(:line_item_for_variant_id) { li } - expect(cart_service.send(:varies_from_cart, {variant_id: variant.id, quantity: '1', max_quantity: '2'})).to be false + expect(cart_service.send(:varies_from_cart, variant_id: variant.id, quantity: '1', max_quantity: '2')).to be false end end @@ -137,12 +137,12 @@ describe CartService do it "returns nothing when all items in the cart are provided" do allow(cart_service).to receive(:variant_ids_in_cart) { [123] } - expect(cart_service.send(:variants_removed, [{variant_id: '123'}])).to eq([]) + expect(cart_service.send(:variants_removed, [{ variant_id: '123' }])).to eq([]) end it "returns nothing when items are added to cart" do allow(cart_service).to receive(:variant_ids_in_cart) { [123] } - expect(cart_service.send(:variants_removed, [{variant_id: '123'}, {variant_id: '456'}])).to eq([]) + expect(cart_service.send(:variants_removed, [{ variant_id: '123' }, { variant_id: '456' }])).to eq([]) end it "does not return duplicates" do @@ -288,7 +288,6 @@ describe CartService do end end - describe "support" do describe "checking if order cycle is required for a variant" do it "requires an order cycle when the product has no product distributions" do @@ -307,8 +306,7 @@ describe CartService do it "provides the distributor and order cycle for the order" do expect(order).to receive(:distributor).and_return(distributor) expect(order).to receive(:order_cycle).and_return(order_cycle) - expect(cart_service.send(:distributor_and_order_cycle)).to eq([distributor, - order_cycle]) + expect(cart_service.send(:distributor_and_order_cycle)).to eq([distributor, order_cycle]) end end end diff --git a/spec/services/variants_stock_levels_spec.rb b/spec/services/variants_stock_levels_spec.rb index 531e30754a..463c8ac99a 100644 --- a/spec/services/variants_stock_levels_spec.rb +++ b/spec/services/variants_stock_levels_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' describe VariantsStockLevels do let(:order) { create(:order) } - let!(:line_item) { create(:line_item, order: order, variant: variant_in_the_order, quantity: 2, max_quantity: 3) } + let!(:line_item) do + create(:line_item, order: order, variant: variant_in_the_order, quantity: 2, max_quantity: 3) + end let!(:variant_in_the_order) { create(:variant, count_on_hand: 4) } let!(:variant_not_in_the_order) { create(:variant, count_on_hand: 2) } @@ -26,7 +28,8 @@ describe VariantsStockLevels do end it "includes an empty quantity entry for variants that aren't in the order" do - expect(variant_stock_levels.call(order, [variant_in_the_order.id, variant_not_in_the_order.id])).to eq( + variant_ids = [variant_in_the_order.id, variant_not_in_the_order.id] + expect(variant_stock_levels.call(order, variant_ids)).to eq( variant_in_the_order.id => { quantity: 2, max_quantity: 3, on_hand: 4 }, variant_not_in_the_order.id => { quantity: 0, max_quantity: 0, on_hand: 2 } )