Merge pull request #10704 from abdellani/hide-customers-with-no-compl…

…eted-orders

Hide users with no completed orders from a hub's customers list

app/controllers/admin/customers_controller.rb

@@ -36,8 +36,10 @@ def show
     end
 
     def create
-      @customer = Customer.new(customer_params)
+      @customer = Customer.find_or_initialize_by(customer_params.slice(:email, :enterprise_id))
+
       if user_can_create_customer?
+        @customer.created_manually = true
         if @customer.save
           tag_rule_mapping = TagRule.mapping_for(Enterprise.where(id: @customer.enterprise))
           render_as_json @customer, tag_rule_mapping: tag_rule_mapping
@@ -83,7 +85,7 @@ def collection
     def customers
       return @customers if @customers.present?
 
-      @customers = Customer.managed_by(spree_current_user)
+      @customers = Customer.visible.managed_by(spree_current_user)
       return @customers if params[:enterprise_id].blank?
 
       @customers = @customers.where(enterprise_id: params[:enterprise_id])

app/controllers/api/v0/customers_controller.rb

@@ -6,7 +6,7 @@ class CustomersController < Api::V0::BaseController
       skip_authorization_check only: :index
 
       def index
-        @customers = current_api_user.customers
+        @customers = current_api_user.customers.visible
         render json: @customers, each_serializer: CustomerSerializer
       end
 

app/controllers/api/v1/customers_controller.rb

@@ -29,7 +29,8 @@ def show
 
       def create
         authorize! :update, Enterprise.find(customer_params[:enterprise_id])
-        customer = Customer.new(customer_params)
+        customer = Customer.find_or_initialize_by(customer_params.slice(:email, :enterprise_id))
+        customer.assign_attributes(customer_params)
 
         if customer.save
           render json: Api::V1::CustomerSerializer.new(customer), status: :created
@@ -80,7 +81,7 @@ def search_customers
       end
 
       def visible_customers
-        Customer.managed_by(current_api_user)
+        Customer.visible.managed_by(current_api_user)
       end
 
       def customer_params
@@ -96,6 +97,7 @@ def customer_params
           ]
         ).to_h
 
+        attributes.merge!(created_manually: true)
         attributes.merge!(tag_list: params[:tags]) if params.key?(:tags)
 
         transform_address!(attributes, :billing_address, :bill_address)

app/models/customer.rb

@@ -41,6 +41,8 @@ class Customer < ApplicationRecord
 
   scope :of, ->(enterprise) { where(enterprise_id: enterprise) }
   scope :managed_by, ->(user) { user&.persisted? ? where(user: user).or(of(Enterprise.managed_by(user))) : none }
+  scope :created_manually, -> { where(created_manually: true) }
+  scope :visible, -> { where(id: Spree::Order.complete.select(:customer_id)).or(created_manually) }
 
   before_create :associate_user
 

db/migrate/20230516072511_add_created_manually_flag_to_customer.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class AddCreatedManuallyFlagToCustomer < ActiveRecord::Migration[7.0]
+  def change
+    add_column :customers, :created_manually, :boolean, null: false, default: false
+    add_index :customers, :created_manually
+  end
+end

db/migrate/20230525081252_update_created_manually_flag_on_customers.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class UpdateCreatedManuallyFlagOnCustomers < ActiveRecord::Migration[7.0]
+  class Customer < ApplicationRecord
+    has_many :orders
+  end
+
+  class Order < ApplicationRecord
+    self.table_name = 'spree_orders'
+  end
+
+  def up
+    Customer.where.missing(:orders).update_all(created_manually: true)
+  end
+
+  def down
+    Customer.where(created_manually: true).update_all(created_manually: false)
+  end
+end

db/schema.rb

@@ -93,7 +93,9 @@
     t.datetime "terms_and_conditions_accepted_at", precision: nil
     t.string "first_name", default: "", null: false
     t.string "last_name", default: "", null: false
+    t.boolean "created_manually", default: false, null: false
     t.index ["bill_address_id"], name: "index_customers_on_bill_address_id"
+    t.index ["created_manually"], name: "index_customers_on_created_manually"
     t.index ["email"], name: "index_customers_on_email"
     t.index ["enterprise_id", "code"], name: "index_customers_on_enterprise_id_and_code", unique: true
     t.index ["ship_address_id"], name: "index_customers_on_ship_address_id"

spec/controllers/admin/customers_controller_spec.rb

@@ -22,7 +22,7 @@ module Admin
       end
 
       context "json" do
-        let!(:customer) { create(:customer, enterprise: enterprise) }
+        let!(:customer) { create(:customer, enterprise: enterprise, created_manually: true) }
 
         context "where I manage the enterprise" do
           before do
@@ -31,7 +31,7 @@ module Admin
 
           context "and enterprise_id is given in params" do
             let(:user){ enterprise.users.first }
-            let(:customers){ Customer.managed_by(user).where(enterprise_id: enterprise.id) }
+            let(:customers){ Customer.visible.managed_by(user).where(enterprise_id: enterprise.id) }
             let(:params) { { format: :json, enterprise_id: enterprise.id } }
 
             it "scopes @collection to customers of that enterprise" do
@@ -205,6 +205,7 @@ def create_customer(enterprise)
 
           it "allows me to create the customer" do
             expect { create_customer enterprise }.to change(Customer, :count).by(1)
+            expect(Customer.reorder(:id).last.created_manually).to eq true
           end
         end
 

spec/controllers/api/v0/customers_controller_spec.rb

@@ -10,8 +10,8 @@ module Api
     let(:user) { create(:user) }
 
     describe "index" do
-      let!(:customer1) { create(:customer) }
-      let!(:customer2) { create(:customer) }
+      let!(:customer1) { create(:customer, created_manually: true) }
+      let!(:customer2) { create(:customer, created_manually: true) }
 
       before do
         user.customers << customer1

spec/models/customer_spec.rb

@@ -116,5 +116,30 @@
         expect(Customer.managed_by(guest)).to match_array []
       end
     end
+
+    context "with split checkout enabled" do
+      before do
+        Flipper.enable(:split_checkout)
+      end
+
+      context 'visible' do
+        let!(:customer) { create(:customer) }
+        let!(:customer2) { create(:customer) }
+        let!(:customer3) { create(:customer) }
+        let!(:customer4) { create(:customer) }
+        let!(:customer5) { create(:customer, created_manually: true) }
+
+        let!(:order_ready_for_details) { create(:order_ready_for_details, customer: customer) }
+        let!(:order_ready_for_payment) { create(:order_ready_for_payment, customer: customer2) }
+        let!(:order_ready_for_confirmation) {
+          create(:order_ready_for_confirmation, customer: customer3)
+        }
+        let!(:completed_order) { create(:completed_order_with_totals, customer: customer4) }
+
+        it 'returns customers with completed orders' do
+          expect(Customer.visible).to match_array [customer4, customer5]
+        end
+      end
+    end
   end
 end

spec/requests/api/v1/customers_spec.rb

@@ -11,13 +11,14 @@
     create(
       :customer,
       enterprise: enterprise1,
+      created_manually: true,
       terms_and_conditions_accepted_at: Time.zone.parse("2000-01-01"),
       tag_list: ["long-term"],
       ship_address: create(:address),
     )
   }
-  let!(:customer2) { create(:customer, enterprise: enterprise1) }
-  let!(:customer3) { create(:customer, enterprise: enterprise2) }
+  let!(:customer2) { create(:customer, enterprise: enterprise1, created_manually: true,) }
+  let!(:customer3) { create(:customer, enterprise: enterprise2, created_manually: true,) }
 
   before do
     Flipper.enable(:api_v1)
@@ -189,6 +190,9 @@
             allow_charges: false,
             terms_and_conditions_accepted_at: nil,
           )
+
+          customer = Customer.find(json_response[:data][:attributes][:id])
+          expect(customer.created_manually).to eq true
         end
       end
 

spec/system/admin/customers_spec.rb

@@ -16,11 +16,17 @@
     describe "using the customers index" do
       let!(:customer1) {
         create(:customer, first_name: 'John', last_name: 'Doe', enterprise: managed_distributor1, 
-code: nil)
+code: nil, created_manually: true)
+      }
+      let!(:customer2) {
+        create(:customer, enterprise: managed_distributor1, created_manually: true, code: nil)
+      }
+      let!(:customer3) {
+        create(:customer, enterprise: unmanaged_distributor, created_manually: true,)
+      }
+      let!(:customer4) {
+        create(:customer, enterprise: managed_distributor2, created_manually: true,)
       }
-      let!(:customer2) { create(:customer, enterprise: managed_distributor1, code: nil) }
-      let!(:customer3) { create(:customer, enterprise: unmanaged_distributor) }
-      let!(:customer4) { create(:customer, enterprise: managed_distributor2) }
 
       before do
         login_as user
@@ -346,6 +352,7 @@ def wait_for_modal_fade_in(time = 0.4)
         context "when a shop is selected" do
           before do
             select2_select managed_distributor1.name, from: "shop_id"
+            customer1.update!(created_manually: false)
           end
 
           it "creates customers when the email provided is valid" do
@@ -366,21 +373,30 @@ def wait_for_modal_fade_in(time = 0.4)
                                             text: "Email is invalid"
             }.to_not change{ Customer.of(managed_distributor1).count }
 
-            # When an existing email is used
-            expect{
-              fill_in 'email', with: customer1.email
-              click_button 'Add Customer'
-              expect(page).to have_selector "#new-customer-dialog .error",
-                                            text: "Email is associated with an existing customer"
-            }.to_not change{ Customer.of(managed_distributor1).count }
-
             # When a new valid email is used
             expect{
               fill_in 'email', with: "[email protected]"
               click_button 'Add Customer'
               expect(page).not_to have_selector "#new-customer-dialog"
             }.to change{ Customer.of(managed_distributor1).count }.from(2).to(3)
           end
+
+          it "shows a hidden customer when trying to create it" do
+            click_link('New Customer')
+            fill_in 'email', with: customer1.email
+
+            expect do
+              click_button 'Add Customer'
+              expect(page).not_to have_selector "#new-customer-dialog"
+              customer1.reload
+            end
+              .to change { customer1.created_manually }.from(false).to(true)
+              .and change { Customer.count }.by(0)
+
+            expect(page).to have_content customer1.email
+            expect(page).to have_field "first_name", with: "John"
+            expect(page).to have_field "last_name", with: "Doe"
+          end
         end
       end
     end

spec/system/consumer/account/cards_spec.rb

@@ -9,7 +9,7 @@
 
   describe "as a logged in user" do
     let(:user) { create(:user) }
-    let!(:customer) { create(:customer, user: user) }
+    let!(:customer) { create(:customer, user: user, created_manually: true) }
     let!(:default_card) {
       create(:stored_credit_card, user_id: user.id, gateway_customer_profile_id: 'cus_AZNMJ',
                                   is_default: true)