Bug 21801 - paycollect.pl should pass library_id when adding accountlines
Summary: paycollect.pl should pass library_id when adding accountlines
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Josef Moravec
URL:
Keywords:
Depends on: 19066
Blocks:
  Show dependency treegraph
 
Reported: 2018-11-09 14:33 UTC by Tomás Cohen Arazi
Modified: 2020-06-04 20:32 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.11.00


Attachments
Bug 21801: Make paycollect.pl pass library_id when calling ->pay (1.69 KB, patch)
2018-11-09 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21801: (follow-up) Use ->account (3.17 KB, patch)
2018-11-09 14:59 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 21801: Make paycollect.pl pass library_id when calling ->pay (1.74 KB, patch)
2019-07-03 02:29 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 21801: (follow-up) Use ->account (3.31 KB, patch)
2019-07-03 02:29 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 21801: Make paycollect.pl pass library_id when calling ->pay (1.79 KB, patch)
2019-07-03 21:33 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21801: (follow-up) Use ->account (3.37 KB, patch)
2019-07-03 21:33 UTC, Josef Moravec
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 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