All account handling should go via the Koha::Account class.
*** Bug 21748 has been marked as a duplicate of this bug. ***
Created attachment 81890 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust
Created attachment 83130 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83234 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust Known Side Effect: Prior to this patch fines did not set an 'action' in the FineLog when FinsLog is enabled. After this patch, if the FinesLog is enabled then the 'action' will be recorded as `create_fine`. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 83252 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust Known Side Effect: The format of the FinesLog, if enabled, is changed after this patch. Prior to this patch the $actionname was left undefined and the $infos field contained the string: `"due=".$due." amount=".$amount." itemnumber=".$itemnum` After this patch, the logs are more consistent with other FINES logs, with an $actionname of 'CREATE' or 'UPDATE' and the $infos field containing a Dumper of fine data. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
(In reply to Martin Renvoize from comment #5) > Created attachment 83252 [details] [review] [review] > Bug 21747: Use Koha::Account:: routines in UpdateFine > > Set to use Koha::Account->add_debit and Koha::Account::Line->adjust > > Known Side Effect: The format of the FinesLog, if enabled, is changed > after this patch. Prior to this patch the $actionname was left undefined > and the $infos field contained the string: > > `"due=".$due." amount=".$amount." itemnumber=".$itemnum` > > After this patch, the logs are more consistent with other FINES logs, > with an $actionname of 'CREATE' or 'UPDATE' and the $infos field > containing a Dumper of fine data. > > Signed-off-by: Josef Moravec <josef.moravec@gmail.com> I think we need to consider the backdated returns use case, and have another offset for those, as fine_increment doesn't look right.
(In reply to Tomás Cohen Arazi from comment #6) > > I think we need to consider the backdated returns use case, and have another > offset for those, as fine_increment doesn't look right. What I meant here is that FU (Fine Update) should be splitted into 'fine_increment' and 'fine_decrement'. Not all 'fine_increment'. And in the case of backdated returns/dropbox mode we could pass a specific offset_type to ->adjust.
Moving to failed QA as of Tomas last comments.
Created attachment 84725 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust Known Side Effect: The format of the FinesLog, if enabled, is changed after this patch. Prior to this patch the $actionname was left undefined and the $infos field contained the string: `"due=".$due." amount=".$amount." itemnumber=".$itemnum` After this patch, the logs are more consistent with other FINES logs, with an $actionname of 'CREATE' or 'UPDATE' and the $infos field containing a Dumper of fine data. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84726 [details] [review] Bug 21747: (follow-up) Intelligent rename of offsets This patch intelligently renames the account_offset types for updateing fines from `Fine Update` to `fine_increment` and `fine_decrement` depending on the sign of the calculated difference of the adjustment. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84807 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust Known Side Effect: The format of the FinesLog, if enabled, is changed after this patch. Prior to this patch the $actionname was left undefined and the $infos field contained the string: `"due=".$due." amount=".$amount." itemnumber=".$itemnum` After this patch, the logs are more consistent with other FINES logs, with an $actionname of 'CREATE' or 'UPDATE' and the $infos field containing a Dumper of fine data. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84808 [details] [review] Bug 21747: (follow-up) Intelligent rename of offsets This patch intelligently renames the account_offset types for updateing fines from `Fine Update` to `fine_increment` and `fine_decrement` depending on the sign of the calculated difference of the adjustment. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 84810 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust Known Side Effect: The format of the FinesLog, if enabled, is changed after this patch. Prior to this patch the $actionname was left undefined and the $infos field contained the string: `"due=".$due." amount=".$amount." itemnumber=".$itemnum` After this patch, the logs are more consistent with other FINES logs, with an $actionname of 'CREATE' or 'UPDATE' and the $infos field containing a Dumper of fine data. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84811 [details] [review] Bug 21747: (follow-up) Intelligent rename of offsets This patch intelligently renames the account_offset types for updateing fines from `Fine Update` to `fine_increment` and `fine_decrement` depending on the sign of the calculated difference of the adjustment. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I've implemented the follow-up as suggested now so that we use 'fine_update' as the 'type' to pass to the method and then 'fine_increment' or 'fine_decrement' for the two cases in the account_offsets table.. I'm not 100% sure 'increment' and 'decrement' are the clearest terms whilst we're here.. they may be clearer as 'increase' and 'decrease'... just a thought.
Created attachment 84816 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust Known Side Effect: The format of the FinesLog, if enabled, is changed after this patch. Prior to this patch the $actionname was left undefined and the $infos field contained the string: `"due=".$due." amount=".$amount." itemnumber=".$itemnum` After this patch, the logs are more consistent with other FINES logs, with an $actionname of 'CREATE' or 'UPDATE' and the $infos field containing a Dumper of fine data. Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84817 [details] [review] Bug 21747: (follow-up) Intelligent rename of offsets This patch intelligently renames the account_offset types for updateing fines from `Fine Update` to `fine_increment` and `fine_decrement` depending on the sign of the calculated difference of the adjustment. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Updated to use _increase and _decrease as Katrin agreed they're clearer terms for the average user.
Created attachment 85179 [details] [review] Bug 21747: Use Koha::Account:: routines in UpdateFine Set to use Koha::Account->add_debit and Koha::Account::Line->adjust Known Side Effect: The format of the FinesLog, if enabled, is changed after this patch. Prior to this patch the $actionname was left undefined and the $infos field contained the string: `"due=".$due." amount=".$amount." itemnumber=".$itemnum` After this patch, the logs are more consistent with other FINES logs, with an $actionname of 'CREATE' or 'UPDATE' and the $infos field containing a Dumper of fine data. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 85180 [details] [review] Bug 21747: (follow-up) Intelligent rename of offsets This patch intelligently renames the account_offset types for updateing fines from `Fine Update` to `fine_increment` and `fine_decrement` depending on the sign of the calculated difference of the adjustment. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Awesome work all! Pushed to master for 19.05
Created attachment 85795 [details] [review] Bug 21747: Insert fine_increase and fine_decrease for new installs This fixes Circulation.t koha_1 | DBD::mysql::st execute failed: Cannot add or update a child row: a foreign key constraint fails (`koha_kohadev`.`account_offsets`, CONSTRAINT `account_offsets_ibfk_t` FOREIGN KEY (`type`) REFERENCES `account_offset_types` (`type`) ON DELETE CASCADE ON UPDATE CASCADE) [for Statement "INSERT INTO `account_offsets` ( `amount`, `debit_id`, `type`) VALUES ( ?, ?, ? )" with ParamValues: 0=1, 1='22', 2='fine_increase'] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
(In reply to Jonathan Druart from comment #22) > Created attachment 85795 [details] [review] [review] > Bug 21747: Insert fine_increase and fine_decrease for new installs > > This fixes Circulation.t > koha_1 | DBD::mysql::st execute failed: Cannot add or update a child > row: a foreign key constraint fails (`koha_kohadev`.`account_offsets`, > CONSTRAINT `account_offsets_ibfk_t` FOREIGN KEY (`type`) REFERENCES > `account_offset_types` (`type`) ON DELETE CASCADE ON UPDATE CASCADE) [for > Statement "INSERT INTO `account_offsets` ( `amount`, `debit_id`, `type`) > VALUES ( ?, ?, ? )" with ParamValues: 0=1, 1='22', 2='fine_increase'] at > /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1832. > > Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Follow up pushed to master
Enhancement, will not be backported to 18.11.x series.
*** Bug 22374 has been marked as a duplicate of this bug. ***
*** Bug 24431 has been marked as a duplicate of this bug. ***