Bug 17894

Summary: Remove and Replace WriteOffFee
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Architecture, internals, and plumbingAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, katrin.fischer
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 15894    
Attachments: Bug 17894 - Update pay() and use it internally for WriteOffFee
Bug 17894 - Add unit tests
Bug 17894 - Remove and replace WriteOffFee
[SIGNED-OFF] Bug 17894 - Update pay() and use it internally for WriteOffFee
[SIGNED-OFF] Bug 17894 - Add unit tests
[SIGNED-OFF] Bug 17894 - Remove and replace WriteOffFee
Bug 17894 - Update pay() and use it internally for WriteOffFee
Bug 17894 - Add unit tests
Bug 17894 - Remove and replace WriteOffFee

Description Kyle M Hall 2017-01-12 18:48:22 UTC
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 ).
Comment 1 Kyle M Hall 2017-01-12 18:57:29 UTC
Created attachment 58883 [details] [review]
Bug 17894 - Update pay() and use it internally for WriteOffFee
Comment 2 Kyle M Hall 2017-01-12 18:57:35 UTC
Created attachment 58884 [details] [review]
Bug 17894 - Add unit tests
Comment 3 Kyle M Hall 2017-01-12 18:57:38 UTC
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
Comment 4 Josef Moravec 2017-01-13 06:26:36 UTC
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>
Comment 5 Josef Moravec 2017-01-13 06:26:43 UTC
Created attachment 58909 [details] [review]
[SIGNED-OFF] Bug 17894 - Add unit tests

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 6 Josef Moravec 2017-01-13 06:26:49 UTC
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>
Comment 7 Jonathan Druart 2017-01-17 08:56:32 UTC
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.
Comment 8 Kyle M Hall 2017-01-18 15:17:32 UTC
(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.
Comment 9 Jonathan Druart 2017-01-19 09:12:47 UTC
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>
Comment 10 Jonathan Druart 2017-01-19 09:12:52 UTC
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>
Comment 11 Jonathan Druart 2017-01-19 09:12:56 UTC
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>
Comment 12 Kyle M Hall 2017-01-19 11:15:10 UTC
Pushed to master for 17.05!
Comment 13 Katrin Fischer 2017-01-22 16:54:40 UTC
This won't get ported back to 16.11.x as it is an enhancement.