This change removes two recently added hooks. No deprecation cycle
because they have only existed for a week, so it's extremely unlikely
that anyone's using them.
RevBank reads the new products file on every interaction (e.g. pressing
enter), and then fires hooks like `product_changed`. Every running
instance gets those hooks, but the price tage should be generated only
once.
There was a bug with the example product defined as:
+smk,matekrat 1.50@+statiegeld "..."
Only the id `+smk` was considered, and no total price was calculated. This
broke the accessible id `matekrat`. The fix is to consider the keys of
the products in the hash, instead of the `id` field.
- 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.
A sufficiently big number, i.e. longer than a long long, had interesting
effects. Perl would promote it to a float, and format it as -1 in
sprintf, which RevBank::Amount didn't handle correctly. In extreme cases
the number got rounded to Inf and would no longer round-trip.
As a result, numbers returned by RevBank::Amount are now Math::BigInt
and Math::BigFloat objects. Those should be transparent to all existing
code. It's amazing to see the unit tests pass.
I don't think there is any actual use case in RevBank for numbers this
large and I don't think anyone will have actually encountered the
aforementioned weird effects. Mostly, the input would be parsed with
parse_amount which refuses any number greater than 99900 anyway. Only
where parse_string was used directly, such large numbers could actually
have been used, but in stock RevBank that is only done when reading the
accounts file.
This change also introduces a new global function parse_any_amount that
is like parse_amount but doesn't complain about negative or large
numbers, to further improve the adduser plugin (see previous commit) in
insane edge cases. It differs from RevBank::Amount->parse_string in that
it does support addition and subtraction operators.
Since the first versions of RevBank, negative and huge amounts are
handled centrally, and since v2 (2013) they've been implemented through
an exception that caused the pending transaction to be aborted. Since v3
(2019), RevBank has had a retry mechanism for rejected input to improve
the user experience, but it required a REJECT return message from a
plugin, not an exception. Now there's an exception class to trigger the
same semantics.
This fixes the bug that empty lines would be inserted after each prompt,
starting from the first use of ^D.
Readline considers ^D end-of-file even when it's not, and for whatever
reason then adds a \n after BRACK_PASTE_FINI, which results in empty
lines after subsequent prompts.
With readline's internal loop, rl_found_eof gets reset to false, but
users of a custom loop don't get that luxury, and Term::ReadLine::Gnu
doesn't expose rl_found_eof (which was added to readline's API only a
few years ago) to do it manually.
One workaround, used briefly but not committed, would be to disable
bracketed paste.
A better workaround, as implemented by this commit, is to abandon the
custom loop and use readline's blocking call instead.
Wanted to move split_input() to a package for unit testing, thought I'd
move prompt() too since the main executable has become messy, and this
would be a good first step in resolving that.
I can't imagine this to be important but throughout the years it's been
expected by users that "abort" can be quoted and passed to a plugin like
one that prints barcodes.
It's still not possible to pass a literal string `abort` to a follow-up
prompt, leaving this feature only available to advanced users who (hope
to) know what they're doing.
The ancient decision to let undo perform the checkout by itself still
makes sense from a UX perspective, but keeps requiring specific handling
of edge cases.
In this case, the easiest way to deal with trailing input is to just
abort entirely.
Also: updated lib/RevBank/Plugins.pm to import 'isa' and get up to 5.32
level.
There's a slight mismatch between what users experience as a command,
and how commands are defined in RevBank. Specifically, the common input
"<productid> <username>" is two separate commands: the first adds the
product to the cart, the second finalizes the transaction. This also
means that "<productid> <username> <productid> <username>" was four
separate commands, resulting in TWO transactions.
That's all fine and useful, but when using this advanced input method,
where input is split on whitespace, it lead to unexpected results if
there are insufficient arguments for the follow-up questions of a
command. For example, "take jantje 10 take pietje 10" will interpret the
second "take" as the description, then "pietje" als the first command of
a new transaction, and finally, "10" which is typically not a valid
command. It is much more likely that the user intended two separate
"take" commands and just forgot to provide the description argument, but
RevBank had no way of inferring that intent.
From this commit on, whenever the user intends to enter further input
words beyond the one that finalizes a transaction ($cart->checkout), a
';' is required. If trailing input is present, the checkout is refused
and the user gets a retry prompt.
Similarly, if the user indicates the intention of having finished a
command by inserting a ';' while there are insufficient words in the
command line to satisfy all follow-up prompts (command arguments), the
rest of the command line is rejected with a retry prompt.
There is, however, still no specific requirement for a ';' separator
after a command that does not finalize a transaction (e.g. "<productid>
<username>" or even "<productid> x2 <productid> <username>" remains
valid), or for a command that precedes a ';' to finalize a transaction
(e.g. "<productid>; <username>;" is also valid).
This change catches many, but not all, mistakes.
Was already implicitly required (since 59387ddb) because RevBank::Amount
uses the "isa" feature, which was introduced in Perl 5.32 (but no longer
experimental since 5.36, not 5.32 as the old comment said).
Perl 5.32 was released in June 2020, and ships with Debian bullseye
("oldstable") which was released in August 2021.
A space had a custom plugin that died during hook_checkout, which caused
the CHECKOUT lines to be logged without the corresponding BALANCE, and
indeed no account balances were updated. While the plugin had a bug, it
should not cause a half transaction in RevBank.
After some hesitation, I went with ON ERROR RESUME NEXT because if a
hook throws an exception, that should not interfere with other plugins
(the hook can return ABORT if this it was intentional), including the
calling plugin. An error message is printed (but not logged... TODO: add
hook_plugin_fail to plugins/log) but the show must go on.
During hook_checkout_prepare, however, nothing is set in stone yet, so
this could be used for something that might die, and this instance of
call_hooks() is now the one place where a failing hook should result in
the transaction getting aborted. For this, call_hooks() now returns a
success status boolean. Maybe it would make sense in more places, but I
didn't identify any such calls yet.
RevBank::Cart->checkout used to return a success status boolean, but it
could just as well just die (indirectly, to abort the transaction) since
it can't be called a second time within the same transaction anyway
(because ->set_user must be called exactly once), so continuing with the
same transaction can't result in anything useful anyway.
In some places, error messages were slightly improved to contain a bit
more information.
