From 555a3fe2ac6af8c52d6dc652b0672231aa2b4d13 Mon Sep 17 00:00:00 2001 From: Kotmin <70173732+Kotmin@users.noreply.github.com> Date: Tue, 23 Dec 2025 15:14:18 +0100 Subject: [PATCH 1/2] Reject invalid coupon discount --- ecommerce/pricing/test/coupons_test.rb | 14 ++++++++++ .../app/controllers/coupons_controller.rb | 27 ++++++++++++++++--- .../app/views/coupons/new.html.erb | 6 ++--- .../test/integration/coupons_test.rb | 21 +++++++++++++++ 4 files changed, 61 insertions(+), 7 deletions(-) diff --git a/ecommerce/pricing/test/coupons_test.rb b/ecommerce/pricing/test/coupons_test.rb index 55d04fe7f..d3f25eb5a 100644 --- a/ecommerce/pricing/test/coupons_test.rb +++ b/ecommerce/pricing/test/coupons_test.rb @@ -36,5 +36,19 @@ def test_100_is_ok def test_0_01_is_ok register_coupon(@uid, fake_name, @code, "0.01") end + + def test_negative_discount_is_rejected + assert_raises(Infra::Command::Invalid) do + register_coupon(@uid, fake_name, @code, -0.01) + end + end + + def test_negative_discount_string_is_rejected + assert_raises(Infra::Command::Invalid) do + register_coupon(@uid, fake_name, @code, "-150") + end + end + + end end diff --git a/rails_application/app/controllers/coupons_controller.rb b/rails_application/app/controllers/coupons_controller.rb index 301ac610a..5abd37805 100644 --- a/rails_application/app/controllers/coupons_controller.rb +++ b/rails_application/app/controllers/coupons_controller.rb @@ -8,10 +8,13 @@ def new end def create - coupon_id = params[:coupon_id] + @coupon_id = params[:coupon_id].presence || SecureRandom.uuid + + discount = validated_discount(params[:discount]) + return render_invalid_discount unless discount ActiveRecord::Base.transaction do - create_coupon(coupon_id) + create_coupon(@coupon_id, discount) end rescue Pricing::Coupon::AlreadyRegistered flash[:notice] = "Coupon is already registered" @@ -22,13 +25,13 @@ def create private - def create_coupon(coupon_id) + def create_coupon(coupon_id,discount) command_bus.( Pricing::RegisterCoupon.new( coupon_id: coupon_id, name: params[:name], code: params[:code], - discount: params[:discount] + discount: discount ) ) command_bus.( @@ -39,4 +42,20 @@ def create_coupon(coupon_id) ) end + def validated_discount(raw) + return nil if raw.blank? + + value = BigDecimal(raw.to_s) + return nil unless value > 0 && value <= 100 + + value + rescue ArgumentError + nil + end + + def render_invalid_discount + flash.now[:alert] = "Discount must be greater than 0 and less than or equal to 100" + render "new", status: :unprocessable_entity + end + end diff --git a/rails_application/app/views/coupons/new.html.erb b/rails_application/app/views/coupons/new.html.erb index a2972eb90..14d63ba59 100644 --- a/rails_application/app/views/coupons/new.html.erb +++ b/rails_application/app/views/coupons/new.html.erb @@ -18,15 +18,15 @@ - <%= text_field_tag :name, "", required: true, class: "mt-1 focus:ring-blue-500 focus:border-blue-500 block shadow-sm sm:text-sm border-gray-300 rounded-md" %> + <%= text_field_tag :name, params[:name], required: true, class: "mt-1 focus:ring-blue-500 focus:border-blue-500 block shadow-sm sm:text-sm border-gray-300 rounded-md" %> - <%= text_field_tag :code, "", required: true, class: "mt-1 focus:ring-blue-500 focus:border-blue-500 block shadow-sm sm:text-sm border-gray-300 rounded-md" %> + <%= text_field_tag :code, params[:code], required: true, class: "mt-1 focus:ring-blue-500 focus:border-blue-500 block shadow-sm sm:text-sm border-gray-300 rounded-md" %> - <%= number_field_tag :discount, nil, min: 1, max: 100, step: 0.01, id: "discount", required: true, class: "mt-1 focus:ring-blue-500 focus:border-blue-500 block shadow-sm sm:text-sm border-gray-300 rounded-md" %> + <%= number_field_tag :discount, params[:discount], min: 1, max: 100, step: 0.01, id: "discount", required: true, class: "mt-1 focus:ring-blue-500 focus:border-blue-500 block shadow-sm sm:text-sm border-gray-300 rounded-md" %> <% end %> diff --git a/rails_application/test/integration/coupons_test.rb b/rails_application/test/integration/coupons_test.rb index cf72d15e3..bdfa97ea4 100644 --- a/rails_application/test/integration/coupons_test.rb +++ b/rails_application/test/integration/coupons_test.rb @@ -23,6 +23,27 @@ def test_creation assert_select("td", "6.69") end + def test_creation_with_negative_discount_is_rejected + register_store("Test Store") + + get "/coupons/new" + coupon_id = css_select("input[name='coupon_id']").first["value"] + + post "/coupons", params: { + coupon_id: coupon_id, + name: "Bad Coupon", + code: "NEG", + discount: "-150" + } + + assert_response :unprocessable_entity + + get "/coupons" + assert_response :success + assert_select("td", text: "Bad Coupon", count: 0) + assert_select("td", text: "NEG", count: 0) + end + private def register_coupon(name, code, discount) From 98deae02fe83ebb59e6e9a10db5aee04fdfbf5e9 Mon Sep 17 00:00:00 2001 From: Kotmin <70173732+Kotmin@users.noreply.github.com> Date: Wed, 21 Jan 2026 23:10:39 +0100 Subject: [PATCH 2/2] convert coupons controller with CouponDiscount more vo approach --- ecommerce/pricing/lib/pricing.rb | 1 + ecommerce/pricing/lib/pricing/coupon.rb | 12 +++--- .../pricing/lib/pricing/coupon_discount.rb | 32 ++++++++++++++++ ecommerce/pricing/lib/pricing/offer.rb | 6 ++- .../pricing/test/coupon_discount_test.rb | 28 ++++++++++++++ .../app/controllers/coupons_controller.rb | 38 +++++++------------ 6 files changed, 85 insertions(+), 32 deletions(-) create mode 100644 ecommerce/pricing/lib/pricing/coupon_discount.rb create mode 100644 ecommerce/pricing/test/coupon_discount_test.rb diff --git a/ecommerce/pricing/lib/pricing.rb b/ecommerce/pricing/lib/pricing.rb index a3b54d075..0de775dcd 100644 --- a/ecommerce/pricing/lib/pricing.rb +++ b/ecommerce/pricing/lib/pricing.rb @@ -1,5 +1,6 @@ require "infra" require_relative "pricing/discounts" +require_relative "pricing/coupon_discount" require_relative "pricing/coupon" require_relative "pricing/commands" require_relative "pricing/events" diff --git a/ecommerce/pricing/lib/pricing/coupon.rb b/ecommerce/pricing/lib/pricing/coupon.rb index 22c343f0a..e1ea2d950 100644 --- a/ecommerce/pricing/lib/pricing/coupon.rb +++ b/ecommerce/pricing/lib/pricing/coupon.rb @@ -1,4 +1,5 @@ -require_relative 'events' +require_relative "events" +require_relative "coupon_discount" module Pricing class Coupon @@ -10,22 +11,23 @@ def initialize(id) @id = id end - def register(name, code, discount) + def register(name, code, discount_raw) raise AlreadyRegistered if @registered + discount = discount_raw.is_a?(CouponDiscount) ? discount_raw : CouponDiscount.parse(discount_raw) + apply CouponRegistered.new( data: { coupon_id: @id, name: name, code: code, - discount: discount + discount: discount.to_d } ) end - on CouponRegistered do |event| + on CouponRegistered do |_event| @registered = true end end end - diff --git a/ecommerce/pricing/lib/pricing/coupon_discount.rb b/ecommerce/pricing/lib/pricing/coupon_discount.rb new file mode 100644 index 000000000..6b48d9080 --- /dev/null +++ b/ecommerce/pricing/lib/pricing/coupon_discount.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "bigdecimal" +module Pricing + class CouponDiscount + UnacceptableRange = Class.new(StandardError) + Unparseable = Class.new(StandardError) + + def self.parse(raw) + new(raw) + rescue ArgumentError + raise Unparseable, "Discount must be a number" + end + + attr_reader :value + + def initialize(raw) + @value = BigDecimal(raw.to_s).freeze + + raise UnacceptableRange, "Discount must be greater than 0" if @value <= 0 + raise UnacceptableRange, "Discount must be less than or equal to 100" if @value > 100 + end + + def to_d + value + end + + def to_s + value.to_s("F") + end + end +end \ No newline at end of file diff --git a/ecommerce/pricing/lib/pricing/offer.rb b/ecommerce/pricing/lib/pricing/offer.rb index 85d048716..5c85d928c 100644 --- a/ecommerce/pricing/lib/pricing/offer.rb +++ b/ecommerce/pricing/lib/pricing/offer.rb @@ -92,12 +92,14 @@ def remove_free_product(order_id, product_id) ) end - def use_coupon(coupon_id, discount) + def use_coupon(coupon_id, discount_raw) + discount = discount_raw.is_a?(CouponDiscount) ? discount_raw : CouponDiscount.parse(discount_raw) + apply CouponUsed.new( data: { order_id: @id, coupon_id: coupon_id, - discount: discount + discount: discount.to_d } ) end diff --git a/ecommerce/pricing/test/coupon_discount_test.rb b/ecommerce/pricing/test/coupon_discount_test.rb new file mode 100644 index 000000000..cf538f204 --- /dev/null +++ b/ecommerce/pricing/test/coupon_discount_test.rb @@ -0,0 +1,28 @@ +require_relative "test_helper" + +module Pricing + class CouponDiscountTest < Test + cover "Pricing::CouponDiscount*" + + def test_rejects_zero + assert_raises(CouponDiscount::UnacceptableRange) { CouponDiscount.parse("0") } + end + + def test_rejects_negative + assert_raises(CouponDiscount::UnacceptableRange) { CouponDiscount.parse("-0.01") } + end + + def test_rejects_over_100 + assert_raises(CouponDiscount::UnacceptableRange) { CouponDiscount.parse("100.01") } + end + + def test_rejects_non_numeric + assert_raises(CouponDiscount::Unparseable) { CouponDiscount.parse("abc") } + end + + def test_accepts_string_decimal + discount = CouponDiscount.parse("0.01") + assert_equal(BigDecimal("0.01"), discount.to_d) + end + end +end \ No newline at end of file diff --git a/rails_application/app/controllers/coupons_controller.rb b/rails_application/app/controllers/coupons_controller.rb index 5abd37805..820f8e35a 100644 --- a/rails_application/app/controllers/coupons_controller.rb +++ b/rails_application/app/controllers/coupons_controller.rb @@ -9,29 +9,34 @@ def new def create @coupon_id = params[:coupon_id].presence || SecureRandom.uuid - - discount = validated_discount(params[:discount]) - return render_invalid_discount unless discount + + discount = Pricing::CouponDiscount.parse(params[:discount]) ActiveRecord::Base.transaction do create_coupon(@coupon_id, discount) end + rescue Pricing::CouponDiscount::UnacceptableRange, Pricing::CouponDiscount::Unparseable + flash.now[:alert] = "Discount must be greater than 0 and less than or equal to 100" + render "new", status: :unprocessable_entity rescue Pricing::Coupon::AlreadyRegistered - flash[:notice] = "Coupon is already registered" - render "new" + flash.now[:alert] = "Coupon is already registered" + render "new", status: :unprocessable_entity + rescue Infra::Command::Invalid + flash.now[:alert] = "Invalid coupon data" + render "new", status: :unprocessable_entity else redirect_to coupons_path, notice: "Coupon was successfully created" end private - def create_coupon(coupon_id,discount) + def create_coupon(coupon_id, discount) command_bus.( Pricing::RegisterCoupon.new( coupon_id: coupon_id, name: params[:name], code: params[:code], - discount: discount + discount: discount.to_d ) ) command_bus.( @@ -41,21 +46,4 @@ def create_coupon(coupon_id,discount) ) ) end - - def validated_discount(raw) - return nil if raw.blank? - - value = BigDecimal(raw.to_s) - return nil unless value > 0 && value <= 100 - - value - rescue ArgumentError - nil - end - - def render_invalid_discount - flash.now[:alert] = "Discount must be greater than 0 and less than or equal to 100" - render "new", status: :unprocessable_entity - end - -end +end \ No newline at end of file