We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made.
Created attachment 64210 [details] [review] Bug 18790 - Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts
The sandbox you've requested is not ready. Some problems occurred applying patches from bug 18790: Cannot rebuild zebra: The command perl /home/koha/src/misc/migration_tools/rebuild_zebra.pl -a -b -r --wait-for-lock fails with the following error: 'perl /home/koha/src/misc/migration_tools/rebuild_zebra.pl -a -b -r --wait-for-lock' exited with value 2 The error message is: Can't locate Koha/Account/Offsets.pm in @INC (you may need to install the Koha::Account::Offsets module) (@INC contains: /home/koha/src /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.22.1 /usr/local/share/perl/5.22.1 /usr/lib/x86_64-linux-gnu/perl5/5.22 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.22 /usr/share/perl/5.22 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base .) at /home/koha/src/Koha/Account/Line.pm line 23. BEGIN failed--compilation aborted at /home/koha/src/Koha/Account/Line.pm line 23. Compilation failed in require at /home/koha/src/Koha/Account.pm line 28. BEGIN failed--compilation aborted at /home/koha/src/Koha/Account.pm line 28. Compilation failed in require at /home/koha/src/C4/Accounts.pm line 28. BEGIN failed--compilation aborted at /home/koha/src/C4/Accounts.pm line 28. Compilation failed in require at /home/koha/src/C4/Overdues.pm line 33. BEGIN failed--compilation aborted at /home/koha/src/C4/Overdues.pm line 33. Compilation failed in require at /home/koha/src/C4/Members.pm line 30. BEGIN failed--compilation aborted at /home/koha/src/C4/Members.pm line 30. Compilation failed in require at /home/koha/src/C4/Reserves.pm line 28. BEGIN failed--compilation aborted at /home/koha/src/C4/Reserves.pm line 28. Compilation failed in require at /home/koha/src/C4/Circulation.pm line 28. BEGIN failed--compilation aborted at /home/koha/src/C4/Circulation.pm line 28. Compilation failed in require at /home/koha/src/C4/XSLT.pm line 30. BEGIN failed--compilation aborted at /home/koha/src/C4/XSLT.pm line 30. Compilation failed in require at /home/koha/src/C4/Search.pm line 29. BEGIN failed--compilation aborted at /home/koha/src/C4/Search.pm line 29. Compilation failed in require at /home/koha/src/C4/AuthoritiesMarc.pm line 25. BEGIN failed--compilation aborted at /home/koha/src/C4/AuthoritiesMarc.pm line 25. Compilation failed in require at /home/koha/src/misc/migration_tools/rebuild_zebra.pl line 26. BEGIN failed--compilation aborted at /home/koha/src/misc/migration_tools/rebuild_zebra.pl line 26. at /home/koha/lib/contrib/sandbox/cron/../Run.pm line 28.
Created attachment 65468 [details] [review] Bug 18790 - Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts
Patch tested with a sandbox, by Koha-us conference <koha-us@koha-us.net>
Created attachment 65932 [details] [review] Bug 18790 - Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts Signed-off-by: Koha-us conference <koha-us@koha-us.net> Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Everything looks good - void doesn't show up on Account Offsets table, but fines restored successfully.
Created attachment 67478 [details] [review] Bug 18790 - Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts Signed-off-by: Koha-us conference <koha-us@koha-us.net> Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Created attachment 69158 [details] [review] Bug 18790 - Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts Signed-off-by: Koha-us conference <koha-us@koha-us.net> Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Created attachment 69159 [details] [review] Bug 18790 [QA Followup] - Store the new offset for the void
Created attachment 69161 [details] [review] Bug 18790 - Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts Signed-off-by: Koha-us conference <koha-us@koha-us.net> Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Should we consider logging reversing or voiding payments somewhere? We have talked to libraries that have to report regularly on money collected and changing payments later on could cause confusing results if not used very carefully.
Comment on attachment 69161 [details] [review] Bug 18790 - Add ability to void payments Review of attachment 69161 [details] [review]: ----------------------------------------------------------------- ::: installer/data/mysql/account_offset_types.sql @@ +9,5 @@ > ('Dropbox'), > ('Rental Fee'), > ('Fine Update'), > +('Fine'), > +('Void Payment'); This needs to be added as atomic update too.
Also, wrong capitalization?
(In reply to Katrin Fischer from comment #12) > Should we consider logging reversing or voiding payments somewhere? We have > talked to libraries that have to report regularly on money collected and > changing payments later on could cause confusing results if not used very > carefully. I agree, we definitely need this for financial reports.
(In reply to Katrin Fischer from comment #14) > Also, wrong capitalization? Yes, wrong capitalization too - for all the values, but these values are only db keys, they are not showing anywhere (as I know), so no blocker for me.
Created attachment 72017 [details] [review] Bug 18790 - Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts Signed-off-by: Koha-us conference <koha-us@koha-us.net> Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Created attachment 72018 [details] [review] Bug 18790: Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts Signed-off-by: Koha-us conference <koha-us@koha-us.net> Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Created attachment 72019 [details] [review] Bug 18790: Add logging for voided payments
Created attachment 72020 [details] [review] Bug 18790: Add new account offsets to atomic update
Created attachment 72021 [details] [review] Bug 18790: Add new account offsets to atomic update
(In reply to Josef Moravec from comment #15) > (In reply to Katrin Fischer from comment #12) > > Should we consider logging reversing or voiding payments somewhere? We have > > talked to libraries that have to report regularly on money collected and > > changing payments later on could cause confusing results if not used very > > carefully. > > I agree, we definitely need this for financial reports. This patch set already includes account offsets for voided payments. All modifications to accountlines are recorded in the account_offsets table to make it a complete record of changes to a patron's account. In addition to that, I've just added a followup to add payment voids to the action logs as well!
Created attachment 72035 [details] [review] Bug 18790: Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts Signed-off-by: Koha-us conference <koha-us@koha-us.net> Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 72036 [details] [review] Bug 18790: Add logging for voided payments Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 72037 [details] [review] Bug 18790: Add new account offsets to atomic update Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
(In reply to Kyle M Hall from comment #22) > > In addition to that, I've just added a followup to add payment voids to the > action logs as well! Great! And I just make the atomic update own file for this bug - in case it is not pushed together with bug 18786
(In reply to Josef Moravec from comment #26) > (In reply to Kyle M Hall from comment #22) > > > > In addition to that, I've just added a followup to add payment voids to the > > action logs as well! > > Great! > > And I just make the atomic update own file for this bug - in case it is not > pushed together with bug 18786 Thanks Josef!
Please add POD to the new method.
* +use Koha::Patron::Images; line readded by these patches * Koha::Account::Line->void must use a transaction to make sure the log will be done only if the offset has been created successfully.
(In reply to Jonathan Druart from comment #29) > * > +use Koha::Patron::Images; > > line readded by these patches I'm not seeing this line in any of the patches. Can you point me to it? > > * Koha::Account::Line->void must use a transaction to make sure the log will > be done only if the offset has been created successfully. Good idea!
Comment on attachment 72036 [details] [review] Bug 18790: Add logging for voided payments Review of attachment 72036 [details] [review]: ----------------------------------------------------------------- ::: Koha/Account/Line.pm @@ +18,4 @@ > use Modern::Perl; > > use Carp; > +use Data::Dumper; Left over from debugging?
Comment on attachment 72035 [details] [review] Bug 18790: Add ability to void payments Review of attachment 72035 [details] [review]: ----------------------------------------------------------------- Was looking for the line - found some other small stuff instead (sorry!) ::: installer/data/mysql/account_offset_types.sql @@ +9,5 @@ > ('Dropbox'), > ('Rental Fee'), > ('Fine Update'), > +('Fine'), > +('Void Payment'); Capitalization... but that's already a little late here. ::: koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt @@ +60,4 @@ > [% CASE 'Pay00' %]Payment, thanks (cash via SIP2) > [% CASE 'Pay01' %]Payment, thanks (VISA via SIP2) > [% CASE 'Pay02' %]Payment, thanks (credit card via SIP2) > + [% CASE 'VOID' %]Payment, Voided Capitalization - not too late here :)
Created attachment 74157 [details] [review] Bug 18790: Add ability to void payments We've had the ability to 'reverse' a payment for a long time, but it would be much better if we had a true void payment function that replaces the paid amounts into the fee so that it appears as if the payment was never made. Test Plan: 1) Apply this patch and dependent patches 2) Run updatedatabase.pl 3) Create some fines 4) Pay those fines 5) Use the new 'void' button to void the payments 6) Note the fines were restored to their pre-payment amounts Signed-off-by: Koha-us conference <koha-us@koha-us.net> Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 74158 [details] [review] Bug 18790: Add logging for voided payments Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 74159 [details] [review] Bug 18790: Add new account offsets to atomic update Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 74160 [details] [review] Bug 18790 [QA Followup]: Remove use of Data::Dumper, fix capitalization
(In reply to Katrin Fischer from comment #31) > Comment on attachment 72036 [details] [review] [review] > Bug 18790: Add logging for voided payments > > Review of attachment 72036 [details] [review] [review]: > ----------------------------------------------------------------- > > ::: Koha/Account/Line.pm > @@ +18,4 @@ > > use Modern::Perl; > > > > use Carp; > > +use Data::Dumper; > > Left over from debugging? Just realized Dumper is used for action logs here. I'll restore that.
Created attachment 74161 [details] [review] Bug 18790 [QA Followup]: Fix capitalization
Created attachment 74162 [details] [review] Bug 18790 [QA Followup]: Use a transaction for voiding
Created attachment 74576 [details] [review] Bug 18790: Prevent the tests to fail if precision changes
Pushed to master for 18.05, thanks to everybody involved!