From d4c6c1be355102c4788bdd6969bd17052b3bb02f Mon Sep 17 00:00:00 2001 From: Juerd Waalboer Date: Thu, 5 Jan 2023 20:46:46 +0100 Subject: [PATCH] Replace add_info() with extra parameter for add_contra() add_info was a thing that grew organically to account for hidden contras, but just wasn't right. The assumption was that if the contra account is hidden, the contra itself should be hidden from view - the sign of the amount would be wrong anyway. The correct approach, however, would of course to flip the sign so it matches the user's perspective, and to add a separate description string to display to the user. --- lib/RevBank/Cart/Entry.pm | 49 +++++++++++++++++++-------------------- plugins/products | 22 ++++++++---------- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/lib/RevBank/Cart/Entry.pm b/lib/RevBank/Cart/Entry.pm index a1bb822..3b058ab 100644 --- a/lib/RevBank/Cart/Entry.pm +++ b/lib/RevBank/Cart/Entry.pm @@ -18,7 +18,7 @@ sub new($class, $amount, $description, $attributes = {}) { description => $description, attributes => { %$attributes }, user => undef, - contras => [], # infos + contras + contras => [], caller => List::Util::first(sub { !/^RevBank::Cart/ }, map { (caller $_)[3] } 1..10) || (caller 1)[3], }; @@ -26,7 +26,10 @@ sub new($class, $amount, $description, $attributes = {}) { return bless $self, $class; } -sub add_contra($self, $user, $amount, $description) { +sub add_contra($self, $user, $amount, $description, $display = undef) { + # $display should be given for either ALL or NONE of the contras, + # with the exception of contras with $amount == 0.00; + $amount = RevBank::Amount->parse_string($amount) if not ref $amount; $user = RevBank::Users::assert_user($user); @@ -35,23 +38,8 @@ sub add_contra($self, $user, $amount, $description) { push @{ $self->{contras} }, { user => $user, amount => $amount, # should usually have opposite sign (+/-) - description => $description, - }; - - $self->attribute('changed', 1); - - return $self; # for method chaining -} - -sub add_info($self, $amount, $description) { - $amount = RevBank::Amount->parse_string($amount) if not ref $amount; - - $description =~ s/\$you/$self->{user}/g if defined $self->{user}; - - push @{ $self->{contras} }, { - user => undef, - amount => $amount, # should usually have SAME sign (+/-) - description => $description, + description => $description, # contra user's perspective + display => $display, # interactive user's perspective }; $self->attribute('changed', 1); @@ -100,7 +88,7 @@ sub multiplied($self) { sub contras($self) { # Shallow copy suffices for now, because there is no depth. - return map +{ %$_ }, grep defined $_->{user}, @{ $self->{contras} }; + return map +{ %$_ }, @{ $self->{contras} }; } sub as_printable($self) { @@ -114,11 +102,23 @@ sub as_printable($self) { for my $c (@{ $self->{contras} }) { my $description; - if (defined $c->{user}) { - next if RevBank::Users::is_hidden($c->{user}) and not $ENV{REVBANK_DEBUG}; - $description = join " ", ($c->{amount}->cents > 0 ? "->" : "<-"), $c->{user}; + my $amount = $self->{amount}; + my $hidden = RevBank::Users::is_hidden($c->{user}); + my $fromto = $c->{amount}->cents < 0 ? "<-" : "->"; + $fromto .= " $c->{user}"; + + if ($c->{display}) { + $description = + $hidden + ? ($ENV{REVBANK_DEBUG} ? "($fromto:) $c->{display}" : $c->{display}) + : "$fromto: $c->{display}"; + + $amount *= -1; + } elsif ($hidden) { + next unless $ENV{REVBANK_DEBUG}; + $description = "($fromto: $c->{description})"; } else { - $description = $c->{description}; + $description = $fromto; } push @s, sprintf( "%11s %s", @@ -139,7 +139,6 @@ sub as_loggable($self) { my @s; for ($self, @{ $self->{contras} }) { - next if not defined $_->{user}; my $total = $quantity * $_->{amount}; my $description = diff --git a/plugins/products b/plugins/products index 2ab6256..3ebd172 100644 --- a/plugins/products +++ b/plugins/products @@ -77,6 +77,12 @@ sub command :Tab(&tab) ($self, $cart, $command, @) { 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->{description}, @@ -85,16 +91,10 @@ sub command :Tab(&tab) ($self, $cart, $command, @) { $entry->add_contra( $product->{contra}, +$price, - $contra_desc + $contra_desc, + $display ); - my @addons = @{ $product->{addons} } - or return ACCEPT; - - - $entry->add_info($price, "Product") if $price->cents > 0; - $entry->add_info($price, "Reimbursement") if $price->cents < 0; - my %ids_seen = ($product->{id} => 1); while (my $addon_id = shift @addons) { @@ -120,13 +120,11 @@ sub command :Tab(&tab) ($self, $cart, $command, @) { $entry->amount( $entry->amount - $addon_price ); - $entry->add_info($addon_price, $addon->{description}) - if $addon->{contra} =~ /^[-+]/; - $entry->add_contra( $addon->{contra}, $addon_price, - "$addon->{description} ($contra_desc)" + "$addon->{description} ($contra_desc)", + $addon->{description} ); push @addons, @{ $addon->{addons} };