From 17a5b5e620822332425fbcff5ac2d0629b2f7424 Mon Sep 17 00:00:00 2001 From: Carlos Chitty Date: Tue, 29 Apr 2025 14:20:50 -0400 Subject: [PATCH 1/4] autocorrect Style/ArrayIntersect offenses --- .rubocop_todo.yml | 12 ------------ app/models/tag_rule/filter_order_cycles.rb | 2 +- app/models/tag_rule/filter_payment_methods.rb | 2 +- app/models/tag_rule/filter_products.rb | 2 +- app/models/tag_rule/filter_shipping_methods.rb | 2 +- lib/open_food_network/tag_rule_applicator.rb | 2 +- spec/support/matchers/select2_matchers.rb | 2 +- 7 files changed, 6 insertions(+), 18 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d20c350e60..414d0dd5d8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -479,18 +479,6 @@ Security/Open: Exclude: - 'app/services/image_importer.rb' -# Offense count: 7 -# This cop supports unsafe autocorrection (--autocorrect-all). -Style/ArrayIntersect: - Exclude: - - 'app/models/spree/ability.rb' - - 'app/models/tag_rule/filter_order_cycles.rb' - - 'app/models/tag_rule/filter_payment_methods.rb' - - 'app/models/tag_rule/filter_products.rb' - - 'app/models/tag_rule/filter_shipping_methods.rb' - - 'lib/open_food_network/tag_rule_applicator.rb' - - 'spec/support/matchers/select2_matchers.rb' - # Offense count: 23 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: EnforcedStyle. diff --git a/app/models/tag_rule/filter_order_cycles.rb b/app/models/tag_rule/filter_order_cycles.rb index dd32314e3b..35fd5d9c58 100644 --- a/app/models/tag_rule/filter_order_cycles.rb +++ b/app/models/tag_rule/filter_order_cycles.rb @@ -7,7 +7,7 @@ class TagRule::FilterOrderCycles < TagRule def tags_match?(order_cycle) exchange_tags = exchange_for(order_cycle)&.tag_list || [] preferred_tags = preferred_exchange_tags.split(",") - ( exchange_tags & preferred_tags ).any? + exchange_tags.intersect?(preferred_tags) end def reject_matched? diff --git a/app/models/tag_rule/filter_payment_methods.rb b/app/models/tag_rule/filter_payment_methods.rb index aee60a46fb..bdf35fbff8 100644 --- a/app/models/tag_rule/filter_payment_methods.rb +++ b/app/models/tag_rule/filter_payment_methods.rb @@ -7,7 +7,7 @@ class TagRule::FilterPaymentMethods < TagRule def tags_match?(payment_method) payment_method_tags = payment_method&.tag_list || [] preferred_tags = preferred_payment_method_tags.split(",") - ( payment_method_tags & preferred_tags ).any? + payment_method_tags.intersect?(preferred_tags) end def reject_matched? diff --git a/app/models/tag_rule/filter_products.rb b/app/models/tag_rule/filter_products.rb index fb3908fefa..86095f1c17 100644 --- a/app/models/tag_rule/filter_products.rb +++ b/app/models/tag_rule/filter_products.rb @@ -12,7 +12,7 @@ class TagRule def tags_match?(variant) variant_tags = variant&.[]("tag_list") || [] preferred_tags = preferred_variant_tags.split(",") - (variant_tags & preferred_tags).any? + variant_tags.intersect?(preferred_tags) end def reject_matched? diff --git a/app/models/tag_rule/filter_shipping_methods.rb b/app/models/tag_rule/filter_shipping_methods.rb index e0cdd404c3..ddd28ec46f 100644 --- a/app/models/tag_rule/filter_shipping_methods.rb +++ b/app/models/tag_rule/filter_shipping_methods.rb @@ -11,6 +11,6 @@ class TagRule::FilterShippingMethods < TagRule def tags_match?(shipping_method) shipping_method_tags = shipping_method&.tag_list || [] preferred_tags = preferred_shipping_method_tags.split(",") - ( shipping_method_tags & preferred_tags ).any? + shipping_method_tags.intersect?(preferred_tags) end end diff --git a/lib/open_food_network/tag_rule_applicator.rb b/lib/open_food_network/tag_rule_applicator.rb index d37d485e1d..2bfdf87042 100644 --- a/lib/open_food_network/tag_rule_applicator.rb +++ b/lib/open_food_network/tag_rule_applicator.rb @@ -59,7 +59,7 @@ module OpenFoodNetwork def customer_tags_match?(rule) preferred_tags = rule.preferred_customer_tags.split(",") - (customer_tags & preferred_tags).any? + customer_tags.intersect?(preferred_tags) end end end diff --git a/spec/support/matchers/select2_matchers.rb b/spec/support/matchers/select2_matchers.rb index 93d176958d..8e7be8f87e 100644 --- a/spec/support/matchers/select2_matchers.rb +++ b/spec/support/matchers/select2_matchers.rb @@ -56,7 +56,7 @@ RSpec::Matchers.define :have_select2 do |id, options = {}| # if options.key? :without_options end - if (options.keys & %i(selected options without_options)).any? + if options.keys.intersect?(%i(selected options without_options)) raise "Not yet implemented" end From 63168086e72390fc64ef6f9f2ca93e45d58ffd3a Mon Sep 17 00:00:00 2001 From: Carlos Chitty Date: Tue, 29 Apr 2025 14:27:31 -0400 Subject: [PATCH 2/4] Autocorrect Style/HashConversion offenses --- .rubocop_todo.yml | 14 -------------- .../admin/column_preferences_controller.rb | 4 ++-- .../admin/variant_overrides_controller.rb | 2 +- .../spree/admin/products_controller.rb | 6 +++--- app/models/product_import/product_importer.rb | 2 +- .../api/admin/exchange_serializer.rb | 2 +- app/services/variants_stock_levels.rb | 18 ++++++++---------- .../admin/inventory_items_controller_spec.rb | 4 ++-- .../admin/variant_overrides_controller_spec.rb | 4 ++-- 9 files changed, 20 insertions(+), 36 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 414d0dd5d8..c21ecf39d6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -508,20 +508,6 @@ Style/ClassAndModuleChildren: - 'lib/open_food_network/locking.rb' - 'spec/models/spree/payment_method_spec.rb' -# Offense count: 10 -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: AllowSplatArgument. -Style/HashConversion: - Exclude: - - 'app/controllers/admin/column_preferences_controller.rb' - - 'app/controllers/admin/variant_overrides_controller.rb' - - 'app/controllers/spree/admin/products_controller.rb' - - 'app/models/product_import/product_importer.rb' - - 'app/serializers/api/admin/exchange_serializer.rb' - - 'app/services/variants_stock_levels.rb' - - 'spec/controllers/admin/inventory_items_controller_spec.rb' - - 'spec/controllers/admin/variant_overrides_controller_spec.rb' - # Offense count: 13 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: AllowedReceivers. diff --git a/app/controllers/admin/column_preferences_controller.rb b/app/controllers/admin/column_preferences_controller.rb index 5403a9a5ee..79f70d9ff1 100644 --- a/app/controllers/admin/column_preferences_controller.rb +++ b/app/controllers/admin/column_preferences_controller.rb @@ -40,8 +40,8 @@ module Admin respond_to do |format| format.json do - collection_attributes = Hash[permitted_params[:column_preferences]. - each_with_index.map { |cp, i| [i, cp] }] + collection_attributes = permitted_params[:column_preferences]. + each_with_index.to_h { |cp, i| [i, cp] } collection_attributes.select!{ |_i, cp| cp[:action_name] == permitted_params[:action_name] } diff --git a/app/controllers/admin/variant_overrides_controller.rb b/app/controllers/admin/variant_overrides_controller.rb index d88c25da10..00ba18f101 100644 --- a/app/controllers/admin/variant_overrides_controller.rb +++ b/app/controllers/admin/variant_overrides_controller.rb @@ -70,7 +70,7 @@ module Admin end def load_collection - collection_hash = Hash[variant_overrides_params.each_with_index.map { |vo, i| [i, vo] }] + collection_hash = variant_overrides_params.each_with_index.to_h { |vo, i| [i, vo] } # Reset count_on_hand when switching to producer settings: collection_hash.each_value do |vo| diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 1c693ea32d..f51ba5503e 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -134,9 +134,9 @@ module Spree end def product_set_from_params - collection_hash = Hash[products_bulk_params[:products].each_with_index.map { |p, i| - [i, p] - } ] + collection_hash = products_bulk_params[:products].each_with_index.to_h { |p, i| + [i, p] + } Sets::ProductSet.new(collection_attributes: collection_hash) end diff --git a/app/models/product_import/product_importer.rb b/app/models/product_import/product_importer.rb index 2cdfba52bd..e92006cea8 100644 --- a/app/models/product_import/product_importer.rb +++ b/app/models/product_import/product_importer.rb @@ -292,7 +292,7 @@ module ProductImport def build_entries_from_rows(rows, offset = 0) rows.each_with_index.inject([]) do |entries, (row, i)| - row_data = Hash[[headers, row].transpose] + row_data = [headers, row].transpose.to_h entry = SpreadsheetEntry.new(row_data) entry.line_number = offset + i + 2 entries.push entry diff --git a/app/serializers/api/admin/exchange_serializer.rb b/app/serializers/api/admin/exchange_serializer.rb index 90828f0ba1..39505ca67b 100644 --- a/app/serializers/api/admin/exchange_serializer.rb +++ b/app/serializers/api/admin/exchange_serializer.rb @@ -11,7 +11,7 @@ module Api def variants variants = object.incoming? ? visible_incoming_variants : visible_outgoing_variants - Hash[object.variants.merge(variants).map { |v| [v.id, true] }] + object.variants.merge(variants).to_h { |v| [v.id, true] } end private diff --git a/app/services/variants_stock_levels.rb b/app/services/variants_stock_levels.rb index e76c1fef99..454658b220 100644 --- a/app/services/variants_stock_levels.rb +++ b/app/services/variants_stock_levels.rb @@ -25,17 +25,15 @@ class VariantsStockLevels private def variant_stock_levels(line_items) - Hash[ - line_items.map do |line_item| - variant = scoped_variant(line_item.order.distributor, line_item.variant) + line_items.to_h do |line_item| + variant = scoped_variant(line_item.order.distributor, line_item.variant) - [variant.id, - { quantity: line_item.quantity, - max_quantity: line_item.max_quantity, - on_hand: variant.on_hand, - on_demand: variant.on_demand }] - end - ] + [variant.id, + { quantity: line_item.quantity, + max_quantity: line_item.max_quantity, + on_hand: variant.on_hand, + on_demand: variant.on_demand }] + end end def scoped_variant(distributor, variant) diff --git a/spec/controllers/admin/inventory_items_controller_spec.rb b/spec/controllers/admin/inventory_items_controller_spec.rb index be33be4871..49d2adb03d 100644 --- a/spec/controllers/admin/inventory_items_controller_spec.rb +++ b/spec/controllers/admin/inventory_items_controller_spec.rb @@ -68,7 +68,7 @@ RSpec.describe Admin::InventoryItemsController, type: :controller do it "returns an error message" do expect{ spree_post :create, bad_params }.to change{ InventoryItem.count }.by(0) - expect(response.body).to eq Hash[:errors, ["Visible must be true or false"]].to_json + expect(response.body).to eq { :errors => ["Visible must be true or false"] }.to_json end end end @@ -134,7 +134,7 @@ RSpec.describe Admin::InventoryItemsController, type: :controller do it "returns an error message" do expect{ spree_put :update, bad_params }.to change{ InventoryItem.count }.by(0) - expect(response.body).to eq Hash[:errors, ["Visible must be true or false"]].to_json + expect(response.body).to eq { :errors => ["Visible must be true or false"] }.to_json end end end diff --git a/spec/controllers/admin/variant_overrides_controller_spec.rb b/spec/controllers/admin/variant_overrides_controller_spec.rb index 13848a22d1..807e4a26c7 100644 --- a/spec/controllers/admin/variant_overrides_controller_spec.rb +++ b/spec/controllers/admin/variant_overrides_controller_spec.rb @@ -60,7 +60,7 @@ RSpec.describe Admin::VariantOverridesController, type: :controller do put :bulk_update, as: format, params: { variant_overrides: variant_override_params } expect(assigns[:hubs]).to eq [hub] expect(assigns[:producers]).to eq [variant.supplier] - expect(assigns[:hub_permissions]).to eq Hash[hub.id, [variant.supplier.id]] + expect(assigns[:hub_permissions]).to eq({ hub.id => [variant.supplier.id] }) expect(assigns[:inventory_items]).to eq [inventory_item] end @@ -163,7 +163,7 @@ RSpec.describe Admin::VariantOverridesController, type: :controller do put(:bulk_reset, params:) expect(assigns[:hubs]).to eq [hub] expect(assigns[:producers]).to eq [producer] - expect(assigns[:hub_permissions]).to eq Hash[hub.id, [producer.id]] + expect(assigns[:hub_permissions]).to eq({ hub.id => [producer.id] }) expect(assigns[:inventory_items]).to eq [] end From 446be6e127ad3d96f17fb816beffc6669bd3898b Mon Sep 17 00:00:00 2001 From: Carlos Chitty Date: Tue, 29 Apr 2025 15:05:52 -0400 Subject: [PATCH 3/4] Fix test failure due to hash interpreted as block in matcher Wrap hash in parentheses to ensure it's passed as an argument rather than a block. --- spec/controllers/admin/inventory_items_controller_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/controllers/admin/inventory_items_controller_spec.rb b/spec/controllers/admin/inventory_items_controller_spec.rb index 49d2adb03d..58650e4aa5 100644 --- a/spec/controllers/admin/inventory_items_controller_spec.rb +++ b/spec/controllers/admin/inventory_items_controller_spec.rb @@ -68,7 +68,7 @@ RSpec.describe Admin::InventoryItemsController, type: :controller do it "returns an error message" do expect{ spree_post :create, bad_params }.to change{ InventoryItem.count }.by(0) - expect(response.body).to eq { :errors => ["Visible must be true or false"] }.to_json + expect(response.body).to eq({ errors: ["Visible must be true or false"] }.to_json) end end end @@ -134,7 +134,7 @@ RSpec.describe Admin::InventoryItemsController, type: :controller do it "returns an error message" do expect{ spree_put :update, bad_params }.to change{ InventoryItem.count }.by(0) - expect(response.body).to eq { :errors => ["Visible must be true or false"] }.to_json + expect(response.body).to eq({ errors: ["Visible must be true or false"] }.to_json) end end end From d0c687650ef87fc3b5c91f6891b43f21a5fe55cd Mon Sep 17 00:00:00 2001 From: Carlos Chitty Date: Tue, 29 Apr 2025 14:31:50 -0400 Subject: [PATCH 4/4] Autocorrect Style/HashEachMethods offenses --- .rubocop_todo.yml | 16 ---------------- app/controllers/admin/enterprises_controller.rb | 6 +++--- .../spree/admin/shipping_methods_controller.rb | 2 +- app/forms/enterprise_fees_bulk_update.rb | 6 +++--- app/models/product_import/entry_processor.rb | 2 +- app/models/spree/preferences/configuration.rb | 2 +- app/services/sets/model_set.rb | 2 +- .../sales_tax/sales_tax_totals_by_producer.rb | 2 +- spec/models/product_importer_spec.rb | 2 +- spec/support/cancan_helper.rb | 2 +- 10 files changed, 13 insertions(+), 29 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c21ecf39d6..d631082ed8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -508,22 +508,6 @@ Style/ClassAndModuleChildren: - 'lib/open_food_network/locking.rb' - 'spec/models/spree/payment_method_spec.rb' -# Offense count: 13 -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: AllowedReceivers. -# AllowedReceivers: Thread.current -Style/HashEachMethods: - Exclude: - - 'app/controllers/admin/enterprises_controller.rb' - - 'app/controllers/spree/admin/shipping_methods_controller.rb' - - 'app/forms/enterprise_fees_bulk_update.rb' - - 'app/models/product_import/entry_processor.rb' - - 'app/models/spree/preferences/configuration.rb' - - 'app/services/sets/model_set.rb' - - 'lib/reporting/reports/sales_tax/sales_tax_totals_by_producer.rb' - - 'spec/models/product_importer_spec.rb' - - 'spec/support/cancan_helper.rb' - # Offense count: 4 # This cop supports unsafe autocorrection (--autocorrect-all). Style/MapToHash: diff --git a/app/controllers/admin/enterprises_controller.rb b/app/controllers/admin/enterprises_controller.rb index 1fd7d38a90..beb3f77b82 100644 --- a/app/controllers/admin/enterprises_controller.rb +++ b/app/controllers/admin/enterprises_controller.rb @@ -254,7 +254,7 @@ module Admin # methods that are specific to each class do not become available until after the # record is persisted. This problem is compounded by the use of calculators. @object.transaction do - tag_rules_attributes.select{ |_i, attrs| attrs[:type].present? }.each do |_i, attrs| + tag_rules_attributes.select{ |_i, attrs| attrs[:type].present? }.each_value do |attrs| rule = @object.tag_rules.find_by(id: attrs.delete(:id)) || attrs[:type].constantize.new(enterprise: @object) @@ -293,7 +293,7 @@ module Admin def check_can_change_bulk_sells return if spree_current_user.admin? - params[:sets_enterprise_set][:collection_attributes].each do |_i, enterprise_params| + params[:sets_enterprise_set][:collection_attributes].each_value do |enterprise_params| unless spree_current_user == Enterprise.find_by(id: enterprise_params[:id]).owner enterprise_params.delete :sells end @@ -327,7 +327,7 @@ module Admin def check_can_change_bulk_owner return if spree_current_user.admin? - bulk_params[:collection_attributes].each do |_i, enterprise_params| + bulk_params[:collection_attributes].each_value do |enterprise_params| enterprise_params.delete :owner_id end end diff --git a/app/controllers/spree/admin/shipping_methods_controller.rb b/app/controllers/spree/admin/shipping_methods_controller.rb index 58f51ac858..25433b213f 100644 --- a/app/controllers/spree/admin/shipping_methods_controller.rb +++ b/app/controllers/spree/admin/shipping_methods_controller.rb @@ -104,7 +104,7 @@ module Spree return unless shipping_fees - shipping_fees.each do |_, shipping_amount| + shipping_fees.each_value do |shipping_amount| unless shipping_amount.nil? || Float(shipping_amount, exception: false) flash[:error] = I18n.t(:calculator_preferred_value_error) return redirect_to location_after_save diff --git a/app/forms/enterprise_fees_bulk_update.rb b/app/forms/enterprise_fees_bulk_update.rb index 09c138ab1b..a553b99954 100644 --- a/app/forms/enterprise_fees_bulk_update.rb +++ b/app/forms/enterprise_fees_bulk_update.rb @@ -30,7 +30,7 @@ class EnterpriseFeesBulkUpdate private def check_enterprise_fee_input - enterprise_fee_bulk_params['collection_attributes'].each do |_, fee_row| + enterprise_fee_bulk_params['collection_attributes'].each_value do |fee_row| enterprise_fees = fee_row['calculator_attributes']&.slice( :preferred_flat_percent, :preferred_amount, :preferred_first_item, :preferred_additional_item, @@ -40,7 +40,7 @@ class EnterpriseFeesBulkUpdate next unless enterprise_fees - enterprise_fees.each do |_, enterprise_amount| + enterprise_fees.each_value do |enterprise_amount| unless enterprise_amount.nil? || Float(enterprise_amount, exception: false) @errors.add(:base, I18n.t(:calculator_preferred_value_error)) end @@ -49,7 +49,7 @@ class EnterpriseFeesBulkUpdate end def check_calculators_compatibility_with_taxes - enterprise_fee_bulk_params['collection_attributes'].each do |_, enterprise_fee| + enterprise_fee_bulk_params['collection_attributes'].each_value do |enterprise_fee| next unless enterprise_fee['inherits_tax_category'] == "true" next unless EnterpriseFee::PER_ORDER_CALCULATORS.include?(enterprise_fee['calculator_type']) diff --git a/app/models/product_import/entry_processor.rb b/app/models/product_import/entry_processor.rb index 6393f14578..c658fdf8b1 100644 --- a/app/models/product_import/entry_processor.rb +++ b/app/models/product_import/entry_processor.rb @@ -46,7 +46,7 @@ module ProductImport end def count_existing_items - @spreadsheet_data.enterprises_index.each do |_enterprise_name, attrs| + @spreadsheet_data.enterprises_index.each_value do |attrs| enterprise_id = attrs[:id] next unless enterprise_id && permission_by_id?(enterprise_id) diff --git a/app/models/spree/preferences/configuration.rb b/app/models/spree/preferences/configuration.rb index a2997451a6..b534a31cfc 100644 --- a/app/models/spree/preferences/configuration.rb +++ b/app/models/spree/preferences/configuration.rb @@ -36,7 +36,7 @@ module Spree end def reset - preferences.each do |name, _value| + preferences.each_key do |name| set_preference name, preference_default(name) end end diff --git a/app/services/sets/model_set.rb b/app/services/sets/model_set.rb index 1db02abf92..854180ba2f 100644 --- a/app/services/sets/model_set.rb +++ b/app/services/sets/model_set.rb @@ -21,7 +21,7 @@ module Sets end def collection_attributes=(collection_attributes) - collection_attributes.each do |_k, attributes| + collection_attributes.each_value do |attributes| # attributes == {:id => 123, :next_collection_at => '...'} found_element = @collection.detect do |element| element.id.to_s == attributes[:id].to_s && !element.id.nil? diff --git a/lib/reporting/reports/sales_tax/sales_tax_totals_by_producer.rb b/lib/reporting/reports/sales_tax/sales_tax_totals_by_producer.rb index a2c00ea082..29bbe483e6 100644 --- a/lib/reporting/reports/sales_tax/sales_tax_totals_by_producer.rb +++ b/lib/reporting/reports/sales_tax/sales_tax_totals_by_producer.rb @@ -37,7 +37,7 @@ module Reporting hash[:line_item].order.distributor_id, hash[:line_item].order.order_cycle_id ] - end.each do |_, v| + end.each_value do |v| v.map!{ |item| item[:line_item] } end end diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index 1d6392f7a4..839d286a7c 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -1059,7 +1059,7 @@ end def filter(type, entries) valid_count = 0 - entries.each do |_line_number, entry| + entries.each_value do |entry| validates_as = entry['validates_as'] valid_count += 1 if type == 'valid' && (validates_as != '') diff --git a/spec/support/cancan_helper.rb b/spec/support/cancan_helper.rb index 11b7723db2..74c087b306 100644 --- a/spec/support/cancan_helper.rb +++ b/spec/support/cancan_helper.rb @@ -17,7 +17,7 @@ module Spree member.merge(i => true) } end - ability_hash.each do |action, _true_or_false| + ability_hash.each_key do |action| @ability_result[action] = ability.can?(action, target) end