From 5415fa2db85ca30758c24c9eea940a52a262646f Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 10:45:38 +0000 Subject: [PATCH 01/12] Fix offense constant definition in block in import_product_images_rake.rb Add a test to import product images rake --- .rubocop_todo.yml | 1 - lib/tasks/import_product_images.rake | 6 +- .../tasks/import_product_images_rake_spec.rb | 84 +++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) create mode 100644 spec/lib/tasks/import_product_images_rake_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 5709efec0e..62292766ca 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'lib/tasks/import_product_images.rake' - 'lib/tasks/users.rake' - 'spec/controllers/spree/admin/base_controller_spec.rb' - 'spec/helpers/serializer_helper_spec.rb' diff --git a/lib/tasks/import_product_images.rake b/lib/tasks/import_product_images.rake index e82d1e55d1..6b1ab14741 100644 --- a/lib/tasks/import_product_images.rake +++ b/lib/tasks/import_product_images.rake @@ -4,15 +4,15 @@ namespace :ofn do namespace :import do desc "Importing images for products from CSV" task :product_images, [:filename] => [:environment] do |_task, args| - COLUMNS = [:producer, :name, :image_url].freeze - puts "Warning: use only with trusted URLs. This script will download whatever it can, " \ "including local secrets, and expose the file as an image file." raise "Filename required" if args[:filename].blank? + columns = %i[producer name image_url].freeze + csv = CSV.read(args[:filename], headers: true, header_converters: :symbol) - raise "CSV columns reqired: #{COLUMNS.map(&:to_s)}" if (COLUMNS - csv.headers).present? + raise "CSV columns reqired: #{columns.map(&:to_s)}" if (columns - csv.headers).present? csv.each.with_index do |entry, index| puts "#{index} #{entry[:producer]}, #{entry[:name]}" diff --git a/spec/lib/tasks/import_product_images_rake_spec.rb b/spec/lib/tasks/import_product_images_rake_spec.rb new file mode 100644 index 0000000000..bfb6dcd571 --- /dev/null +++ b/spec/lib/tasks/import_product_images_rake_spec.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rake' + +RSpec.describe 'ofn:import:product_images' do + before do + Rake.application.load_rakefile + Rake::Task.define_task(:environment) + Rake::Task['ofn:import:product_images'].reenable + end + + after do + Rake::Task['ofn:import:product_images'].clear + end + + describe 'task' do + context "filename is blank" do + it 'raises an error' do + expect { + Rake::Task['ofn:import:product_images'].invoke('') + }.to raise_error(RuntimeError, + 'Filename required') + end + end + + context "invalid CSV format" do + it 'raises an error if CSV columns are missing' do + allow(CSV).to receive(:read).and_return(CSV::Table.new([])) + Rake::Task['ofn:import:product_images'].reenable + + expect { + Rake::Task['ofn:import:product_images'].invoke('path/to/csv/file.csv') + }.to raise_error(RuntimeError, 'CSV columns reqired: ["producer", "name", "image_url"]') + end + end + + context "valid CSV" do + it 'imports images for each product in the CSV that exists and does not have images' do + filename = 'path/to/csv/file.csv' + + csv_data = [ + { producer: 'Producer 1', name: 'Product 1', image_url: 'http://example.com/image1.jpg' }, + { producer: 'Producer 2', name: 'Product 2', image_url: 'http://example.com/image2.jpg' }, + { producer: 'Producer 3', name: 'Product 3', image_url: 'http://example.com/image3.jpg' } + ] + + csv_rows = csv_data.map do |hash| + CSV::Row.new(hash.keys, hash.values) + end + + csv_table = CSV::Table.new(csv_rows) + + allow(CSV).to receive(:read).and_return(csv_table) + + allow(Enterprise).to receive(:find_by!).with(name: 'Producer 1').and_return(double) + allow(Enterprise).to receive(:find_by!).with(name: 'Producer 2').and_return(double) + allow(Enterprise).to receive(:find_by!).with(name: 'Producer 3').and_return(double) + + allow(Spree::Product).to receive(:where).and_return( + class_double('Spree::Product', first: nil), + class_double('Spree::Product', first: instance_double('Spree::Product', image: nil)), + class_double('Spree::Product', first: instance_double('Spree::Product', image: true)) + ) + + allow_any_instance_of(ImageImporter).to receive(:import).and_return(true) + + expected_output = <<~OUTPUT + Warning: use only with trusted URLs. This script will download whatever it can, including local secrets, and expose the file as an image file. + 0 Producer 1, Product 1 + product not found. + 1 Producer 2, Product 2 + image added. + 2 Producer 3, Product 3 + image exists, not updated. + OUTPUT + + expect { + Rake::Task['ofn:import:product_images'].invoke('path/to/csv/file.csv') + }.to output(expected_output).to_stdout + end + end + end +end From d726a0e3cb3f30660142352482e932a31e060d43 Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 10:48:23 +0000 Subject: [PATCH 02/12] Fix offense constant definition in block in users.rake This rake has tests --- .rubocop_todo.yml | 1 - lib/tasks/users.rake | 21 +++------------------ spec/lib/tasks/users_rake_spec.rb | 12 ++++++------ 3 files changed, 9 insertions(+), 25 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 62292766ca..59cfe8eaba 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'lib/tasks/users.rake' - 'spec/controllers/spree/admin/base_controller_spec.rb' - 'spec/helpers/serializer_helper_spec.rb' - 'spec/lib/reports/line_items_spec.rb' diff --git a/lib/tasks/users.rake b/lib/tasks/users.rake index d1249cb9a0..fdf2c110f1 100644 --- a/lib/tasks/users.rake +++ b/lib/tasks/users.rake @@ -5,23 +5,8 @@ require 'csv' namespace :ofn do desc 'remove the limit of enterprises a user can create' task :remove_enterprise_limit, [:user_id] => :environment do |_task, args| - RemoveEnterpriseLimit.new(args.user_id).call - end - - class RemoveEnterpriseLimit - MAX_INTEGER = 2_147_483_647 - - def initialize(user_id) - @user_id = user_id - end - - def call - user = Spree::User.find(user_id) - user.update_attribute(:enterprise_limit, MAX_INTEGER) - end - - private - - attr_reader :user_id + max_integer = 2_147_483_647 + user = Spree::User.find(args.user_id) + user.update_attribute(:enterprise_limit, max_integer) end end diff --git a/spec/lib/tasks/users_rake_spec.rb b/spec/lib/tasks/users_rake_spec.rb index cfddfec7ae..5fdc6a0487 100644 --- a/spec/lib/tasks/users_rake_spec.rb +++ b/spec/lib/tasks/users_rake_spec.rb @@ -4,27 +4,27 @@ require 'spec_helper' require 'rake' describe 'users.rake' do - before(:all) do + before do Rake.application.rake_require 'tasks/users' Rake::Task.define_task(:environment) + Rake::Task['ofn:remove_enterprise_limit'].reenable end describe ':remove_enterprise_limit' do context 'when the user exists' do - it 'sets the enterprise_limit to the maximum integer' do - max_integer = 2_147_483_647 - user = create(:user) + let(:user) { create(:user) } + it 'sets the enterprise_limit to the maximum integer' do Rake.application.invoke_task "ofn:remove_enterprise_limit[#{user.id}]" - expect(user.reload.enterprise_limit).to eq(max_integer) + expect(user.reload.enterprise_limit).to eq(2_147_483_647) end end context 'when the user does not exist' do it 'raises' do expect { - RemoveEnterpriseLimit.new(-1).call + Rake.application.invoke_task "ofn:remove_enterprise_limit[123]" }.to raise_error(ActiveRecord::RecordNotFound) end end From f1309db0f09325036eaa6caa4371ea46ca38332a Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 10:51:00 +0000 Subject: [PATCH 03/12] Fix offense constant definition in block in spree base_controller_spec.rb --- .rubocop_todo.yml | 1 - spec/controllers/spree/admin/base_controller_spec.rb | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 59cfe8eaba..07a98c79d6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'spec/controllers/spree/admin/base_controller_spec.rb' - 'spec/helpers/serializer_helper_spec.rb' - 'spec/lib/reports/line_items_spec.rb' - 'spec/models/spree/ability_spec.rb' diff --git a/spec/controllers/spree/admin/base_controller_spec.rb b/spec/controllers/spree/admin/base_controller_spec.rb index 15d763aee0..21dbc71a4e 100644 --- a/spec/controllers/spree/admin/base_controller_spec.rb +++ b/spec/controllers/spree/admin/base_controller_spec.rb @@ -71,10 +71,9 @@ describe Spree::Admin::BaseController, type: :controller do describe "determining the name of the serializer to be used" do before do - class Api::Admin::AllowedPrefixBaseSerializer; end; - - class Api::Admin::BaseSerializer; end; allow(controller).to receive(:ams_prefix_whitelist) { [:allowed_prefix] } + stub_const('Api::Admin::AllowedPrefixBaseSerializer', Class) + stub_const('Api::Admin::BaseSerializer', Class) end context "when a prefix is passed in" do From cfca7816d53741b3a6875e23b46164761792af33 Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 10:52:20 +0000 Subject: [PATCH 04/12] Fix offense constant definition in block in serializer_helper_spec.rb --- .rubocop_todo.yml | 1 - spec/helpers/serializer_helper_spec.rb | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 07a98c79d6..919cb02439 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'spec/helpers/serializer_helper_spec.rb' - 'spec/lib/reports/line_items_spec.rb' - 'spec/models/spree/ability_spec.rb' - 'spec/models/spree/gateway_spec.rb' diff --git a/spec/helpers/serializer_helper_spec.rb b/spec/helpers/serializer_helper_spec.rb index 16822b89a0..6ba9589abe 100644 --- a/spec/helpers/serializer_helper_spec.rb +++ b/spec/helpers/serializer_helper_spec.rb @@ -4,10 +4,9 @@ require 'spec_helper' describe SerializerHelper, type: :helper do let(:serializer) do - class ExampleEnterpriseSerializer < ActiveModel::Serializer + Class.new(ActiveModel::Serializer) do attributes :id, :name end - ExampleEnterpriseSerializer end describe "#required_attributes" do From 0726e4c1d0007bfb24d22e5e1fef97a76554266c Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 10:54:16 +0000 Subject: [PATCH 05/12] Fix offense constant definition in block in reports/line_items_spec.rb --- .rubocop_todo.yml | 1 - spec/lib/reports/line_items_spec.rb | 52 +++++++++++++++-------------- 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 919cb02439..4a90dba13a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'spec/lib/reports/line_items_spec.rb' - 'spec/models/spree/ability_spec.rb' - 'spec/models/spree/gateway_spec.rb' - 'spec/models/spree/preferences/configuration_spec.rb' diff --git a/spec/lib/reports/line_items_spec.rb b/spec/lib/reports/line_items_spec.rb index 0e266352c1..8e04eb714f 100644 --- a/spec/lib/reports/line_items_spec.rb +++ b/spec/lib/reports/line_items_spec.rb @@ -3,35 +3,37 @@ require 'spec_helper' describe Reporting::LineItems do - subject(:reports_line_items) { described_class.new(order_permissions, params) } - # This object lets us add some test coverage despite the very deep coupling between the class # under test and the various objects it depends on. Other more common moking strategies where very # hard. - class FakeOrderPermissions - def initialize(line_items, orders_relation) - @relations = Spree::LineItem.where(id: line_items.map(&:id)) - @orders_relation = orders_relation + let(:fake_order_permissions) do + Class.new do + def initialize(line_items, orders_relation) + @relations = Spree::LineItem.where(id: line_items.map(&:id)) + @orders_relation = orders_relation + end + + def visible_line_items + relations + end + + def editable_line_items + line_item = FactoryBot.create(:line_item) + Spree::LineItem.where(id: line_item.id) + end + + def visible_orders + orders_relation + end + + private + + attr_reader :relations, :orders_relation end - - def visible_line_items - relations - end - - def editable_line_items - line_item = FactoryBot.create(:line_item) - Spree::LineItem.where(id: line_item.id) - end - - def visible_orders - orders_relation - end - - private - - attr_reader :relations, :orders_relation end + subject(:reports_line_items) { described_class.new(order_permissions, params) } + describe '#list' do let!(:order) do create( @@ -44,7 +46,7 @@ describe Reporting::LineItems do let!(:line_item1) { create(:line_item, order:) } let(:orders_relation) { Spree::Order.where(id: order.id) } - let(:order_permissions) { FakeOrderPermissions.new([line_item1], orders_relation) } + let(:order_permissions) { fake_order_permissions.new([line_item1], orders_relation) } let(:params) { {} } it 'returns masked data' do @@ -58,7 +60,7 @@ describe Reporting::LineItems do let!(:line_item2) { create(:line_item, order:) } let!(:line_item3) { create(:line_item, order:) } let(:order_permissions) do - FakeOrderPermissions.new([line_item1, line_item2, line_item3], orders_relation) + fake_order_permissions.new([line_item1, line_item2, line_item3], orders_relation) end let(:params) { { variant_id_in: [line_item3.variant.id, line_item1.variant.id] } } From 3bd6c85f3b3b7ae6696a91b4f7f6f4abf99b87f6 Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 10:56:21 +0000 Subject: [PATCH 06/12] Fix offense constant definition in block in models/spree/ability_spec.rb --- .rubocop_todo.yml | 1 - spec/models/spree/ability_spec.rb | 2 -- 2 files changed, 3 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 4a90dba13a..d64c4f4631 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'spec/models/spree/ability_spec.rb' - 'spec/models/spree/gateway_spec.rb' - 'spec/models/spree/preferences/configuration_spec.rb' - 'spec/models/spree/preferences/preferable_spec.rb' diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index 4d607aa8ae..b787e64a87 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -13,8 +13,6 @@ describe Spree::Ability do user.spree_roles.clear end - TOKEN = 'token123' - after(:each) { user.spree_roles = [] } From fc3d7f84960a025229a053f3f45d2e90899ccf96 Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 10:57:32 +0000 Subject: [PATCH 07/12] Fix offense constant definition in block in models/spree/gateway_spec.rb --- .rubocop_todo.yml | 1 - spec/models/spree/gateway_spec.rb | 18 +++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d64c4f4631..a906c62651 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'spec/models/spree/gateway_spec.rb' - 'spec/models/spree/preferences/configuration_spec.rb' - 'spec/models/spree/preferences/preferable_spec.rb' - 'spec/validators/date_time_string_validator_spec.rb' diff --git a/spec/models/spree/gateway_spec.rb b/spec/models/spree/gateway_spec.rb index 294073a559..9bf43378f4 100644 --- a/spec/models/spree/gateway_spec.rb +++ b/spec/models/spree/gateway_spec.rb @@ -3,20 +3,20 @@ require 'spec_helper' describe Spree::Gateway do - class Provider - def initialize(options); end + let(:test_gateway) do + Class.new(Spree::Gateway) do + def provider_class + Class.new do + def initialize(options = {}); end - def imaginary_method; end - end - - class TestGateway < Spree::Gateway - def provider_class - Provider + def imaginary_method; end + end + end end end it "passes through all arguments on a method_missing call" do - gateway = TestGateway.new + gateway = test_gateway.new expect(gateway.provider).to receive(:imaginary_method).with('foo') gateway.imaginary_method('foo') end From b18fe8ce35b1378f6c20a467ca51bbfa248aff12 Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 10:59:35 +0000 Subject: [PATCH 08/12] Fix offense constant definition in block in models/spree/preferences/configuration_spec.rb --- .rubocop_todo.yml | 1 - .../spree/preferences/configuration_spec.rb | 19 +++++++++---------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index a906c62651..de77e31cd8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'spec/models/spree/preferences/configuration_spec.rb' - 'spec/models/spree/preferences/preferable_spec.rb' - 'spec/validators/date_time_string_validator_spec.rb' - 'spec/validators/integer_array_validator_spec.rb' diff --git a/spec/models/spree/preferences/configuration_spec.rb b/spec/models/spree/preferences/configuration_spec.rb index 32883d3fb6..fc4b51a709 100644 --- a/spec/models/spree/preferences/configuration_spec.rb +++ b/spec/models/spree/preferences/configuration_spec.rb @@ -3,25 +3,24 @@ require 'spec_helper' describe Spree::Preferences::Configuration do - before :all do - class AppConfig < Spree::Preferences::Configuration + let(:config) do + Class.new(Spree::Preferences::Configuration) do preference :color, :string, default: :blue - end - @config = AppConfig.new + end.new end it "has named methods to access preferences" do - @config.color = 'orange' - expect(@config.color).to eq 'orange' + config.color = 'orange' + expect(config.color).to eq 'orange' end it "uses [ ] to access preferences" do - @config[:color] = 'red' - expect(@config[:color]).to eq 'red' + config[:color] = 'red' + expect(config[:color]).to eq 'red' end it "uses set/get to access preferences" do - @config.set :color, 'green' - expect(@config.get(:color)).to eq 'green' + config.set :color, 'green' + expect(config.get(:color)).to eq 'green' end end From 939605cb7aff375b197237bf44bdca552da0743d Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 11:04:59 +0000 Subject: [PATCH 09/12] Fix offense constant definition in block in models/spree/preferences/preferable_spec.rb --- .rubocop_todo.yml | 1 - .../spree/preferences/preferable_spec.rb | 262 +++++++++--------- 2 files changed, 132 insertions(+), 131 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index de77e31cd8..a1825eba7d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'spec/models/spree/preferences/preferable_spec.rb' - 'spec/validators/date_time_string_validator_spec.rb' - 'spec/validators/integer_array_validator_spec.rb' diff --git a/spec/models/spree/preferences/preferable_spec.rb b/spec/models/spree/preferences/preferable_spec.rb index f350c1a891..4cf412c392 100644 --- a/spec/models/spree/preferences/preferable_spec.rb +++ b/spec/models/spree/preferences/preferable_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Spree::Preferences::Preferable do - before :all do - class A + a_class = A = + Class.new do include Spree::Preferences::Preferable attr_reader :id @@ -15,16 +15,38 @@ describe Spree::Preferences::Preferable do preference :color, :string, default: 'green', description: "My Favorite Color" end - class B < A + b_class = B = + Class.new(a_class) do preference :flavor, :string end - end - before :each do - @a = A.new - allow(@a).to receive_messages(persisted?: true) - @b = B.new - allow(@b).to receive_messages(persisted?: true) + let(:a) { a_class.new } + let(:b) { b_class.new } + + create_pref_test = + Class.new(ActiveRecord::Migration[4.2]) do + def self.up + create_table :pref_tests do |t| + t.string :col + end + end + + def self.down + drop_table :pref_tests + end + end + + pref_test_class = + Class.new(ApplicationRecord) do + self.table_name = 'pref_tests' + + preference :pref_test_pref, :string, default: 'abc' + preference :pref_test_any, :any, default: [] + end + + before do + allow(a).to receive_messages(persisted?: true) + allow(b).to receive_messages(persisted?: true) # ensure we're persisting as that is the default # @@ -34,235 +56,213 @@ describe Spree::Preferences::Preferable do describe "preference definitions" do it "parent should not see child definitions" do - expect(@a.has_preference?(:color)).to be_truthy - expect(@a.has_preference?(:flavor)).not_to be_truthy + expect(a.has_preference?(:color)).to be_truthy + expect(a.has_preference?(:flavor)).not_to be_truthy end it "child should have parent and own definitions" do - expect(@b.has_preference?(:color)).to be_truthy - expect(@b.has_preference?(:flavor)).to be_truthy + expect(b.has_preference?(:color)).to be_truthy + expect(b.has_preference?(:flavor)).to be_truthy end it "instances have defaults" do - expect(@a.preferred_color).to eq 'green' - expect(@b.preferred_color).to eq 'green' - expect(@b.preferred_flavor).to be_nil + expect(a.preferred_color).to eq 'green' + expect(b.preferred_color).to eq 'green' + expect(b.preferred_flavor).to be_nil end it "can be asked if it has a preference definition" do - expect(@a.has_preference?(:color)).to be_truthy - expect(@a.has_preference?(:bad)).to be_falsy + expect(a.has_preference?(:color)).to be_truthy + expect(a.has_preference?(:bad)).to be_falsy end it "can be asked and raises" do expect { - @a.has_preference! :flavor + a.has_preference! :flavor }.to raise_error(NoMethodError, "flavor preference not defined") end it "has a type" do - expect(@a.preferred_color_type).to eq :string - expect(@a.preference_type(:color)).to eq :string + expect(a.preferred_color_type).to eq :string + expect(a.preference_type(:color)).to eq :string end it "has a default" do - expect(@a.preferred_color_default).to eq 'green' - expect(@a.preference_default(:color)).to eq 'green' + expect(a.preferred_color_default).to eq 'green' + expect(a.preference_default(:color)).to eq 'green' end it "has a description" do - expect(@a.preferred_color_description).to eq "My Favorite Color" - expect(@a.preference_description(:color)).to eq "My Favorite Color" + expect(a.preferred_color_description).to eq "My Favorite Color" + expect(a.preference_description(:color)).to eq "My Favorite Color" end it "raises if not defined" do expect { - @a.get_preference :flavor + a.get_preference :flavor }.to raise_error(NoMethodError, "flavor preference not defined") end end describe "preference access" do it "handles ghost methods for preferences" do - @a.preferred_color = 'blue' - expect(@a.preferred_color).to eq 'blue' + a.preferred_color = 'blue' + expect(a.preferred_color).to eq 'blue' - @a.prefers_color = 'green' - expect(@a.prefers_color?).to eq 'green' + a.prefers_color = 'green' + expect(a.prefers_color?).to eq 'green' end it "has genric readers" do - @a.preferred_color = 'red' - expect(@a.prefers?(:color)).to eq 'red' - expect(@a.preferred(:color)).to eq 'red' + a.preferred_color = 'red' + expect(a.prefers?(:color)).to eq 'red' + expect(a.preferred(:color)).to eq 'red' end it "parent and child instances have their own prefs" do - @a.preferred_color = 'red' - @b.preferred_color = 'blue' + a.preferred_color = 'red' + b.preferred_color = 'blue' - expect(@a.preferred_color).to eq 'red' - expect(@b.preferred_color).to eq 'blue' + expect(a.preferred_color).to eq 'red' + expect(b.preferred_color).to eq 'blue' end it "raises when preference not defined" do expect { - @a.set_preference(:bad, :bone) + a.set_preference(:bad, :bone) }.to raise_exception(NoMethodError, "bad preference not defined") end it "builds a hash of preferences" do - @b.preferred_flavor = :strawberry - expect(@b.preferences[:flavor]).to eq 'strawberry' - expect(@b.preferences[:color]).to eq 'green' # default from A + b.preferred_flavor = :strawberry + expect(b.preferences[:flavor]).to eq 'strawberry' + expect(b.preferences[:color]).to eq 'green' # default from A end context "database fallback" do before do - @a.instance_variable_set("@pending_preferences", {}) + a.instance_variable_set("@pending_preferences", {}) end it "retrieves a preference from the database before falling back to default" do preference = double(value: "chatreuse", key: 'a/color/123') expect(Spree::Preference).to receive(:find_by).and_return(preference) - expect(@a.preferred_color).to eq 'chatreuse' + expect(a.preferred_color).to eq 'chatreuse' end it "defaults if no database key exists" do expect(Spree::Preference).to receive(:find_by).and_return(nil) - expect(@a.preferred_color).to eq 'green' + expect(a.preferred_color).to eq 'green' end end context "converts integer preferences to integer values" do before do - A.preference :is_integer, :integer + a_class.preference :is_integer, :integer end it "with strings" do - @a.set_preference(:is_integer, '3') - expect(@a.preferences[:is_integer]).to eq 3 + a.set_preference(:is_integer, '3') + expect(a.preferences[:is_integer]).to eq 3 - @a.set_preference(:is_integer, '') - expect(@a.preferences[:is_integer]).to eq 0 + a.set_preference(:is_integer, '') + expect(a.preferences[:is_integer]).to eq 0 end end context "converts decimal preferences to BigDecimal values" do before do - A.preference :if_decimal, :decimal + a_class.preference :if_decimal, :decimal end it "returns a BigDecimal" do - @a.set_preference(:if_decimal, 3.3) - expect(@a.preferences[:if_decimal].class).to eq BigDecimal + a.set_preference(:if_decimal, 3.3) + expect(a.preferences[:if_decimal].class).to eq BigDecimal end it "with strings" do - @a.set_preference(:if_decimal, '3.3') - expect(@a.preferences[:if_decimal]).to eq 3.3 + a.set_preference(:if_decimal, '3.3') + expect(a.preferences[:if_decimal]).to eq 3.3 - @a.set_preference(:if_decimal, '') - expect(@a.preferences[:if_decimal]).to eq 0.0 + a.set_preference(:if_decimal, '') + expect(a.preferences[:if_decimal]).to eq 0.0 end context "when the value cannot be converted to BigDecimal" do it "returns the original value" do - @a.set_preference(:if_decimal, "invalid") - expect(@a.preferences[:if_decimal]).to eq "invalid" + a.set_preference(:if_decimal, "invalid") + expect(a.preferences[:if_decimal]).to eq "invalid" end end end context "converts boolean preferences to boolean values" do before do - A.preference :is_boolean, :boolean, default: true + a_class.preference :is_boolean, :boolean, default: true end it "with strings" do - @a.set_preference(:is_boolean, '0') - expect(@a.preferences[:is_boolean]).to be_falsy - @a.set_preference(:is_boolean, 'f') - expect(@a.preferences[:is_boolean]).to be_falsy - @a.set_preference(:is_boolean, 't') - expect(@a.preferences[:is_boolean]).to be_truthy + a.set_preference(:is_boolean, '0') + expect(a.preferences[:is_boolean]).to be_falsy + a.set_preference(:is_boolean, 'f') + expect(a.preferences[:is_boolean]).to be_falsy + a.set_preference(:is_boolean, 't') + expect(a.preferences[:is_boolean]).to be_truthy end it "with integers" do - @a.set_preference(:is_boolean, 0) - expect(@a.preferences[:is_boolean]).to be_falsy - @a.set_preference(:is_boolean, 1) - expect(@a.preferences[:is_boolean]).to be_truthy + a.set_preference(:is_boolean, 0) + expect(a.preferences[:is_boolean]).to be_falsy + a.set_preference(:is_boolean, 1) + expect(a.preferences[:is_boolean]).to be_truthy end it "with an empty string" do - @a.set_preference(:is_boolean, '') - expect(@a.preferences[:is_boolean]).to be_falsy + a.set_preference(:is_boolean, '') + expect(a.preferences[:is_boolean]).to be_falsy end it "with an empty hash" do - @a.set_preference(:is_boolean, []) - expect(@a.preferences[:is_boolean]).to be_falsy + a.set_preference(:is_boolean, []) + expect(a.preferences[:is_boolean]).to be_falsy end end context "converts any preferences to any values" do before do - A.preference :product_ids, :any, default: [] - A.preference :product_attributes, :any, default: {} + a_class.preference :product_ids, :any, default: [] + a_class.preference :product_attributes, :any, default: {} end it "with array" do - expect(@a.preferences[:product_ids]).to eq [] - @a.set_preference(:product_ids, [1, 2]) - expect(@a.preferences[:product_ids]).to eq [1, 2] + expect(a.preferences[:product_ids]).to eq [] + a.set_preference(:product_ids, [1, 2]) + expect(a.preferences[:product_ids]).to eq [1, 2] end it "with hash" do - expect(@a.preferences[:product_attributes]).to eq({}) - @a.set_preference(:product_attributes, { id: 1, name: 2 }) + expect(a.preferences[:product_attributes]).to eq({}) + a.set_preference(:product_attributes, { id: 1, name: 2 }) attributes_hash = { id: 1, name: 2 } - expect(@a.preferences[:product_attributes]).to eq attributes_hash + expect(a.preferences[:product_attributes]).to eq attributes_hash end end end describe "persisted preferables" do before(:all) do - class CreatePrefTest < ActiveRecord::Migration[4.2] - def self.up - create_table :pref_tests do |t| - t.string :col - end - end - - def self.down - drop_table :pref_tests - end - end - - @migration_verbosity = ActiveRecord::Migration.verbose ActiveRecord::Migration.verbose = false - CreatePrefTest.migrate(:up) - - class PrefTest < ApplicationRecord - preference :pref_test_pref, :string, default: 'abc' - preference :pref_test_any, :any, default: [] - end + create_pref_test.migrate(:up) end after(:all) do - CreatePrefTest.migrate(:down) - ActiveRecord::Migration.verbose = @migration_verbosity - end - - before(:each) do - @pt = PrefTest.create + create_pref_test.migrate(:down) + ActiveRecord::Migration.verbose = true end describe "pending preferences for new activerecord objects" do it "saves preferences after record is saved" do - pr = PrefTest.new + pr = pref_test_class.new pr.set_preference(:pref_test_pref, 'XXX') expect(pr.get_preference(:pref_test_pref)).to eq 'XXX' pr.save! @@ -270,7 +270,7 @@ describe Spree::Preferences::Preferable do end it "saves preferences for serialized object" do - pr = PrefTest.new + pr = pref_test_class.new pr.set_preference(:pref_test_any, [1, 2]) expect(pr.get_preference(:pref_test_any)).to eq [1, 2] pr.save! @@ -280,7 +280,7 @@ describe Spree::Preferences::Preferable do describe "requires a valid id" do it "for cache_key" do - pref_test = PrefTest.new + pref_test = pref_test_class.new expect(pref_test.preference_cache_key(:pref_test_pref)).to be_nil pref_test.save @@ -288,46 +288,48 @@ describe Spree::Preferences::Preferable do end it "but returns default values" do - pref_test = PrefTest.new + pref_test = pref_test_class.new expect(pref_test.get_preference(:pref_test_pref)).to eq 'abc' end it "adds prefs in a pending hash until after_create" do - pref_test = PrefTest.new + pref_test = pref_test_class.new expect(pref_test).to receive(:add_pending_preference).with(:pref_test_pref, 'XXX') pref_test.set_preference(:pref_test_pref, 'XXX') end end + let!(:pt) { pref_test_class.create } + it "clear preferences" do - @pt.set_preference(:pref_test_pref, 'xyz') - expect(@pt.preferred_pref_test_pref).to eq 'xyz' - @pt.clear_preferences - expect(@pt.preferred_pref_test_pref).to eq 'abc' + pt.set_preference(:pref_test_pref, 'xyz') + expect(pt.preferred_pref_test_pref).to eq 'xyz' + pt.clear_preferences + expect(pt.preferred_pref_test_pref).to eq 'abc' end it "clear preferences when record is deleted" do - @pt.save! - @pt.preferred_pref_test_pref = 'lmn' - @pt.save! - @pt.destroy - @pt1 = PrefTest.new(col: 'aaaa') - @pt1.id = @pt.id - @pt1.save! - expect(@pt1.get_preference(:pref_test_pref)).not_to eq 'lmn' - expect(@pt1.get_preference(:pref_test_pref)).to eq 'abc' + pt.save! + pt.preferred_pref_test_pref = 'lmn' + pt.save! + pt.destroy + pt1 = pref_test_class.new(col: 'aaaa') + pt1.id = pt.id + pt1.save! + expect(pt1.get_preference(:pref_test_pref)).not_to eq 'lmn' + expect(pt1.get_preference(:pref_test_pref)).to eq 'abc' end end it "builds cache keys" do - expect(@a.preference_cache_key(:color)).to match %r{a/color/\d+} + expect(a.preference_cache_key(:color)).to match %r{a/color/\d+} end it "can add and remove preferences" do - A.preference :test_temp, :boolean, default: true - expect(@a.preferred_test_temp).to be_truthy - A.remove_preference :test_temp - expect(@a.has_preference?(:test_temp)).to be_falsy - expect(@a.respond_to?(:preferred_test_temp)).to be_falsy + a_class.preference :test_temp, :boolean, default: true + expect(a.preferred_test_temp).to be_truthy + a_class.remove_preference :test_temp + expect(a.has_preference?(:test_temp)).to be_falsy + expect(a.respond_to?(:preferred_test_temp)).to be_falsy end end From 0aea14832abd13cd8612f0e904058a8a659573c8 Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 11:06:47 +0000 Subject: [PATCH 10/12] Fix offense constant definition in block in validators/date_time_string_validator_spec.rb --- .rubocop_todo.yml | 1 - .../date_time_string_validator_spec.rb | 17 ++++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index a1825eba7d..e1e44cee6f 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -11,7 +11,6 @@ # AllowedMethods: enums Lint/ConstantDefinitionInBlock: Exclude: - - 'spec/validators/date_time_string_validator_spec.rb' - 'spec/validators/integer_array_validator_spec.rb' # Offense count: 2 diff --git a/spec/validators/date_time_string_validator_spec.rb b/spec/validators/date_time_string_validator_spec.rb index 5476b0f6a0..a4b5d7a468 100644 --- a/spec/validators/date_time_string_validator_spec.rb +++ b/spec/validators/date_time_string_validator_spec.rb @@ -3,14 +3,6 @@ require "spec_helper" describe DateTimeStringValidator do - class TestModel - include ActiveModel::Validations - - attr_accessor :timestamp - - validates :timestamp, date_time_string: true - end - describe "internationalization" do it "has translation for NOT_STRING_ERROR" do expect(described_class.not_string_error).not_to be_blank @@ -22,7 +14,14 @@ describe DateTimeStringValidator do end describe "validation" do - let(:instance) { TestModel.new } + let(:instance) do + Class.new do + include ActiveModel::Validations + attr_accessor :timestamp + + validates :timestamp, date_time_string: true + end.new + end it "does not add error when nil" do instance.timestamp = nil From 061ff9178611f111adaa56db973b6d2c98806fdc Mon Sep 17 00:00:00 2001 From: Ana Nunes da Silva Date: Tue, 26 Mar 2024 11:08:53 +0000 Subject: [PATCH 11/12] Fix offense constant definition in block in validators/integer_array_validator_spec.rb --- .rubocop_todo.yml | 7 ------- spec/validators/integer_array_validator_spec.rb | 17 ++++++++--------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e1e44cee6f..bc831cf76c 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -6,13 +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: AllowedMethods. -# AllowedMethods: enums -Lint/ConstantDefinitionInBlock: - Exclude: - - 'spec/validators/integer_array_validator_spec.rb' - # Offense count: 2 Lint/DuplicateMethods: Exclude: diff --git a/spec/validators/integer_array_validator_spec.rb b/spec/validators/integer_array_validator_spec.rb index 0fbb08fd20..0643d02806 100644 --- a/spec/validators/integer_array_validator_spec.rb +++ b/spec/validators/integer_array_validator_spec.rb @@ -3,14 +3,6 @@ require "spec_helper" describe IntegerArrayValidator do - class TestModel - include ActiveModel::Validations - - attr_accessor :ids - - validates :ids, integer_array: true - end - describe "internationalization" do it "has translation for NOT_ARRAY_ERROR" do expect(described_class.not_array_error).not_to be_blank @@ -22,7 +14,14 @@ describe IntegerArrayValidator do end describe "validation" do - let(:instance) { TestModel.new } + let(:instance) do + Class.new do + include ActiveModel::Validations + attr_accessor :ids + + validates :ids, integer_array: true + end.new + end it "does not add error when nil" do instance.ids = nil From c0010319af3678685f6b4ffe50430164830e59a2 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 9 Apr 2024 12:04:42 +1000 Subject: [PATCH 12/12] Avoid duplicate loading of task in spec The new product image import spec was loading rake tasks multiple times. That make the spec for enterprise deletion fail when executed afterwards because the deletion task was executed twice and failed the second time. --- spec/lib/tasks/enterprises_rake_spec.rb | 8 +++++--- .../lib/tasks/import_product_images_rake_spec.rb | 16 +++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/spec/lib/tasks/enterprises_rake_spec.rb b/spec/lib/tasks/enterprises_rake_spec.rb index 5e3b02873e..dfcb91cebd 100644 --- a/spec/lib/tasks/enterprises_rake_spec.rb +++ b/spec/lib/tasks/enterprises_rake_spec.rb @@ -4,14 +4,16 @@ require 'spec_helper' require 'rake' describe 'enterprises.rake' do + before(:all) do + Rake.application.rake_require("tasks/enterprises") + Rake::Task.define_task(:environment) + end + describe ':remove_enterprise' do context 'when the enterprises exists' do it 'removes the enterprise' do enterprise = create(:enterprise) - Rake.application.rake_require 'tasks/enterprises' - Rake::Task.define_task(:environment) - expect { Rake.application.invoke_task "ofn:remove_enterprise[#{enterprise.id}]" }.to change { Enterprise.count }.by(-1) diff --git a/spec/lib/tasks/import_product_images_rake_spec.rb b/spec/lib/tasks/import_product_images_rake_spec.rb index bfb6dcd571..7a6451b0f2 100644 --- a/spec/lib/tasks/import_product_images_rake_spec.rb +++ b/spec/lib/tasks/import_product_images_rake_spec.rb @@ -4,21 +4,20 @@ require 'spec_helper' require 'rake' RSpec.describe 'ofn:import:product_images' do - before do - Rake.application.load_rakefile + before(:all) do + Rake.application.rake_require("tasks/import_product_images") Rake::Task.define_task(:environment) - Rake::Task['ofn:import:product_images'].reenable end - after do - Rake::Task['ofn:import:product_images'].clear + before do + Rake::Task['ofn:import:product_images'].reenable end describe 'task' do context "filename is blank" do it 'raises an error' do expect { - Rake::Task['ofn:import:product_images'].invoke('') + Rake.application.invoke_task('ofn:import:product_images') }.to raise_error(RuntimeError, 'Filename required') end @@ -27,10 +26,9 @@ RSpec.describe 'ofn:import:product_images' do context "invalid CSV format" do it 'raises an error if CSV columns are missing' do allow(CSV).to receive(:read).and_return(CSV::Table.new([])) - Rake::Task['ofn:import:product_images'].reenable expect { - Rake::Task['ofn:import:product_images'].invoke('path/to/csv/file.csv') + Rake.application.invoke_task('ofn:import:product_images["path/to/csv/file.csv"]') }.to raise_error(RuntimeError, 'CSV columns reqired: ["producer", "name", "image_url"]') end end @@ -76,7 +74,7 @@ RSpec.describe 'ofn:import:product_images' do OUTPUT expect { - Rake::Task['ofn:import:product_images'].invoke('path/to/csv/file.csv') + Rake.application.invoke_task('ofn:import:product_images["path/to/csv/file.csv"]') }.to output(expected_output).to_stdout end end