To recreate: 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.
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