Summary: | Writeoff causes 500 error if RequirePaymentType is on | ||
---|---|---|---|
Product: | Koha | Reporter: | Lucas Gass (lukeg) <lucas> |
Component: | Fines and fees | Assignee: | Lucas Gass (lukeg) <lucas> |
Status: | RESOLVED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | major | ||
Priority: | P5 - low | CC: | david, fridolin.somers, kyle, m.de.rooy, martin.renvoize |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This fixes writing off a charge when the RequirePaymentType system preference is set to required. The write-off now completes successfully without generating an error page (Patrons > [patron account] > Accounting > Make a payment > Write off an individual charge).
|
Version(s) released in: |
23.11.00,23.05.04,22.11.10
|
Circulation function: | |||
Bug Depends on: | 33176 | ||
Bug Blocks: | |||
Attachments: |
Bug 34620: Do not throw expection if payment type is writeoff
Bug 34620: Do not throw expection if payment type is writeoff Bug 34620: Do not throw expection if payment type is writeoff |
Description
Lucas Gass (lukeg)
2023-08-25 15:47:59 UTC
Created attachment 154835 [details] [review] Bug 34620: Do not throw expection if payment type is writeoff To test: 1. Turn on RequirePaymentType 2. Create a manual invoice and then attempt to write it off. 3. 500 error 4. Turn of RequirePaymentType, no error. 5. Apply patch, restart_all 6. Try step 2 again, you should not get an error 7. prove t/db_dependent/Koha/Account.t 8. Make sure tests pass Created attachment 154850 [details] [review] Bug 34620: Do not throw expection if payment type is writeoff To test: 1. Turn on RequirePaymentType 2. Create a manual invoice and then attempt to write it off. 3. 500 error 4. Turn of RequirePaymentType, no error. 5. Apply patch, restart_all 6. Try step 2 again, you should not get an error 7. prove t/db_dependent/Koha/Account.t 8. Make sure tests pass Signed-off-by: David Nind <david@davidnind.com> Looking here Hmm. Are we solving this problem at the right spot? Yes, we can prevent an exception by putting an if around it. But if we require payment type, why not fill this column correctly when we add the writeoff? Not sure what is the exact difference here between type and payment type? (In reply to Marcel de Rooy from comment #4) > Hmm. Are we solving this problem at the right spot? > Yes, we can prevent an exception by putting an if around it. > But if we require payment type, why not fill this column correctly when we > add the writeoff? payment_type = 'WRITEOFF' ? (In reply to Marcel de Rooy from comment #4) > Hmm. Are we solving this problem at the right spot? > Yes, we can prevent an exception by putting an if around it. > But if we require payment type, why not fill this column correctly when we > add the writeoff? > > Not sure what is the exact difference here between type and payment type? To Koha, writeoffs are not a type of payment. Both are types of credits ( along with forgiveness ). It's as simple as that. That is why we have (credit) type and payment type. To change that would require alterations to fundamental assumptions about accounting in Koha that I don't think we need to get into to fix this bug ;) Initially I didn't like having the two types, though they come historically from accountlines type prior to the clarification of credit_type and debit_type. I now appreciate the clarity of having two fields. I wonder if we aught to have tied the require feature to the credit types rather than at a system level though.. that's a feature I think I'd get behind. Created attachment 155192 [details] [review] Bug 34620: Do not throw expection if payment type is writeoff To test: 1. Turn on RequirePaymentType 2. Create a manual invoice and then attempt to write it off. 3. 500 error 4. Turn of RequirePaymentType, no error. 5. Apply patch, restart_all 6. Try step 2 again, you should not get an error 7. prove t/db_dependent/Koha/Account.t 8. Make sure tests pass Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.04 Nice work everyone! Pushed to oldstable for 22.11.x |