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

Allow creating custom vat rates #377

Merged
merged 4 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
5 changes: 2 additions & 3 deletions ecommerce/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@

module Ecommerce
class Configuration
def initialize(number_generator: nil, payment_gateway: nil, available_vat_rates: [])
def initialize(number_generator: nil, payment_gateway: nil)
@number_generator = number_generator
@payment_gateway = payment_gateway
@available_vat_rates = available_vat_rates
end

def call(event_store, command_bus)
Expand All @@ -37,7 +36,7 @@ def configure_bounded_contexts(event_store, command_bus)
Payments::Configuration.new(@payment_gateway),
Shipping::Configuration.new,
Pricing::Configuration.new,
Taxes::Configuration.new(@available_vat_rates),
Taxes::Configuration.new,
ProductCatalog::Configuration.new,
Fulfillment::Configuration.new
].each { |c| c.call(event_store, command_bus) }
Expand Down
10 changes: 1 addition & 9 deletions ecommerce/taxes/lib/taxes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,10 @@

module Taxes
class Configuration
def self.available_vat_rates
@@available_vat_rates
end

def initialize(available_vat_rates = [])
@available_vat_rates = available_vat_rates
end

def call(event_store, command_bus)
@@available_vat_rates = @available_vat_rates
command_bus.register(SetVatRate, SetVatRateHandler.new(event_store))
command_bus.register(DetermineVatRate, DetermineVatRateHandler.new(event_store))
command_bus.register(AddAvailableVatRate, AddAvailableVatRateHandler.new(event_store))
end
end
end
7 changes: 6 additions & 1 deletion ecommerce/taxes/lib/taxes/commands.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,9 @@ class DetermineVatRate < Infra::Command
attribute :product_id, Infra::Types::UUID
attribute :order_id, Infra::Types::UUID
end
end

class AddAvailableVatRate < Infra::Command
attribute :available_vat_rate_id, Infra::Types::UUID
attribute :vat_rate, Infra::Types::VatRate
end
end
7 changes: 6 additions & 1 deletion ecommerce/taxes/lib/taxes/events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,9 @@ class VatRateDetermined < Infra::Event
attribute :product_id, Infra::Types::UUID
attribute :vat_rate, Infra::Types::VatRate
end
end

class AvailableVatRateAdded < Infra::Event
attribute :available_vat_rate_id, Infra::Types::UUID
attribute :vat_rate, Infra::Types::VatRate
end
end
8 changes: 4 additions & 4 deletions ecommerce/taxes/lib/taxes/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ def initialize(id)
@id = id
end

def set_vat_rate(vat_rate)
raise VatRateNotApplicable unless vat_rate_applicable?(vat_rate)
def set_vat_rate(vat_rate, catalog)
raise VatRateNotApplicable unless vat_rate_applicable?(vat_rate.code, catalog)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The validation checking if the vat rate is available should happen outside of the domain model

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks @lukaszreszke! Can you help me figure out where to put it? Is command handler a better place for it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes - command handler is better place for checking such a logic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

apply(VatRateSet.new(data: { product_id: @id, vat_rate: vat_rate }))
end

private

def vat_rate_applicable?(vat_rate)
Configuration.available_vat_rates.include?(vat_rate)
def vat_rate_applicable?(vat_rate_code, catalog)
catalog.vat_rate_available?(vat_rate_code)
end

on(VatRateSet) { |_| }
Expand Down
36 changes: 34 additions & 2 deletions ecommerce/taxes/lib/taxes/services.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
module Taxes
VatRateAlreadyExists = Class.new(StandardError)
class SetVatRateHandler
def initialize(event_store)
@repository = Infra::AggregateRootRepository.new(event_store)
@catalog = VatRateCatalog.new(event_store)
end

def call(cmd)
@repository.with_aggregate(Product, cmd.product_id) do |product|
product.set_vat_rate(cmd.vat_rate)
product.set_vat_rate(cmd.vat_rate, @catalog)
end
end
end
Expand Down Expand Up @@ -34,4 +36,34 @@ def stream_name(order_id)
"Taxes::Order$#{order_id}"
end
end
end

class AddAvailableVatRateHandler
def initialize(event_store)
@catalog = VatRateCatalog.new(event_store)
@event_store = event_store
end

def call(cmd)
raise VatRateAlreadyExists if catalog.vat_rate_available?(cmd.vat_rate.code)

event_store.publish(available_vat_rate_added_event(cmd), stream_name: stream_name(cmd))
end

private

attr_reader :event_store, :catalog

def available_vat_rate_added_event(cmd)
AvailableVatRateAdded.new(
data: {
available_vat_rate_id: cmd.available_vat_rate_id,
vat_rate: cmd.vat_rate
}
)
end

def stream_name(cmd)
"Taxes::AvailableVatRate$#{cmd.vat_rate.code}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm using code instead of uuid in the stream name.
This helps ensure uniqueness of vat rates and simplifies some logic.

Please let me know what you think.

Copy link
Collaborator

@lukaszreszke lukaszreszke Aug 14, 2024

Choose a reason for hiding this comment

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

Technically it is a good choice. However I am wondering if the model is correct. But I don't have an answer yet. It is good for first iteration I guess.

end
end
end
10 changes: 9 additions & 1 deletion ecommerce/taxes/lib/taxes/vat_rate_catalog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,13 @@ def vat_rate_for(product_id)
&.data
&.fetch(:vat_rate)
end

def vat_rate_available?(vat_rate_code)
@event_store
.read
.stream("Taxes::AvailableVatRate$#{vat_rate_code}")
.to_a
.any?
end
end
end
end
47 changes: 35 additions & 12 deletions ecommerce/taxes/test/taxes_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,32 +3,59 @@
module Taxes
class TaxesTest < Test
def test_setting_available_vat_rate
vat_rate = Infra::Types::VatRate.new(code: "50", rate: 50)
add_available_vat_rate(vat_rate)

product_id = SecureRandom.uuid
vat_rate_set = VatRateSet.new(data: { product_id: product_id, vat_rate: available_vat_rate })
vat_rate_set = VatRateSet.new(data: { product_id: product_id, vat_rate: vat_rate })
Copy link
Collaborator

Choose a reason for hiding this comment

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

small tip, you could do

vat_rate_set = VatRateSet.new(data: { product_id:, vat_rate: })

instead :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would prefer to leave this as is for now, as it is consistent with the rest of the code in the project. In the future we can add a rubocop rule to enforce style guides :)

assert_events("Taxes::Product$#{product_id}", vat_rate_set) do
set_vat_rate(product_id, available_vat_rate)
set_vat_rate(product_id, vat_rate)
end
end

def test_setting_unavailable_vat_rate_should_raise_error
product_id = SecureRandom.uuid
unavailable_vat_rate = Infra::Types::VatRate.new(code: "20", rate: 20)

assert_raises(Product::VatRateNotApplicable) do
set_vat_rate(product_id, unavailable_vat_rate)
end
end

def test_determining_vat_rate
vat_rate = Infra::Types::VatRate.new(code: "50", rate: 50)
add_available_vat_rate(vat_rate)

order_id = SecureRandom.uuid
product_id = SecureRandom.uuid
another_product_id = SecureRandom.uuid

set_vat_rate(product_id, available_vat_rate)
vat_rate_determined = VatRateDetermined.new(data: { order_id: order_id, product_id: product_id, vat_rate: available_vat_rate })
set_vat_rate(product_id, vat_rate)
vat_rate_determined = VatRateDetermined.new(data: { order_id: order_id, product_id: product_id, vat_rate: vat_rate })
assert_events("Taxes::Order$#{order_id}", vat_rate_determined) do
determine_vat_rate(order_id, product_id, available_vat_rate)
determine_vat_rate(order_id, product_id, vat_rate)
end
assert_events("Taxes::Order$#{order_id}") do
determine_vat_rate(order_id, another_product_id, available_vat_rate)
determine_vat_rate(order_id, another_product_id, vat_rate)
end
end

def test_adding_available_vat_rate
available_vat_rate_id = SecureRandom.uuid
vat_rate = Infra::Types::VatRate.new(code: "50", rate: 50)
available_vat_rate_added = AvailableVatRateAdded.new(data: { available_vat_rate_id: available_vat_rate_id, vat_rate: vat_rate })

assert_events("Taxes::AvailableVatRate$#{vat_rate.code}", available_vat_rate_added) do
add_available_vat_rate(vat_rate, available_vat_rate_id)
end
end

def test_should_not_allow_for_double_registration
vat_rate = Infra::Types::VatRate.new(code: "50", rate: 50)
add_available_vat_rate(vat_rate)

assert_raises(VatRateAlreadyExists) do
add_available_vat_rate(vat_rate)
end
end

Expand All @@ -42,12 +69,8 @@ def determine_vat_rate(order_id, product_id, vat_rate)
run_command(DetermineVatRate.new(order_id: order_id, product_id: product_id, vat_rate: vat_rate))
end

def available_vat_rate
Configuration.available_vat_rates.first
end

def unavailable_vat_rate
Infra::Types::VatRate.new(code: "50", rate: 50)
def add_available_vat_rate(vat_rate, available_vat_rate_id = SecureRandom.uuid)
run_command(AddAvailableVatRate.new(available_vat_rate_id: available_vat_rate_id, vat_rate: vat_rate))
end
end
end
8 changes: 1 addition & 7 deletions ecommerce/taxes/test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@ class Test < Infra::InMemoryTest

def before_setup
super
Configuration.new([dummy_vat_rate]).call(event_store, command_bus)
end

private

def dummy_vat_rate
Infra::Types::VatRate.new(code: "20", rate: 20)
Configuration.new.call(event_store, command_bus)
end
end
end
38 changes: 38 additions & 0 deletions ecommerce/taxes/test/vat_rate_catalog_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
require_relative "test_helper"

module Taxes
class VatRateCatalogTest < Test
class VatRateAvailableTest < VatRateCatalogTest
def test_returns_true_when_vat_rate_is_available
vat_rate = Infra::Types::VatRate.new(code: "50", rate: 50)
add_available_vat_rate(vat_rate)

assert catalog.vat_rate_available?(vat_rate.code)
end

def test_returns_false_when_vat_rate_is_not_available
vat_rate = Infra::Types::VatRate.new(code: "50", rate: 50)

refute catalog.vat_rate_available?(vat_rate.code)
end

def test_returns_false_when_vat_rate_is_not_available_even_when_other_vat_rates_are_available
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't quite like this test but mutant complained without it:

@event_store
        .read
        .stream("Taxes::AvailableVatRate$#{vat_rate_code}")
        .to_a
        .any?

Mutant indicated that this code passes all the tests without the stream filter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the test if fine, but the name of it could be improved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had to change the functionality of VatRateCatalog so I removed these tests.

vat_rate = Infra::Types::VatRate.new(code: "50", rate: 50)
add_available_vat_rate(vat_rate)

refute catalog.vat_rate_available?("60")
end
end


private

def catalog
VatRateCatalog.new(@event_store)
end

def add_available_vat_rate(vat_rate, available_vat_rate_id = SecureRandom.uuid)
run_command(AddAvailableVatRate.new(available_vat_rate_id: available_vat_rate_id, vat_rate: vat_rate))
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
class AvailableVatRatesController < ApplicationController
class AvailableVatRateForm
include ActiveModel::Model
include ActiveModel::Validations

attr_reader :code, :rate

def initialize(params)
@code = params[:code]
@rate = params[:rate]
end
Comment on lines +6 to +11
Copy link
Collaborator Author

Choose a reason for hiding this comment

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


validates :code, presence: true
validates :rate, presence: true, numericality: { only_numeric: true, greater_than: 0 }
end

def new
end

def create
available_vat_rate_id = SecureRandom.uuid
available_vat_rate_form = AvailableVatRateForm.new(available_vat_rate_params)

unless available_vat_rate_form.valid?
return render "new", locals: { errors: available_vat_rate_form.errors }, status: :unprocessable_entity
end

add_available_vat_rate(available_vat_rate_form.code, available_vat_rate_form.rate, available_vat_rate_id)
rescue Taxes::VatRateAlreadyExists
flash.now[:notice] = "VAT rate already exists"
render "new", status: :unprocessable_entity
else
redirect_to available_vat_rates_path, notice: "VAT rate was successfully created"
end

def index
@available_vat_rates = VatRates::AvailableVatRate.all
end

private

def add_available_vat_rate(code, rate, available_vat_rate_id)
command_bus.(add_available_vat_rate_cmd(code, rate, available_vat_rate_id))
end

def add_available_vat_rate_cmd(code, rate, available_vat_rate_id)
Taxes::AddAvailableVatRate.new(
available_vat_rate_id: available_vat_rate_id,
vat_rate: Infra::Types::VatRate.new(code: code, rate: rate)
)
end

def available_vat_rate_params
params.permit(:code, :rate)
end
end
4 changes: 2 additions & 2 deletions rails_application/app/controllers/products_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ def set_future_product_price(product_id, price, valid_since)
command_bus.(set_product_future_price_cmd(product_id, price, valid_since))
end

def set_product_vat_rate(product_id, vat_rate_code)
vat_rate = Taxes::Configuration.available_vat_rates.find{|rate| rate.code == vat_rate_code}
def set_product_vat_rate(product_id, vat_rate)
vat_rate = VatRates::AvailableVatRate.find_by!(code: vat_rate).to_value
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think that the controller should validate if the vat rate exists

Copy link
Collaborator

Choose a reason for hiding this comment

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

Especially as it happens in the domain model (which is also not correct).

Would you move it to the SetVatRateHandler?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done 👍

command_bus.(set_product_vat_rate_cmd(product_id, vat_rate))
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module VatRates
class AddAvailableVatRate
def call(event)
AvailableVatRate.create!(
uid: event.data.fetch(:available_vat_rate_id),
code: event.data.fetch(:vat_rate).fetch(:code),
rate: event.data.fetch(:vat_rate).fetch(:rate)
)
end
end
end
15 changes: 15 additions & 0 deletions rails_application/app/read_models/vat_rates/configuration.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module VatRates
class AvailableVatRate < ApplicationRecord
self.table_name = "available_vat_rates"

def to_value
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is not used anymore, you can remove this method in next PR :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I already removed it :)

Infra::Types::VatRate.new(code: code, rate: rate)
end
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Other models don't have any methods but this was very useful. I can remove and refactor if we don't want to have any methods in models

end

class Configuration
def call(event_store)
event_store.subscribe(AddAvailableVatRate, to: [Taxes::AvailableVatRateAdded])
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<%= navigation_link "Coupons", coupons_path %>
<%= navigation_link "Time Promotions", time_promotions_path %>
<%= navigation_link "Customers", customers_path %>
<%= navigation_link "VAT Rates", available_vat_rates_path %>
<%= navigation_link "Client View", clients_path %>
</div>
</div>
Expand Down
Loading