diff --git a/CHANGELOG.md b/CHANGELOG.md index fa7f0d20a..b4c240aca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ - Fix a bug: portuguese time formatting (#405) - Fix a bug: admin users groups being overriden by SSO group_id (#404) - Fix a bug: no statistics on trainings and spaces reservations +- Fix a bug: invalid ventilation for amount coupons +- Fix a bug: invalid VAT for invoices using amount coupons +- Fix a bug: invalid 1 cent rounding for invoices using coupons - Fix a security issue: updated nokogiri to 1.13.9 to fix [GHSA-2qc6-mcvw-92cw](https://github.com/advisories/GHSA-2qc6-mcvw-92cw) - [TODO DEPLOY] `rails fablab:maintenance:regenerate_statistics[2021,6]` - [TODO DEPLOY] `rails fablab:setup:set_admins_group` diff --git a/app/models/cart_item/machine_reservation.rb b/app/models/cart_item/machine_reservation.rb index ef692fe13..e456b37bd 100644 --- a/app/models/cart_item/machine_reservation.rb +++ b/app/models/cart_item/machine_reservation.rb @@ -47,6 +47,6 @@ class CartItem::MachineReservation < CartItem::Reservation return 0 if @plan.nil? machine_credit = @plan.machine_credits.find { |credit| credit.creditable_id == @reservable.id } - credits_hours(machine_credit, @new_subscription) + credits_hours(machine_credit, new_plan_being_bought: @new_subscription) end end diff --git a/app/models/cart_item/reservation.rb b/app/models/cart_item/reservation.rb index 5bb11af3d..2c86364ee 100644 --- a/app/models/cart_item/reservation.rb +++ b/app/models/cart_item/reservation.rb @@ -23,7 +23,7 @@ class CartItem::Reservation < CartItem::BaseItem amount = 0 hours_available = credits - grouped_slots.values.each do |slots| + grouped_slots.each_value do |slots| prices = applicable_prices(slots) slots.each_with_index do |slot, index| amount += get_slot_price_from_prices(prices, slot, is_privileged, @@ -100,7 +100,7 @@ class CartItem::Reservation < CartItem::BaseItem slot_minutes = (slot[:slot_attributes][:end_at].to_time - slot[:slot_attributes][:start_at].to_time) / SECONDS_PER_MINUTE price = prices[:prices].find { |p| p[:duration] <= slot_minutes && p[:duration].positive? } price = prices[:prices].first if price.nil? - hourly_rate = (price[:price].amount.to_f / price[:price].duration) * MINUTES_PER_HOUR + hourly_rate = ((Rational(price[:price].amount.to_f) / Rational(price[:price].duration)) * Rational(MINUTES_PER_HOUR)).to_f # apply the base price to the real slot duration real_price = get_slot_price(hourly_rate, slot, is_privileged, options) @@ -130,7 +130,7 @@ class CartItem::Reservation < CartItem::BaseItem slot_minutes = (slot[:slot_attributes][:end_at].to_time - slot[:slot_attributes][:start_at].to_time) / SECONDS_PER_MINUTE # apply the base price to the real slot duration real_price = if options[:is_division] - (slot_rate / MINUTES_PER_HOUR) * slot_minutes + ((Rational(slot_rate) / Rational(MINUTES_PER_HOUR)) * Rational(slot_minutes)).to_f else slot_rate end @@ -138,7 +138,7 @@ class CartItem::Reservation < CartItem::BaseItem if real_price.positive? && options[:prepaid][:minutes]&.positive? consumed = slot_minutes consumed = options[:prepaid][:minutes] if slot_minutes > options[:prepaid][:minutes] - real_price = (slot_minutes - consumed) * (slot_rate / MINUTES_PER_HOUR) + real_price = (Rational(slot_minutes - consumed) * (Rational(slot_rate) / Rational(MINUTES_PER_HOUR))).to_f options[:prepaid][:minutes] -= consumed end @@ -158,7 +158,9 @@ class CartItem::Reservation < CartItem::BaseItem # and the base price (1 hours), we use the 7 hours price, then 3 hours price, and finally the base price twice (7+3+1+1 = 12). # All these prices are returned to be applied to the reservation. def applicable_prices(slots) - total_duration = slots.map { |slot| (slot[:slot_attributes][:end_at].to_time - slot[:slot_attributes][:start_at].to_time) / SECONDS_PER_MINUTE }.reduce(:+) + total_duration = slots.map do |slot| + (slot[:slot_attributes][:end_at].to_time - slot[:slot_attributes][:start_at].to_time) / SECONDS_PER_MINUTE + end.reduce(:+) rates = { prices: [] } remaining_duration = total_duration @@ -182,7 +184,7 @@ class CartItem::Reservation < CartItem::BaseItem ## # Compute the number of remaining hours in the users current credits (for machine or space) ## - def credits_hours(credits, new_plan_being_bought = false) + def credits_hours(credits, new_plan_being_bought: false) return 0 unless credits hours_available = credits.hours diff --git a/app/models/cart_item/space_reservation.rb b/app/models/cart_item/space_reservation.rb index 36a0c36cd..e8463b8da 100644 --- a/app/models/cart_item/space_reservation.rb +++ b/app/models/cart_item/space_reservation.rb @@ -41,6 +41,6 @@ class CartItem::SpaceReservation < CartItem::Reservation return 0 if @plan.nil? space_credit = @plan.space_credits.find { |credit| credit.creditable_id == @reservable.id } - credits_hours(space_credit, @new_subscription) + credits_hours(space_credit, new_plan_being_bought: @new_subscription) end end diff --git a/app/models/invoice_item.rb b/app/models/invoice_item.rb index 193dbdec1..526f0e3be 100644 --- a/app/models/invoice_item.rb +++ b/app/models/invoice_item.rb @@ -4,8 +4,8 @@ class InvoiceItem < Footprintable belongs_to :invoice - has_one :invoice_item # associates invoice_items of an invoice to invoice_items of an Avoir - has_one :payment_gateway_object, as: :item + has_one :invoice_item, dependent: :destroy # associates invoice_items of an invoice to invoice_items of an Avoir + has_one :payment_gateway_object, as: :item, dependent: :destroy belongs_to :object, polymorphic: true @@ -15,7 +15,8 @@ class InvoiceItem < Footprintable def amount_after_coupon # deduct coupon discount coupon_service = CouponService.new - coupon_service.ventilate(invoice.total, amount, invoice.coupon) + total_without_coupon = coupon_service.invoice_total_no_coupon(invoice) + coupon_service.ventilate(total_without_coupon, amount, invoice.coupon) end # return the item amount, coupon discount deducted, if any, and VAT excluded, if applicable diff --git a/app/services/coupon_service.rb b/app/services/coupon_service.rb index e849c1d57..364f928bb 100644 --- a/app/services/coupon_service.rb +++ b/app/services/coupon_service.rb @@ -26,11 +26,14 @@ class CouponService return price if coupon_object.nil? if coupon_object.status(user_id, total) == 'active' - if coupon_object.type == 'percent_off' - price -= (price * coupon_object.percent_off / 100.00).truncate - elsif coupon_object.type == 'amount_off' + case coupon_object.type + when 'percent_off' + price -= (Rational(price * coupon_object.percent_off) / Rational(100.0)).to_f.ceil + when 'amount_off' # do not apply cash coupon unless it has a lower amount that the total price price -= coupon_object.amount_off if coupon_object.amount_off <= price + else + raise InvalidCouponError("unsupported coupon type #{coupon_object.type}") end end @@ -61,14 +64,15 @@ class CouponService def ventilate(total, amount, coupon) price = amount if !coupon.nil? && total != 0 - if coupon.type == 'percent_off' - price = amount - (amount * coupon.percent_off / 100.00) - elsif coupon.type == 'amount_off' - ratio = (coupon.amount_off / 100.00) / total - discount = (amount * ratio.abs) * 100 - price = amount - discount + case coupon.type + when 'percent_off' + price = amount - (Rational(amount * coupon.percent_off) / Rational(100.00)).to_f.round + when 'amount_off' + ratio = Rational(amount) / Rational(total) + discount = (coupon.amount_off * ratio.abs) + price = (amount - discount).to_f.round else - raise InvalidCouponError + raise InvalidCouponError("unsupported coupon type #{coupon.type}") end end price diff --git a/app/services/vat_history_service.rb b/app/services/vat_history_service.rb index 479c96dcc..d4685323b 100644 --- a/app/services/vat_history_service.rb +++ b/app/services/vat_history_service.rb @@ -37,7 +37,9 @@ class VatHistoryService private - def vat_history(vat_rate_type) + # This method is really complex and cannot be simplified using the current data model + # As a futur improvement, we should save the VAT rate for each invoice_item in the DB + def vat_history(vat_rate_type) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity chronology = [] end_date = DateTime.current Setting.find_by(name: 'invoice_VAT-active').history_values.order(created_at: 'DESC').each do |v| @@ -92,7 +94,7 @@ class VatHistoryService # when the VAT rate was enabled, set the date it was enabled and the rate range = chronology.find { |p| rate.created_at.to_i.between?(p[:start].to_i, p[:end].to_i) } date = range[:enabled] ? rate.created_at : range[:end] - date_rates.push(date: date, rate: rate.value.to_i) unless date_rates.find { |d| d[:date] == date } + date_rates.push(date: date, rate: rate.value.to_f) unless date_rates.find { |d| d[:date] == date } end chronology.reverse_each do |period| # when the VAT rate was disabled, set the date it was disabled and rate=0 diff --git a/test/fixtures/plans.yml b/test/fixtures/plans.yml index e6de5e3cb..254ef45b5 100644 --- a/test/fixtures/plans.yml +++ b/test/fixtures/plans.yml @@ -51,7 +51,7 @@ plan_3: type: Plan base_name: Mensuel tarif réduit ui_weight: 0 - interval_count: 1* + interval_count: 1 slug: mensuel-tarif-reduit plan_schedulable: @@ -72,3 +72,21 @@ plan_schedulable: interval_count: 1 monthly_payment: true slug: abonnement-mensualisable + +plan_5: + id: 5 + name: Abonnement crazy fabbers + amount: 1498 + interval: month + group_id: 2 + stp_plan_id: mensuel-tarif-reduit-student-month-20160404171827 + created_at: 2022-10-26 17:08:21.681615000 Z + updated_at: 2022-10-26 17:08:21.681615000 Z + training_credit_nb: 1 + is_rolling: + description: + type: Plan + base_name: Crazy fabbers + ui_weight: 1 + interval_count: 2 + slug: crazy-fabbers diff --git a/test/fixtures/prices.yml b/test/fixtures/prices.yml index 78781b2d6..79bff0440 100644 --- a/test/fixtures/prices.yml +++ b/test/fixtures/prices.yml @@ -460,3 +460,80 @@ price_54: duration: 60 created_at: 2016-04-04 15:18:28.860220000 Z updated_at: 2016-04-04 15:18:50.517702000 Z + +price_55: + id: 55 + group_id: 2 + plan_id: 5 + priceable_id: 1 + priceable_type: Machine + amount: 121 + duration: 60 + created_at: 2022-10-26 17:08:21.681615000 Z + updated_at: 2022-10-26 17:08:21.681615000 Z + +price_56: + id: 56 + group_id: 2 + plan_id: 5 + priceable_id: 2 + priceable_type: Machine + amount: 1116 + duration: 60 + created_at: 2022-10-26 17:08:21.681615000 Z + updated_at: 2022-10-26 17:08:21.681615000 Z + +price_57: + id: 57 + group_id: 2 + plan_id: 5 + priceable_id: 3 + priceable_type: Machine + amount: 1423 + duration: 60 + created_at: 2022-10-26 17:08:21.681615000 Z + updated_at: 2022-10-26 17:08:21.681615000 Z + +price_58: + id: 58 + group_id: 2 + plan_id: 5 + priceable_id: 4 + priceable_type: Machine + amount: 1238 + duration: 60 + created_at: 2022-10-26 17:08:21.681615000 Z + updated_at: 2022-10-26 17:08:21.681615000 Z + +price_59: + id: 59 + group_id: 2 + plan_id: 5 + priceable_id: 5 + priceable_type: Machine + amount: 724 + duration: 60 + created_at: 2022-10-26 17:08:21.681615000 Z + updated_at: 2022-10-26 17:08:21.681615000 Z + +price_60: + id: 60 + group_id: 2 + plan_id: 5 + priceable_id: 6 + priceable_type: Machine + amount: 666 + duration: 60 + created_at: 2022-10-26 17:08:21.681615000 Z + updated_at: 2022-10-26 17:08:21.681615000 Z + +price_61: + id: 61 + group_id: 2 + plan_id: 5 + priceable_id: 1 + priceable_type: Space + amount: 1373 + duration: 60 + created_at: 2022-10-26 17:08:21.681615000 Z + updated_at: 2022-10-26 17:08:21.681615000 Z diff --git a/test/integration/invoices/round_test.rb b/test/integration/invoices/round_test.rb new file mode 100644 index 000000000..d8ca83e76 --- /dev/null +++ b/test/integration/invoices/round_test.rb @@ -0,0 +1,265 @@ +# frozen_string_literal: true + +require 'test_helper' + +module Invoices; end + +class Invoices::RoundTest < ActionDispatch::IntegrationTest + def setup + @vlonchamp = User.find_by(username: 'vlonchamp') + @admin = User.find_by(username: 'admin') + login_as(@admin, scope: :user) + end + + test 'invoice using percent coupon rounded up' do + machine = Machine.first + availability = machine.availabilities.first + plan = Plan.find(5) + + # enable the VAT + Setting.set('invoice_VAT-active', true) + Setting.set('invoice_VAT-rate', 20) + + post '/api/local_payment/confirm_payment', params: { + customer_id: @vlonchamp.id, + coupon_code: 'REDUC20', + items: [ + { + reservation: { + reservable_id: machine.id, + reservable_type: machine.class.name, + slots_reservations_attributes: [ + { + slot_id: availability.slots.first.id + } + ] + } + }, + { + subscription: { + plan_id: plan.id + } + } + ] + }.to_json, headers: default_headers + + # Check response format & status + assert_equal 201, response.status, response.body + assert_equal Mime[:json], response.content_type + + # in the invoice, we should have: + # - machine reservation = 121 (97, coupon applied) + # - subscription = 1498 (1198, coupon applied) + ### intermediate total = 1619 + # - coupon (20%) = -323.8 => round to 324 + ### total incl. taxes = 1295 + # - vat = 216 + # - total exct. taxes = 1079 + ### amount paid = 1295 + + invoice = Invoice.last + assert_equal 121, invoice.invoice_items.first.amount + assert_equal 1498, invoice.invoice_items.last.amount + assert_equal 1295, invoice.total + + coupon_service = CouponService.new + total_without_coupon = coupon_service.invoice_total_no_coupon(invoice) + assert_equal 97, coupon_service.ventilate(total_without_coupon, invoice.invoice_items.first.amount, invoice.coupon) + assert_equal 1198, coupon_service.ventilate(total_without_coupon, invoice.invoice_items.last.amount, invoice.coupon) + assert_equal 324, total_without_coupon - invoice.total + + vat_service = VatHistoryService.new + vat_rate_groups = vat_service.invoice_vat(invoice) + assert_equal 216, vat_rate_groups.values.pluck(:total_vat).reduce(:+) + assert_equal 1079, invoice.invoice_items.map(&:net_amount).reduce(:+) + end + + test 'invoice using percent coupon rounded down' do + machine = Machine.find(3) + availability = machine.availabilities.first + plan = Plan.find(5) + + # enable the VAT + Setting.set('invoice_VAT-active', true) + Setting.set('invoice_VAT-rate', 20) + + post '/api/local_payment/confirm_payment', params: { + customer_id: @vlonchamp.id, + coupon_code: 'REDUC20', + items: [ + { + reservation: { + reservable_id: machine.id, + reservable_type: machine.class.name, + slots_reservations_attributes: [ + { + slot_id: availability.slots.first.id + } + ] + } + }, + { + subscription: { + plan_id: plan.id + } + } + ] + }.to_json, headers: default_headers + + # Check response format & status + assert_equal 201, response.status, response.body + assert_equal Mime[:json], response.content_type + + # in the invoice, we should have: + # - machine reservation = 1423 (1138, coupon applied) + # - subscription = 1498 (1198, coupon applied) + ### intermediate total = 2921 + # - coupon (20%) = -584.2 => round to 585 + ### total incl. taxes = 2336 + # - vat = 390 + # - total exct. taxes = 1946 + ### amount paid = 2336 + + invoice = Invoice.last + assert_equal 1423, invoice.invoice_items.first.amount + assert_equal 1498, invoice.invoice_items.last.amount + assert_equal 2336, invoice.total + + coupon_service = CouponService.new + total_without_coupon = coupon_service.invoice_total_no_coupon(invoice) + assert_equal 1138, coupon_service.ventilate(total_without_coupon, invoice.invoice_items.first.amount, invoice.coupon) + assert_equal 1198, coupon_service.ventilate(total_without_coupon, invoice.invoice_items.last.amount, invoice.coupon) + assert_equal 585, total_without_coupon - invoice.total + + vat_service = VatHistoryService.new + vat_rate_groups = vat_service.invoice_vat(invoice) + assert_equal 390, vat_rate_groups.values.pluck(:total_vat).reduce(:+) + assert_equal 1946, invoice.invoice_items.map(&:net_amount).reduce(:+) + end + + test 'invoice using amount coupon rounded up' do + machine = Machine.first + availability = machine.availabilities.first + plan = Plan.find(5) + + # enable the VAT + Setting.set('invoice_VAT-active', true) + Setting.set('invoice_VAT-rate', 19.6) + + post '/api/local_payment/confirm_payment', params: { + customer_id: @vlonchamp.id, + coupon_code: 'GIME3EUR', + items: [ + { + reservation: { + reservable_id: machine.id, + reservable_type: machine.class.name, + slots_reservations_attributes: [ + { + slot_id: availability.slots.first.id + } + ] + } + }, + { + subscription: { + plan_id: plan.id + } + } + ] + }.to_json, headers: default_headers + + # Check response format & status + assert_equal 201, response.status, response.body + assert_equal Mime[:json], response.content_type + + # in the invoice, we should have: + # - machine reservation = 121 (99, coupon applied) + # - subscription = 1498 (1220, coupon applied) + ### intermediate total = 1619 + # - coupon (20%) = -300 + ### total incl. taxes = 1319 + # - vat = 216 + # - total exct. taxes = 1103 + ### amount paid = 1319 + + invoice = Invoice.last + assert_equal 121, invoice.invoice_items.first.amount + assert_equal 1498, invoice.invoice_items.last.amount + assert_equal 1319, invoice.total + + coupon_service = CouponService.new + total_without_coupon = coupon_service.invoice_total_no_coupon(invoice) + assert_equal 99, coupon_service.ventilate(total_without_coupon, invoice.invoice_items.first.amount, invoice.coupon) + assert_equal 1220, coupon_service.ventilate(total_without_coupon, invoice.invoice_items.last.amount, invoice.coupon) + assert_equal 300, total_without_coupon - invoice.total + + vat_service = VatHistoryService.new + vat_rate_groups = vat_service.invoice_vat(invoice) + assert_equal 216, vat_rate_groups.values.pluck(:total_vat).reduce(:+) + assert_equal 1103, invoice.invoice_items.map(&:net_amount).reduce(:+) + end + + test 'invoice using amount coupon rounded down' do + machine = Machine.find(3) + availability = machine.availabilities.first + plan = Plan.find(5) + + # enable the VAT + Setting.set('invoice_VAT-active', true) + Setting.set('invoice_VAT-rate', 20) + + post '/api/local_payment/confirm_payment', params: { + customer_id: @vlonchamp.id, + coupon_code: 'GIME3EUR', + items: [ + { + reservation: { + reservable_id: machine.id, + reservable_type: machine.class.name, + slots_reservations_attributes: [ + { + slot_id: availability.slots.first.id + } + ] + } + }, + { + subscription: { + plan_id: plan.id + } + } + ] + }.to_json, headers: default_headers + + # Check response format & status + assert_equal 201, response.status, response.body + assert_equal Mime[:json], response.content_type + + # in the invoice, we should have: + # - machine reservation = 1423 (1277, coupon applied) + # - subscription = 1498 (1344, coupon applied) + ### intermediate total = 2921 + # - coupon (20%) = -300 + ### total incl. taxes = 2621 + # - vat = 430 + # - total exct. taxes = 2191 + ### amount paid = 2621 + + invoice = Invoice.last + assert_equal 1423, invoice.invoice_items.first.amount + assert_equal 1498, invoice.invoice_items.last.amount + assert_equal 2621, invoice.total + + coupon_service = CouponService.new + total_without_coupon = coupon_service.invoice_total_no_coupon(invoice) + assert_equal 1277, coupon_service.ventilate(total_without_coupon, invoice.invoice_items.first.amount, invoice.coupon) + assert_equal 1344, coupon_service.ventilate(total_without_coupon, invoice.invoice_items.last.amount, invoice.coupon) + assert_equal 300, total_without_coupon - invoice.total + + vat_service = VatHistoryService.new + vat_rate_groups = vat_service.invoice_vat(invoice) + assert_equal 437, vat_rate_groups.values.pluck(:total_vat).reduce(:+) + assert_equal 2184, invoice.invoice_items.map(&:net_amount).reduce(:+) + end +end