From d9153f2c0075df507496b8608df05a2dc5712dc6 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 1 Jun 2023 19:09:54 +0100 Subject: [PATCH 01/24] Introduce "zero priced order" concept with no required payment This applies to cases where an order contains items with zero price or where applied vouchers have reduced the order's total to zero. --- app/models/spree/order.rb | 8 +++++++- app/views/spree/shared/_order_details.html.haml | 14 +++++++++----- config/locales/en.yml | 1 + 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index 027bff09821..f5ab00ea9cf 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -23,7 +23,7 @@ class Order < ApplicationRecord go_to_state :delivery go_to_state :payment, if: ->(order) { order.update_totals - order.payment_required? + order.payment_required? || order.zero_priced_order? } go_to_state :confirmation go_to_state :complete @@ -220,6 +220,12 @@ def payment_required? total.to_f > 0.0 && !skip_payment_for_subscription? end + # There are items present in the order, but either the items have zero price, + # or the order's total has been modified (maybe discounted) to zero. + def zero_priced_order? + valid? && line_items.count.positive? && total.zero? + end + # Returns the relevant zone (if any) to be used for taxation purposes. # Uses default tax zone unless there is a specific match def tax_zone diff --git a/app/views/spree/shared/_order_details.html.haml b/app/views/spree/shared/_order_details.html.haml index b559074bf51..b3e56509840 100644 --- a/app/views/spree/shared/_order_details.html.haml +++ b/app/views/spree/shared/_order_details.html.haml @@ -11,11 +11,15 @@ %strong = order.display_total.to_html .pad - .text-big - = t :order_payment - %strong= last_payment_method(order)&.name - %p.text-small.text-skinny.pre-line.word-wrap - %em= last_payment_method(order)&.description + - if (order_payment_method = last_payment_method(order)) + .text-big + = t :order_payment + %strong= order_payment_method&.name + %p.text-small.text-skinny.pre-line.word-wrap + %em= order_payment_method&.description + - else + .text-big + = t(:no_payment_required) .order-summary.text-small %strong diff --git a/config/locales/en.yml b/config/locales/en.yml index 395a6cfaae9..53f09d49415 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2117,6 +2117,7 @@ en: order_not_paid: NOT PAID order_total: Total order order_payment: "Paying via:" + no_payment_required: "No payment required" order_billing_address: Billing address order_delivery_on: Delivery on order_delivery_address: Delivery address From 24b2ff3fbc7553ad0ba4d5fadb80cc3dff383993 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 1 Jun 2023 19:11:16 +0100 Subject: [PATCH 02/24] Simplify voucher controller --- app/controllers/admin/vouchers_controller.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/controllers/admin/vouchers_controller.rb b/app/controllers/admin/vouchers_controller.rb index efd9ac9b7d8..3f190a8001b 100644 --- a/app/controllers/admin/vouchers_controller.rb +++ b/app/controllers/admin/vouchers_controller.rb @@ -9,14 +9,11 @@ def new end def create - voucher_params = permitted_resource_params.merge(enterprise: @enterprise) - @voucher = Voucher.create(voucher_params) + @voucher = Voucher.create(permitted_resource_params.merge(enterprise: @enterprise)) if @voucher.save - redirect_to( - "#{edit_admin_enterprise_path(@enterprise)}#vouchers_panel", - flash: { success: flash_message_for(@voucher, :successfully_created) } - ) + flash[:success] = flash_message_for(@voucher, :successfully_created) + redirect_to edit_admin_enterprise_path(@enterprise, anchor: :vouchers_panel) else flash[:error] = @voucher.errors.full_messages.to_sentence render :new From da11241a14c12d7961cda85f2efb954c811b6061 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 00:12:33 +0100 Subject: [PATCH 03/24] Move form definition down into each checkout step --- app/views/split_checkout/_details.html.haml | 277 ++++++++++---------- app/views/split_checkout/_form.html.haml | 7 +- app/views/split_checkout/_payment.html.haml | 67 ++--- app/views/split_checkout/_summary.html.haml | 177 ++++++------- 4 files changed, 264 insertions(+), 264 deletions(-) diff --git a/app/views/split_checkout/_details.html.haml b/app/views/split_checkout/_details.html.haml index e81c8c08a79..53b4e29fddb 100644 --- a/app/views/split_checkout/_details.html.haml +++ b/app/views/split_checkout/_details.html.haml @@ -1,155 +1,156 @@ -.medium-6 - = f.fields :bill_address, model: @order.bill_address do |bill_address| - %div.checkout-substep - -# YOUR DETAILS - %div.checkout-title - = t("split_checkout.step1.contact_information.title") - - .two-columns-inputs - %div.checkout-input.with-floating-label{ "data-controller": "floating-label" } - = f.label :email, t("split_checkout.step1.contact_information.email.label") - = f.text_field :email, { placeholder: " " } - = f.error_message_on :email - - %div.checkout-input.with-floating-label{ "data-controller": "floating-label" } - = bill_address.label :phone, t("split_checkout.step1.contact_information.phone.label") - = bill_address.text_field :phone, { placeholder: " " } - = f.error_message_on "bill_address.phone" += form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| + .medium-6 + = f.fields :bill_address, model: @order.bill_address do |bill_address| + %div.checkout-substep + -# YOUR DETAILS + %div.checkout-title + = t("split_checkout.step1.contact_information.title") + + .two-columns-inputs + %div.checkout-input.with-floating-label{ "data-controller": "floating-label" } + = f.label :email, t("split_checkout.step1.contact_information.email.label") + = f.text_field :email, { placeholder: " " } + = f.error_message_on :email + + %div.checkout-input.with-floating-label{ "data-controller": "floating-label" } + = bill_address.label :phone, t("split_checkout.step1.contact_information.phone.label") + = bill_address.text_field :phone, { placeholder: " " } + = f.error_message_on "bill_address.phone" + + %div.checkout-substep + -# BILLING ADDRESS + %div.checkout-title + = t("split_checkout.step1.billing_address.title") + + .two-columns-inputs + %div.checkout-input.with-floating-label{ "data-controller": "floating-label" } + = bill_address.label :firstname, t("split_checkout.step1.billing_address.first_name.label") + = bill_address.text_field :firstname, { placeholder: " " } + = f.error_message_on "bill_address.firstname" + + %div.checkout-input.with-floating-label{ "data-controller": "floating-label" } + = bill_address.label :lastname, t("split_checkout.step1.billing_address.last_name.label") + = bill_address.text_field :lastname, { placeholder: " " } + = f.error_message_on "bill_address.lastname" - %div.checkout-substep - -# BILLING ADDRESS - %div.checkout-title - = t("split_checkout.step1.billing_address.title") - - .two-columns-inputs - %div.checkout-input.with-floating-label{ "data-controller": "floating-label" } - = bill_address.label :firstname, t("split_checkout.step1.billing_address.first_name.label") - = bill_address.text_field :firstname, { placeholder: " " } - = f.error_message_on "bill_address.firstname" - - %div.checkout-input.with-floating-label{ "data-controller": "floating-label" } - = bill_address.label :lastname, t("split_checkout.step1.billing_address.last_name.label") - = bill_address.text_field :lastname, { placeholder: " " } - = f.error_message_on "bill_address.lastname" - - %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} - = bill_address.label :address1, t("split_checkout.step1.address.address1.label") - = bill_address.text_field :address1, { placeholder: " " } - = f.error_message_on "bill_address.address1" - - %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} - = bill_address.label :address2, t("split_checkout.step1.address.address2.label") - = bill_address.text_field :address2, { placeholder: " " } - = f.error_message_on "bill_address.address2" - - %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} - = bill_address.label :city, t("split_checkout.step1.address.city.label") - = bill_address.text_field :city, { placeholder: " " } - = f.error_message_on "bill_address.city" - - %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} - = bill_address.label :zipcode, t("split_checkout.step1.address.zipcode.label") - = bill_address.text_field :zipcode, { placeholder: " " } - = f.error_message_on "bill_address.zipcode" - - %div{ "data-controller": "dependent-select", "data-dependent-select-options-value": countries_with_states } - - bill_address_country = @order.bill_address.country || DefaultCountry.country - - %div.checkout-input - = bill_address.label :country_id, t("split_checkout.step1.address.country_id.label") - = bill_address.select :country_id, countries, { selected: bill_address_country.id }, { "data-dependent-select-target": "source", "data-action": "dependent-select#handleSelectChange" } - - %div.checkout-input - = bill_address.label :state_id, t("split_checkout.step1.address.state_id.label") - = bill_address.select :state_id, states_for_country(bill_address_country), { selected: @order.bill_address&.state_id }, { "data-dependent-select-target": "select" } - - - if spree_current_user - %div.checkout-input - = f.check_box :save_bill_address - = f.label :save_bill_address, t(:checkout_default_bill_address) - - %div.checkout-substep{ "data-controller": "toggle shippingmethod" } - - selected_shipping_method = @order.shipping_method&.id || params[:shipping_method_id] - %div.checkout-title - = t("split_checkout.step1.shipping_info.title") - - - display_ship_address = false - - ship_method_description = nil - - - selected_shipping_method ||= @shipping_methods[0].id if @shipping_methods.length == 1 - - @shipping_methods.each do |shipping_method| - - ship_method_is_selected = shipping_method.id == selected_shipping_method.to_i - %div.checkout-input.checkout-input-radio - = fields_for shipping_method do |shipping_method_form| - = shipping_method_form.radio_button :name, shipping_method.id, - id: "shipping_method_#{shipping_method.id}", - checked: ship_method_is_selected, - name: "shipping_method_id", - "data-requireAddress": shipping_method.require_ship_address, - "data-action": "toggle#toggle shippingmethod#selectShippingMethod", - "data-toggle-show": shipping_method.require_ship_address - = shipping_method_form.label shipping_method.id, shipping_method.name, {for: "shipping_method_" + shipping_method.id.to_s } - %em.fees= payment_or_shipping_price(shipping_method, @order) - - display_ship_address = display_ship_address || (ship_method_is_selected && shipping_method.require_ship_address) - %div.checkout-input{"data-shippingmethod-target": "shippingMethodDescription", "data-shippingmethodid": shipping_method.id , style: "display: #{ship_method_is_selected ? 'block' : 'none'}" } - #distributor_address.panel - - if shipping_method.description.present? - %span #{shipping_method.description} - %br/ - %br/ - - if @order.order_cycle.pickup_time_for(@order.distributor) - = t :checkout_ready_for - = @order.order_cycle.pickup_time_for(@order.distributor) - - = f.error_message_on :shipping_method, standalone: true - - %div.checkout-input{ "data-toggle-target": "content", style: "display: #{display_ship_address ? 'block' : 'none'}" } - = f.check_box :ship_address_same_as_billing, { id: "ship_address_same_as_billing", name: "ship_address_same_as_billing", "data-action": "shippingmethod#showHideShippingAddress", "data-shippingmethod-target": "shippingAddressCheckbox", checked: shipping_and_billing_match?(@order) }, 1, nil - = f.label :ship_address_same_as_billing, t(:checkout_address_same), { for: "ship_address_same_as_billing" } - - %div{"data-shippingmethod-target": "shippingMethodAddress", style: "display: #{!display_ship_address || shipping_and_billing_match?(@order) ? 'none' : 'block'}" } - = f.fields :ship_address, model: @order.ship_address do |ship_address| %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} - = ship_address.label :address1, t("split_checkout.step1.address.address1.label") - = ship_address.text_field :address1, { placeholder: " " } - = f.error_message_on "ship_address.address1" + = bill_address.label :address1, t("split_checkout.step1.address.address1.label") + = bill_address.text_field :address1, { placeholder: " " } + = f.error_message_on "bill_address.address1" %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} - = ship_address.label :address2, t("split_checkout.step1.address.address2.label") - = ship_address.text_field :address2, { placeholder: " " } - = f.error_message_on "ship_address.address2" + = bill_address.label :address2, t("split_checkout.step1.address.address2.label") + = bill_address.text_field :address2, { placeholder: " " } + = f.error_message_on "bill_address.address2" %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} - = ship_address.label :city, t("split_checkout.step1.address.city.label") - = ship_address.text_field :city, { placeholder: " " } - = f.error_message_on "ship_address.city" + = bill_address.label :city, t("split_checkout.step1.address.city.label") + = bill_address.text_field :city, { placeholder: " " } + = f.error_message_on "bill_address.city" %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} - = ship_address.label :zipcode, t("split_checkout.step1.address.zipcode.label") - = ship_address.text_field :zipcode, { placeholder: " " } - = f.error_message_on "ship_address.zipcode" + = bill_address.label :zipcode, t("split_checkout.step1.address.zipcode.label") + = bill_address.text_field :zipcode, { placeholder: " " } + = f.error_message_on "bill_address.zipcode" %div{ "data-controller": "dependent-select", "data-dependent-select-options-value": countries_with_states } - - ship_address_country = @order.ship_address.country || DefaultCountry.country + - bill_address_country = @order.bill_address.country || DefaultCountry.country %div.checkout-input - = ship_address.label :country_id, t("split_checkout.step1.address.country_id.label") - = ship_address.select :country_id, countries, { selected: ship_address_country.id }, { "data-dependent-select-target": "source", "data-action": "dependent-select#handleSelectChange" } + = bill_address.label :country_id, t("split_checkout.step1.address.country_id.label") + = bill_address.select :country_id, countries, { selected: bill_address_country.id }, { "data-dependent-select-target": "source", "data-action": "dependent-select#handleSelectChange" } %div.checkout-input - = ship_address.label :state_id, t("split_checkout.step1.address.state_id.label") - = ship_address.select :state_id, states_for_country(ship_address_country), { selected: @order.ship_address&.state_id }, { "data-dependent-select-target": "select" } + = bill_address.label :state_id, t("split_checkout.step1.address.state_id.label") + = bill_address.select :state_id, states_for_country(bill_address_country), { selected: @order.bill_address&.state_id }, { "data-dependent-select-target": "select" } - - if spree_current_user - %div.checkout-input{ "data-toggle-target": "content", style: "display: #{display_ship_address ? 'block' : 'none'}" } - = f.check_box :save_ship_address - = f.label :save_ship_address, t(:checkout_default_ship_address) + - if spree_current_user + %div.checkout-input + = f.check_box :save_bill_address + = f.label :save_bill_address, t(:checkout_default_bill_address) + + %div.checkout-substep{ "data-controller": "toggle shippingmethod" } + - selected_shipping_method = @order.shipping_method&.id || params[:shipping_method_id] + %div.checkout-title + = t("split_checkout.step1.shipping_info.title") + + - display_ship_address = false + - ship_method_description = nil + + - selected_shipping_method ||= @shipping_methods[0].id if @shipping_methods.length == 1 + - @shipping_methods.each do |shipping_method| + - ship_method_is_selected = shipping_method.id == selected_shipping_method.to_i + %div.checkout-input.checkout-input-radio + = fields_for shipping_method do |shipping_method_form| + = shipping_method_form.radio_button :name, shipping_method.id, + id: "shipping_method_#{shipping_method.id}", + checked: ship_method_is_selected, + name: "shipping_method_id", + "data-requireAddress": shipping_method.require_ship_address, + "data-action": "toggle#toggle shippingmethod#selectShippingMethod", + "data-toggle-show": shipping_method.require_ship_address + = shipping_method_form.label shipping_method.id, shipping_method.name, {for: "shipping_method_" + shipping_method.id.to_s } + %em.fees= payment_or_shipping_price(shipping_method, @order) + - display_ship_address = display_ship_address || (ship_method_is_selected && shipping_method.require_ship_address) + %div.checkout-input{"data-shippingmethod-target": "shippingMethodDescription", "data-shippingmethodid": shipping_method.id , style: "display: #{ship_method_is_selected ? 'block' : 'none'}" } + #distributor_address.panel + - if shipping_method.description.present? + %span #{shipping_method.description} + %br/ + %br/ + - if @order.order_cycle.pickup_time_for(@order.distributor) + = t :checkout_ready_for + = @order.order_cycle.pickup_time_for(@order.distributor) + + = f.error_message_on :shipping_method, standalone: true - .div.checkout-input - = f.label :special_instructions, t(:checkout_instructions) - = f.text_area :special_instructions, size: "60x4" + %div.checkout-input{ "data-toggle-target": "content", style: "display: #{display_ship_address ? 'block' : 'none'}" } + = f.check_box :ship_address_same_as_billing, { id: "ship_address_same_as_billing", name: "ship_address_same_as_billing", "data-action": "shippingmethod#showHideShippingAddress", "data-shippingmethod-target": "shippingAddressCheckbox", checked: shipping_and_billing_match?(@order) }, 1, nil + = f.label :ship_address_same_as_billing, t(:checkout_address_same), { for: "ship_address_same_as_billing" } + + %div{"data-shippingmethod-target": "shippingMethodAddress", style: "display: #{!display_ship_address || shipping_and_billing_match?(@order) ? 'none' : 'block'}" } + = f.fields :ship_address, model: @order.ship_address do |ship_address| + %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} + = ship_address.label :address1, t("split_checkout.step1.address.address1.label") + = ship_address.text_field :address1, { placeholder: " " } + = f.error_message_on "ship_address.address1" + + %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} + = ship_address.label :address2, t("split_checkout.step1.address.address2.label") + = ship_address.text_field :address2, { placeholder: " " } + = f.error_message_on "ship_address.address2" + + %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} + = ship_address.label :city, t("split_checkout.step1.address.city.label") + = ship_address.text_field :city, { placeholder: " " } + = f.error_message_on "ship_address.city" + + %div.checkout-input.with-floating-label{ "data-controller": "floating-label"} + = ship_address.label :zipcode, t("split_checkout.step1.address.zipcode.label") + = ship_address.text_field :zipcode, { placeholder: " " } + = f.error_message_on "ship_address.zipcode" + + %div{ "data-controller": "dependent-select", "data-dependent-select-options-value": countries_with_states } + - ship_address_country = @order.ship_address.country || DefaultCountry.country + + %div.checkout-input + = ship_address.label :country_id, t("split_checkout.step1.address.country_id.label") + = ship_address.select :country_id, countries, { selected: ship_address_country.id }, { "data-dependent-select-target": "source", "data-action": "dependent-select#handleSelectChange" } + + %div.checkout-input + = ship_address.label :state_id, t("split_checkout.step1.address.state_id.label") + = ship_address.select :state_id, states_for_country(ship_address_country), { selected: @order.ship_address&.state_id }, { "data-dependent-select-target": "select" } - %div.checkout-submit - = f.submit t("split_checkout.step1.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false - %a.button.cancel{href: main_app.cart_path} - = t("split_checkout.step1.cancel") + - if spree_current_user + %div.checkout-input{ "data-toggle-target": "content", style: "display: #{display_ship_address ? 'block' : 'none'}" } + = f.check_box :save_ship_address + = f.label :save_ship_address, t(:checkout_default_ship_address) + + .div.checkout-input + = f.label :special_instructions, t(:checkout_instructions) + = f.text_area :special_instructions, size: "60x4" + + %div.checkout-submit + = f.submit t("split_checkout.step1.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false + %a.button.cancel{href: main_app.cart_path} + = t("split_checkout.step1.cancel") diff --git a/app/views/split_checkout/_form.html.haml b/app/views/split_checkout/_form.html.haml index 0d32cac7c5e..afedf10ebd0 100644 --- a/app/views/split_checkout/_form.html.haml +++ b/app/views/split_checkout/_form.html.haml @@ -1,8 +1,5 @@ - content_for :injection_data do = inject_saved_credit_cards -%div.checkout-step{"class": if checkout_step?(:summary) then "checkout-summary" end} - = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, - data: { remote: "true" } do |form| - - = render "split_checkout/#{checkout_step}", f: form +%div.checkout-step{ class: "#{'checkout-summary' if checkout_step?(:summary)}" } + = render "split_checkout/#{checkout_step}" diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml index 82d3adcb3a6..a85fe0d922b 100644 --- a/app/views/split_checkout/_payment.html.haml +++ b/app/views/split_checkout/_payment.html.haml @@ -1,37 +1,38 @@ -.medium-6 - %div.checkout-substep{"data-controller": "paymentmethod"} - = render partial: "split_checkout/voucher_section", formats: [:cable_ready], locals: { order: @order, voucher_adjustment: @voucher_adjustment } - - %div.checkout-title - = t("split_checkout.step2.payment_method.title") += form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| + .medium-6 + %div.checkout-substep{"data-controller": "paymentmethod"} + = render partial: "split_checkout/voucher_section", formats: [:cable_ready], locals: { order: @order, voucher_adjustment: @voucher_adjustment } - - selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id - - selected_payment_method ||= available_payment_methods[0].id if available_payment_methods.length == 1 - - available_payment_methods.each do |payment_method| - %div.checkout-input.checkout-input-radio - = f.radio_button :payment_method_id, payment_method.id, - id: "payment_method_#{payment_method.id}", - name: "order[payments_attributes][][payment_method_id]", - checked: (payment_method.id == selected_payment_method), - "data-action": "paymentmethod#selectPaymentMethod", - "data-paymentmethod-id": "#{payment_method.id}", - "data-paymentmethod-target": "input" - = f.label :payment_method_id, "#{payment_method.name}", for: "payment_method_#{payment_method.id}" - %em.fees=payment_or_shipping_price(payment_method, @order) + %div.checkout-title + = t("split_checkout.step2.payment_method.title") - .paymentmethod-container{"data-paymentmethod-id": "#{payment_method.id}", style: "display: #{payment_method.id == selected_payment_method ? "block" : "none"}"} - - if payment_method.description && !payment_method.description.empty? - .paymentmethod-description.panel - #{payment_method.description} - .paymentmethod-form - = render partial: "split_checkout/payment/#{payment_method.method_type}", locals: { payment_method: payment_method, f: f } + - selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id + - selected_payment_method ||= available_payment_methods[0].id if available_payment_methods.length == 1 + - available_payment_methods.each do |payment_method| + %div.checkout-input.checkout-input-radio + = f.radio_button :payment_method_id, payment_method.id, + id: "payment_method_#{payment_method.id}", + name: "order[payments_attributes][][payment_method_id]", + checked: (payment_method.id == selected_payment_method), + "data-action": "paymentmethod#selectPaymentMethod", + "data-paymentmethod-id": "#{payment_method.id}", + "data-paymentmethod-target": "input" + = f.label :payment_method_id, "#{payment_method.name}", for: "payment_method_#{payment_method.id}" + %em.fees=payment_or_shipping_price(payment_method, @order) - = f.error_message_on :payment_method, standalone: true - - %div.checkout-substep - = t("split_checkout.step2.explaination") + .paymentmethod-container{"data-paymentmethod-id": "#{payment_method.id}", style: "display: #{payment_method.id == selected_payment_method ? "block" : "none"}"} + - if payment_method.description && !payment_method.description.empty? + .paymentmethod-description.panel + #{payment_method.description} + .paymentmethod-form + = render partial: "split_checkout/payment/#{payment_method.method_type}", locals: { payment_method: payment_method, f: f } - %div.checkout-submit - = f.submit t("split_checkout.step2.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false - %a.button.cancel{href: main_app.checkout_step_path(:details)} - = t("split_checkout.step2.cancel") + = f.error_message_on :payment_method, standalone: true + + %div.checkout-substep + = t("split_checkout.step2.explaination") + + %div.checkout-submit + = f.submit t("split_checkout.step2.submit"), class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false + %a.button.cancel{href: main_app.checkout_step_path(:details)} + = t("split_checkout.step2.cancel") diff --git a/app/views/split_checkout/_summary.html.haml b/app/views/split_checkout/_summary.html.haml index e395bd7411a..bd14fd3e7d3 100644 --- a/app/views/split_checkout/_summary.html.haml +++ b/app/views/split_checkout/_summary.html.haml @@ -1,103 +1,104 @@ -.summary-main - = render partial: "split_checkout/already_ordered" if show_bought_items? && checkout_step?(:summary) - .checkout-substep - .checkout-title - = t("split_checkout.step3.delivery_details.title") - %a.summary-edit{href: main_app.checkout_step_path(:details)} - = t("split_checkout.step3.delivery_details.edit") += form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| + .summary-main + = render partial: "split_checkout/already_ordered" if show_bought_items? && checkout_step?(:summary) + .checkout-substep + .checkout-title + = t("split_checkout.step3.delivery_details.title") + %a.summary-edit{href: main_app.checkout_step_path(:details)} + = t("split_checkout.step3.delivery_details.edit") - .summary-subtitle - = @order.shipping_method.name - %em.fees= payment_or_shipping_price(@order.shipping_method, @order) - .two-columns - %div - .summary-subtitle - = t("split_checkout.step3.delivery_details.address") - %span - = @order.bill_address.firstname - = @order.bill_address.lastname - %div - = @order.bill_address.phone + .summary-subtitle + = @order.shipping_method.name + %em.fees= payment_or_shipping_price(@order.shipping_method, @order) + .two-columns %div - = @order.user.email if @order.user - %br - %div - = @order.bill_address.address1 - - unless @order.bill_address.address2.blank? + .summary-subtitle + = t("split_checkout.step3.delivery_details.address") + %span + = @order.bill_address.firstname + = @order.bill_address.lastname %div - = @order.bill_address.address2 - %div - = @order.bill_address.city - %div - = @order.bill_address.state - %div - = @order.bill_address.zipcode - %div - = @order.bill_address.country - - if @order.special_instructions.present? + = @order.bill_address.phone + %div + = @order.user.email if @order.user %br - %em - = @order.special_instructions - - if @order.shipping_method.description.present? - %div - .summary-subtitle - = t("split_checkout.step3.delivery_details.instructions") %div - = @order.shipping_method.description - - %hr + = @order.bill_address.address1 + - unless @order.bill_address.address2.blank? + %div + = @order.bill_address.address2 + %div + = @order.bill_address.city + %div + = @order.bill_address.state + %div + = @order.bill_address.zipcode + %div + = @order.bill_address.country + - if @order.special_instructions.present? + %br + %em + = @order.special_instructions + - if @order.shipping_method.description.present? + %div + .summary-subtitle + = t("split_checkout.step3.delivery_details.instructions") + %div + = @order.shipping_method.description + + %hr - .checkout-substep - .checkout-title - = t("split_checkout.step3.payment_method.title") - %a.summary-edit{href: main_app.checkout_step_path(:payment)} - = t("split_checkout.step3.payment_method.edit") - .two-columns - %div - - payment_method = last_payment_method(@order) - = payment_method&.name - %em.fees=payment_or_shipping_price(payment_method, @order) - - if payment_method&.description.present? + .checkout-substep + .checkout-title + = t("split_checkout.step3.payment_method.title") + %a.summary-edit{href: main_app.checkout_step_path(:payment)} + = t("split_checkout.step3.payment_method.edit") + .two-columns %div - .summary-subtitle - = t("split_checkout.step3.payment_method.instructions") + - payment_method = last_payment_method(@order) + = payment_method&.name + %em.fees=payment_or_shipping_price(payment_method, @order) + - if payment_method&.description.present? %div - = last_payment_method(@order)&.description + .summary-subtitle + = t("split_checkout.step3.payment_method.instructions") + %div + = last_payment_method(@order)&.description - %div.checkout-substep - %div.checkout-title - = t("split_checkout.step3.order.title") - %a.summary-edit{href: main_app.cart_path} - = t("split_checkout.step3.order.edit") - - = render 'spree/orders/summary', order: @order, display_footer: false + %div.checkout-substep + %div.checkout-title + = t("split_checkout.step3.order.title") + %a.summary-edit{href: main_app.cart_path} + = t("split_checkout.step3.order.edit") + = render 'spree/orders/summary', order: @order, display_footer: false -.summary-right{ "data-controller": "sticky", "data-sticky-target": "container" } - .summary-right-line.total - .summary-right-line-label= t :order_total_price - .summary-right-line-value#order_total= @order.display_total.to_html - - .summary-right-line - .summary-right-line-label= t :order_produce - .summary-right-line-value= display_checkout_subtotal(@order) - - checkout_adjustments_for(@order, exclude: [:line_item]).reverse_each do |adjustment| - .summary-right-line - -if adjustment.originator_type == 'Voucher' - .summary-right-line-label.voucher= adjustment.label - .summary-right-line-value.voucher= adjustment.display_amount.to_html - -else - .summary-right-line-label= adjustment.label - .summary-right-line-value= adjustment.display_amount.to_html + .summary-right{ "data-controller": "sticky", "data-sticky-target": "container" } + .summary-right-line.total + .summary-right-line-label= t :order_total_price + .summary-right-line-value#order_total= @order.display_total.to_html - - if @order.total_tax > 0 .summary-right-line - .summary-right-line-label= t :order_includes_tax - .summary-right-line-value#tax-row= display_checkout_tax_total(@order) - - .checkout-submit - - if any_terms_required?(@order.distributor) - = render partial: "terms_and_conditions", locals: { f: f } - = f.submit t("split_checkout.step3.submit"), name: "confirm_order", class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false + .summary-right-line-label= t :order_produce + .summary-right-line-value= display_checkout_subtotal(@order) + + - checkout_adjustments_for(@order, exclude: [:line_item]).reverse_each do |adjustment| + .summary-right-line + -if adjustment.originator_type == 'Voucher' + .summary-right-line-label.voucher= adjustment.label + .summary-right-line-value.voucher= adjustment.display_amount.to_html + -else + .summary-right-line-label= adjustment.label + .summary-right-line-value= adjustment.display_amount.to_html + + - if @order.total_tax > 0 + .summary-right-line + .summary-right-line-label= t :order_includes_tax + .summary-right-line-value#tax-row= display_checkout_tax_total(@order) + + .checkout-submit + - if any_terms_required?(@order.distributor) + = render partial: "terms_and_conditions", locals: { f: f } + = f.submit t("split_checkout.step3.submit"), name: "confirm_order", class: "button primary", disabled: @terms_and_conditions_accepted == false || @platform_tos_accepted == false From 35011014754ce0f2120a32657e9a5a5b846ddfa9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 00:20:19 +0100 Subject: [PATCH 04/24] Move voucher section out of main checkout form --- app/views/split_checkout/_payment.html.haml | 8 +++-- .../_voucher_section.cable_ready.haml | 35 +++++++++---------- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml index a85fe0d922b..fb626ae4f9a 100644 --- a/app/views/split_checkout/_payment.html.haml +++ b/app/views/split_checkout/_payment.html.haml @@ -1,8 +1,10 @@ -= form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| - .medium-6 - %div.checkout-substep{"data-controller": "paymentmethod"} +.medium-6 + - if @order.distributor.vouchers.present? + %div.checkout-substep = render partial: "split_checkout/voucher_section", formats: [:cable_ready], locals: { order: @order, voucher_adjustment: @voucher_adjustment } + = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| + %div.checkout-substep{"data-controller": "paymentmethod"} %div.checkout-title = t("split_checkout.step2.payment_method.title") diff --git a/app/views/split_checkout/_voucher_section.cable_ready.haml b/app/views/split_checkout/_voucher_section.cable_ready.haml index a005b5698b9..059acfa00a9 100644 --- a/app/views/split_checkout/_voucher_section.cable_ready.haml +++ b/app/views/split_checkout/_voucher_section.cable_ready.haml @@ -1,19 +1,18 @@ %div#voucher-section - - if order.distributor.vouchers.present? - .checkout-title - = t("split_checkout.step2.voucher.apply_voucher") - .checkout-input - .two-columns-inputs.voucher{"data-controller": "toggle-button-disabled"} - - if voucher_adjustment.present? - %span.button.voucher-added - %i.ofn-i_051-check-big - = t("split_checkout.step2.voucher.voucher", voucher_amount: voucher_adjustment.originator.display_value) - = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") } - - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ? - - if voucher_adjustment.originator.amount > order.total - .checkout-input - %span.formError.standalone - = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") - - else - = text_field_tag "[order][voucher_code]", params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher" - = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } + .checkout-title + = t("split_checkout.step2.voucher.apply_voucher") + .checkout-input + .two-columns-inputs.voucher{"data-controller": "toggle-button-disabled"} + - if voucher_adjustment.present? + %span.button.voucher-added + %i.ofn-i_051-check-big + = t("split_checkout.step2.voucher.voucher", voucher_amount: voucher_adjustment.originator.display_value) + = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") } + - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ? + - if voucher_adjustment.originator.amount > order.total + .checkout-input + %span.formError.standalone + = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") + - else + = text_field_tag "[order][voucher_code]", params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher" + = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } From 7978aa82a7e83fa4e05ab0268ffb01f77e90f48f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 00:35:16 +0100 Subject: [PATCH 05/24] Add separate voucher form --- app/views/split_checkout/_payment.html.haml | 2 +- ...r_section.cable_ready.haml => _voucher_section.html.haml} | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) rename app/views/split_checkout/{_voucher_section.cable_ready.haml => _voucher_section.html.haml} (66%) diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml index fb626ae4f9a..2b0fd81aa09 100644 --- a/app/views/split_checkout/_payment.html.haml +++ b/app/views/split_checkout/_payment.html.haml @@ -1,7 +1,7 @@ .medium-6 - if @order.distributor.vouchers.present? %div.checkout-substep - = render partial: "split_checkout/voucher_section", formats: [:cable_ready], locals: { order: @order, voucher_adjustment: @voucher_adjustment } + = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @voucher_adjustment } = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| %div.checkout-substep{"data-controller": "paymentmethod"} diff --git a/app/views/split_checkout/_voucher_section.cable_ready.haml b/app/views/split_checkout/_voucher_section.html.haml similarity index 66% rename from app/views/split_checkout/_voucher_section.cable_ready.haml rename to app/views/split_checkout/_voucher_section.html.haml index 059acfa00a9..d50aad39331 100644 --- a/app/views/split_checkout/_voucher_section.cable_ready.haml +++ b/app/views/split_checkout/_voucher_section.html.haml @@ -14,5 +14,6 @@ %span.formError.standalone = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") - else - = text_field_tag "[order][voucher_code]", params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher" - = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } + = form_with url: voucher_adjustments_path, method: :post, data: { remote: true } do |form| + = form.text_field :voucher_code, value: params[:voucher_code], data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher" + = form.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } From 0e93946b978276b1e9e756be16ac8daaecd91c51 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 00:58:06 +0100 Subject: [PATCH 06/24] Move voucher processing out of checkout controller --- app/controllers/split_checkout_controller.rb | 57 ---------------- .../voucher_adjustments_controller.rb | 66 ++++++++++++++----- config/routes.rb | 2 +- spec/requests/voucher_adjustments_spec.rb | 42 +++++------- 4 files changed, 66 insertions(+), 101 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 31f2368b695..7b3d57977b9 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -29,8 +29,6 @@ def edit end def update - return process_voucher if params[:apply_voucher].present? - if confirm_order || update_order return if performed? @@ -60,27 +58,6 @@ def render_error replace("#flashes", partial("shared/flashes", locals: { flashes: flash })) end - def render_voucher_section_or_redirect - respond_to do |format| - format.cable_ready { render_voucher_section } - format.html { redirect_to checkout_step_path(:payment) } - end - end - - # Using the power of cable_car we replace only the #voucher_section instead of reloading the page - def render_voucher_section - render( - status: :ok, - cable_ready: cable_car.replace( - "#voucher-section", - partial( - "split_checkout/voucher_section", - locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first } - ) - ) - ) - end - def order_error_messages # Remove ship_address.* errors if no shipping method is not selected remove_ship_address_errors if no_ship_address_needed? @@ -201,40 +178,6 @@ def shipping_method_ship_address_not_required? selected_shipping_method.first.require_ship_address == false end - def process_voucher - if add_voucher - VoucherAdjustmentsService.calculate(@order) - render_voucher_section_or_redirect - elsif @order.errors.present? - render_error - end - end - - def add_voucher - if params.dig(:order, :voucher_code).blank? - @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found')) - return false - end - - # Fetch Voucher - voucher = Voucher.find_by(code: params[:order][:voucher_code], enterprise: @order.distributor) - - if voucher.nil? - @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found')) - return false - end - - adjustment = voucher.create_adjustment(voucher.code, @order) - - unless adjustment.valid? - @order.errors.add(:voucher, I18n.t('split_checkout.errors.add_voucher_error')) - adjustment.errors.each { |error| @order.errors.import(error) } - return false - end - - true - end - def summary_step? params[:step] == "summary" end diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb index b45b60fd9b9..050e71d1185 100644 --- a/app/controllers/voucher_adjustments_controller.rb +++ b/app/controllers/voucher_adjustments_controller.rb @@ -1,32 +1,66 @@ # frozen_string_literal: true class VoucherAdjustmentsController < BaseController - include CablecarResponses + before_action :set_order - def destroy - @order = current_order + def create + if add_voucher + render_voucher_section + elsif @order.errors.present? + render_error + end + end + def destroy @order.voucher_adjustments.find_by(id: params[:id])&.destroy - respond_to do |format| - format.cable_ready { render_voucher_section } - format.html { redirect_to checkout_step_path(:payment) } - end + render_voucher_section end private - # Using the power of cable_car we replace only the #voucher_section instead of reloading the page + def set_order + @order = current_order + end + + def add_voucher + if params[:voucher_code].blank? + @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found')) + return false + end + + voucher = Voucher.find_by(code: params[:voucher_code], enterprise: @order.distributor) + + if voucher.nil? + @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found')) + return false + end + + adjustment = voucher.create_adjustment(voucher.code, @order) + + if !adjustment.valid? + @order.errors.add(:voucher, I18n.t('split_checkout.errors.add_voucher_error')) + adjustment.errors.each { |error| @order.errors.import(error) } + return false + end + + true + end + def render_voucher_section - render( - status: :ok, - cable_ready: cable_car.replace( - "#voucher-section", - partial( - "split_checkout/voucher_section", - locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first } - ) + render cable_ready: cable_car.replace( + selector: "#voucher-section", + html: render_to_string( + partial: "split_checkout/voucher_section", + locals: { order: @order,voucher_adjustment: @order.voucher_adjustments.first } ) ) end + + def render_error + flash.now[:error] = @order.errors.full_messages.to_sentence + + render status: :unprocessable_entity, cable_ready: cable_car. + replace("#flashes", partial("shared/flashes", locals: { flashes: flash })) + end end diff --git a/config/routes.rb b/config/routes.rb index 1d933363ac7..c92481cb64c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -112,7 +112,7 @@ get '/:id/shop', to: 'enterprises#shop', as: 'enterprise_shop' get "/enterprises/:permalink", to: redirect("/") # Legacy enterprise URL - resources :voucher_adjustments, only: [:destroy] + resources :voucher_adjustments, only: [:create, :destroy] get 'sitemap.xml', to: 'sitemap#index', defaults: { format: 'xml' } diff --git a/spec/requests/voucher_adjustments_spec.rb b/spec/requests/voucher_adjustments_spec.rb index 9154f7ea327..fc533473b86 100644 --- a/spec/requests/voucher_adjustments_spec.rb +++ b/spec/requests/voucher_adjustments_spec.rb @@ -18,41 +18,29 @@ end describe "DELETE voucher_adjustments/:id" do - let(:cable_ready_header) { { accept: "text/vnd.cable-ready.json" } } + it "deletes the voucher adjustment" do + delete "/voucher_adjustments/#{adjustment.id}" - context "with a cable ready request" do - it "deletes the voucher adjustment" do - delete("/voucher_adjustments/#{adjustment.id}", headers: cable_ready_header) - - expect(order.voucher_adjustments.length).to eq(0) - end - - it "render a succesful response" do - delete("/voucher_adjustments/#{adjustment.id}", headers: cable_ready_header) - - expect(response).to be_successful - end + expect(order.voucher_adjustments.length).to eq(0) + end - context "when adjustment doesn't exits" do - it "does nothing" do - delete "/voucher_adjustments/-1", headers: cable_ready_header + it "render a succesful response" do + delete "/voucher_adjustments/#{adjustment.id}" - expect(order.voucher_adjustments.length).to eq(1) - end + expect(response).to be_successful + end - it "render a succesful response" do - delete "/voucher_adjustments/-1", headers: cable_ready_header + context "when adjustment doesn't exits" do + it "does nothing" do + delete "/voucher_adjustments/-1" - expect(response).to be_successful - end + expect(order.voucher_adjustments.length).to eq(1) end - end - context "with an html request" do - it "redirect to checkout payment step" do - delete "/voucher_adjustments/#{adjustment.id}" + it "render a succesful response" do + delete "/voucher_adjustments/-1" - expect(response).to redirect_to(checkout_step_path(:payment)) + expect(response).to be_successful end end end From 2b98d2e76e2a5b2e2e4a012cf19289d8e8a4950b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 14:31:28 +0100 Subject: [PATCH 07/24] Move voucher adjustment calculations out of checkout controller --- app/controllers/voucher_adjustments_controller.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb index 050e71d1185..b9f5c0524e0 100644 --- a/app/controllers/voucher_adjustments_controller.rb +++ b/app/controllers/voucher_adjustments_controller.rb @@ -5,6 +5,8 @@ class VoucherAdjustmentsController < BaseController def create if add_voucher + VoucherAdjustmentsService.calculate(@order) + render_voucher_section elsif @order.errors.present? render_error From 930d4b1454e25414cfc55062b48ae801d147420b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 14:33:51 +0100 Subject: [PATCH 08/24] Don't apply tax calculations if there's no tax --- app/services/voucher_adjustments_service.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/services/voucher_adjustments_service.rb b/app/services/voucher_adjustments_service.rb index 2569010d95c..ccadb8eb752 100644 --- a/app/services/voucher_adjustments_service.rb +++ b/app/services/voucher_adjustments_service.rb @@ -19,8 +19,10 @@ def self.calculate(order) # For now we just assume it is either all tax included in price or all tax excluded from price. if order.additional_tax_total.positive? handle_tax_excluded_from_price(order, amount) - else + elsif order.included_tax_total.positive? handle_tax_included_in_price(order, amount) + else + adjustment.amount = amount end # Move to closed state From 28a8aa7cf3defc0c184a76054c36aa2ccc26657c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 15:15:15 +0100 Subject: [PATCH 09/24] Drop superfluous method --- app/controllers/split_checkout_controller.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 7b3d57977b9..c74b3f0ba12 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -25,7 +25,9 @@ def edit redirect_to_step_based_on_order unless params[:step] check_step if params[:step] - flash_error_when_no_shipping_method_available if available_shipping_methods.none? + return if available_shipping_methods.any? + + flash[:error] = I18n.t('split_checkout.errors.no_shipping_methods_available') end def update @@ -102,10 +104,6 @@ def bill_address_error_order(error) end end - def flash_error_when_no_shipping_method_available - flash[:error] = I18n.t('split_checkout.errors.no_shipping_methods_available') - end - def check_payments_adjustments @order.payments.each(&:ensure_correct_adjustment) end From 2e5ae8a0a9b353ad22927368dca5621c01252440 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 15:41:39 +0100 Subject: [PATCH 10/24] Extract voucher tests to separate controller spec --- .../split_checkout_controller_spec.rb | 67 ---------------- .../voucher_adjustments_controller_spec.rb | 78 +++++++++++++++++++ 2 files changed, 78 insertions(+), 67 deletions(-) create mode 100644 spec/controllers/voucher_adjustments_controller_spec.rb diff --git a/spec/controllers/split_checkout_controller_spec.rb b/spec/controllers/split_checkout_controller_spec.rb index 114a947c395..7783717e280 100644 --- a/spec/controllers/split_checkout_controller_spec.rb +++ b/spec/controllers/split_checkout_controller_spec.rb @@ -233,73 +233,6 @@ expect(order.payments.first.source.id).to eq saved_card.id end end - - describe "Vouchers" do - let(:voucher) { create(:voucher, code: 'some_code', enterprise: distributor) } - - describe "adding a voucher" do - let(:checkout_params) do - { - apply_voucher: "true", - order: { - voucher_code: voucher.code - } - } - end - - it "adds a voucher to the order" do - # Set the headers to simulate a cable_ready request - request.headers["accept"] = "text/vnd.cable-ready.json" - - put :update, params: params - - expect(response.status).to eq(200) - expect(order.reload.voucher_adjustments.length).to eq(1) - end - - context "when voucher doesn't exist" do - let(:checkout_params) do - { - apply_voucher: "true", - order: { - voucher_code: "non_voucher" - } - } - end - - it "returns 422 and an error message" do - put :update, params: params - - expect(response.status).to eq 422 - expect(flash[:error]).to match "Voucher Not found" - end - end - - context "when adding fails" do - it "returns 422 and an error message" do - # Create a non valid adjustment - adjustment = build(:adjustment, label: nil) - allow(voucher).to receive(:create_adjustment).and_return(adjustment) - allow(Voucher).to receive(:find_by).and_return(voucher) - - put :update, params: params - - expect(response.status).to eq 422 - expect(flash[:error]).to match( - "There was an error while adding the voucher and Label can't be blank" - ) - end - end - - context "with an html request" do - it "redirects to the payment step" do - put :update, params: params - - expect(response).to redirect_to(checkout_step_path(:payment)) - end - end - end - end end context "summary step" do diff --git a/spec/controllers/voucher_adjustments_controller_spec.rb b/spec/controllers/voucher_adjustments_controller_spec.rb new file mode 100644 index 00000000000..d9366dae174 --- /dev/null +++ b/spec/controllers/voucher_adjustments_controller_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe VoucherAdjustmentsController, type: :controller do + let(:user) { order.user } + let(:address) { create(:address) } + let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) } + let(:order_cycle) { create(:order_cycle, distributors: [distributor]) } + let(:exchange) { order_cycle.exchanges.outgoing.first } + let(:order) { + create(:order_with_line_items, line_items_count: 1, distributor: distributor, + order_cycle: order_cycle, bill_address: address, + ship_address: address) + } + let(:payment_method) { distributor.payment_methods.first } + let(:shipping_method) { distributor.shipping_methods.first } + + let(:voucher) { create(:voucher, code: 'some_code', enterprise: distributor) } + + before do + exchange.variants << order.line_items.first.variant + order.select_shipping_method shipping_method.id + OrderWorkflow.new(order).advance_to_payment + + allow(controller).to receive(:current_order) { order } + allow(controller).to receive(:spree_current_user) { user } + end + + describe "#create" do + describe "adding a voucher" do + let(:params) { { voucher_code: voucher.code } } + + it "adds a voucher to the user's current order" do + post :create, params: params + + expect(response.status).to eq(200) + expect(order.reload.voucher_adjustments.length).to eq(1) + end + + context "when voucher doesn't exist" do + let(:params) { { voucher_code: "non_voucher" } } + + it "returns 422 and an error message" do + post :create, params: params + + expect(response.status).to eq 422 + expect(flash[:error]).to match "Voucher Not found" + end + end + + context "when adding fails" do + it "returns 422 and an error message" do + # Create a non valid adjustment + adjustment = build(:adjustment, label: nil) + allow(voucher).to receive(:create_adjustment).and_return(adjustment) + allow(Voucher).to receive(:find_by).and_return(voucher) + + post :create, params: params + + expect(response.status).to eq 422 + expect(flash[:error]).to match( + "There was an error while adding the voucher and Label can't be blank" + ) + end + end + end + end + + describe "#destroy" do + it "removes the voucher from the current order" do + put :destroy, params: { id: voucher.id } + + expect(order.reload.voucher_adjustments.count).to eq 0 + expect(response.status).to eq 200 + end + end +end From f23cda50248494393704b6f49d6580313f3913f4 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 20:18:14 +0100 Subject: [PATCH 11/24] Introduce "zero priced orders" to checkout UI and order state flow --- app/controllers/split_checkout_controller.rb | 1 + app/helpers/checkout_helper.rb | 2 + app/models/spree/order.rb | 4 ++ app/views/split_checkout/_payment.html.haml | 44 +++++++++++--------- app/views/split_checkout/_summary.html.haml | 13 ++++-- 5 files changed, 40 insertions(+), 24 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index c74b3f0ba12..6d714c258d5 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -203,6 +203,7 @@ def validate_details! def validate_payment! return true if params.dig(:order, :payments_attributes, 0, :payment_method_id).present? + return true if @order.zero_priced_order? @order.errors.add :payment_method, I18n.t('split_checkout.errors.select_a_payment_method') end diff --git a/app/helpers/checkout_helper.rb b/app/helpers/checkout_helper.rb index 7b4ac43f2e6..4c1154b5ecb 100644 --- a/app/helpers/checkout_helper.rb +++ b/app/helpers/checkout_helper.rb @@ -120,6 +120,8 @@ def payment_method_price(method, order) end def payment_or_shipping_price(method, order) + return unless method + price = method.compute_amount(order) if price.zero? t('checkout_method_free') diff --git a/app/models/spree/order.rb b/app/models/spree/order.rb index f5ab00ea9cf..1bb2bf1c71f 100644 --- a/app/models/spree/order.rb +++ b/app/models/spree/order.rb @@ -622,6 +622,10 @@ def process_each_payment raise Core::GatewayError, Spree.t(:no_pending_payments) if pending_payments.empty? pending_payments.each do |payment| + if payment.amount.zero? && zero_priced_order? + payment.update_columns(state: "completed", captured_at: Time.zone.now) + end + break if payment_total >= total yield payment diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml index 2b0fd81aa09..12a94405ee2 100644 --- a/app/views/split_checkout/_payment.html.haml +++ b/app/views/split_checkout/_payment.html.haml @@ -8,28 +8,32 @@ %div.checkout-title = t("split_checkout.step2.payment_method.title") - - selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id - - selected_payment_method ||= available_payment_methods[0].id if available_payment_methods.length == 1 - - available_payment_methods.each do |payment_method| - %div.checkout-input.checkout-input-radio - = f.radio_button :payment_method_id, payment_method.id, - id: "payment_method_#{payment_method.id}", - name: "order[payments_attributes][][payment_method_id]", - checked: (payment_method.id == selected_payment_method), - "data-action": "paymentmethod#selectPaymentMethod", - "data-paymentmethod-id": "#{payment_method.id}", - "data-paymentmethod-target": "input" - = f.label :payment_method_id, "#{payment_method.name}", for: "payment_method_#{payment_method.id}" - %em.fees=payment_or_shipping_price(payment_method, @order) + - if @order.zero_priced_order? + %h3= t(:no_payment_required) + = hidden_field_tag "order[payments_attributes][][amount]", 0 + - else + - selected_payment_method = @order.payments&.with_state(:checkout)&.first&.payment_method_id + - selected_payment_method ||= available_payment_methods[0].id if available_payment_methods.length == 1 + - available_payment_methods.each do |payment_method| + %div.checkout-input.checkout-input-radio + = f.radio_button :payment_method_id, payment_method.id, + id: "payment_method_#{payment_method.id}", + name: "order[payments_attributes][][payment_method_id]", + checked: (payment_method.id == selected_payment_method), + "data-action": "paymentmethod#selectPaymentMethod", + "data-paymentmethod-id": "#{payment_method.id}", + "data-paymentmethod-target": "input" + = f.label :payment_method_id, "#{payment_method.name}", for: "payment_method_#{payment_method.id}" + %em.fees=payment_or_shipping_price(payment_method, @order) - .paymentmethod-container{"data-paymentmethod-id": "#{payment_method.id}", style: "display: #{payment_method.id == selected_payment_method ? "block" : "none"}"} - - if payment_method.description && !payment_method.description.empty? - .paymentmethod-description.panel - #{payment_method.description} - .paymentmethod-form - = render partial: "split_checkout/payment/#{payment_method.method_type}", locals: { payment_method: payment_method, f: f } + .paymentmethod-container{"data-paymentmethod-id": "#{payment_method.id}", style: "display: #{payment_method.id == selected_payment_method ? "block" : "none"}"} + - if payment_method.description && !payment_method.description.empty? + .paymentmethod-description.panel + #{payment_method.description} + .paymentmethod-form + = render partial: "split_checkout/payment/#{payment_method.method_type}", locals: { payment_method: payment_method, f: f } - = f.error_message_on :payment_method, standalone: true + = f.error_message_on :payment_method, standalone: true %div.checkout-substep = t("split_checkout.step2.explaination") diff --git a/app/views/split_checkout/_summary.html.haml b/app/views/split_checkout/_summary.html.haml index bd14fd3e7d3..0317ee6232d 100644 --- a/app/views/split_checkout/_summary.html.haml +++ b/app/views/split_checkout/_summary.html.haml @@ -54,16 +54,21 @@ %a.summary-edit{href: main_app.checkout_step_path(:payment)} = t("split_checkout.step3.payment_method.edit") .two-columns + - payment_method = last_payment_method(@order) %div - - payment_method = last_payment_method(@order) - = payment_method&.name - %em.fees=payment_or_shipping_price(payment_method, @order) + - if payment_method + = payment_method.name + %em.fees + = payment_or_shipping_price(payment_method, @order) + - elsif @order.zero_priced_order? + %h4= t(:no_payment_required) + - if payment_method&.description.present? %div .summary-subtitle = t("split_checkout.step3.payment_method.instructions") %div - = last_payment_method(@order)&.description + = payment_method&.description %div.checkout-substep From 0c3877e8e40bb7ef14a125d0ea3d07ee64aa45bf Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 20:31:49 +0100 Subject: [PATCH 12/24] Introduce "zero priced orders" in admin order payments UI and helper --- app/helpers/spree/payment_methods_helper.rb | 5 +++-- app/views/spree/admin/payments/_list.html.haml | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/helpers/spree/payment_methods_helper.rb b/app/helpers/spree/payment_methods_helper.rb index 50fe34d4570..531848ab147 100644 --- a/app/helpers/spree/payment_methods_helper.rb +++ b/app/helpers/spree/payment_methods_helper.rb @@ -4,12 +4,13 @@ module Spree module PaymentMethodsHelper def payment_method(payment) # hack to allow us to retrieve the name of a "deleted" payment method - id = payment.payment_method_id + return unless (id = payment.payment_method_id) + Spree::PaymentMethod.find_with_destroyed(id) end def payment_method_name(payment) - payment_method(payment).name + payment_method(payment)&.name end end end diff --git a/app/views/spree/admin/payments/_list.html.haml b/app/views/spree/admin/payments/_list.html.haml index 4e1a1ec0669..cbae3b95c60 100644 --- a/app/views/spree/admin/payments/_list.html.haml +++ b/app/views/spree/admin/payments/_list.html.haml @@ -11,7 +11,9 @@ %tr{class: "#{cycle('odd', 'even')}"} %td= pretty_time(payment.created_at) %td.align-center= payment.display_amount.to_html - %td.align-center= link_to payment_method_name(payment), spree.admin_order_payment_path(@order, payment) + %td.align-center + - if payment.payment_method_id + = link_to payment_method_name(payment), spree.admin_order_payment_path(@order, payment) %td.align-center %span{class: "state #{payment.state}"}= t(payment.state, scope: "spree.payment_states", default: payment.state.capitalize) %td.actions From 53e5ee516ac5199b9d5b66d3a19a312ffa640d5a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 21:30:42 +0100 Subject: [PATCH 13/24] Move loading of saved cards out of checkout concern --- app/controllers/concerns/checkout_callbacks.rb | 7 +------ app/views/split_checkout/payment/_stripe_sca.html.haml | 8 +++++--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb index 159c6041eab..18f413d0e25 100644 --- a/app/controllers/concerns/checkout_callbacks.rb +++ b/app/controllers/concerns/checkout_callbacks.rb @@ -14,7 +14,7 @@ module CheckoutCallbacks prepend_before_action :require_order_cycle prepend_before_action :require_distributor_chosen - before_action :load_order, :associate_user, :load_saved_addresses, :load_saved_credit_cards + before_action :load_order, :associate_user, :load_saved_addresses before_action :load_shipping_methods, if: -> { params[:step] == "details" } before_action :ensure_order_not_completed @@ -43,11 +43,6 @@ def load_saved_addresses @order.ship_address ||= finder.ship_address end - def load_saved_credit_cards - @saved_credit_cards = spree_current_user&.credit_cards&.with_payment_profile.to_a - @selected_card = nil - end - def load_shipping_methods @shipping_methods = available_shipping_methods.sort { |a, b| a.name.casecmp(b.name) } end diff --git a/app/views/split_checkout/payment/_stripe_sca.html.haml b/app/views/split_checkout/payment/_stripe_sca.html.haml index 144ca2559af..4503c2829ed 100644 --- a/app/views/split_checkout/payment/_stripe_sca.html.haml +++ b/app/views/split_checkout/payment/_stripe_sca.html.haml @@ -1,15 +1,17 @@ +- saved_credit_cards = spree_current_user&.credit_cards&.with_payment_profile.to_a + %div{"data-controller": "stripe-cards", "data-paymentmethod-id": "#{payment_method.id}" } - - if @saved_credit_cards.any? + - if saved_credit_cards.any? .checkout-input %label = t('split_checkout.step2.form.stripe.use_saved_card') = select_tag :existing_card_id, - options_for_select(stripe_card_options(@saved_credit_cards) + [[t('split_checkout.step2.form.stripe.create_new_card'), ""]], @selected_card), + options_for_select(stripe_card_options(saved_credit_cards) + [[t('split_checkout.step2.form.stripe.create_new_card'), ""]], nil), { "data-action": "change->stripe-cards#onSelectCard", "data-stripe-cards-target": "select" } %div{"data-stripe-cards-target": "stripeelements"} .checkout-input - - if @saved_credit_cards.none? + - if saved_credit_cards.none? %label = t('split_checkout.step2.form.stripe.use_new_card') From f92ceb63738079ae33c21d22645ff6c4633f8159 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 21:31:20 +0100 Subject: [PATCH 14/24] Remove @voucher_adjustment instance variable --- app/controllers/concerns/checkout_callbacks.rb | 2 -- app/views/split_checkout/_payment.html.haml | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb index 18f413d0e25..7da81a592b9 100644 --- a/app/controllers/concerns/checkout_callbacks.rb +++ b/app/controllers/concerns/checkout_callbacks.rb @@ -30,8 +30,6 @@ def load_order @order.manual_shipping_selection = true @order.checkout_processing = true - @voucher_adjustment = @order.voucher_adjustments.first - redirect_to(main_app.shop_path) && return if redirect_to_shop? redirect_to_cart_path && return unless valid_order_line_items? end diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml index 12a94405ee2..e142b894e66 100644 --- a/app/views/split_checkout/_payment.html.haml +++ b/app/views/split_checkout/_payment.html.haml @@ -1,7 +1,7 @@ .medium-6 - if @order.distributor.vouchers.present? %div.checkout-substep - = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @voucher_adjustment } + = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first } = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| %div.checkout-substep{"data-controller": "paymentmethod"} From 49499a9a5d61ec07eebca3f3e5d9c02bba741d25 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 21:34:30 +0100 Subject: [PATCH 15/24] Show/hide payment methods if voucher changes order total to zero --- app/controllers/voucher_adjustments_controller.rb | 14 ++++++-------- app/views/split_checkout/_payment.html.haml | 4 ++-- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb index b9f5c0524e0..4ee0c3d3fef 100644 --- a/app/controllers/voucher_adjustments_controller.rb +++ b/app/controllers/voucher_adjustments_controller.rb @@ -6,8 +6,9 @@ class VoucherAdjustmentsController < BaseController def create if add_voucher VoucherAdjustmentsService.calculate(@order) + @order.update_totals_and_states - render_voucher_section + update_payment_section elsif @order.errors.present? render_error end @@ -16,7 +17,7 @@ def create def destroy @order.voucher_adjustments.find_by(id: params[:id])&.destroy - render_voucher_section + update_payment_section end private @@ -49,13 +50,10 @@ def add_voucher true end - def render_voucher_section + def update_payment_section render cable_ready: cable_car.replace( - selector: "#voucher-section", - html: render_to_string( - partial: "split_checkout/voucher_section", - locals: { order: @order,voucher_adjustment: @order.voucher_adjustments.first } - ) + selector: "#checkout-payment-methods", + html: render_to_string(partial: "split_checkout/payment", locals: { step: "payment" }) ) end diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml index e142b894e66..e32e65b245a 100644 --- a/app/views/split_checkout/_payment.html.haml +++ b/app/views/split_checkout/_payment.html.haml @@ -1,9 +1,9 @@ -.medium-6 +.medium-6#checkout-payment-methods - if @order.distributor.vouchers.present? %div.checkout-substep = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first } - = form_with url: checkout_update_path(checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| + = form_with url: checkout_update_path(local_assigns[:step] || checkout_step), model: @order, method: :put, data: { remote: "true" } do |f| %div.checkout-substep{"data-controller": "paymentmethod"} %div.checkout-title = t("split_checkout.step2.payment_method.title") From 264abcc0c47a906fa03846e09cb44a3c9d369a79 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 2 Jun 2023 21:38:41 +0100 Subject: [PATCH 16/24] Clarify named vouchers in UI --- app/views/split_checkout/_summary.html.haml | 8 +++++--- app/views/spree/orders/_totals_footer.html.haml | 2 ++ config/locales/en.yml | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/views/split_checkout/_summary.html.haml b/app/views/split_checkout/_summary.html.haml index 0317ee6232d..e1e230043da 100644 --- a/app/views/split_checkout/_summary.html.haml +++ b/app/views/split_checkout/_summary.html.haml @@ -91,10 +91,12 @@ - checkout_adjustments_for(@order, exclude: [:line_item]).reverse_each do |adjustment| .summary-right-line - -if adjustment.originator_type == 'Voucher' - .summary-right-line-label.voucher= adjustment.label + - if adjustment.originator_type == 'Voucher' + .summary-right-line-label.voucher + = "#{t(:voucher)}:" + = adjustment.label .summary-right-line-value.voucher= adjustment.display_amount.to_html - -else + - else .summary-right-line-label= adjustment.label .summary-right-line-value= adjustment.display_amount.to_html diff --git a/app/views/spree/orders/_totals_footer.html.haml b/app/views/spree/orders/_totals_footer.html.haml index ab8130210ef..556160804b9 100644 --- a/app/views/spree/orders/_totals_footer.html.haml +++ b/app/views/spree/orders/_totals_footer.html.haml @@ -12,6 +12,8 @@ %tr.total %td.text-right{:colspan => "3"} %strong + - if adjustment.originator_type == "Voucher" + = "#{t(:voucher)}:" = adjustment.label %td.text-right.total %span= adjustment.display_amount.to_html diff --git a/config/locales/en.yml b/config/locales/en.yml index 53f09d49415..5b4fdfe8a1f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -450,6 +450,7 @@ en: none: None notes: Notes error: Error + voucher: Voucher processing_payment: "Processing payment..." no_pending_payments: "No pending payments" invalid_payment_state: "Invalid payment state: %{state}" From 0bf0f38d5947efd658f356eb41c6e2f1e1e3090a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 5 Jun 2023 18:14:38 +0100 Subject: [PATCH 17/24] Re-enable voucher test --- spec/system/consumer/split_checkout_tax_not_incl_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb index d4a7ddda411..1109c1c600e 100644 --- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb @@ -115,7 +115,7 @@ expect(page).to have_selector('#tax-row', text: with_currency(1.30)) end - pending "when using a voucher" do + context "when using a voucher" do let!(:voucher) do create(:voucher, code: 'some_code', enterprise: distributor, amount: 10) end From 657a9a387fd1f6e9b60139cf6c127558934bd97c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 8 Jun 2023 19:33:26 +0100 Subject: [PATCH 18/24] Improve feature toggling --- app/views/split_checkout/_payment.html.haml | 2 +- spec/system/consumer/split_checkout_spec.rb | 2 ++ spec/system/consumer/split_checkout_tax_incl_spec.rb | 2 ++ spec/system/consumer/split_checkout_tax_not_incl_spec.rb | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/views/split_checkout/_payment.html.haml b/app/views/split_checkout/_payment.html.haml index e32e65b245a..675fbfd820f 100644 --- a/app/views/split_checkout/_payment.html.haml +++ b/app/views/split_checkout/_payment.html.haml @@ -1,5 +1,5 @@ .medium-6#checkout-payment-methods - - if @order.distributor.vouchers.present? + - if feature?(:vouchers, spree_current_user) && @order.distributor.vouchers.present? %div.checkout-substep = render partial: "split_checkout/voucher_section", locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first } diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb index 2bcc9f1cd0f..abf8a6f2f2b 100644 --- a/spec/system/consumer/split_checkout_spec.rb +++ b/spec/system/consumer/split_checkout_spec.rb @@ -707,6 +707,8 @@ end describe "vouchers" do + before { Flipper.enable :vouchers } + context "with no voucher available" do before do visit checkout_step_path(:payment) diff --git a/spec/system/consumer/split_checkout_tax_incl_spec.rb b/spec/system/consumer/split_checkout_tax_incl_spec.rb index 66edeff0080..6663c6a970a 100644 --- a/spec/system/consumer/split_checkout_tax_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_incl_spec.rb @@ -106,6 +106,8 @@ end context "when using a voucher" do + before { Flipper.enable :vouchers } + let!(:voucher) do create(:voucher, code: 'some_code', enterprise: distributor, amount: 10) end diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb index 1109c1c600e..5d9f9bcd5b7 100644 --- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb @@ -63,6 +63,7 @@ before do # assures tax is charged in dependence of shipping address Spree::Config.set(tax_using_ship_address: true) + Flipper.enable :vouchers end describe "a not-included tax" do From 472cf6faecfb10cc6964a0ff3b73357d54cac772 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 8 Jun 2023 19:34:21 +0100 Subject: [PATCH 19/24] Fix rubocop complaint --- app/controllers/voucher_adjustments_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb index 4ee0c3d3fef..267e2d367b6 100644 --- a/app/controllers/voucher_adjustments_controller.rb +++ b/app/controllers/voucher_adjustments_controller.rb @@ -41,8 +41,8 @@ def add_voucher adjustment = voucher.create_adjustment(voucher.code, @order) - if !adjustment.valid? - @order.errors.add(:voucher, I18n.t('split_checkout.errors.add_voucher_error')) + unless adjustment.valid? + @order.errors.add(:voucher_code, I18n.t('split_checkout.errors.add_voucher_error')) adjustment.errors.each { |error| @order.errors.import(error) } return false end From 3d3370fca83e212e68064e930416f1e08716a25f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 8 Jun 2023 19:37:23 +0100 Subject: [PATCH 20/24] Update use of params --- app/controllers/voucher_adjustments_controller.rb | 12 ++++++++---- app/views/split_checkout/_voucher_section.html.haml | 4 ++-- .../voucher_adjustments_controller_spec.rb | 6 +++--- spec/system/consumer/split_checkout_spec.rb | 2 +- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb index 267e2d367b6..9923733fbb8 100644 --- a/app/controllers/voucher_adjustments_controller.rb +++ b/app/controllers/voucher_adjustments_controller.rb @@ -27,15 +27,15 @@ def set_order end def add_voucher - if params[:voucher_code].blank? - @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found')) + if voucher_params[:voucher_code].blank? + @order.errors.add(:voucher_code, I18n.t('split_checkout.errors.voucher_not_found')) return false end - voucher = Voucher.find_by(code: params[:voucher_code], enterprise: @order.distributor) + voucher = Voucher.find_by(code: voucher_params[:voucher_code], enterprise: @order.distributor) if voucher.nil? - @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found')) + @order.errors.add(:voucher_code, I18n.t('split_checkout.errors.voucher_not_found')) return false end @@ -63,4 +63,8 @@ def render_error render status: :unprocessable_entity, cable_ready: cable_car. replace("#flashes", partial("shared/flashes", locals: { flashes: flash })) end + + def voucher_params + params.require(:order).permit(:voucher_code) + end end diff --git a/app/views/split_checkout/_voucher_section.html.haml b/app/views/split_checkout/_voucher_section.html.haml index d50aad39331..c46dce4ec01 100644 --- a/app/views/split_checkout/_voucher_section.html.haml +++ b/app/views/split_checkout/_voucher_section.html.haml @@ -14,6 +14,6 @@ %span.formError.standalone = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") - else - = form_with url: voucher_adjustments_path, method: :post, data: { remote: true } do |form| - = form.text_field :voucher_code, value: params[:voucher_code], data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher" + = form_with url: voucher_adjustments_path, model: @order, method: :post, data: { remote: true } do |form| + = form.text_field :voucher_code, value: params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher" = form.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } diff --git a/spec/controllers/voucher_adjustments_controller_spec.rb b/spec/controllers/voucher_adjustments_controller_spec.rb index d9366dae174..df11d9c06b1 100644 --- a/spec/controllers/voucher_adjustments_controller_spec.rb +++ b/spec/controllers/voucher_adjustments_controller_spec.rb @@ -29,7 +29,7 @@ describe "#create" do describe "adding a voucher" do - let(:params) { { voucher_code: voucher.code } } + let(:params) { { order: { voucher_code: voucher.code } } } it "adds a voucher to the user's current order" do post :create, params: params @@ -39,13 +39,13 @@ end context "when voucher doesn't exist" do - let(:params) { { voucher_code: "non_voucher" } } + let(:params) { { order: { voucher_code: "non_voucher" } } } it "returns 422 and an error message" do post :create, params: params expect(response.status).to eq 422 - expect(flash[:error]).to match "Voucher Not found" + expect(flash[:error]).to match "Voucher code Not found" end end diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb index abf8a6f2f2b..db6fbb32366 100644 --- a/spec/system/consumer/split_checkout_spec.rb +++ b/spec/system/consumer/split_checkout_spec.rb @@ -771,7 +771,7 @@ fill_in "Enter voucher code", with: "non_code" click_button("Apply") - expect(page).to have_content("Voucher Not found") + expect(page).to have_content("Voucher code Not found") end end end From 821b57947261a00ad4d10769279c2b30857c36c3 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 8 Jun 2023 19:38:46 +0100 Subject: [PATCH 21/24] Fix CSS/layout issues --- .../voucher_adjustments_controller.rb | 9 ++++- .../split_checkout/_voucher_section.html.haml | 34 +++++++++++-------- .../css/darkswarm/split-checkout.scss | 20 +++++------ spec/system/consumer/split_checkout_spec.rb | 3 +- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/app/controllers/voucher_adjustments_controller.rb b/app/controllers/voucher_adjustments_controller.rb index 9923733fbb8..c34570269c3 100644 --- a/app/controllers/voucher_adjustments_controller.rb +++ b/app/controllers/voucher_adjustments_controller.rb @@ -61,7 +61,14 @@ def render_error flash.now[:error] = @order.errors.full_messages.to_sentence render status: :unprocessable_entity, cable_ready: cable_car. - replace("#flashes", partial("shared/flashes", locals: { flashes: flash })) + replace("#flashes", partial("shared/flashes", locals: { flashes: flash })). + replace( + "#voucher-section", + partial( + "split_checkout/voucher_section", + locals: { order: @order, voucher_adjustment: @order.voucher_adjustments.first } + ) + ) end def voucher_params diff --git a/app/views/split_checkout/_voucher_section.html.haml b/app/views/split_checkout/_voucher_section.html.haml index c46dce4ec01..19298da4096 100644 --- a/app/views/split_checkout/_voucher_section.html.haml +++ b/app/views/split_checkout/_voucher_section.html.haml @@ -1,19 +1,25 @@ %div#voucher-section .checkout-title = t("split_checkout.step2.voucher.apply_voucher") - .checkout-input - .two-columns-inputs.voucher{"data-controller": "toggle-button-disabled"} + .checkout-input{"data-controller": "toggle-button-disabled"} + = form_with url: voucher_adjustments_path, model: @order, method: :post, data: { remote: true } do |form| - if voucher_adjustment.present? - %span.button.voucher-added - %i.ofn-i_051-check-big - = t("split_checkout.step2.voucher.voucher", voucher_amount: voucher_adjustment.originator.display_value) - = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") } - - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ? - - if voucher_adjustment.originator.amount > order.total - .checkout-input - %span.formError.standalone - = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") + .two-columns-inputs.voucher + %span.button.voucher-added + %i.ofn-i_051-check-big + = t("split_checkout.step2.voucher.voucher", voucher_amount: voucher_adjustment.originator.display_value) + = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") } + + - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ? + - if voucher_adjustment.originator.amount > order.total + .checkout-input + %span.formError.standalone + = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") - else - = form_with url: voucher_adjustments_path, model: @order, method: :post, data: { remote: true } do |form| - = form.text_field :voucher_code, value: params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher" - = form.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } + .two-columns-inputs + %div.checkout-input + = form.text_field :voucher_code, value: params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged" }, placeholder: t("split_checkout.step2.voucher.placeholder"), class: "voucher" + = form.error_message_on :voucher_code + + %div.checkout-input + = form.submit t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher-button", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } diff --git a/app/webpacker/css/darkswarm/split-checkout.scss b/app/webpacker/css/darkswarm/split-checkout.scss index 3d9c05b929f..1fc90b98151 100644 --- a/app/webpacker/css/darkswarm/split-checkout.scss +++ b/app/webpacker/css/darkswarm/split-checkout.scss @@ -412,22 +412,18 @@ justify-content: normal; align-items: center; - input { - width: 50%; - } - a { color: inherit; } + } - .button { - &.cancel { - width: 30%; - border-radius: 0.5em; - padding:0; - height: 2.5em; - background-color: $teal-400 - } + .voucher-button { + &.cancel { + width: 30%; + border-radius: 0.35em; + padding:0; + height: 2.5em; + background-color: $teal-400 } } diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb index db6fbb32366..148f2a212c0 100644 --- a/spec/system/consumer/split_checkout_spec.rb +++ b/spec/system/consumer/split_checkout_spec.rb @@ -788,9 +788,10 @@ click_on "Remove code" end - within '.voucher' do + within '#voucher-section' do expect(page).to have_button("Apply", disabled: true) end + expect(order.voucher_adjustments.length).to eq(0) end end From eced839455143a7c9632f55f6aca076941068db6 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 8 Jun 2023 20:05:51 +0100 Subject: [PATCH 22/24] Fix flaky test --- spec/system/consumer/split_checkout_tax_not_incl_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb index 5d9f9bcd5b7..046a6ac4833 100644 --- a/spec/system/consumer/split_checkout_tax_not_incl_spec.rb +++ b/spec/system/consumer/split_checkout_tax_not_incl_spec.rb @@ -127,9 +127,11 @@ choose "Delivery" click_button "Next - Payment method" + # add Voucher fill_in "Enter voucher code", with: voucher.code click_button("Apply") + expect(page).to have_selector ".voucher-added" click_on "Next - Order summary" click_on "Complete order" From 10855572e957fc0e238a9c137019b2329936721d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 8 Jun 2023 22:31:00 +0100 Subject: [PATCH 23/24] Add nil safety in reports for zero priced orders with no payment method --- lib/reporting/reports/payments/payment_totals.rb | 2 +- lib/reporting/reports/payments/payments_by_payment_type.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/reporting/reports/payments/payment_totals.rb b/lib/reporting/reports/payments/payment_totals.rb index 06a6d1cdb42..4e1b5619947 100644 --- a/lib/reporting/reports/payments/payment_totals.rb +++ b/lib/reporting/reports/payments/payment_totals.rb @@ -23,7 +23,7 @@ def columns def total_by_payment_method(orders, pay_method) orders.map(&:payments).flatten.select { |payment| - payment.completed? && payment.payment_method.name.to_s.include?(pay_method) + payment.completed? && payment.payment_method&.name.to_s.include?(pay_method) }.sum(&:amount) end end diff --git a/lib/reporting/reports/payments/payments_by_payment_type.rb b/lib/reporting/reports/payments/payments_by_payment_type.rb index 67d96236be5..89003205df7 100644 --- a/lib/reporting/reports/payments/payments_by_payment_type.rb +++ b/lib/reporting/reports/payments/payments_by_payment_type.rb @@ -17,7 +17,7 @@ def columns { payment_state: proc { |payments| payment_state(payments.first.order) }, distributor: proc { |payments| payments.first.order.distributor.name }, - payment_type: proc { |payments| payments.first.payment_method.name }, + payment_type: proc { |payments| payments.first.payment_method&.name }, total_price: proc { |payments| payments.sum(&:amount) } } end From a211605b9b9caa4d8696eeba1d9eec2393a8dd7e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 8 Jun 2023 23:37:08 +0100 Subject: [PATCH 24/24] Use pre_discount_total when comparing to voucher amount --- app/views/split_checkout/_voucher_section.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/split_checkout/_voucher_section.html.haml b/app/views/split_checkout/_voucher_section.html.haml index 19298da4096..fcbebda20b3 100644 --- a/app/views/split_checkout/_voucher_section.html.haml +++ b/app/views/split_checkout/_voucher_section.html.haml @@ -11,7 +11,7 @@ = link_to t("split_checkout.step2.voucher.remove_code"), voucher_adjustment_path(id: voucher_adjustment.id), method: "delete", data: { confirm: t("split_checkout.step2.voucher.confirm_delete") } - # This might not be true, ie payment method including a fee which wouldn't be covered by voucher or tax implication raising total to be bigger than the voucher amount ? - - if voucher_adjustment.originator.amount > order.total + - if voucher_adjustment.originator.amount > order.pre_discount_total .checkout-input %span.formError.standalone = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount")