From 690d65d83929ce6c213fc10a65f68878e846bc8e Mon Sep 17 00:00:00 2001 From: marlena-b Date: Mon, 26 Aug 2024 10:48:15 +0200 Subject: [PATCH] Better error when submitting order with out of stock product --- .gitignore | 1 + ecommerce/crm/lib/crm/order.rb | 5 +- .../crm/test/assign_customer_to_order_test.rb | 27 +++++- ecommerce/processes/lib/processes.rb | 1 + ecommerce/processes/lib/processes/events.rb | 10 +++ .../lib/processes/reservation_process.rb | 36 +++++--- .../test/reservation_process_test.rb | 43 ++++++---- rails_application/.gitignore | 2 +- rails_application/.mutant.yml | 1 + .../controllers/client/orders_controller.rb | 11 ++- .../app/controllers/orders_controller.rb | 18 ++-- .../app/services/application_service.rb | 17 ++++ .../app/services/orders/submit_service.rb | 42 ++++++++++ rails_application/app/services/result.rb | 14 ++++ .../test/integration/orders_test.rb | 16 ++++ .../services/orders/submit_service_test.rb | 84 +++++++++++++++++++ .../test/services/result_test.rb | 44 ++++++++++ 17 files changed, 321 insertions(+), 51 deletions(-) create mode 100644 ecommerce/processes/lib/processes/events.rb create mode 100644 rails_application/app/services/application_service.rb create mode 100644 rails_application/app/services/orders/submit_service.rb create mode 100644 rails_application/app/services/result.rb create mode 100644 rails_application/test/services/orders/submit_service_test.rb create mode 100644 rails_application/test/services/result_test.rb diff --git a/.gitignore b/.gitignore index eaa6e54c0..bf3ad63e0 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ .ruby-version +.tool-versions diff --git a/ecommerce/crm/lib/crm/order.rb b/ecommerce/crm/lib/crm/order.rb index be6bc955d..653a5dca9 100644 --- a/ecommerce/crm/lib/crm/order.rb +++ b/ecommerce/crm/lib/crm/order.rb @@ -1,14 +1,13 @@ module Crm class Order include AggregateRoot - CustomerAlreadyAssigned = Class.new(StandardError) def initialize(id) @id = id end def set_customer(customer_id) - raise CustomerAlreadyAssigned if @customer_id + return if @customer_id == customer_id apply CustomerAssignedToOrder.new(data: { order_id: @id, customer_id: customer_id }) end @@ -18,4 +17,4 @@ def set_customer(customer_id) @customer_id = event.data[:customer_id] end end -end \ No newline at end of file +end diff --git a/ecommerce/crm/test/assign_customer_to_order_test.rb b/ecommerce/crm/test/assign_customer_to_order_test.rb index cbddf33ed..070a2854b 100644 --- a/ecommerce/crm/test/assign_customer_to_order_test.rb +++ b/ecommerce/crm/test/assign_customer_to_order_test.rb @@ -17,10 +17,31 @@ def test_customer_should_get_assigned def test_customer_should_not_get_assigned_twice customer_id = SecureRandom.uuid order_id = SecureRandom.uuid + + register_customer(customer_id, fake_name) + + expected_event = CustomerAssignedToOrder.new(data: {customer_id: customer_id, order_id: order_id}) + + assert_events("Crm::Order$#{order_id}", expected_event) do + assign_customer_to_order(order_id, customer_id) + assign_customer_to_order(order_id, customer_id) + end + end + + def test_customer_can_be_reassigned + customer_id = SecureRandom.uuid + another_customer_id = SecureRandom.uuid + order_id = SecureRandom.uuid + register_customer(customer_id, fake_name) - assign_customer_to_order(order_id, customer_id) - assert_raises(Order::CustomerAlreadyAssigned) do + register_customer(another_customer_id, fake_name) + + expected_event_1 = CustomerAssignedToOrder.new(data: {customer_id: customer_id, order_id: order_id}) + expected_event_2 = CustomerAssignedToOrder.new(data: {customer_id: another_customer_id, order_id: order_id}) + + assert_events("Crm::Order$#{order_id}", expected_event_1, expected_event_2) do assign_customer_to_order(order_id, customer_id) + assign_customer_to_order(order_id, another_customer_id) end end @@ -40,4 +61,4 @@ def assign_customer_to_order(order_id, customer_id) run_command(AssignCustomerToOrder.new(order_id: order_id, customer_id: customer_id)) end end -end \ No newline at end of file +end diff --git a/ecommerce/processes/lib/processes.rb b/ecommerce/processes/lib/processes.rb index 153c90df6..3b37af413 100644 --- a/ecommerce/processes/lib/processes.rb +++ b/ecommerce/processes/lib/processes.rb @@ -9,6 +9,7 @@ require_relative "../../invoicing/lib/invoicing" require_relative "../../fulfillment/lib/fulfillment" require_relative 'processes/confirm_order_on_payment_captured' +require_relative 'processes/events' require_relative 'processes/release_payment_process' require_relative 'processes/shipment_process' require_relative 'processes/determine_vat_rates_on_order_placed' diff --git a/ecommerce/processes/lib/processes/events.rb b/ecommerce/processes/lib/processes/events.rb new file mode 100644 index 000000000..1c270b930 --- /dev/null +++ b/ecommerce/processes/lib/processes/events.rb @@ -0,0 +1,10 @@ +module Processes + class ReservationProcessFailed < Infra::Event + attribute :order_id, Infra::Types::UUID + attribute :unavailable_products, Infra::Types::Array.of(Infra::Types::UUID) + end + + class ReservationProcessSuceeded < Infra::Event + attribute :order_id, Infra::Types::UUID + end +end diff --git a/ecommerce/processes/lib/processes/reservation_process.rb b/ecommerce/processes/lib/processes/reservation_process.rb index fa920cea2..16c1f6dc7 100644 --- a/ecommerce/processes/lib/processes/reservation_process.rb +++ b/ecommerce/processes/lib/processes/reservation_process.rb @@ -10,14 +10,7 @@ def call(event) state = build_state(event) case event.event_type when 'Ordering::OrderSubmitted' - begin - reserve_stock(state) - rescue Inventory::InventoryEntry::InventoryNotAvailable - release_stock(state) - reject_order(state) - else - accept_order(state) - end + order_side_effects(state) { reserve_stock(state) } when 'Fulfillment::OrderCancelled' release_stock(state) when 'Fulfillment::OrderConfirmed' @@ -28,10 +21,29 @@ def call(event) private def reserve_stock(state) + unavailable_products = [] state.order_lines.each do |product_id, quantity| command_bus.(Inventory::Reserve.new(product_id: product_id, quantity: quantity)) state.product_reserved(product_id) + rescue Inventory::InventoryEntry::InventoryNotAvailable + unavailable_products << product_id + end + + if unavailable_products.empty? + event = ReservationProcessSuceeded.new(data: { order_id: state.order_id }) + else + release_stock(state) + event = ReservationProcessFailed.new(data: { order_id: state.order_id, unavailable_products: unavailable_products }) end + event_store.publish(event, stream_name: stream_name(state.order_id)) + end + + def order_side_effects(state) + event_store + .within { yield } + .subscribe(to: ReservationProcessFailed) { reject_order(state) } + .subscribe(to: ReservationProcessSuceeded) { accept_order(state) } + .call end def release_stock(state) @@ -54,8 +66,12 @@ def reject_order(state) command_bus.(Ordering::RejectOrder.new(order_id: state.order_id)) end + def stream_name(order_id) + "ReservationProcess$#{order_id}" + end + def build_state(event) - stream_name = "ReservationProcess$#{event.data.fetch(:order_id)}" + stream_name = stream_name(event.data.fetch(:order_id)) begin past_events = event_store.read.stream(stream_name).to_a last_stored = past_events.size - 1 @@ -93,4 +109,4 @@ def product_reserved(product_id) end end end -end \ No newline at end of file +end diff --git a/ecommerce/processes/test/reservation_process_test.rb b/ecommerce/processes/test/reservation_process_test.rb index a71aa122c..6e0edd8c8 100644 --- a/ecommerce/processes/test/reservation_process_test.rb +++ b/ecommerce/processes/test/reservation_process_test.rb @@ -6,7 +6,9 @@ class ReservationProcessTest < Test def test_happy_path process = ReservationProcess.new - given([order_submitted]).each { |event| process.call(event) } + assert_success_event do + given([order_submitted]).each { |event| process.call(event) } + end assert_all_commands( Inventory::Reserve.new(product_id: product_id, quantity: 1), Inventory::Reserve.new(product_id: another_product_id, quantity: 2), @@ -26,28 +28,19 @@ def call(command) end end - def test_reject_order_command_is_dispatched_when_sth_is_unavailable + def test_rejects_order_and_compensates_stock_when_sth_is_unavailable failing_command = Inventory::Reserve.new(product_id: product_id, quantity: 1) enhanced_command_bus = EnhancedFakeCommandBus.new(command_bus, failing_command => Inventory::InventoryEntry::InventoryNotAvailable) process = ReservationProcess.new process.command_bus = enhanced_command_bus - given([order_submitted]).each { |event| process.call(event) } - assert_all_commands( - failing_command, - Ordering::RejectOrder.new(order_id: order_id) - ) - end - def test_compensation_when_sth_is_unavailable - failing_command = Inventory::Reserve.new(product_id: another_product_id, quantity: 2) - enhanced_command_bus = EnhancedFakeCommandBus.new(command_bus, failing_command => Inventory::InventoryEntry::InventoryNotAvailable) - process = ReservationProcess.new - process.command_bus = enhanced_command_bus - given([order_submitted]).each { |event| process.call(event) } + assert_failure_event do + given([order_submitted]).each { |event| process.call(event) } + end assert_all_commands( - Inventory::Reserve.new(product_id: product_id, quantity: 1), failing_command, - Inventory::Release.new(product_id: product_id, quantity: 1), + Inventory::Reserve.new(product_id: another_product_id, quantity: 2), + Inventory::Release.new(product_id: another_product_id, quantity: 2), Ordering::RejectOrder.new(order_id: order_id) ) end @@ -104,5 +97,21 @@ def order_cancelled } ) end + + def assert_success_event(&block) + assert_events_contain( + "ReservationProcess$#{order_id}", + ReservationProcessSuceeded.new(data: { order_id: order_id }), + &block + ) + end + + def assert_failure_event(&block) + assert_events_contain( + "ReservationProcess$#{order_id}", + ReservationProcessFailed.new(data: { order_id: order_id, unavailable_products: [product_id] }), + &block + ) + end end -end \ No newline at end of file +end diff --git a/rails_application/.gitignore b/rails_application/.gitignore index 7714eaa69..551d8dddd 100644 --- a/rails_application/.gitignore +++ b/rails_application/.gitignore @@ -34,4 +34,4 @@ yarn-debug.log* # Event to handlers and handler to events mappings generated by big_picture.rb script /lib/event_to_handlers.rb -/lib/handler_to_events.rb \ No newline at end of file +/lib/handler_to_events.rb diff --git a/rails_application/.mutant.yml b/rails_application/.mutant.yml index 4e7857ab8..c0914a614 100644 --- a/rails_application/.mutant.yml +++ b/rails_application/.mutant.yml @@ -37,3 +37,4 @@ matcher: - Orders::UpdateOrderTotalValue* - Orders::SubmitOrder* - Orders::AssignCustomerToOrder* + - Orders::SubmitService#submit_order diff --git a/rails_application/app/controllers/client/orders_controller.rb b/rails_application/app/controllers/client/orders_controller.rb index 970243cb2..813899bc4 100644 --- a/rails_application/app/controllers/client/orders_controller.rb +++ b/rails_application/app/controllers/client/orders_controller.rb @@ -12,13 +12,12 @@ def new end def create - ActiveRecord::Base.transaction do - command_bus.(Ordering::SubmitOrder.new(order_id: params[:order_id])) - command_bus.(Crm::AssignCustomerToOrder.new(customer_id: cookies[:client_id], order_id: params[:order_id])) + Orders::SubmitService.new(order_id: params[:order_id], customer_id: cookies[:client_id]).call.then do |result| + result.path(:success) { redirect_to client_order_path(params[:order_id]), notice: "Your order is being submitted" } + result.path(:products_out_of_stock) { |unavailable_products| redirect_to edit_client_order_path(params[:order_id]), + alert: "Order can not be submitted! #{unavailable_products.join(", ")} not available in requested quantity!"} + result.path(:order_is_empty) { redirect_to edit_client_order_path(params[:order_id]), alert: "You can't submit an empty order" } end - redirect_to client_order_path(params[:order_id]), notice: "Your order is being submitted" - rescue Ordering::Order::IsEmpty - redirect_to edit_client_order_path(params[:order_id]), alert: "You can't submit an empty order" end def show diff --git a/rails_application/app/controllers/orders_controller.rb b/rails_application/app/controllers/orders_controller.rb index 0ca77e2a4..0ff99611a 100644 --- a/rails_application/app/controllers/orders_controller.rb +++ b/rails_application/app/controllers/orders_controller.rb @@ -74,12 +74,13 @@ def remove_item end def create - ApplicationRecord.transaction { submit_order(params[:order_id], params[:customer_id]) } - redirect_to order_path(params[:order_id]), notice: "Your order is being submitted" - rescue Ordering::Order::IsEmpty - redirect_to edit_order_path(params[:order_id]), alert: "You can't submit an empty order" - rescue Crm::Customer::NotExists - redirect_to order_path(params[:order_id]), alert: "Order can not be submitted! Customer does not exist." + Orders::SubmitService.new(order_id: params[:order_id], customer_id: params[:customer_id]).call.then do |result| + result.path(:success) { redirect_to order_path(params[:order_id]), notice: "Your order is being submitted" } + result.path(:products_out_of_stock) { |unavailable_products| redirect_to edit_order_path(params[:order_id]), + alert: "Order can not be submitted! #{unavailable_products.join(", ")} not available in requested quantity!"} + result.path(:order_is_empty) { redirect_to edit_order_path(params[:order_id]), alert: "You can't submit an empty order" } + result.path(:customer_not_exists) { redirect_to order_path(params[:order_id]), alert: "Order can not be submitted! Customer does not exist." } + end end def expire @@ -113,11 +114,6 @@ def cancel private - def submit_order(order_id, customer_id) - command_bus.(Ordering::SubmitOrder.new(order_id: order_id)) - command_bus.(Crm::AssignCustomerToOrder.new(order_id: order_id, customer_id: customer_id)) - end - def authorize_payment(order_id) command_bus.call(authorize_payment_cmd(order_id)) end diff --git a/rails_application/app/services/application_service.rb b/rails_application/app/services/application_service.rb new file mode 100644 index 000000000..73f75e101 --- /dev/null +++ b/rails_application/app/services/application_service.rb @@ -0,0 +1,17 @@ +class ApplicationService + def self.call(...) + new(...).call + end + + def call + raise NotImplementedError + end + + def event_store + Rails.configuration.event_store + end + + def command_bus + Rails.configuration.command_bus + end +end diff --git a/rails_application/app/services/orders/submit_service.rb b/rails_application/app/services/orders/submit_service.rb new file mode 100644 index 000000000..f26f9cd98 --- /dev/null +++ b/rails_application/app/services/orders/submit_service.rb @@ -0,0 +1,42 @@ +module Orders + class SubmitService < ApplicationService + def initialize(order_id:, customer_id:) + @order_id = order_id + @customer_id = customer_id + end + + def call + success = true + unavailable_products = nil + + event_store + .within { submit_order } + .subscribe(to: Processes::ReservationProcessFailed) do |event| + success = false + unavailable_products = Products::Product.where(id: event.data.fetch(:unavailable_products)).pluck(:name) + end + .call + + if success + Result.new(:success) + else + Result.new(:products_out_of_stock, unavailable_products) + end + rescue Ordering::Order::IsEmpty + Result.new(:order_is_empty) + rescue Crm::Customer::NotExists + Result.new(:customer_not_exists) + end + + private + + attr_reader :order_id, :customer_id + + def submit_order + ActiveRecord::Base.transaction do + command_bus.(Ordering::SubmitOrder.new(order_id: order_id)) + command_bus.(Crm::AssignCustomerToOrder.new(order_id: order_id, customer_id: customer_id)) + end + end + end +end diff --git a/rails_application/app/services/result.rb b/rails_application/app/services/result.rb new file mode 100644 index 000000000..12fa16087 --- /dev/null +++ b/rails_application/app/services/result.rb @@ -0,0 +1,14 @@ +class Result + attr_reader :status, :args + + def initialize(status, *args) + @status = status + @args = args + end + + def path(name, &block) + return unless @status == name.to_sym + + block.call(*@args) + end +end diff --git a/rails_application/test/integration/orders_test.rb b/rails_application/test/integration/orders_test.rb index 8c3c1610a..327d75c96 100644 --- a/rails_application/test/integration/orders_test.rb +++ b/rails_application/test/integration/orders_test.rb @@ -203,6 +203,22 @@ def test_empty_order_cannot_be_submitted assert_select "#alert", "You can't submit an empty order" end + def test_order_cannot_be_submitted_with_out_of_stock_product + product_id = register_product("Fearless Refactoring", 4, 10) + shopify_id = register_customer("Shopify") + + supply_product(product_id, 1) + order_1_id = SecureRandom.uuid + order_2_id = SecureRandom.uuid + post "/orders/#{order_1_id}/add_item?product_id=#{product_id}" + post "/orders/#{order_2_id}/add_item?product_id=#{product_id}" + + post "/orders", params: { order_id: order_1_id, customer_id: shopify_id } + post "/orders", params: { order_id: order_2_id, customer_id: shopify_id } + + assert_equal "Order can not be submitted! Fearless Refactoring not available in requested quantity!", flash[:alert] + end + private def assert_remove_buttons_visible(async_remote_id, fearless_id, order_id) diff --git a/rails_application/test/services/orders/submit_service_test.rb b/rails_application/test/services/orders/submit_service_test.rb new file mode 100644 index 000000000..6c699f50d --- /dev/null +++ b/rails_application/test/services/orders/submit_service_test.rb @@ -0,0 +1,84 @@ +require "test_helper" + +module Orders + class SubmitServiceTest < InMemoryTestCase + cover Orders::SubmitService + + def test_successful_order_submission + order_id = SecureRandom.uuid + customer_id = SecureRandom.uuid + product_id = SecureRandom.uuid + + run_command(Crm::RegisterCustomer.new(customer_id: customer_id, name: "John Doe")) + prepare_product(product_id, "Async Remote", 49) + run_command(Ordering::AddItemToBasket.new(order_id: order_id, product_id: product_id)) + + result = Orders::SubmitService.new(order_id: order_id, customer_id: customer_id).call + + assert_equal result.status, :success + end + + def test_order_submission_with_unavailable_products + order_id = SecureRandom.uuid + customer_id = SecureRandom.uuid + product_id = SecureRandom.uuid + another_product_id = SecureRandom.uuid + + run_command(Crm::RegisterCustomer.new(customer_id: customer_id, name: "John Doe")) + prepare_product(product_id, "Async Remote", 49) + run_command(Inventory::Supply.new(product_id: product_id, quantity: 1)) + run_command(Inventory::Reserve.new(product_id: product_id, quantity: 1)) + run_command(Ordering::AddItemToBasket.new(order_id: order_id, product_id: product_id)) + prepare_product(another_product_id, "Fearless Refactoring", 49) + run_command(Ordering::AddItemToBasket.new(order_id: order_id, product_id: another_product_id)) + + result = Orders::SubmitService.new(order_id: order_id, customer_id: customer_id).call + + assert_equal result.status, :products_out_of_stock + assert_equal result.args, [["Async Remote"]] + end + + def test_order_submission_with_empty_order + order_id = SecureRandom.uuid + customer_id = SecureRandom.uuid + + result = Orders::SubmitService.new(order_id: order_id, customer_id: customer_id).call + + assert_equal result.status, :order_is_empty + end + + def test_order_submission_with_non_existing_customer + order_id = SecureRandom.uuid + customer_id = SecureRandom.uuid + product_id = SecureRandom.uuid + + prepare_product(product_id, "Async Remote", 49) + run_command(Ordering::AddItemToBasket.new(order_id: order_id, product_id: product_id)) + + result = Orders::SubmitService.new(order_id: order_id, customer_id: customer_id).call + + assert_equal result.status, :customer_not_exists + end + + private + + def event_store + Rails.configuration.event_store + end + + def prepare_product(product_id, name, price) + run_command( + ProductCatalog::RegisterProduct.new( + product_id: product_id, + ) + ) + run_command( + ProductCatalog::NameProduct.new( + product_id: product_id, + name: name + ) + ) + run_command(Pricing::SetPrice.new(product_id: product_id, price: price)) + end + end +end diff --git a/rails_application/test/services/result_test.rb b/rails_application/test/services/result_test.rb new file mode 100644 index 000000000..78080201f --- /dev/null +++ b/rails_application/test/services/result_test.rb @@ -0,0 +1,44 @@ +require "test_helper" + +class ResultTest < InMemoryTestCase + cover Result + + def test_executes_path + result = Result.new(:first_path) + executed_path = nil + + result.path(:first_path) { executed_path = :first } + result.path(:other_path) { executed_path = :other } + + assert_equal :first, executed_path + end + + def test_executes_path_with_argument + result = Result.new(:first_path, "argument") + executed_path = nil + + result.path(:first_path) do |arg| + assert arg == "argument" + + executed_path = :first + end + result.path(:other_path) { |_arg| executed_path = :other } + + assert_equal :first, executed_path + end + + def test_executes_path_with_multiple_arguments + result = Result.new(:first_path, "argument1", "argument2") + executed_path = nil + + result.path(:first_path) do |arg1, arg2| + assert arg1 == "argument1" + assert arg2 == "argument2" + + executed_path = :first + end + result.path(:other_path) { |_arg| executed_path = :other } + + assert_equal :first, executed_path + end +end