diff --git a/app/controllers/admin/customers_controller.rb b/app/controllers/admin/customers_controller.rb index 71cc8fc2c8..28fcf235ba 100644 --- a/app/controllers/admin/customers_controller.rb +++ b/app/controllers/admin/customers_controller.rb @@ -99,7 +99,7 @@ module Admin def customer_params params.require(:customer).permit( - :enterprise_id, :name, :email, :code, :tag_list, + :enterprise_id, :first_name, :last_name, :email, :code, :tag_list, ship_address_attributes: PermittedAttributes::Address.attributes, bill_address_attributes: PermittedAttributes::Address.attributes, ) diff --git a/app/controllers/spree/admin/search_controller.rb b/app/controllers/spree/admin/search_controller.rb index fc3fbad955..1ddb023b3a 100644 --- a/app/controllers/spree/admin/search_controller.rb +++ b/app/controllers/spree/admin/search_controller.rb @@ -21,7 +21,8 @@ module Spree @customers = [] if spree_current_user.enterprises.pluck(:id).include? search_params[:distributor_id].to_i @customers = Customer. - ransack(m: 'or', email_start: search_params[:q], name_start: search_params[:q]). + ransack(m: 'or', email_start: search_params[:q], first_name_start: search_params[:q], + last_name_start: search_params[:q]). result. where(enterprise_id: search_params[:distributor_id].to_i) end diff --git a/app/models/customer.rb b/app/models/customer.rb index 96a14d01d2..ec4d78235c 100644 --- a/app/models/customer.rb +++ b/app/models/customer.rb @@ -5,7 +5,7 @@ class Customer < ApplicationRecord acts_as_taggable - searchable_attributes :name, :email, :code + searchable_attributes :first_name, :last_name, :email, :code belongs_to :enterprise belongs_to :user, class_name: "Spree::User" @@ -34,6 +34,10 @@ class Customer < ApplicationRecord attr_accessor :gateway_recurring_payment_client_secret, :gateway_shop_id + def full_name + "#{first_name} #{last_name}".strip + end + private def downcase_email diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index c73f4453f0..2693ef5bb9 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -84,8 +84,7 @@ module Spree before_validation :set_currency before_validation :generate_order_number, on: :create before_validation :clone_billing_address, if: :use_billing? - before_validation :associate_customer, unless: :customer_id? - before_validation :ensure_customer, unless: :customer_is_valid? + before_validation :ensure_customer before_create :link_by_email after_create :create_tax_charge! @@ -711,23 +710,25 @@ module Spree def associate_customer return customer if customer.present? - self.customer = Customer.of(distributor).find_by(email: email_for_customer) + Customer.of(distributor).find_by(email: email_for_customer) end - def ensure_customer - return if associate_customer + def create_customer + return if customer_is_valid? - self.customer = Customer.new( + Customer.create( enterprise: distributor, email: email_for_customer, user: user, - name: bill_address&.full_name, + first_name: bill_address&.first_name.to_s, + last_name: bill_address&.last_name.to_s, bill_address: bill_address&.clone, ship_address: ship_address&.clone ) - customer.save + end - Bugsnag.notify(customer.errors.full_messages.join(", ")) unless customer.persisted? + def ensure_customer + self.customer = associate_customer || create_customer end def update_adjustment!(adjustment) diff --git a/app/serializers/api/admin/customer_serializer.rb b/app/serializers/api/admin/customer_serializer.rb index 30c067349b..14e8478e49 100644 --- a/app/serializers/api/admin/customer_serializer.rb +++ b/app/serializers/api/admin/customer_serializer.rb @@ -3,14 +3,14 @@ module Api module Admin class CustomerSerializer < ActiveModel::Serializer - attributes :id, :email, :enterprise_id, :user_id, :code, :tags, :tag_list, :name, - :allow_charges, :default_card_present? + attributes :id, :email, :enterprise_id, :user_id, :code, :tags, :tag_list, :first_name, + :last_name, :allow_charges, :default_card_present? has_one :ship_address, serializer: Api::AddressSerializer has_one :bill_address, serializer: Api::AddressSerializer - def name - object.name.presence || object.bill_address&.full_name + def full_name + object.full_name.presence || object.bill_address&.full_name end def tag_list diff --git a/app/serializers/api/admin/subscription_serializer.rb b/app/serializers/api/admin/subscription_serializer.rb index c75657ba99..cdaecf432e 100644 --- a/app/serializers/api/admin/subscription_serializer.rb +++ b/app/serializers/api/admin/subscription_serializer.rb @@ -4,8 +4,8 @@ module Api module Admin class SubscriptionSerializer < ActiveModel::Serializer attributes :id, :shop_id, :customer_id, :schedule_id, :payment_method_id, :shipping_method_id, - :begins_at, :ends_at, - :customer_email, :customer_name, :schedule_name, :edit_path, :canceled_at, :paused_at, :state, + :begins_at, :ends_at, :customer_email, :customer_first_name, :customer_last_name, + :customer_full_name, :schedule_name, :edit_path, :canceled_at, :paused_at, :state, :shipping_fee_estimate, :payment_fee_estimate has_many :subscription_line_items, serializer: Api::Admin::SubscriptionLineItemSerializer @@ -34,8 +34,16 @@ module Api object.customer&.email end - def customer_name - object.customer&.name + def customer_first_name + object.customer&.first_name + end + + def customer_last_name + object.customer&.last_name + end + + def customer_full_name + object.customer&.full_name end def schedule_name diff --git a/app/serializers/api/customer_serializer.rb b/app/serializers/api/customer_serializer.rb index 053313cb30..72423714ba 100644 --- a/app/serializers/api/customer_serializer.rb +++ b/app/serializers/api/customer_serializer.rb @@ -2,7 +2,7 @@ module Api class CustomerSerializer < ActiveModel::Serializer - attributes :id, :enterprise_id, :name, :code, :email, :allow_charges + attributes :id, :enterprise_id, :first_name, :last_name, :code, :email, :allow_charges def attributes hash = super diff --git a/app/views/admin/customers/index.html.haml b/app/views/admin/customers/index.html.haml index e2bff1832f..1124120693 100644 --- a/app/views/admin/customers/index.html.haml +++ b/app/views/admin/customers/index.html.haml @@ -50,7 +50,8 @@ %table.index#customers %col.email{ width: "20%", 'ng-show' => 'columns.email.visible' } - %col.name{ width: "20%", 'ng-show' => 'columns.name.visible' } + %col.first_name{ width: "20%", 'ng-show' => 'columns.first_name.visible' } + %col.last_name{ width: "20%", 'ng-show' => 'columns.last_name.visible' } %col.code{ width: "10%", 'ng-show' => 'columns.code.visible' } %col.tags{ width: "20%", 'ng-show' => 'columns.tags.visible' } %col.bill_address{ width: "10%", 'ng-show' => 'columns.bill_address.visible' } @@ -63,8 +64,10 @@ -# %input{ :type => "checkbox", :name => 'toggle_bulk', 'ng-click' => 'toggleAllCheckboxes()', 'ng-checked' => "allBoxesChecked()" } %th.email{ 'ng-show' => 'columns.email.visible' } %a{ :href => '', 'ng-click' => "sorting.toggle('email')" }=t('admin.email') - %th.name{ 'ng-show' => 'columns.name.visible' } - %a{ :href => '', 'ng-click' => "sorting.toggle('name')" }=t('admin.name') + %th.first_name{ 'ng-show' => 'columns.first_name.visible' } + %a{ :href => '', 'ng-click' => "sorting.toggle('first_name')" }=t('admin.first_name') + %th.last_name{ 'ng-show' => 'columns.last_name.visible' } + %a{ :href => '', 'ng-click' => "sorting.toggle('last_name')" }=t('admin.last_name') %th.code{ 'ng-show' => 'columns.code.visible' } %a{ :href => '', 'ng-click' => "sorting.toggle('code')" }=t('admin.customers.index.code') %th.tags{ 'ng-show' => 'columns.tags.visible' }=t('admin.tags') @@ -78,8 +81,10 @@ %td.email{ 'ng-show' => 'columns.email.visible'} %span{ 'ng-bind' => '::customer.email' } %span.guest-label{ 'ng-show' => 'customer.user_id == null' }= t('.guest_label') - %td.name{ 'ng-show' => 'columns.name.visible'} - %input{ type: 'text', name: 'name', ng: { model: 'customer.name' }, 'obj-for-update' => 'customer', 'attr-for-update' => 'name'} + %td.first_name{ 'ng-show' => 'columns.first_name.visible'} + %input{ type: 'text', name: 'first_name', ng: { model: 'customer.first_name' }, 'obj-for-update' => 'customer', 'attr-for-update' => 'first_name'} + %td.last_name{ 'ng-show' => 'columns.last_name.visible'} + %input{ type: 'text', name: 'last_name', ng: { model: 'customer.last_name' }, 'obj-for-update' => 'customer', 'attr-for-update' => 'last_name'} %td.code{ 'ng-show' => 'columns.code.visible' } %input{ type: 'text', name: 'code', ng: {model: 'customer.code', change: 'checkForDuplicateCodes()'}, "obj-for-update" => "customer", "attr-for-update" => "code" } %i.icon-warning-sign{ ng: {if: 'duplicate'} } diff --git a/app/views/admin/subscriptions/_table.html.haml b/app/views/admin/subscriptions/_table.html.haml index c4eb12b891..67dced6881 100644 --- a/app/views/admin/subscriptions/_table.html.haml +++ b/app/views/admin/subscriptions/_table.html.haml @@ -41,7 +41,7 @@ %td.customer.text-center{ ng: { show: 'columns.customer.visible'}} %span{ "ng-bind": '::subscription.customer_email' } %br - %span{ "ng-bind": '::subscription.customer_name' } + %span{ "ng-bind": '::subscription.customer_full_name' } %td.schedule.text-center{ ng: { show: 'columns.schedule.visible', bind: '::subscription.schedule_name' } } %td.items.panel-toggle.text-center{ name: 'products', ng: { show: 'columns.items.visible' } } %h5{ ng: { bind: 'itemCount(subscription)' } } diff --git a/config/locales/en.yml b/config/locales/en.yml index 6c23d73022..e639a9faa5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -447,6 +447,8 @@ en: ends_at: Ends At ends_on: Ends On name: Name + first_name: First Name + last_name: Last Name on_hand: On Hand on_demand: On Demand on_demand?: On Demand? diff --git a/db/migrate/20220105085729_split_customers_name.rb b/db/migrate/20220105085729_split_customers_name.rb new file mode 100644 index 0000000000..d52f122b9c --- /dev/null +++ b/db/migrate/20220105085729_split_customers_name.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class SplitCustomersName < ActiveRecord::Migration[6.1] + def up + add_column :customers, :first_name, :string, null: false, default: "" + add_column :customers, :last_name, :string, null: false, default: "" + end + + def down + remove_column :customers, :first_name + remove_column :customers, :last_name + end +end diff --git a/db/migrate/20220105085730_migrate_customers_data.rb b/db/migrate/20220105085730_migrate_customers_data.rb new file mode 100644 index 0000000000..9968d8e400 --- /dev/null +++ b/db/migrate/20220105085730_migrate_customers_data.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +class MigrateCustomersData < ActiveRecord::Migration[6.1] + class SpreeAddress < ApplicationRecord; end + + class Customer < ApplicationRecord + belongs_to :bill_address, class_name: "SpreeAddress" + end + + def up + migrate_customer_name_data! + end + + def migrate_customer_name_data! + customers_with_bill_addresses.find_each do |customer| + if bill_address_name_matches?(customer) + apply_name_from_bill_address!(customer) + next + end + + split_customer_name!(customer) + end + + customers_without_bill_addresses.find_each do |customer| + split_customer_name!(customer) + end + end + + def customers_with_bill_addresses + Customer.joins(:bill_address).where(first_name: "", last_name: "").where.not(name: [nil, ""]) + end + + def customers_without_bill_addresses + Customer.where(bill_address_id: nil, first_name: "", last_name: "").where.not(name: [nil, ""]) + end + + def bill_address_name_matches?(customer) + address_name = customer.bill_address.firstname + customer.bill_address.lastname + customer.name.delete(" ") == address_name.delete(" ") + end + + def split_customer_name!(customer) + return if (name_parts = customer.name.split(' ')).empty? + + customer.update_columns( + first_name: name_parts.first, + last_name: name_parts[1..].join(' '), + updated_at: Time.zone.now + ) + end + + def apply_name_from_bill_address!(customer) + customer.update_columns( + first_name: customer.bill_address.firstname, + last_name: customer.bill_address.lastname, + updated_at: Time.zone.now + ) + end +end diff --git a/db/schema.rb b/db/schema.rb index b1c9499ddb..ef2c12e7d2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -54,6 +54,8 @@ ActiveRecord::Schema.define(version: 2022_01_18_053107) do t.string "name", limit: 255 t.boolean "allow_charges", default: false, null: false t.datetime "terms_and_conditions_accepted_at" + t.string "first_name", default: "", null: false + t.string "last_name", default: "", null: false t.index ["bill_address_id"], name: "index_customers_on_bill_address_id" t.index ["email"], name: "index_customers_on_email" t.index ["enterprise_id", "code"], name: "index_customers_on_enterprise_id_and_code", unique: true diff --git a/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb b/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb index 59a5cd0470..0be3d77921 100644 --- a/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb +++ b/engines/order_management/app/services/order_management/reports/enterprise_fee_summary/scope.rb @@ -374,7 +374,7 @@ module OrderManagement hubs.name AS hub_name, enterprises.name AS enterprise_name, enterprise_fees.fee_type AS fee_type, - customers.name AS customer_name, + TRIM(CONCAT(customers.first_name, ' ', customers.last_name)) AS customer_name, customers.email AS customer_email, enterprise_fees.name AS fee_name, spree_tax_categories.name AS tax_category_name, diff --git a/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb b/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb index 44cf80200d..28d240a296 100644 --- a/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb +++ b/engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_service_spec.rb @@ -36,8 +36,8 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do let!(:variant) { prepare_variant } # Create customers. - let!(:customer) { create(:customer, name: "Sample Customer") } - let!(:another_customer) { create(:customer, name: "Another Customer") } + let!(:customer) { create(:customer, first_name: "Sample", last_name: "Customer") } + let!(:another_customer) { create(:customer, first_name: "Another", last_name: "Customer") } # Setup up permissions and report. let!(:current_user) { create(:admin_user) } @@ -438,9 +438,9 @@ describe OrderManagement::Reports::EnterpriseFeeSummary::ReportService do context "filtering by completion date" do let(:timestamp) { Time.zone.local(2018, 1, 5, 14, 30, 5) } - let!(:customer_a) { create(:customer, name: "Customer A") } - let!(:customer_b) { create(:customer, name: "Customer B") } - let!(:customer_c) { create(:customer, name: "Customer C") } + let!(:customer_a) { create(:customer, first_name: "Customer", last_name: "A") } + let!(:customer_b) { create(:customer, first_name: "Customer", last_name: "B") } + let!(:customer_c) { create(:customer, first_name: "Customer", last_name: "C") } let!(:order_placed_before_timestamp) do prepare_order(customer: customer_a).tap do |order| diff --git a/lib/open_food_network/column_preference_defaults.rb b/lib/open_food_network/column_preference_defaults.rb index 3f135f6b61..cd162d2638 100644 --- a/lib/open_food_network/column_preference_defaults.rb +++ b/lib/open_food_network/column_preference_defaults.rb @@ -30,9 +30,10 @@ module OpenFoodNetwork node = 'admin.customers.index' { email: { name: I18n.t("admin.email"), visible: true }, - name: { name: I18n.t("admin.name"), visible: true }, - code: { name: I18n.t("#{node}.code"), visible: true }, - tags: { name: I18n.t("admin.tags"), visible: true }, + first_name: { name: I18n.t("admin.first_name"), visible: true }, + last_name: { name: I18n.t("admin.last_name"), visible: true }, + code: { name: I18n.t("#{node}.code"), visible: true }, + tags: { name: I18n.t("admin.tags"), visible: true }, bill_address: { name: I18n.t("#{node}.bill_address"), visible: true }, ship_address: { name: I18n.t("#{node}.ship_address"), visible: true }, balance: { name: I18n.t("#{node}.balance"), visible: true } diff --git a/spec/controllers/spree/admin/search_controller_spec.rb b/spec/controllers/spree/admin/search_controller_spec.rb index 7000620af2..370dae9468 100644 --- a/spec/controllers/spree/admin/search_controller_spec.rb +++ b/spec/controllers/spree/admin/search_controller_spec.rb @@ -35,7 +35,7 @@ describe Spree::Admin::SearchController, type: :controller do describe 'searching for customers' do let!(:customer_1) { create(:customer, enterprise: enterprise, email: 'test1@email.com') } - let!(:customer_2) { create(:customer, enterprise: enterprise, name: 'test2') } + let!(:customer_2) { create(:customer, enterprise: enterprise, first_name: 'test2') } let!(:customer_3) { create(:customer, email: 'test3@email.com') } describe 'when search owned enterprises' do diff --git a/spec/factories.rb b/spec/factories.rb index f766c0cb19..73a46e9109 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -113,14 +113,6 @@ FactoryBot.define do property end - factory :customer, class: Customer do - email { generate(:random_email) } - enterprise - code { SecureRandom.base64(150) } - user - bill_address { create(:address) } - end - factory :stripe_account do enterprise { FactoryBot.create(:distributor_enterprise) } stripe_user_id { "abc123" } diff --git a/spec/factories/customer_factory.rb b/spec/factories/customer_factory.rb new file mode 100644 index 0000000000..a661255146 --- /dev/null +++ b/spec/factories/customer_factory.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :customer, class: Customer do + email { generate(:random_email) } + enterprise + code { SecureRandom.base64(150) } + user + bill_address { create(:address) } + end +end diff --git a/spec/migrations/split_customer_names_spec.rb b/spec/migrations/split_customer_names_spec.rb new file mode 100644 index 0000000000..76c3803553 --- /dev/null +++ b/spec/migrations/split_customer_names_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../../db/migrate/20220105085730_migrate_customers_data' + +describe MigrateCustomersData do + let!(:customer1) { + create(:customer, name: "Timmy Test", first_name: "", last_name: "", bill_address: nil) + } + let!(:customer2) { + create(:customer, + name: "Frank Lee Ridiculous", first_name: "", last_name: "", + bill_address: create(:address, first_name: "Frank Lee", last_name: "Ridiculous")) + } + let!(:customer3) { + create(:customer, name: "Shia Le Boeuf", first_name: "", last_name: "", + bill_address: create(:address, first_name: "Shia", last_name: "Le Boeuf")) + } + let!(:customer4) { + create(:customer, name: "No Eyed Deer", first_name: "", last_name: "", bill_address: nil) + } + let!(:customer5) { + create(:customer, name: " Space Invader ", first_name: "", last_name: "", + bill_address: nil) + } + let!(:customer6) { + create(:customer, name: "How Many Names Do You Need?", first_name: "", last_name: "", + bill_address: nil) + } + let!(:customer7) { + create(:customer, + name: "Customer Name", first_name: "", last_name: "", + bill_address: create(:address, first_name: "Different", last_name: "AddressName")) + } + + it "migrates customer names" do + subject.up + + [ + customer1, customer2, customer3, customer4, + customer5, customer6, customer7, + ].map(&:reload) + + expect([customer1.first_name, customer1.last_name]).to eq ["Timmy", "Test"] + expect([customer2.first_name, customer2.last_name]).to eq ["Frank Lee", "Ridiculous"] + expect([customer3.first_name, customer3.last_name]).to eq ["Shia", "Le Boeuf"] + expect([customer4.first_name, customer4.last_name]).to eq ["No", "Eyed Deer"] + expect([customer5.first_name, customer5.last_name]).to eq ["Space", "Invader"] + expect([customer6.first_name, customer6.last_name]).to eq ["How", "Many Names Do You Need?"] + expect([customer7.first_name, customer7.last_name]).to eq ["Customer", "Name"] + end +end diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index fe31ab000a..2a87a04009 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -940,125 +940,56 @@ describe Spree::Order do context "when creating an order" do it "does not create a customer" do - order = create(:order, distributor: distributor) - expect(order.customer).to be_nil - end - end - - context "when updating the order" do - let!(:order) { create(:order, distributor: distributor) } - - before do - order.state = "complete" - order.save! - end - - it "creates a customer" do - expect(order.customer).not_to be_nil - end - end - end - - describe "#associate_customer" do - let(:distributor) { create(:distributor_enterprise) } - let!(:order) { create(:order, distributor: distributor) } - - context "when an email address is available for the order" do - before { allow(order).to receive(:email_for_customer) { "existing@email.com" } } - - context "and a customer for order.distributor and order#email_for_customer already exists" do - let!(:customer) { create(:customer, enterprise: distributor, email: "existing@email.com" ) } - - it "associates the order with the existing customer, and returns the customer" do - result = order.send(:associate_customer) - expect(order.customer).to eq customer - expect(result).to eq customer - end - end - - context "and a customer for order.distributor and order.user.email does not alread exist" do - let!(:customer) { - create(:customer, enterprise: distributor, email: 'some-other-email@email.com') + expect { + create(:order, distributor: distributor) + }.to_not change { + Customer.count } - - it "does not set the customer and returns nil" do - result = order.send(:associate_customer) - expect(order.customer).to be_nil - expect(result).to be_nil - end end - end - context "when an email address is not available for the order" do - let!(:customer) { create(:customer, enterprise: distributor) } - before { allow(order).to receive(:email_for_customer) { nil } } + it "associates an existing customer" do + customer = create( + :customer, + user: user, + email: user.email, + enterprise: distributor + ) + order = create(:order, user: user, distributor: distributor) - it "does not set the customer and returns nil" do - result = order.send(:associate_customer) - expect(order.customer).to be_nil - expect(result).to be_nil - end - end - end - - describe "ensuring a customer is linked" do - let(:distributor) { create(:distributor_enterprise) } - let!(:order) { create(:order, distributor: distributor) } - - context "when a customer has already been linked to the order" do - let!(:customer) { create(:customer, enterprise: distributor, email: "existing@email.com" ) } - before { order.update_attribute(:customer_id, customer.id) } - - it "does nothing" do - order.send(:ensure_customer) expect(order.customer).to eq customer end end - context "when a customer not been linked to the order" do - context "but one matching order#email_for_customer already exists" do - let!(:customer) { - create(:customer, enterprise: distributor, email: 'some-other-email@email.com') - } - before { allow(order).to receive(:email_for_customer) { 'some-other-email@email.com' } } - - it "links the customer customer to the order" do - expect(order.customer).to be_nil - expect{ order.send(:ensure_customer) }.to_not change{ Customer.count } - expect(order.customer).to eq customer - end + context "when updating the order" do + before do + order.update!(distributor: distributor) end - context "and order#email_for_customer does not match any existing customers" do - before do - order.bill_address = create(:address) - order.ship_address = create(:address) - end + it "associates an existing customer" do + customer = create( + :customer, + user: user, + email: user.email, + enterprise: distributor + ) - context "and the customer is not valid" do - before do - order.distributor = nil - order.user = nil - order.email = nil - end + order.update!(state: "complete") - it "sends an error to Bugsnag" do - expect(Bugsnag) - .to receive(:notify).with("Email can't be blank, Enterprise can't be blank") - order.send(:ensure_customer) - end - end + expect(order.customer).to eq customer + end - context "and the customer is valid" do - it "creates a new customer with defaut name and addresses" do - expect(order.customer).to be_nil - expect { order.send(:ensure_customer) }.to change{ Customer.count }.by 1 + it "doesn't create a customer before needed" do + expect(order.customer).to eq nil + end - expect(order.customer.name).to eq order.bill_address.full_name - expect(order.customer.bill_address.same_as?(order.bill_address)).to be true - expect(order.customer.ship_address.same_as?(order.ship_address)).to be true - end - end + it "creates a customer" do + expect { + order.update!(state: "complete") + }.to change { + Customer.count + }.by(1) + + expect(order.customer).to be_present end end end diff --git a/spec/system/admin/customers_spec.rb b/spec/system/admin/customers_spec.rb index 3692d90bf4..eb4fca8363 100644 --- a/spec/system/admin/customers_spec.rb +++ b/spec/system/admin/customers_spec.rb @@ -14,7 +14,9 @@ describe 'Customers' do let(:unmanaged_distributor) { create(:distributor_enterprise) } describe "using the customers index" do - let!(:customer1) { create(:customer, enterprise: managed_distributor1, code: nil) } + let!(:customer1) { + create(:customer, first_name: 'John', last_name: 'Doe', enterprise: managed_distributor1, code: nil) + } let!(:customer2) { create(:customer, enterprise: managed_distributor1, code: nil) } let!(:customer3) { create(:customer, enterprise: unmanaged_distributor) } let!(:customer4) { create(:customer, enterprise: managed_distributor2) } @@ -177,13 +179,14 @@ describe 'Customers' do select2_select managed_distributor1.name, from: "shop_id" within "tr#c_#{customer1.id}" do - expect(find_field('name').value).to eq 'John Doe' + expect(find_field('first_name').value).to eq 'John' + expect(find_field('last_name').value).to eq 'Doe' fill_in "code", with: "new-customer-code" expect(page).to have_css "input[name=code].update-pending" - fill_in "name", with: "customer abc" - expect(page).to have_css "input[name=name].update-pending" + fill_in "first_name", with: "customer abc" + expect(page).to have_css "input[name=first_name].update-pending" find(:css, "tags-input .tags input").set "awesome\n" expect(page).to have_css ".tag_watcher.update-pending" @@ -193,12 +196,12 @@ describe 'Customers' do # Every says it updated expect(page).to have_css "input[name=code].update-success" - expect(page).to have_css "input[name=name].update-success" + expect(page).to have_css "input[name=first_name].update-success" expect(page).to have_css ".tag_watcher.update-success" # And it actually did expect(customer1.reload.code).to eq "new-customer-code" - expect(customer1.reload.name).to eq "customer abc" + expect(customer1.reload.first_name).to eq "customer abc" expect(customer1.tag_list).to eq ["awesome"] # Clearing attributes @@ -206,8 +209,8 @@ describe 'Customers' do fill_in "code", with: "" expect(page).to have_css "input[name=code].update-pending" - fill_in "name", with: "" - expect(page).to have_css "input[name=name].update-pending" + fill_in "first_name", with: "" + expect(page).to have_css "input[name=first_name].update-pending" find("tags-input li.tag-item a.remove-button").click expect(page).to have_css ".tag_watcher.update-pending" @@ -216,12 +219,12 @@ describe 'Customers' do # Every says it updated expect(page).to have_css "input[name=code].update-success" - expect(page).to have_css "input[name=name].update-success" + expect(page).to have_css "input[name=first_name].update-success" expect(page).to have_css ".tag_watcher.update-success" # And it actually did expect(customer1.reload.code).to be nil - expect(customer1.reload.name).to eq '' + expect(customer1.reload.first_name).to eq '' expect(customer1.tag_list).to eq [] end diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb index dae71d2d86..7182da0851 100644 --- a/spec/system/admin/order_spec.rb +++ b/spec/system/admin/order_spec.rb @@ -239,7 +239,7 @@ describe ' new_customer = Customer.last - expect(new_customer.name).to eq('Clark Kent') + expect(new_customer.full_name).to eq('Clark Kent') expect(new_customer.bill_address.address1).to eq('Smallville') expect(new_customer.bill_address.city).to eq('Kansas') expect(new_customer.bill_address.zipcode).to eq('SP1 M11') diff --git a/spec/system/admin/subscriptions_spec.rb b/spec/system/admin/subscriptions_spec.rb index 41807ec0e4..01b9fdab3a 100644 --- a/spec/system/admin/subscriptions_spec.rb +++ b/spec/system/admin/subscriptions_spec.rb @@ -19,7 +19,7 @@ describe 'Subscriptions' do let!(:subscription) { create(:subscription, shop: shop, with_items: true, with_proxy_orders: true) } - let!(:customer) { create(:customer, name: "Customer A") } + let!(:customer) { create(:customer, first_name: "Timmy", last_name: "Test") } let!(:other_subscription) { create(:subscription, shop: shop, customer: customer, with_items: true, with_proxy_orders: true) @@ -81,8 +81,13 @@ describe 'Subscriptions' do expect(page).to have_selector "tr#so_#{other_subscription.id}" expect(page).to have_no_selector "tr#so_#{subscription.id}" - # Using the Quick Search: filter by name - fill_in 'query', with: other_subscription.customer.name + # Using the Quick Search: filter by first_name + fill_in 'query', with: other_subscription.customer.first_name + expect(page).to have_selector "tr#so_#{other_subscription.id}" + expect(page).to have_no_selector "tr#so_#{subscription.id}" + + # Using the Quick Search: filter by last_name + fill_in 'query', with: other_subscription.customer.last_name expect(page).to have_selector "tr#so_#{other_subscription.id}" expect(page).to have_no_selector "tr#so_#{subscription.id}"