From b22cc4c99737e64b30d6f3bfc9d450da2404b2d3 Mon Sep 17 00:00:00 2001 From: Juerd Waalboer Date: Thu, 26 Dec 2024 01:36:55 +0100 Subject: [PATCH] Move price calculation from `products` plugin to RevBank::Products - Adds price tag calculation. Addons tagged #OPAQUE are excluded from the price tag. - BREAKING CHANGE: instead of abusing $product->{price} for a percent, $product->{percent} is no longer a boolean but the actual percent, so $product->{price} is the calculated amount. The total price of a product is now calculated in two places, once when reading the product list, and once as the result of adding the entry and its contras when adding the product. Although this involves some duplication and the sums are calculated in different ways, it hinges on the existing assertion to make sure that the entry is balanced to ensure that both sums are the same. Because of that, this code duplication actually strengthens the integrity. --- lib/RevBank/Products.pm | 51 +++++++++++++++++++++++++++++++++++++---- plugins/products | 33 ++++++++------------------ 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/lib/RevBank/Products.pm b/lib/RevBank/Products.pm index 2adab62..7dcafb1 100644 --- a/lib/RevBank/Products.pm +++ b/lib/RevBank/Products.pm @@ -87,9 +87,10 @@ sub read_products($filename = "revbank.products", $default_contra = "+sales/prod warn "Percentage invalid for non-addon at $filename line $linenr.\n"; next; } - $price = 0 + $price; + $percent = $sign * (0 + $price); + $price = undef; # calculated later } else { - $price = eval { parse_amount($price) }; + $price = $sign * eval { parse_amount($price) }; if (not defined $price) { warn "Invalid price for '$ids[0]' at $filename line $linenr.\n"; next; @@ -100,18 +101,25 @@ sub read_products($filename = "revbank.products", $default_contra = "+sales/prod $products{$id} = { id => $ids[0], - price => $sign * $price, - percent => $percent, description => $desc, contra => $contra || $default_contra, _addon_ids => \@addon_ids, line => $linenr, tags => \%tags, config => $canonical, + + percent => $percent, + price => $price, # base price + + # The following are calculated below, for top-level products only: + # tag_price => base price + sum of transparent addons + # hidden_fees => sum of opaque addons + # total_price => tag_price + hidden_fees }; } } + # Resolve addons PRODUCT: for my $product (values %products) { my %ids_seen = ($product->{id} => 1); my @addon_ids = @{ $product->{_addon_ids} }; @@ -124,7 +132,7 @@ sub read_products($filename = "revbank.products", $default_contra = "+sales/prod next PRODUCT; } - my $addon = $products{$addon_id}; + my $addon = { %{ $products{$addon_id} } }; # shallow copy to overwrite ->{price} later if (not $addon) { warn "Addon '$addon_id' does not exist for '$product->{id}' at $filename line $product->{line}.\n"; next PRODUCT; @@ -135,6 +143,39 @@ sub read_products($filename = "revbank.products", $default_contra = "+sales/prod } } + # Calculate tag and total price + PRODUCT: for my $product (values %products) { + next if $product->{id} =~ /^\+/; + + my $tag_price = $product->{price} || 0; + my $hidden = 0; + + my @seen = ($product); + for my $addon (@{ $product->{addons} }) { + if ($addon->{percent}) { + my $sum = List::Util::sum map { + $_->{price} + } grep { + $_->{contra} eq $addon->{contra} + } @seen; + + $addon->{price} = $addon->{percent} / 100 * $sum; + } + + if ($addon->{tags}{OPAQUE}) { + $hidden += $addon->{price}; + } else { + $tag_price += $addon->{price}; + } + + push @seen, $addon; + } + + $product->{tag_price} = $tag_price; + $product->{hidden_fees} = $hidden; + $product->{total_price} = $tag_price + $hidden; + } + return $cache{$filename} = \%products; } diff --git a/plugins/products b/plugins/products index f02b253..5b0044e 100644 --- a/plugins/products +++ b/plugins/products @@ -8,19 +8,12 @@ sub command :Tab(&tab) ($self, $cart, $command, @) { $command =~ /^\+/ and return NEXT; my $products = read_products; + my $product = $products->{ $command } or return NEXT; my $price = $product->{price}; - my $contra_desc = "\$you bought $product->{description}"; - - my @addons = @{ $product->{addons} // [] }; - - my $display = undef; - $display = "Product" if @addons and $price->cents > 0; - $display = "Reimbursement" if @addons and $price->cents < 0; - my $entry = $cart->add( - -$price, + -$product->{total_price}, $product->{description}, { product_id => $product->{id}, @@ -29,6 +22,13 @@ sub command :Tab(&tab) ($self, $cart, $command, @) { deduplicate => join("/", $self->id, $product->{id}), } ); + + my $contra_desc = "\$you bought $product->{description}"; + my @addons = @{ $product->{addons} // [] }; + my $display = undef; + $display = "Product" if @addons and $price->cents > 0; + $display = "Reimbursement" if @addons and $price->cents < 0; + $entry->add_contra( $product->{contra}, +$price, @@ -37,22 +37,9 @@ sub command :Tab(&tab) ($self, $cart, $command, @) { ); for my $addon (@addons) { - my $addon_price = $addon->{price}; - if ($addon->{percent}) { - my $sum = List::Util::sum map { - $_->{amount} - } grep { - $_->{user} eq $addon->{contra} - } $entry->contras; - - $addon_price = $addon_price / 100 * $sum; - } - - $entry->amount( $entry->amount - $addon_price ); - $entry->add_contra( $addon->{contra}, - $addon_price, + $addon->{price}, "$addon->{description} ($contra_desc)", $addon->{description} );