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

Add service to order copy cards #570

Merged
merged 3 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
25 changes: 16 additions & 9 deletions app/models/waste_carriers_engine/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,12 @@ class Order
field :manualOrder, as: :manual_order, type: String
field :order_item_reference, type: String

# TODO: Move to a service
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the intention is to move this to an OrderRenewalService (or something like that) would it be better to do that refactoring first and then add your new service?

As is I'm just not sure I like the idea that if we never get round to this TODO we'll have Order create the order with new_order, set the order items as if its a renewal, but then OrderAdditionalCardsService take that order and simply overwrites the array. Plus we're creating orders in 2 different ways.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This service do not use the code in new_order so I have left the TODO for the next person touching it. We should get there naturally when we work out the front end flow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂ Yep, was pointed out to me I was misreading the changes!

def self.new_order(transient_registration, method, current_user)
order = Order.new
order = new_order_for(current_user)

card_count = transient_registration.temp_cards

order[:order_id] = order.generate_id
order[:order_code] = order[:order_id]
order[:currency] = "GBP"

order[:date_created] = Time.current
order[:date_last_updated] = order[:date_created]
order[:updated_by_user] = current_user.email

order[:order_items] = [OrderItem.new_renewal_item]
order[:order_items] << OrderItem.new_type_change_item if transient_registration.registration_type_changed?
# TODO: Review whether card_count.present? is still necessary - this was a fix put in to deal with WC-498
Expand All @@ -53,6 +46,20 @@ def self.new_order(transient_registration, method, current_user)
order
end

def self.new_order_for(user)
order = Order.new

order[:order_id] = order.generate_id
order[:order_code] = order[:order_id]
order[:currency] = "GBP"

order[:date_created] = Time.current
order[:date_last_updated] = order[:date_created]
order[:updated_by_user] = user.email

order
end

def self.valid_world_pay_status?(response_type, status)
allowed_statuses = {
success: %w[AUTHORISED],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# frozen_string_literal: true

module WasteCarriersEngine
class OrderAdditionalCardsService < BaseService
def run(cards_count:, user:, registration:, payment_method:)
finance_details = registration.finance_details
order = additional_cards_order(user, cards_count, payment_method)

finance_details[:orders] << order

finance_details.update_balance
finance_details.save!
end

private

def additional_cards_order(user, cards_count, payment_method)
order = Order.new_order_for(user)
new_item = OrderItem.new_copy_cards_item(cards_count)

order[:order_items] = [new_item]

order.generate_description

order[:total_amount] = new_item[:amount]

order.add_bank_transfer_attributes if payment_method == :bank_transfer
order.add_worldpay_attributes if payment_method == :worldpay

order
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# frozen_string_literal: true

require "rails_helper"

module WasteCarriersEngine
RSpec.describe OrderAdditionalCardsService do
describe ".run" do
let(:user) { double(:user) }
let(:registration) { double(:registration) }
let(:order) { double(:order) }

before do
finance_details = double(:finance_details)
order_item = double(:order_item)
orders = double(:orders)

expect(registration).to receive(:finance_details).and_return(finance_details)
expect(Order).to receive(:new_order_for).with(user).and_return(order)
expect(OrderItem).to receive(:new_copy_cards_item).with(2).and_return(order_item)
expect(order).to receive(:generate_description)
expect(order).to receive(:[]=).with(:order_items, [order_item])
expect(order_item).to receive(:[]).with(:amount).and_return(10)
expect(order).to receive(:[]=).with(:total_amount, 10)

expect(finance_details).to receive(:[]).with(:orders).and_return(orders)
expect(orders).to receive(:<<).with(order)
expect(finance_details).to receive(:update_balance)
expect(finance_details).to receive(:save!)
end

context "when the payment method is bank transfer" do
let(:payment_method) { :bank_transfer }

it "updates the registration's finance details with a new order for the given copy cards" do
expect(order).to receive(:add_bank_transfer_attributes)

described_class.run(cards_count: 2, user: user, registration: registration, payment_method: payment_method)
end
end

context "when the payment method is worldpay" do
let(:payment_method) { :worldpay }

it "updates the registration's finance details with a new order for the given copy cards" do
expect(order).to receive(:add_worldpay_attributes)

described_class.run(cards_count: 2, user: user, registration: registration, payment_method: payment_method)
end
end
end
end
end