Summary: | Remove and Replace WriteOffFee | ||
---|---|---|---|
Product: | Koha | Reporter: | Kyle M Hall (khall) <kyle> |
Component: | Architecture, internals, and plumbing | Assignee: | Kyle M Hall (khall) <kyle> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | jonathan.druart |
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: | ||
Circulation function: | |||
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 (khall)
2017-01-12 18:48:22 UTC
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. |