WriteOffFee is the last of the "payment" subroutines that need to be merged into Koha::Account::pay ( as a writeoff is really just a special case payment ).
Created attachment 58883 [details] [review] Bug 17894 - Update pay() and use it internally for WriteOffFee
Created attachment 58884 [details] [review] Bug 17894 - Add unit tests
Created attachment 58885 [details] [review] Bug 17894 - Remove and replace WriteOffFee WriteOffFee is the last of the "payment" subroutines that need to be merged into Koha::Account::pay ( as a writeoff is really just type of payment ). Test Plan: 1) Apply this patch 2) Verify the writeoff, and writeoff all buttons still work
Created attachment 58908 [details] [review] [SIGNED-OFF] Bug 17894 - Update pay() and use it internally for WriteOffFee Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58909 [details] [review] [SIGNED-OFF] Bug 17894 - Add unit tests Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 58910 [details] [review] [SIGNED-OFF] Bug 17894 - Remove and replace WriteOffFee WriteOffFee is the last of the "payment" subroutines that need to be merged into Koha::Account::pay ( as a writeoff is really just type of payment ). Test Plan: 1) Apply this patch 2) Verify the writeoff, and writeoff all buttons still work Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Before this patch, the action logged contained the itemnumber, not now. Would it make sense to keep it? We have the accountlines_id, so we could get the itemnumber from it. But it could be handy to have it directly. I am not sure this is relevant but I prefer to ask.
(In reply to Jonathan Druart from comment #7) > Before this patch, the action logged contained the itemnumber, not now. > Would it make sense to keep it? > We have the accountlines_id, so we could get the itemnumber from it. But it > could be handy to have it directly. > I am not sure this is relevant but I prefer to ask. After serious consideration, I think we should *not* be logging the itemnumber directly . This was actually making the logs messier. It is predicated on the assumption that a single writeoff action only affects a single fee. Basically, a "write off all" action was indistinguishable to making many individual writeoffs in the way it was coded. Now the writeoff all action works in a more sensible manner, it makes no sense to have an itemnumber key. For these reasons I recommend do not add itemnumber directly.
Created attachment 59211 [details] [review] Bug 17894 - Update pay() and use it internally for WriteOffFee Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 59212 [details] [review] Bug 17894 - Add unit tests Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 59213 [details] [review] Bug 17894 - Remove and replace WriteOffFee WriteOffFee is the last of the "payment" subroutines that need to be merged into Koha::Account::pay ( as a writeoff is really just type of payment ). Test Plan: 1) Apply this patch 2) Verify the writeoff, and writeoff all buttons still work Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Pushed to master for 17.05!
This won't get ported back to 16.11.x as it is an enhancement.