Adding/Removing items to refunds #425

marlena-b · 2025-01-07T09:28:59Z

Issue: #154

This is the second step to allow order refunds. First step is here: #424.

add/remove buttons now works, the form can be filled
the form cannot be submitted yet - it will be added in another PR :)

Nagranie.z.ekranu.2025-01-7.o.10.07.34.mov

rails_application/app/views/orders/show.html.erb

ecommerce/ordering/lib/ordering/refund.rb

lukaszreszke · 2025-01-15T08:55:24Z

ecommerce/ordering/lib/ordering/refund.rb

+    end
+
+    def remove_item(product_id)
+      raise ProductNotFoundError unless @refund_items.quantity(product_id).positive?


I wonder if this error could have more meaningful business name. Something like RefundHaveNotBeenRequestedForThisProduct

Good idea, I changed the name 👍

lukaszreszke · 2025-01-15T08:56:40Z

rails_application/app/read_models/refunds/remove_item_from_refund.rb

+    def call(event)
+      refund_id = event.data.fetch(:refund_id)
+      product_id = event.data.fetch(:product_id)
+      item = find(refund_id, product_id)


Is this event handler idempotent?

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?

ecommerce/ordering/lib/ordering/projections.rb

rails_application/app/controllers/refunds_controller.rb

lukaszreszke · 2025-01-15T09:08:12Z

ecommerce/ordering/lib/ordering/service.rb

-        refund.add_item(command.product_id)
+        refund.add_item(
+          command.product_id,
+          available_quantity_to_refund(command.order_id, command.product_id)


I think that the available quantity to refund information should be passed to Refund once it is requested. We don't want this value to be changed.

I see it that way:

Order has been paid

Someone wants to refund part of it or whole

Refund is created, it knows what products (or how many this case*) were there in the order

It allows refunding those products or not

Perhaps it should know which products were in the order initially, not only quantity of them?

Are we talking about refunding items or product? Method names of refund imply items, but projection's method indicates product.

Thanks for the suggestion! I'm not sure if I understand what you mean.
You propose that when creating a Refund, we should pass in all the quantities per product (e.g., { product_1_id: 13, product_2_id: 3 }), and store this information within the Refund aggregate. This would allow us to validate refund operations against the original available quantities without needing to query the projection each time.
Is that what you mean?

lukaszreszke · 2025-02-17T09:30:35Z

rails_application/test/orders/item_added_to_refund_test.rb

+
+      AddItemToRefund.new.call(item_added_to_refund(refund_id, order_id, product_id))
+
+      assert_equal(Refunds::RefundItem.count, 1)


Parameters are reversed. First. parameter is "expected", the other one is actual value.

Fixed, thank you! :)

lukaszreszke · 2025-02-17T09:30:55Z

rails_application/test/orders/item_added_to_refund_test.rb

+      assert_equal(1, refund_item.quantity)
+      assert_equal(50, refund_item.price)
+
+      assert_equal(Refunds::Refund.count, 1)


Same here (reversed parameters)

Fixed, thank you!

lukaszreszke · 2025-02-17T09:31:21Z

rails_application/test/orders/item_removed_from_refund_test.rb

+
+      RemoveItemFromRefund.new.call(item_removed_from_refund(refund_id, order_id, product_id))
+
+      assert_equal(Refunds::RefundItem.count, 1)


Same here (reversed parameters)

Fixed, thank you!

lukaszreszke · 2025-02-17T09:31:33Z

rails_application/test/orders/item_removed_from_refund_test.rb

+      assert_equal(1, refund_item.quantity)
+      assert_equal(30, refund_item.price)
+
+      assert_equal(Refunds::Refund.count, 1)


Same here (reversed parameters)

lukaszreszke · 2025-02-17T09:31:46Z

rails_application/test/orders/item_removed_from_refund_test.rb

+
+      assert_equal(Refunds::Refund.count, 1)
+      refund = Refunds::Refund.find_by(uid: refund_id)
+      assert_equal(refund.status, "Draft")


Same here (reversed parameters)

netlify · 2025-02-17T10:04:43Z