-
Notifications
You must be signed in to change notification settings - Fork 72
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
Adding/Removing items to refunds #425
base: add-new-refund-form
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
module Ordering | ||
class AddItemToRefund < Infra::Command | ||
attribute :refund_id, Infra::Types::UUID | ||
attribute :order_id, Infra::Types::UUID | ||
attribute :product_id, Infra::Types::UUID | ||
|
||
alias aggregate_id refund_id | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
module Ordering | ||
class RemoveItemFromRefund < Infra::Command | ||
attribute :refund_id, Infra::Types::UUID | ||
attribute :order_id, Infra::Types::UUID | ||
attribute :product_id, Infra::Types::UUID | ||
|
||
alias aggregate_id refund_id | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
module Ordering | ||
class ItemAddedToRefund < Infra::Event | ||
attribute :refund_id, Infra::Types::UUID | ||
attribute :order_id, Infra::Types::UUID | ||
attribute :product_id, Infra::Types::UUID | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
module Ordering | ||
class ItemRemovedFromRefund < Infra::Event | ||
attribute :refund_id, Infra::Types::UUID | ||
attribute :order_id, Infra::Types::UUID | ||
attribute :product_id, Infra::Types::UUID | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
require_relative "test_helper" | ||
|
||
module Ordering | ||
class AddItemToRefundTest < Test | ||
cover "Ordering::OnAddItemToRefund*" | ||
|
||
def test_add_item_to_refund | ||
order_id = SecureRandom.uuid | ||
aggregate_id = SecureRandom.uuid | ||
product_id = SecureRandom.uuid | ||
stream = "Ordering::Refund$#{aggregate_id}" | ||
|
||
arrange( | ||
CreateDraftRefund.new( | ||
refund_id: aggregate_id, | ||
order_id: order_id | ||
) | ||
) | ||
|
||
expected_events = [ | ||
ItemAddedToRefund.new( | ||
data: { | ||
refund_id: aggregate_id, | ||
order_id: order_id, | ||
product_id: product_id | ||
} | ||
) | ||
] | ||
|
||
assert_events(stream, *expected_events) do | ||
act( | ||
AddItemToRefund.new( | ||
refund_id: aggregate_id, | ||
order_id: order_id, | ||
product_id: product_id | ||
) | ||
) | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
require_relative "test_helper" | ||
|
||
module Ordering | ||
class RemoveItemFromRefundTest < Test | ||
cover "Ordering::OnRemoveItemFromRefund*" | ||
|
||
def test_removing_items_from_refund | ||
order_id = SecureRandom.uuid | ||
aggregate_id = SecureRandom.uuid | ||
product_id = SecureRandom.uuid | ||
stream = "Ordering::Refund$#{aggregate_id}" | ||
|
||
arrange( | ||
CreateDraftRefund.new( | ||
refund_id: aggregate_id, | ||
order_id: order_id | ||
), | ||
AddItemToRefund.new( | ||
refund_id: aggregate_id, | ||
order_id: order_id, | ||
product_id: product_id | ||
) | ||
) | ||
|
||
expected_events = [ | ||
ItemRemovedFromRefund.new( | ||
data: { | ||
refund_id: aggregate_id, | ||
order_id: order_id, | ||
product_id: product_id | ||
} | ||
) | ||
] | ||
|
||
assert_events(stream, *expected_events) do | ||
act( | ||
RemoveItemFromRefund.new( | ||
refund_id: aggregate_id, | ||
order_id: order_id, | ||
product_id: product_id | ||
) | ||
) | ||
end | ||
end | ||
|
||
def test_can_remove_only_added_items | ||
order_id = SecureRandom.uuid | ||
aggregate_id = SecureRandom.uuid | ||
product_id = SecureRandom.uuid | ||
|
||
arrange( | ||
CreateDraftRefund.new( | ||
refund_id: aggregate_id, | ||
order_id: order_id | ||
) | ||
) | ||
|
||
assert_raises(Refund::ProductNotFoundError) do | ||
act( | ||
RemoveItemFromRefund.new( | ||
refund_id: aggregate_id, | ||
order_id: order_id, | ||
product_id: product_id | ||
) | ||
) | ||
end | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,14 @@ def create | |
redirect_to edit_order_refund_path(refund_id, order_id: params[:order_id]) | ||
end | ||
|
||
def add_item | ||
add_item_to_refund | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if this abstraction brings any benefit. Do you think it is more readable this way? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tbh I followed the same pattern as is in e.g. |
||
end | ||
|
||
def remove_item | ||
remove_item_from_refund | ||
end | ||
|
||
private | ||
|
||
def create_draft_refund_cmd(refund_id) | ||
|
@@ -21,4 +29,20 @@ def create_draft_refund_cmd(refund_id) | |
def create_draft_refund(refund_id) | ||
command_bus.(create_draft_refund_cmd(refund_id)) | ||
end | ||
|
||
def add_item_to_refund_cmd | ||
Ordering::AddItemToRefund.new(refund_id: params[:id], order_id: params[:order_id], product_id: params[:product_id]) | ||
end | ||
|
||
def add_item_to_refund | ||
command_bus.(add_item_to_refund_cmd) | ||
end | ||
|
||
def remove_item_from_refund_cmd | ||
Ordering::RemoveItemFromRefund.new(refund_id: params[:id], order_id: params[:order_id], product_id: params[:product_id]) | ||
end | ||
|
||
def remove_item_from_refund | ||
command_bus.(remove_item_from_refund_cmd) | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
module Refunds | ||
class AddItemToRefund | ||
def call(event) | ||
refund_id = event.data.fetch(:refund_id) | ||
Refund.find_or_create_by!(uid: refund_id) | ||
product_id = event.data.fetch(:product_id) | ||
item = | ||
find(refund_id, product_id) || | ||
create(refund_id, product_id) | ||
item.quantity += 1 | ||
item.save! | ||
end | ||
|
||
private | ||
|
||
def event_store | ||
Rails.configuration.event_store | ||
end | ||
|
||
def find(refund_id, product_id) | ||
Refund | ||
.find_by_uid(refund_id) | ||
.refund_items | ||
.where(product_uid: product_id) | ||
.first | ||
end | ||
|
||
def create(refund_id, product_id) | ||
product = Orders::Product.find_by_uid(product_id) | ||
Refund | ||
.find_by(uid: refund_id) | ||
.refund_items | ||
.create( | ||
product_uid: product_id, | ||
price: product.price, | ||
quantity: 0 | ||
) | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
module Refunds | ||
class RemoveItemFromRefund | ||
def call(event) | ||
refund_id = event.data.fetch(:refund_id) | ||
product_id = event.data.fetch(:product_id) | ||
item = find(refund_id, product_id) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this event handler idempotent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, it isn't, but most, if not all, of our event handlers aren't idempotent 🤔 I'm not sure if I know how to make it idempotent, maybe you have some ideas? |
||
item.quantity -= 1 | ||
item.quantity > 0 ? item.save! : item.destroy! | ||
end | ||
|
||
private | ||
def find(order_uid, product_id) | ||
Refund | ||
.find_by_uid(order_uid) | ||
.refund_items | ||
.where(product_uid: product_id) | ||
.first | ||
end | ||
|
||
def event_store | ||
Rails.configuration.event_store | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this error could have more meaningful business name. Something like RefundHaveNotBeenRequestedForThisProduct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, I changed the name 👍