Created attachment 125463 [details] [review] Bug 29139: $line->debit_offsets doesn't honor list context Being based on _new_from_dbic (discussion on bug 28883), makes the assignment incorrect: my @account_offsets = $payment->debit_offsets; This patch explicitly makes the resultset be assigned as a list by calling *as_list*. To test: 1. Have UseEmailReceipts disabled 2. Have a patron with a debt of 6 3. Make a payment of 2 => SUCCESS: All good 4. Enable UseEmailReceipts 5. Repeat 3 => FAIL: You get something like: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Account::Offsets->debit is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 595 Koha::Objects::AUTOLOAD('Koha::Account::Offsets=HASH(0x561cbe2ac930)') called at input text line 6 eval {...} at input text line 6 eval {...} at input text line 23 6. Apply this patch 7. Repeat 3 => SUCCESS: It doesn't explode anymore! 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125464 [details] [review] Bug 29139: $line->debit_offsets doesn't honor list context Being based on _new_from_dbic (discussion on bug 28883), makes the assignment incorrect: my @account_offsets = $payment->debit_offsets; This patch explicitly makes the resultset be assigned as a list by calling *as_list*. To test: 1. Have UseEmailReceipts disabled 2. Have a patron with a debt of 6 3. Make a payment of 2 => SUCCESS: All good 4. Enable UseEmailReceipts 5. Repeat 3 => FAIL: You get something like: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Account::Offsets->debit is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 595 Koha::Objects::AUTOLOAD('Koha::Account::Offsets=HASH(0x561cbe2ac930)') called at input text line 6 eval {...} at input text line 6 eval {...} at input text line 23 6. Apply this patch 7. Repeat 3 => SUCCESS: It doesn't explode anymore! 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Should have been caught by tests... Looks like it's in stable. Created attachment 125474 [details] [review] Bug 29139: $line->debit_offsets doesn't honor list context Being based on _new_from_dbic (discussion on bug 28883), makes the assignment incorrect: my @account_offsets = $payment->debit_offsets; This patch explicitly makes the resultset be assigned as a list by calling *as_list*. To test: 1. Have UseEmailReceipts disabled 2. Have a patron with a debt of 6 3. Make a payment of 2 => SUCCESS: All good 4. Enable UseEmailReceipts 5. Repeat 3 => FAIL: You get something like: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Account::Offsets->debit is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 595 Koha::Objects::AUTOLOAD('Koha::Account::Offsets=HASH(0x561cbe2ac930)') called at input text line 6 eval {...} at input text line 6 eval {...} at input text line 23 6. Apply this patch 7. Repeat 3 => SUCCESS: It doesn't explode anymore! 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125475 [details] [review] Bug 29139: Fix incorrect relation call The bug here was worse than originally thought. We were calling the wrong relation too.. we should probably add some exceptions to catch this, it confuses me every single time! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Tomas told me he's working on tests already.. to save duplication of work I'll wait to see what he produces and take a look at those, Created attachment 125476 [details] [review] Bug 29139: Add exceptions to relation accessors We already had exceptions on the many-to-many links, but we didn't have them for the middle table. The underlying dbic relations make it clear which id's are being used for linking. A 'credit' has 'credit_offsets', a 'debit' has 'debit_offsets'. Created attachment 125504 [details] [review] Bug 29139: $line->debit_offsets doesn't honor list context Being based on _new_from_dbic (discussion on bug 28883), makes the assignment incorrect: my @account_offsets = $payment->debit_offsets; This patch explicitly makes the resultset be assigned as a list by calling *as_list*. To test: 1. Have UseEmailReceipts disabled 2. Have a patron with a debt of 6 3. Make a payment of 2 => SUCCESS: All good 4. Enable UseEmailReceipts 5. Repeat 3 => FAIL: You get something like: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Account::Offsets->debit is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 595 Koha::Objects::AUTOLOAD('Koha::Account::Offsets=HASH(0x561cbe2ac930)') called at input text line 6 eval {...} at input text line 6 eval {...} at input text line 23 6. Apply this patch 7. Repeat 3 => SUCCESS: It doesn't explode anymore! 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125505 [details] [review] Bug 29139: Fix incorrect relation call The bug here was worse than originally thought. We were calling the wrong relation too.. we should probably add some exceptions to catch this, it confuses me every single time! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125506 [details] [review] Bug 29139: Add exceptions to relation accessors We already had exceptions on the many-to-many links, but we didn't have them for the middle table. The underlying dbic relations make it clear which id's are being used for linking. A 'credit' has 'credit_offsets', a 'debit' has 'debit_offsets'. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125507 [details] [review] Bug 29139: Add regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125508 [details] [review] Bug 29139: Only pass the offsets for lines the credit was applied to On writing the regression tests, I noticed the CREATE offset was added to the template. The idea behind passing the offsets is that we can print information about the lines that got the credit applied. Having the CREATE offset is meaningless, and (worse) would require users to add logic to skip it. And all the payment information is already passed in the 'credit' variable anyway. This patch filters the credit_offsets by type, leaving the APPLY ones only. To test: 1. Apply up to the regression tests 2. Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: 3 offsets, including the CREATE one, boo! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! Only the two APPLY offsets are returned! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 125509 [details] [review] Bug 29139: $line->debit_offsets doesn't honor list context Being based on _new_from_dbic (discussion on bug 28883), makes the assignment incorrect: my @account_offsets = $payment->debit_offsets; This patch explicitly makes the resultset be assigned as a list by calling *as_list*. To test: 1. Have UseEmailReceipts disabled 2. Have a patron with a debt of 6 3. Make a payment of 2 => SUCCESS: All good 4. Enable UseEmailReceipts 5. Repeat 3 => FAIL: You get something like: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Account::Offsets->debit is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 595 Koha::Objects::AUTOLOAD('Koha::Account::Offsets=HASH(0x561cbe2ac930)') called at input text line 6 eval {...} at input text line 6 eval {...} at input text line 23 6. Apply this patch 7. Repeat 3 => SUCCESS: It doesn't explode anymore! 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 125510 [details] [review] Bug 29139: Fix incorrect relation call The bug here was worse than originally thought. We were calling the wrong relation too.. we should probably add some exceptions to catch this, it confuses me every single time! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 125511 [details] [review] Bug 29139: Add exceptions to relation accessors We already had exceptions on the many-to-many links, but we didn't have them for the middle table. The underlying dbic relations make it clear which id's are being used for linking. A 'credit' has 'credit_offsets', a 'debit' has 'debit_offsets'. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 125512 [details] [review] Bug 29139: Add regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 125513 [details] [review] Bug 29139: Only pass the offsets for lines the credit was applied to On writing the regression tests, I noticed the CREATE offset was added to the template. The idea behind passing the offsets is that we can print information about the lines that got the credit applied. Having the CREATE offset is meaningless, and (worse) would require users to add logic to skip it. And all the payment information is already passed in the 'credit' variable anyway. This patch filters the credit_offsets by type, leaving the APPLY ones only. To test: 1. Apply up to the regression tests 2. Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: 3 offsets, including the CREATE one, boo! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! Only the two APPLY offsets are returned! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 125592 [details] [review] Bug 29139: $line->debit_offsets doesn't honor list context Being based on _new_from_dbic (discussion on bug 28883), makes the assignment incorrect: my @account_offsets = $payment->debit_offsets; This patch explicitly makes the resultset be assigned as a list by calling *as_list*. To test: 1. Have UseEmailReceipts disabled 2. Have a patron with a debt of 6 3. Make a payment of 2 => SUCCESS: All good 4. Enable UseEmailReceipts 5. Repeat 3 => FAIL: You get something like: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Account::Offsets->debit is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 595 Koha::Objects::AUTOLOAD('Koha::Account::Offsets=HASH(0x561cbe2ac930)') called at input text line 6 eval {...} at input text line 6 eval {...} at input text line 23 6. Apply this patch 7. Repeat 3 => SUCCESS: It doesn't explode anymore! 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125593 [details] [review] Bug 29139: Fix incorrect relation call The bug here was worse than originally thought. We were calling the wrong relation too.. we should probably add some exceptions to catch this, it confuses me every single time! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125594 [details] [review] Bug 29139: Add exceptions to relation accessors We already had exceptions on the many-to-many links, but we didn't have them for the middle table. The underlying dbic relations make it clear which id's are being used for linking. A 'credit' has 'credit_offsets', a 'debit' has 'debit_offsets'. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125595 [details] [review] Bug 29139: Add regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125596 [details] [review] Bug 29139: Only pass the offsets for lines the credit was applied to On writing the regression tests, I noticed the CREATE offset was added to the template. The idea behind passing the offsets is that we can print information about the lines that got the credit applied. Having the CREATE offset is meaningless, and (worse) would require users to add logic to skip it. And all the payment information is already passed in the 'credit' variable anyway. This patch filters the credit_offsets by type, leaving the APPLY ones only. To test: 1. Apply up to the regression tests 2. Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: 3 offsets, including the CREATE one, boo! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! Only the two APPLY offsets are returned! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Final QA run done over the whole patchset... nice work everyone. Passing QA Pushed to master for 21.11, thanks to everybody involved! Created attachment 125844 [details] [review] [21.05.x] Bug 29139: $line->debit_offsets doesn't honor list context Being based on _new_from_dbic (discussion on bug 28883), makes the assignment incorrect: my @account_offsets = $payment->debit_offsets; This patch explicitly makes the resultset be assigned as a list by calling *as_list*. To test: 1. Have UseEmailReceipts disabled 2. Have a patron with a debt of 6 3. Make a payment of 2 => SUCCESS: All good 4. Enable UseEmailReceipts 5. Repeat 3 => FAIL: You get something like: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Account::Offsets->debit is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 595 Koha::Objects::AUTOLOAD('Koha::Account::Offsets=HASH(0x561cbe2ac930)') called at input text line 6 eval {...} at input text line 6 eval {...} at input text line 23 6. Apply this patch 7. Repeat 3 => SUCCESS: It doesn't explode anymore! 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125845 [details] [review] [21.05.x] Bug 29139: Fix incorrect relation call The bug here was worse than originally thought. We were calling the wrong relation too.. we should probably add some exceptions to catch this, it confuses me every single time! Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125846 [details] [review] [21.05.x] Bug 29139: Add exceptions to relation accessors We already had exceptions on the many-to-many links, but we didn't have them for the middle table. The underlying dbic relations make it clear which id's are being used for linking. A 'credit' has 'credit_offsets', a 'debit' has 'debit_offsets'. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125847 [details] [review] [21.05.x] Bug 29139: Add regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 125848 [details] [review] [21.05.x] Bug 29139: Only pass the offsets for lines the credit was applied to On writing the regression tests, I noticed the CREATE offset was added to the template. The idea behind passing the offsets is that we can print information about the lines that got the credit applied. Having the CREATE offset is meaningless, and (worse) would require users to add logic to skip it. And all the payment information is already passed in the 'credit' variable anyway. This patch filters the credit_offsets by type, leaving the APPLY ones only. To test: 1. Apply up to the regression tests 2. Run: $ kshell k$ prove t/db_dependent/Koha/Account.t => FAIL: 3 offsets, including the CREATE one, boo! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! Only the two APPLY offsets are returned! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Pushed to 21.05.x for 21.05.05 Depends on bug 27636 not in 20.11.x |
To reproduce: 1. Have UseEmailReceipts disabled 2. Have a patron with a debt of 4 3. Make a payment of 2 => SUCCESS: All good 4. Enable UseEmailReceipts 5. Repeat 3 => FAIL: You get something like: ERROR PROCESSING TEMPLATE: undef error - The method Koha::Account::Offsets->debit is not covered by tests! Trace begun at /kohadevbox/koha/Koha/Objects.pm line 595 Koha::Objects::AUTOLOAD('Koha::Account::Offsets=HASH(0x561cbe2ac930)') called at input text line 6 eval {...} at input text line 6 eval {...} at input text line 23