Bug 21801

Summary: paycollect.pl should pass library_id when adding accountlines
Product: Koha Reporter: Tomás Cohen Arazi <tomascohen>
Component: Architecture, internals, and plumbingAssignee: Tomás Cohen Arazi <tomascohen>
Status: CLOSED FIXED QA Contact: Josef Moravec <josef.moravec>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, josef.moravec, martin.renvoize, mtompset
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00
Bug Depends on: 19066    
Bug Blocks:    
Attachments: Bug 21801: Make paycollect.pl pass library_id when calling ->pay
Bug 21801: (follow-up) Use ->account
Bug 21801: Make paycollect.pl pass library_id when calling ->pay
Bug 21801: (follow-up) Use ->account
Bug 21801: Make paycollect.pl pass library_id when calling ->pay
Bug 21801: (follow-up) Use ->account

Description Tomás Cohen Arazi 2018-11-09 14:33:06 UTC

    
Comment 1 Tomás Cohen Arazi 2018-11-09 14:59:46 UTC
Created attachment 82172 [details] [review]
Bug 21801: Make paycollect.pl pass library_id when calling ->pay

This patch makes paycollect store the branchcode of the current library
when payments are done.

- Try all the possible options, specially selecting things to pay for (not
using the individual buttons).
- Check on the DB, that the  accountlines for the patron you're playin
with have the branchcode stored correctly in all cases:
  $ sudo koha-mysql kohadev
  > SELECT * FROM accountlines WHERE borrowernumber=<the_bn>;
=> SUCCESS: Things are as expected
- SIgn off :-D
Comment 2 Tomás Cohen Arazi 2018-11-09 14:59:51 UTC
Created attachment 82173 [details] [review]
Bug 21801: (follow-up) Use ->account

This patch is a minor refactoring, that reuses the $account object we
already get from $patron, instead of creating new ones all over the
place. Code gets simpler to read too.
Comment 3 Mark Tompsett 2019-07-03 02:15:51 UTC
(In reply to Tomás Cohen Arazi from comment #2)
> Created attachment 82173 [details] [review] [review]
> Bug 21801: (follow-up) Use ->account
> 
> This patch is a minor refactoring, that reuses the $account object we
> already get from $patron, instead of creating new ones all over the
> place. Code gets simpler to read too.

And given the use of the variable, probably is faster too. Nice. :)
Comment 4 Mark Tompsett 2019-07-03 02:17:12 UTC
Comment on attachment 82173 [details] [review]
Bug 21801: (follow-up) Use ->account

Review of attachment 82173 [details] [review]:
-----------------------------------------------------------------

::: members/paycollect.pl
@@ +59,4 @@
>  my $category       = $patron->category;
>  my $user           = $input->remote_user;
>  
> +my $library_id = C4::Context->userenv->{'branch'};

Just had to rename $branch to library_id to fit the _id structure of thinking. :)
Comment 5 Mark Tompsett 2019-07-03 02:18:20 UTC
Comment on attachment 82172 [details] [review]
Bug 21801: Make paycollect.pl pass library_id when calling ->pay

Review of attachment 82172 [details] [review]:
-----------------------------------------------------------------

::: members/paycollect.pl
@@ +168,4 @@
>                  Koha::Account->new( { patron_id => $borrowernumber } )->pay(
>                      {
>                          amount       => $total_paid,
> +                        library_id   => $branch,

Strange how, it was passed in a place above, but not these two changes.
Comment 6 Mark Tompsett 2019-07-03 02:29:03 UTC
Created attachment 91199 [details] [review]
Bug 21801: Make paycollect.pl pass library_id when calling ->pay

This patch makes paycollect store the branchcode of the current library
when payments are done.

- Try all the possible options, specially selecting things to pay for (not
using the individual buttons).
- Check on the DB, that the  accountlines for the patron you're playin
with have the branchcode stored correctly in all cases:
  $ sudo koha-mysql kohadev
  > SELECT * FROM accountlines WHERE borrowernumber=<the_bn>;
=> SUCCESS: Things are as expected
- SIgn off :-D
Comment 7 Mark Tompsett 2019-07-03 02:29:06 UTC
Created attachment 91200 [details] [review]
Bug 21801: (follow-up) Use ->account

This patch is a minor refactoring, that reuses the $account object we
already get from $patron, instead of creating new ones all over the
place. Code gets simpler to read too.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 8 Josef Moravec 2019-07-03 21:33:54 UTC
Created attachment 91248 [details] [review]
Bug 21801: Make paycollect.pl pass library_id when calling ->pay

This patch makes paycollect store the branchcode of the current library
when payments are done.

- Try all the possible options, specially selecting things to pay for (not
using the individual buttons).
- Check on the DB, that the  accountlines for the patron you're playin
with have the branchcode stored correctly in all cases:
  $ sudo koha-mysql kohadev
  > SELECT * FROM accountlines WHERE borrowernumber=<the_bn>;
=> SUCCESS: Things are as expected
- SIgn off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Josef Moravec 2019-07-03 21:33:58 UTC
Created attachment 91249 [details] [review]
Bug 21801: (follow-up) Use ->account

This patch is a minor refactoring, that reuses the $account object we
already get from $patron, instead of creating new ones all over the
place. Code gets simpler to read too.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Martin Renvoize 2019-07-04 12:57:55 UTC
Nice work!

Pushed to master for 19.11.00
Comment 11 Fridolin Somers 2019-07-25 13:36:11 UTC
Enhancement not pushed to 19.05.x