Bug 29139 - Paying gives ISE if UseEmailReceipts is enabled
Summary: Paying gives ISE if UseEmailReceipts is enabled
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 27636
Blocks: 14825
  Show dependency treegraph
 
Reported: 2021-09-29 19:23 UTC by Tomás Cohen Arazi
Modified: 2021-10-21 04:05 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.05


Attachments
Bug 29139: $line->debit_offsets doesn't honor list context (1.64 KB, patch)
2021-09-29 19:31 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29139: $line->debit_offsets doesn't honor list context (1.71 KB, patch)
2021-09-29 19:35 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29139: $line->debit_offsets doesn't honor list context (1.77 KB, patch)
2021-09-30 08:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29139: Fix incorrect relation call (1.01 KB, patch)
2021-09-30 08:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29139: Add exceptions to relation accessors (1.77 KB, patch)
2021-09-30 08:31 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29139: $line->debit_offsets doesn't honor list context (1.82 KB, patch)
2021-09-30 13:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29139: Fix incorrect relation call (1.06 KB, patch)
2021-09-30 13:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29139: Add exceptions to relation accessors (1.81 KB, patch)
2021-09-30 13:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29139: Add regression tests (2.49 KB, patch)
2021-09-30 13:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29139: Only pass the offsets for lines the credit was applied to (1.58 KB, patch)
2021-09-30 13:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 29139: $line->debit_offsets doesn't honor list context (1.89 KB, patch)
2021-09-30 13:41 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29139: Fix incorrect relation call (1.13 KB, patch)
2021-09-30 13:41 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29139: Add exceptions to relation accessors (1.88 KB, patch)
2021-09-30 13:41 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29139: Add regression tests (2.55 KB, patch)
2021-09-30 13:41 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29139: Only pass the offsets for lines the credit was applied to (1.65 KB, patch)
2021-09-30 13:41 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29139: $line->debit_offsets doesn't honor list context (1.95 KB, patch)
2021-10-01 09:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29139: Fix incorrect relation call (1.19 KB, patch)
2021-10-01 09:16 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29139: Add exceptions to relation accessors (1.94 KB, patch)
2021-10-01 09:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29139: Add regression tests (2.61 KB, patch)
2021-10-01 09:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 29139: Only pass the offsets for lines the credit was applied to (1.71 KB, patch)
2021-10-01 09:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
[21.05.x] Bug 29139: $line->debit_offsets doesn't honor list context (1.96 KB, patch)
2021-10-06 18:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[21.05.x] Bug 29139: Fix incorrect relation call (1.20 KB, patch)
2021-10-06 18:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[21.05.x] Bug 29139: Add exceptions to relation accessors (2.13 KB, patch)
2021-10-06 18:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[21.05.x] Bug 29139: Add regression tests (2.68 KB, patch)
2021-10-06 18:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[21.05.x] Bug 29139: Only pass the offsets for lines the credit was applied to (1.83 KB, patch)
2021-10-06 18:10 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2021-09-29 19:23:51 UTC
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
Comment 1 Tomás Cohen Arazi 2021-09-29 19:31:04 UTC
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>
Comment 2 Andrew Fuerste-Henry 2021-09-29 19:35:09 UTC
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>
Comment 3 Jonathan Druart 2021-09-30 06:38:14 UTC
Should have been caught by tests...
Comment 4 Jonathan Druart 2021-09-30 06:40:16 UTC
Looks like it's in stable.
Comment 5 Martin Renvoize 2021-09-30 08:16:43 UTC
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>
Comment 6 Martin Renvoize 2021-09-30 08:16:46 UTC
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>
Comment 7 Martin Renvoize 2021-09-30 08:17:21 UTC
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,
Comment 8 Martin Renvoize 2021-09-30 08:31:54 UTC
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'.
Comment 9 Tomás Cohen Arazi 2021-09-30 13:10:33 UTC
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>
Comment 10 Tomás Cohen Arazi 2021-09-30 13:10:52 UTC
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>
Comment 11 Tomás Cohen Arazi 2021-09-30 13:11:15 UTC
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>
Comment 12 Tomás Cohen Arazi 2021-09-30 13:11:32 UTC
Created attachment 125507 [details] [review]
Bug 29139: Add regression tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2021-09-30 13:11:51 UTC
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>
Comment 14 Andrew Fuerste-Henry 2021-09-30 13:41:43 UTC
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>
Comment 15 Andrew Fuerste-Henry 2021-09-30 13:41:47 UTC
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>
Comment 16 Andrew Fuerste-Henry 2021-09-30 13:41:51 UTC
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>
Comment 17 Andrew Fuerste-Henry 2021-09-30 13:41:55 UTC
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>
Comment 18 Andrew Fuerste-Henry 2021-09-30 13:41:59 UTC
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>
Comment 19 Martin Renvoize 2021-10-01 09:16:53 UTC
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>
Comment 20 Martin Renvoize 2021-10-01 09:16:57 UTC
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>
Comment 21 Martin Renvoize 2021-10-01 09:17:01 UTC
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>
Comment 22 Martin Renvoize 2021-10-01 09:17:05 UTC
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>
Comment 23 Martin Renvoize 2021-10-01 09:17:09 UTC
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>
Comment 24 Martin Renvoize 2021-10-01 09:17:35 UTC
Final QA run done over the whole patchset... nice work everyone.

Passing QA
Comment 25 Jonathan Druart 2021-10-01 14:30:33 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 26 Tomás Cohen Arazi 2021-10-06 18:10:00 UTC
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>
Comment 27 Tomás Cohen Arazi 2021-10-06 18:10:07 UTC
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>
Comment 28 Tomás Cohen Arazi 2021-10-06 18:10:17 UTC
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>
Comment 29 Tomás Cohen Arazi 2021-10-06 18:10:27 UTC
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>
Comment 30 Tomás Cohen Arazi 2021-10-06 18:10:38 UTC
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>
Comment 31 Kyle M Hall 2021-10-08 13:52:12 UTC
Pushed to 21.05.x for 21.05.05
Comment 32 Fridolin Somers 2021-10-21 04:05:44 UTC
Depends on bug 27636 not in 20.11.x