Fix deduplication bug, refactor deduplication to own plugin

(Bumps version to 3.8 because admins should update the plugin list.)

Deduplication didn't work on quantified additions, i.e. if you added
"20x clubmate" when there was already clubmate in the cart, it would add
just ONE item, and have a lingering message that the next thing would be
multiplied by 20.

This old bug was especially annoying if there is a barcode "20x
clubmate" to scan 20 bottles (which is the size of a crate), and this is
repeated.

The fix also uncovered another bug: newly added entries were selected
too early. There are two hooks, hook_add_entry and hook_added_entry, and
of course the selection should happen in between, not before the former.
No entry in UPGRADING.md, because I think it is extremely unlikely that
any plugin author will have used the selection feature yet, which is
very new.

UPGRADING.md

@@ -1,3 +1,11 @@
+# (2023-02-12) RevBank 3.8
+
+## Update your `revbank.plugins`
+
+Deduplication is moved from individual plugins to a plugin that does that. If
+you want to keep deduplication of cart items, and you probably do want that,
+add `deduplicate` to `revbank.plugins` just below `repeat`.
+
 # (2022-12-25) RevBank 3.6
 
 ## Update your `revbank.plugins`

lib/RevBank/Cart.pm

@@ -21,6 +21,8 @@ sub add_entry($self, $entry) {
 
     push @{ $self->{entries} }, $entry;
     $self->{changed}++;
+    $self->select($entry);
+
     RevBank::Plugins::call_hooks("added_entry", $self, $entry);
 
     return $entry;
@@ -35,9 +37,7 @@ sub add($self, $amount, $description, $data = {}) {
     # ->add($user, ...)  => use $cart->add(...)->add_contra($user, ...)
     # ->add($entry)      => use $cart->add_entry($entry)
 
-    my $entry = $self->add_entry(RevBank::Cart::Entry->new($amount, $description, $data));
-    $self->select($entry);
-    return $entry;
+    return $self->add_entry(RevBank::Cart::Entry->new($amount, $description, $data));
 }
 
 sub select($self, $entry) {

plugins/deduplicate

@@ -0,0 +1,31 @@
+#!perl
+
+# Deduplication merges duplicate entries in the cart, e.g.
+# 3x cola + 4x cola = 7x cola.
+#
+# Plugins that support this, set the "deduplicate" attribute to a string key
+# that is used to determine which entries are equal. It is the responsibility
+# of the plugin that sets this, to ensure that the entries are indeed exactly
+# the same, if their deduplicate keys are equal.
+#
+# The recommended value for the deduplicate attribute is join("/", $plugin_id,
+# $unique_id), where $plugin_id can be obtained from $self->id in interactive
+# methods or $class->id in hooks. Including the plugin id avoids deduplicating
+# across plugins, that are probably not aware of eachothers $unique_id's.
+
+use List::Util qw(sum any);
+
+sub hook_added_entry($class, $cart, $added_entry, @) {
+    my $key = $added_entry->attribute('deduplicate') or return;
+
+    my @dedupe = grep {
+        $_->attribute('deduplicate') eq $key
+    } $cart->entries('deduplicate');
+
+    @dedupe >= 2 or return;
+
+    $dedupe[0]->quantity(sum map { $_->quantity } @dedupe);
+    $cart->select($dedupe[0]);
+
+    $cart->delete($_) for @dedupe[1 .. $#dedupe];
+}

plugins/market

@@ -35,21 +35,14 @@ sub command :Tab(market,&tab) ($self, $cart, $command, @) {
     my $space    = parse_amount($product->{ space })  or return NEXT;
     my $description = $product->{description};
 
-    my @existing = grep {
-        $_->attribute('plugin') eq $self->id and
-        $_->attribute('product_id') eq $command
-    } $cart->entries('plugin');
-
-    if (@existing) {
-        $existing[0]->quantity($existing[0]->quantity + 1);
-        $cart->select($existing[0]);
-        return ACCEPT;
-    }
-
     $cart->add(
         -($seller + $space),
         "$description (sold by $username)",
-        { product_id => $command, plugin => $self->id }
+        {
+            product_id => $command,
+            plugin => $self->id,
+            deduplicate => join("/", $self->id, $command),
+        }
     )->add_contra(
         $username,
         $seller,

plugins/products

@@ -94,17 +94,6 @@ sub command :Tab(&tab) ($self, $cart, $command, @) {
     my $product = $products->{ $command } or return NEXT;
     my $price = $product->{price};
 
-    my @existing = grep {
-        $_->attribute('plugin') eq $self->id and
-        $_->attribute('product_id') eq $product->{id}
-    } $cart->entries('plugin');
-
-    if (@existing) {
-        $existing[0]->quantity($existing[0]->quantity + 1);
-        $cart->select($existing[0]);
-        return ACCEPT;
-    }
-
     my $contra_desc = "\$you bought $product->{description}";
 
     my @addons = @{ $product->{addons} // [] };
@@ -116,7 +105,12 @@ sub command :Tab(&tab) ($self, $cart, $command, @) {
     my $entry = $cart->add(
         -$price,
         $product->{description},
-        { product_id => $product->{id}, plugin => $self->id, product => $product }
+        {
+            product_id => $product->{id},
+            plugin => $self->id,
+            product => $product,
+            deduplicate => join("/", $self->id, $product->{id}),
+        }
     );
     $entry->add_contra(
         $product->{contra},

plugins/revspace_barcode

@@ -20,7 +20,11 @@ sub data($self, $cart, $input, @) {
         ->add(
             -$price,
             "Barcode <$input>",
-            { is_barcode => 1, barcode_data => $input }
+            {
+                is_barcode => 1,
+                barcode_data => $input,
+                deduplicate => join("/", $self->id, $input),
+            }
         )
         ->add_contra(
             "+sales/barcodes",

plugins/statiegeld

@@ -76,20 +76,12 @@ sub command { # args via @_ for mutable alias
 			? "$addon->{description}"
 			: "$addon->{description} ($product->{description})";
 
-		my @existing = grep {
-			$_->attribute('plugin') eq $invocant->id and
-			$_->attribute('addon_id') eq $addon->{id} and
-			$_->{description} eq $d
-		} $cart->entries('plugin');
-
-		if (@existing) {
-			$existing[0]->quantity($existing[0]->quantity + 1);
-			$cart->select($existing[0]);
-			next;
-		}
-
 		$cart
-		->add(+$addon->{price}, $d, { plugin => $invocant->id, addon_id => $addon->{id} })
+		->add(+$addon->{price}, $d, {
+			plugin => $invocant->id,
+			addon_id => $addon->{id},
+			deduplicate => join("/", $invocant->id, $addon->{id}),
+		})
 		->add_contra($addon->{contra}, -$addon->{price}, "$d for \$you");
 	}
 

revbank

@@ -18,7 +18,7 @@ use RevBank::Global;
 use RevBank::Messages;
 use RevBank::Cart;
 
-our $VERSION = "3.7";
+our $VERSION = "3.8";
 our %HELP1 = (
     "abort" => "Abort the current transaction",
 );

revbank.plugins

@@ -27,6 +27,7 @@ dinnerbonus
 # Then, plugins that apply heuristics
 
 repeat
+deduplicate  # wants to be after 'repeat'
 statiegeld
 statiegeld_tokens
 products   # matches product IDs (barcodes)