Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vouchers part 2 #11135

Merged
merged 28 commits into from
Jul 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
3d44f4b
Introduce "zero priced order" concept with no required payment
Matt-Yorkley Jun 1, 2023
75e6ccd
Simplify voucher controller
Matt-Yorkley Jun 1, 2023
e04a3f6
Move form definition down into each checkout step
Matt-Yorkley Jun 1, 2023
3f83da9
Move voucher section out of main checkout form
Matt-Yorkley Jun 1, 2023
9100458
Add separate voucher form
Matt-Yorkley Jun 1, 2023
9e5061f
Move voucher processing out of checkout controller
Matt-Yorkley Jun 1, 2023
47b5a3f
Don't apply tax calculations if there's no tax
Matt-Yorkley Jun 2, 2023
d795061
Drop superfluous method
Matt-Yorkley Jun 2, 2023
a65b3b8
Extract voucher tests to separate controller spec
Matt-Yorkley Jun 2, 2023
bd29a9a
Introduce "zero priced orders" to checkout UI and order state flow
Matt-Yorkley Jun 2, 2023
1cd38c9
Introduce "zero priced orders" in admin order payments UI and helper
Matt-Yorkley Jun 2, 2023
7a0b830
Move loading of saved cards out of checkout concern
Matt-Yorkley Jun 2, 2023
55bce9f
Remove @voucher_adjustment instance variable
Matt-Yorkley Jun 2, 2023
de97e69
Show/hide payment methods if voucher changes order total to zero
Matt-Yorkley Jun 2, 2023
28795ef
Clarify named vouchers in UI
Matt-Yorkley Jun 2, 2023
2f0f3e0
Re-enable voucher test
Matt-Yorkley Jun 5, 2023
9b45b71
Improve feature toggling
Matt-Yorkley Jun 8, 2023
37a4c73
Fix rubocop complaint
Matt-Yorkley Jun 8, 2023
6724001
Update use of params
Matt-Yorkley Jun 8, 2023
f7912a2
Fix CSS/layout issues
Matt-Yorkley Jun 8, 2023
2aa3f8e
Fix flaky test
Matt-Yorkley Jun 8, 2023
ad6d0c1
Add nil safety in reports for zero priced orders with no payment method
Matt-Yorkley Jun 8, 2023
55cc57c
Use pre_discount_total when comparing to voucher amount
Matt-Yorkley Jun 8, 2023
7407394
Move VoucherAdjustmentsController to request specs
rioug Jun 30, 2023
66a5460
Add test coverage for handling zero priced orders
Matt-Yorkley Jul 4, 2023
671dfc7
Don't flush errors when checking zero priced order validity
Matt-Yorkley Jul 4, 2023
7e0042e
Apply code suggestion
dacook Jul 5, 2023
652beac
Drop valid? check from zero_priced_order? method
Matt-Yorkley Jul 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions app/controllers/admin/vouchers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
dacook marked this conversation as resolved.
Show resolved Hide resolved
)
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
Expand Down
9 changes: 1 addition & 8 deletions app/controllers/concerns/checkout_callbacks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -43,11 +41,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
Expand Down
66 changes: 4 additions & 62 deletions app/controllers/split_checkout_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ 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
return process_voucher if params[:apply_voucher].present?

if confirm_order || update_order
return if performed?

Expand Down Expand Up @@ -60,27 +60,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?
Expand Down Expand Up @@ -125,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
Expand Down Expand Up @@ -201,40 +176,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
Expand Down Expand Up @@ -262,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?
dacook marked this conversation as resolved.
Show resolved Hide resolved

@order.errors.add :payment_method, I18n.t('split_checkout.errors.select_a_payment_method')
end
Expand Down
69 changes: 57 additions & 12 deletions app/controllers/voucher_adjustments_controller.rb
Original file line number Diff line number Diff line change
@@ -1,32 +1,77 @@
# frozen_string_literal: true

class VoucherAdjustmentsController < BaseController
include CablecarResponses
before_action :set_order

def create
if add_voucher
VoucherAdjustmentsService.calculate(@order)
Matt-Yorkley marked this conversation as resolved.
Show resolved Hide resolved
@order.update_totals_and_states

update_payment_section
elsif @order.errors.present?
render_error
end
end

def destroy
@order.voucher_adjustments.find_by(id: params[:id])&.destroy

update_payment_section
end

private

def set_order
@order = current_order
end

@order.voucher_adjustments.find_by(id: params[:id])&.destroy
def add_voucher
if voucher_params[:voucher_code].blank?
@order.errors.add(:voucher_code, I18n.t('split_checkout.errors.voucher_not_found'))
return false
end

respond_to do |format|
format.cable_ready { render_voucher_section }
format.html { redirect_to checkout_step_path(:payment) }
voucher = Voucher.find_by(code: voucher_params[:voucher_code], enterprise: @order.distributor)

if voucher.nil?
@order.errors.add(:voucher_code, 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_code, I18n.t('split_checkout.errors.add_voucher_error'))
adjustment.errors.each { |error| @order.errors.import(error) }
return false
end

true
end

private
def update_payment_section
render cable_ready: cable_car.replace(
selector: "#checkout-payment-methods",
html: render_to_string(partial: "split_checkout/payment", locals: { step: "payment" })
)
end

def render_error
flash.now[:error] = @order.errors.full_messages.to_sentence

# 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(
render status: :unprocessable_entity, cable_ready: cable_car.
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
params.require(:order).permit(:voucher_code)
end
end
2 changes: 2 additions & 0 deletions app/helpers/checkout_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
4 changes: 3 additions & 1 deletion app/helpers/spree/payment_methods_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ 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 if id.nil?

Spree::PaymentMethod.find_with_destroyed(id)
end

def payment_method_name(payment)
payment_method(payment).name
payment_method(payment)&.name
end
end
end
12 changes: 11 additions & 1 deletion app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -219,6 +219,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?
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
Expand Down Expand Up @@ -615,6 +621,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
dacook marked this conversation as resolved.
Show resolved Hide resolved

break if payment_total >= total

yield payment
Expand Down
4 changes: 3 additions & 1 deletion app/services/voucher_adjustments_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading