Some transactions with only one contra account, but used multiple times,
or with the actor's own account as the only other contra account, were
erroneously allowed.
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.
The hook gets fired for each product individually, so a single mtime
update is not good enough.
The timestamp needs to be recorded per product or tag. Meh.
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.
- 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.
Prevents (parts of) transactions if a user does not have sufficient
money.
The default configuration allows negative balances for buying products
and for multi-user takes.
When the transaction is still pending, the past tense is incorrect.
This is not relevant for the contras, because those descriptions are
only displayed in logs, after the fact.
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.
Old message was not as intended:
> Name for the new account: 123123123123
> That's way too much money. Enter 'abort' to abort.
Fixed:
> Name for the new account: 123123123123
Sorry, that's too numeric to be a user name. Enter 'abort' to abort.
This somehow escaped change with the introduction of RevBank::Amount in
v3.2 in 2021, which only now became relevant due to the recent change in
v6.1.0 which turns invalid account balances into a feature.
This is weird. I'm sure I did test valueless tags. But apparently
between that and committing, the `?` quantifier in the regex got lost,
and I don't know how that happened.
Rationale in UPGRADING.md
It's a big change technically, but converting the format won't be hard
for admins.
There's a compatibility mode with loud warnings in case the file isn't
converted.
It's too buggy; in some edge cases it results in an infinite input loop
with 100% cpu. If you want to restart, use 'restart' instead of eof'ing
the input with ^D.
Originally, this command didn't have a description parameter. Foo would
use `give xyzzy 10 foo`. Then, a description parameter was added. For
backwards compatibility, if you would enter a username (like `foo` in
this example) in the place of the description, it would finalize the
transaction using that.
However, as the user base grows, several reasonable descriptions exist as
user account names, and that would finalize the transaction under the
wrong user.
It's time to break backward compatibility. If you don't want to leave a
message (it's still optional), that can be done with `x` (like the
`donate` command), or in advanced mode, with `""`.
Because it's likely that people are still very much used to just leaving
the description out, if you enter something that happens to match an
existing username, the input will be rejected.
The current equivalent command line would be `give xyzzy 10 ""; foo` or
`give xyzzy 10 x; foo`. If the `;` (new since v5.0.0) is left out, the
trailing `foo` has to be confirmed with a second press of the enter key.
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.
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.
