From e26d591d2471caeaa3d03530540b6e6e9255903c Mon Sep 17 00:00:00 2001 From: cyrillefr Date: Wed, 12 Mar 2025 21:23:49 +0100 Subject: [PATCH 1/3] Fixes some rubocop linting offenses - part VI --- .rubocop_todo.yml | 16 ---------------- engines/catalog/config/routes.rb | 4 ++-- .../distributor_title_component_spec.rb | 4 ++-- spec/components/example_component_spec.rb | 4 ++-- .../subscription_line_items_controller_spec.rb | 17 ++++++++++------- .../api/v0/shipments_controller_spec.rb | 2 +- spec/controllers/concerns/extra_fields_spec.rb | 4 ++-- spec/factories.rb | 2 ++ spec/factories/enterprise_factory.rb | 4 ++-- spec/jobs/order_cycle_opened_job_spec.rb | 2 +- spec/jobs/subscription_placement_job_spec.rb | 2 +- .../product_import/entry_validator_spec.rb | 6 +++--- 12 files changed, 28 insertions(+), 39 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c3a7a14fe9..1becd96f06 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,22 +6,6 @@ # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 16 -# Configuration parameters: AllowComments, AllowEmptyLambdas. -Lint/EmptyBlock: - Exclude: - - 'engines/catalog/config/routes.rb' - - 'spec/components/distributor_title_component_spec.rb' - - 'spec/components/example_component_spec.rb' - - 'spec/controllers/admin/subscription_line_items_controller_spec.rb' - - 'spec/controllers/api/v0/shipments_controller_spec.rb' - - 'spec/controllers/concerns/extra_fields_spec.rb' - - 'spec/factories.rb' - - 'spec/factories/enterprise_factory.rb' - - 'spec/jobs/order_cycle_opened_job_spec.rb' - - 'spec/jobs/subscription_placement_job_spec.rb' - - 'spec/models/product_import/entry_validator_spec.rb' - # Offense count: 24 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes, Max. Metrics/AbcSize: diff --git a/engines/catalog/config/routes.rb b/engines/catalog/config/routes.rb index 51d03b72a4..55680d5541 100644 --- a/engines/catalog/config/routes.rb +++ b/engines/catalog/config/routes.rb @@ -1,4 +1,4 @@ # frozen_string_literal: true -Openfoodnetwork::Application.routes.append do -end +# Openfoodnetwork::Application.routes.append do +# end diff --git a/spec/components/distributor_title_component_spec.rb b/spec/components/distributor_title_component_spec.rb index c88b76eb67..703a359ecf 100644 --- a/spec/components/distributor_title_component_spec.rb +++ b/spec/components/distributor_title_component_spec.rb @@ -2,9 +2,9 @@ require "spec_helper" -RSpec.describe "DistributorTitle tests", type: :component do +RSpec.describe DistributorTitleComponent, type: :component do it "displays distributor title with its name" do - render_inline(DistributorTitleComponent.new(name: "Freddy's Farm Shop")) {} + render_inline(described_class.new(name: "Freddy's Farm Shop")) expect(page).to have_selector "h3", text: "Freddy's Farm Shop" end end diff --git a/spec/components/example_component_spec.rb b/spec/components/example_component_spec.rb index 540d7a4364..cdb101581c 100644 --- a/spec/components/example_component_spec.rb +++ b/spec/components/example_component_spec.rb @@ -2,9 +2,9 @@ require "spec_helper" -RSpec.describe "ExampleComponent tests", type: :component do +RSpec.describe ExampleComponent, type: :component do it "displays the h1 with the given parameter" do - render_inline(ExampleComponent.new(title: "Hello")) {} + render_inline(described_class.new(title: "Hello")) expect(page).to have_selector "h1", text: "Hello" end end diff --git a/spec/controllers/admin/subscription_line_items_controller_spec.rb b/spec/controllers/admin/subscription_line_items_controller_spec.rb index bf637d92b4..c8379a2f27 100644 --- a/spec/controllers/admin/subscription_line_items_controller_spec.rb +++ b/spec/controllers/admin/subscription_line_items_controller_spec.rb @@ -5,6 +5,15 @@ require 'spec_helper' RSpec.describe Admin::SubscriptionLineItemsController, type: :controller do include AuthenticationHelper + RSpec.configure do |c| + c.before(:each) do |test| + unless test.metadata[:no_outgoing_exchange] + order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], + enterprise_fees: [enterprise_fee]) + end + end + end + describe "build" do let(:user) { create(:user) } let!(:shop) { create(:enterprise, owner: user) } @@ -13,10 +22,6 @@ RSpec.describe Admin::SubscriptionLineItemsController, type: :controller do let!(:variant) { create(:variant, product:, unit_value: '100', price: 15.00) } - let!(:outgoing_exchange) { - order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], - enterprise_fees: [enterprise_fee]) - } let!(:enterprise_fee) { create(:enterprise_fee, amount: 3.50) } let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, @@ -55,9 +60,7 @@ RSpec.describe Admin::SubscriptionLineItemsController, type: :controller do before { params.merge!(shop_id: shop.id) } context "but the shop doesn't have permission to sell product in question" do - let!(:outgoing_exchange) {} - - it "returns an error" do + it "returns an error", :no_outgoing_exchange do spree_post :build, params json_response = response.parsed_body expect(json_response['errors']) diff --git a/spec/controllers/api/v0/shipments_controller_spec.rb b/spec/controllers/api/v0/shipments_controller_spec.rb index 1c2196b36b..56b2404a1e 100644 --- a/spec/controllers/api/v0/shipments_controller_spec.rb +++ b/spec/controllers/api/v0/shipments_controller_spec.rb @@ -369,7 +369,7 @@ RSpec.describe Api::V0::ShipmentsController, type: :controller do before do allow(Spree::Order).to receive(:find_by!) { fee_order } - allow(controller).to receive(:refuse_changing_cancelled_orders) {} + allow(controller).to receive(:refuse_changing_cancelled_orders) allow(fee_order).to receive(:contents) { contents } allow(contents).to receive_messages(add: {}, remove: {}) allow(fee_order).to receive_message_chain(:shipments, :find_by!) { fee_order_shipment } diff --git a/spec/controllers/concerns/extra_fields_spec.rb b/spec/controllers/concerns/extra_fields_spec.rb index 0c6d4a1569..e6cd121086 100644 --- a/spec/controllers/concerns/extra_fields_spec.rb +++ b/spec/controllers/concerns/extra_fields_spec.rb @@ -7,7 +7,7 @@ RSpec.describe ExtraFields do describe "#invalid_query_param" do it "renders error" do - allow(dummy_controller).to receive(:render) {} + allow(dummy_controller).to receive(:render) dummy_controller.invalid_query_param("param", :unprocessable_entity, "error message") expect(dummy_controller).to have_received(:render).with( json: @@ -44,7 +44,7 @@ RSpec.describe ExtraFields do context "when fields not in available fields" do it "calls invalid_query_param" do - allow(dummy_controller).to receive(:invalid_query_param) {} + allow(dummy_controller).to receive(:invalid_query_param) allow(dummy_controller).to receive(:params). and_return({ extra_fields: { customer: "unknown" } }) dummy_controller.extra_fields(:customer, [:balance]) diff --git a/spec/factories.rb b/spec/factories.rb index 90675a7cbb..44b50bfe32 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -61,9 +61,11 @@ FactoryBot.define do end factory :enterprise_relationship do + nil end factory :enterprise_role do + nil end factory :enterprise_group, class: EnterpriseGroup do diff --git a/spec/factories/enterprise_factory.rb b/spec/factories/enterprise_factory.rb index e29445941d..6ff71ea6f4 100644 --- a/spec/factories/enterprise_factory.rb +++ b/spec/factories/enterprise_factory.rb @@ -4,8 +4,8 @@ FactoryBot.define do factory :enterprise, class: Enterprise do transient do users { [] } - logo {} - promo_image {} + logo { nil } + promo_image { nil } end owner factory: :user diff --git a/spec/jobs/order_cycle_opened_job_spec.rb b/spec/jobs/order_cycle_opened_job_spec.rb index caf5ee6ef1..a75f8413f6 100644 --- a/spec/jobs/order_cycle_opened_job_spec.rb +++ b/spec/jobs/order_cycle_opened_job_spec.rb @@ -36,7 +36,7 @@ RSpec.describe OrderCycleOpenedJob do breakpoint.lock allow(OrderCycleOpenedJob).to( receive(:new).and_wrap_original do |method, *args| - breakpoint.synchronize {} + breakpoint.synchronize { nil } method.call(*args) end ) diff --git a/spec/jobs/subscription_placement_job_spec.rb b/spec/jobs/subscription_placement_job_spec.rb index e2828f18b5..26238df68b 100644 --- a/spec/jobs/subscription_placement_job_spec.rb +++ b/spec/jobs/subscription_placement_job_spec.rb @@ -202,7 +202,7 @@ RSpec.describe SubscriptionPlacementJob do breakpoint.lock allow(PlaceProxyOrder).to( receive(:new).and_wrap_original do |method, *args| - breakpoint.synchronize {} + breakpoint.synchronize { nil } method.call(*args) end ) diff --git a/spec/models/product_import/entry_validator_spec.rb b/spec/models/product_import/entry_validator_spec.rb index db240b9498..6583dcfe30 100644 --- a/spec/models/product_import/entry_validator_spec.rb +++ b/spec/models/product_import/entry_validator_spec.rb @@ -110,9 +110,9 @@ RSpec.describe ProductImport::EntryValidator do describe "inventory validation" do before do allow(entry_validator).to receive(:import_into_inventory?) { true } - allow(entry_validator).to receive(:enterprise_validation) {} - allow(entry_validator).to receive(:producer_validation) {} - allow(entry_validator).to receive(:variant_of_product_validation) {} + allow(entry_validator).to receive(:enterprise_validation) + allow(entry_validator).to receive(:producer_validation) + allow(entry_validator).to receive(:variant_of_product_validation) end context "products exist" do From ba58a7b9241e8fa82979d3de0faec5927f9d4d29 Mon Sep 17 00:00:00 2001 From: cyrillefr Date: Thu, 13 Mar 2025 10:08:35 +0100 Subject: [PATCH 2/3] Fix: inconsiderate use of RSpec.configure --- .../admin/subscription_line_items_controller_spec.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/controllers/admin/subscription_line_items_controller_spec.rb b/spec/controllers/admin/subscription_line_items_controller_spec.rb index c8379a2f27..daf83e5975 100644 --- a/spec/controllers/admin/subscription_line_items_controller_spec.rb +++ b/spec/controllers/admin/subscription_line_items_controller_spec.rb @@ -5,12 +5,10 @@ require 'spec_helper' RSpec.describe Admin::SubscriptionLineItemsController, type: :controller do include AuthenticationHelper - RSpec.configure do |c| - c.before(:each) do |test| - unless test.metadata[:no_outgoing_exchange] - order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], - enterprise_fees: [enterprise_fee]) - end + before(:each) do |test| + unless test.metadata[:no_outgoing_exchange] + order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], + enterprise_fees: [enterprise_fee]) end end From b15ffe7a4eb20ac3cce46aef1f66a1d9db24ed36 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 14 Mar 2025 11:26:04 +1100 Subject: [PATCH 3/3] Restore empty block with comment RSpec tags are too complex for this simple setup. --- .../subscription_line_items_controller_spec.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/spec/controllers/admin/subscription_line_items_controller_spec.rb b/spec/controllers/admin/subscription_line_items_controller_spec.rb index daf83e5975..5926abf23d 100644 --- a/spec/controllers/admin/subscription_line_items_controller_spec.rb +++ b/spec/controllers/admin/subscription_line_items_controller_spec.rb @@ -5,13 +5,6 @@ require 'spec_helper' RSpec.describe Admin::SubscriptionLineItemsController, type: :controller do include AuthenticationHelper - before(:each) do |test| - unless test.metadata[:no_outgoing_exchange] - order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], - enterprise_fees: [enterprise_fee]) - end - end - describe "build" do let(:user) { create(:user) } let!(:shop) { create(:enterprise, owner: user) } @@ -20,6 +13,10 @@ RSpec.describe Admin::SubscriptionLineItemsController, type: :controller do let!(:variant) { create(:variant, product:, unit_value: '100', price: 15.00) } + let!(:outgoing_exchange) { + order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], + enterprise_fees: [enterprise_fee]) + } let!(:enterprise_fee) { create(:enterprise_fee, amount: 3.50) } let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, @@ -58,7 +55,11 @@ RSpec.describe Admin::SubscriptionLineItemsController, type: :controller do before { params.merge!(shop_id: shop.id) } context "but the shop doesn't have permission to sell product in question" do - it "returns an error", :no_outgoing_exchange do + let!(:outgoing_exchange) { + # missing exchange should trigger an error + } + + it "returns an error" do spree_post :build, params json_response = response.parsed_body expect(json_response['errors'])