Bug 19066 - Add branchcode to accountlines
Summary: Add branchcode to accountlines
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
: 7595 (view as bug list)
Depends on:
Blocks: 16461 21002 21401 21678 21786 21788 21801 22066
  Show dependency treegraph
 
Reported: 2017-08-09 13:48 UTC by Kyle M Hall
Modified: 2019-06-27 09:24 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
This enhancements adds recording of the branch an account transaction was performed. For payments it will be the signed in branch when payment is collected. For manual invoices/credits it is the signed in branch when the line is created.
Version(s) released in:


Attachments
Bug 19066 - Update Database (1.35 KB, patch)
2017-08-09 17:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 - Update Schema (1.39 KB, patch)
2017-08-09 17:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 - Add branchcode to accountlines (15.82 KB, patch)
2017-08-09 17:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 [QA Followup] - Fix related unit test failures (3.15 KB, patch)
2017-10-06 18:51 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 [QA Followup] - Fix related unit test failures (1.93 KB, patch)
2017-10-06 18:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 - Update Database (1.39 KB, patch)
2018-02-02 16:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 - Add branchcode to accountlines (11.68 KB, patch)
2018-02-02 16:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 - Update Schema (1.39 KB, patch)
2018-02-02 16:53 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 - Update Database (1.42 KB, patch)
2018-03-23 14:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 - Add branchcode to accountlines (12.27 KB, patch)
2018-03-23 14:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066 - Update Schema (1.41 KB, patch)
2018-03-23 14:36 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066: Follow up to reduce C4::Context references (903 bytes, patch)
2018-04-04 02:07 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 19066: Update Database (1.41 KB, patch)
2018-07-26 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066: Add branchcode to accountlines (11.65 KB, patch)
2018-07-26 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066: Update Schema (1.40 KB, patch)
2018-07-26 13:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066: Add branchcode to accountlines (11.73 KB, patch)
2018-09-15 22:06 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 19066: Update Schema (1.50 KB, patch)
2018-09-15 22:06 UTC, Lisette Scheer
Details | Diff | Splinter Review
Bug 19066: Update Database (1.42 KB, patch)
2018-10-25 12:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066: Add branchcode to accountlines (11.76 KB, patch)
2018-10-25 12:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066: Update Schema (1.41 KB, patch)
2018-10-25 12:52 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066: Update Database (1.45 KB, patch)
2018-10-26 08:22 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19066: Add branchcode to accountlines (11.79 KB, patch)
2018-10-26 08:22 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19066: Update Schema (1.45 KB, patch)
2018-10-26 08:22 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Move db update to perl (1.66 KB, patch)
2018-10-31 07:27 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Add handling in add_credit (2.47 KB, patch)
2018-11-02 16:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Remove bad and unecessary test (1.77 KB, patch)
2018-11-02 18:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee (6.51 KB, patch)
2018-11-07 20:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Fix ChargeReserveFee (973 bytes, patch)
2018-11-07 20:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee (6.44 KB, patch)
2018-11-08 14:19 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Fix ChargeReserveFee (973 bytes, patch)
2018-11-08 14:19 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: Add tests for Koha::Account->pay (2.46 KB, patch)
2018-11-08 14:19 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: Update Database (1.46 KB, patch)
2018-11-19 12:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: Add branchcode to accountlines (11.59 KB, patch)
2018-11-19 12:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: Update Schema (1.45 KB, patch)
2018-11-19 12:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Move db update to perl (1.66 KB, patch)
2018-11-19 12:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Add handling in add_credit (2.47 KB, patch)
2018-11-19 12:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Remove bad and unecessary test (1.75 KB, patch)
2018-11-19 12:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee (6.47 KB, patch)
2018-11-19 12:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Fix ChargeReserveFee (973 bytes, patch)
2018-11-19 12:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: Add tests for Koha::Account->pay (3.15 KB, patch)
2018-11-19 12:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 19066: Update Database (1.46 KB, patch)
2018-12-12 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Add branchcode to accountlines (11.59 KB, patch)
2018-12-12 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Update Schema (1.37 KB, patch)
2018-12-12 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Move db update to perl (1.66 KB, patch)
2018-12-12 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Add handling in add_credit (2.34 KB, patch)
2018-12-12 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Remove bad and unecessary test (1.75 KB, patch)
2018-12-12 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee (6.47 KB, patch)
2018-12-12 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Fix ChargeReserveFee (973 bytes, patch)
2018-12-12 08:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Add tests for Koha::Account->pay (3.16 KB, patch)
2018-12-12 08:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Update Database (1.52 KB, patch)
2018-12-12 08:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Add branchcode to accountlines (11.65 KB, patch)
2018-12-12 08:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Update Schema (1.43 KB, patch)
2018-12-12 08:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Move db update to perl (1.72 KB, patch)
2018-12-12 08:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Add handling in add_credit (2.41 KB, patch)
2018-12-12 08:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Remove bad and unecessary test (1.82 KB, patch)
2018-12-12 08:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee (6.53 KB, patch)
2018-12-12 08:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Fix ChargeReserveFee (1.01 KB, patch)
2018-12-12 08:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Add tests for Koha::Account->pay (3.22 KB, patch)
2018-12-12 08:59 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Define foreign key for branchcode (2.35 KB, patch)
2018-12-17 11:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 19066: Update Database (1.51 KB, patch)
2018-12-27 14:07 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: Add branchcode to accountlines (11.64 KB, patch)
2018-12-27 14:07 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: Update Schema (1.06 KB, patch)
2018-12-27 14:07 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Move db update to perl (1.72 KB, patch)
2018-12-27 14:08 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Add handling in add_credit (2.39 KB, patch)
2018-12-27 14:08 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Remove bad and unecessary test (1.81 KB, patch)
2018-12-27 14:08 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee (6.51 KB, patch)
2018-12-27 14:08 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Fix ChargeReserveFee (1.01 KB, patch)
2018-12-27 14:08 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: Add tests for Koha::Account->pay (3.21 KB, patch)
2018-12-27 14:09 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: (QA follow-up) Define foreign key for branchcode (2.36 KB, patch)
2018-12-27 14:09 UTC, Alex Arnaud
Details | Diff | Splinter Review
Bug 19066: (RM follow-up) Fix test count and structure error (1.71 KB, patch)
2019-01-03 19:08 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2017-08-09 13:48:02 UTC
For the purposes of statistics, it appears that it would help many libraries to have branchcode recorded in the accountlines table. For payments, the field would contain the code for the branch the payment was made at. For manual invoices, it would be the code of the library that created the invoice.
Comment 1 Kyle M Hall 2017-08-09 17:04:27 UTC
Created attachment 65769 [details] [review]
Bug 19066 - Update Database
Comment 2 Kyle M Hall 2017-08-09 17:04:33 UTC
Created attachment 65770 [details] [review]
Bug 19066 - Update Schema
Comment 3 Kyle M Hall 2017-08-09 17:04:36 UTC
Created attachment 65771 [details] [review]
Bug 19066 - Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch
Comment 4 Kyle M Hall 2017-08-09 17:04:47 UTC
*** Bug 7595 has been marked as a duplicate of this bug. ***
Comment 5 M. Tompsett 2017-08-10 03:46:35 UTC
Comment on attachment 65771 [details] [review]
Bug 19066 - Add branchcode to accountlines

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

Looking for test that triggers "third" part of Circulation.pm changes.
prove t/db_dependent/Reserves/
prove t/db_dependent/Accounts.t
Just that one little change needs triggering before I'll sign off.
Read last Koha/Account comment, then first, then second.

::: C4/Circulation.pm
@@ +2872,3 @@
>      # Charge a new rental fee, if applicable?
>      my ( $charge, $type ) = GetIssuingCharges( $itemnumber, $borrowernumber );
>      if ( $charge > 0 ) {

Do we have a test that triggers this? I haven't found it yet.

::: Koha/Account.pm
@@ +69,5 @@
>      my $lines           = $params->{lines};
>      my $type            = $params->{type} || 'payment';
> +    my $branchcode      = $params->{branchcode};
> +
> +    $library_id ||= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;

But why line 73 and change the line?

@@ +74,2 @@
>  
>      my $userenv = C4::Context->userenv;

Why not just after $userenv here, and keep the line identical?

@@ -194,5 @@
>              note              => $note,
>          }
>      )->store();
>  
> -    $library_id ||= $userenv ? $userenv->{'branch'} : undef;

Yes, this needs to move.
Comment 6 M. Tompsett 2017-08-10 05:07:28 UTC
prove -r t/db_dependent fails after applying the patch. Constraint errors related to test builder, I suspect.
Comment 7 Tomás Cohen Arazi 2017-10-06 17:34:07 UTC
(In reply to M. Tompsett from comment #6)
> prove -r t/db_dependent fails after applying the patch. Constraint errors
> related to test builder, I suspect.

Can you please make it explicit what tests are failing? Otherwise its around an hour of waiting for something to fail...
Comment 8 Kyle M Hall 2017-10-06 18:51:25 UTC
Created attachment 67756 [details] [review]
Bug 19066 [QA Followup] - Fix related unit test failures
Comment 9 Kyle M Hall 2017-10-06 18:52:37 UTC
Created attachment 67757 [details] [review]
Bug 19066 [QA Followup] - Fix related unit test failures
Comment 10 David Bourgault 2017-11-03 20:14:14 UTC
Patch doesnt apply for me on master.
Comment 11 Kyle M Hall 2018-02-02 16:53:42 UTC
Created attachment 71153 [details] [review]
Bug 19066 - Update Database
Comment 12 Kyle M Hall 2018-02-02 16:53:50 UTC
Created attachment 71154 [details] [review]
Bug 19066 - Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch
Comment 13 Kyle M Hall 2018-02-02 16:53:53 UTC
Created attachment 71155 [details] [review]
Bug 19066 - Update Schema
Comment 14 Kyle M Hall 2018-03-23 14:36:42 UTC
Created attachment 73185 [details] [review]
Bug 19066 - Update Database
Comment 15 Kyle M Hall 2018-03-23 14:36:50 UTC
Created attachment 73186 [details] [review]
Bug 19066 - Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch
Comment 16 Kyle M Hall 2018-03-23 14:36:52 UTC
Created attachment 73187 [details] [review]
Bug 19066 - Update Schema
Comment 17 M. Tompsett 2018-04-02 02:37:00 UTC
Comment on attachment 73186 [details] [review]
Bug 19066 - Add branchcode to accountlines

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

::: Koha/Account.pm
@@ +76,4 @@
>      my $account_type = $params->{account_type};
>      my $offset_type  = $params->{offset_type} || $type eq 'writeoff' ? 'Writeoff' : 'Payment';
>  
> +    $library_id ||= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;

why not below the $userenv below? Less referencing, shorter line, and easier to read. :)
Comment 18 Kyle M Hall 2018-04-03 11:36:52 UTC
(In reply to M. Tompsett from comment #17)
> Comment on attachment 73186 [details] [review] [review]
> Bug 19066 - Add branchcode to accountlines
> 
> Review of attachment 73186 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/Account.pm
> @@ +76,4 @@
> >      my $account_type = $params->{account_type};
> >      my $offset_type  = $params->{offset_type} || $type eq 'writeoff' ? 'Writeoff' : 'Payment';
> >  
> > +    $library_id ||= C4::Context->userenv ? C4::Context->userenv->{'branch'} : undef;
> 
> why not below the $userenv below? Less referencing, shorter line, and easier
> to read. :)

I'm not sure what you are referencing? Can you point me to the line numbers? Or if you'd rather submit a patch I'd be happy to sign it!
Comment 19 M. Tompsett 2018-04-04 02:07:38 UTC
Created attachment 73595 [details] [review]
Bug 19066: Follow up to reduce C4::Context references
Comment 20 Kyle M Hall 2018-04-04 10:45:34 UTC
(In reply to M. Tompsett from comment #19)
> Created attachment 73595 [details] [review] [review]
> Bug 19066: Follow up to reduce C4::Context references

Thanks!
Comment 21 Cab Vinton 2018-07-21 13:54:16 UTC
Issue with Sandbox 06 --

Some problems occurred applying patches from bug 19066:
<h1>Something went wrong !</h1>Applying: Bug 19066 - Update Database
Applying: Bug 19066 - Add branchcode to accountlines
.git/rebase-apply/patch:83: trailing whitespace.
         = C4::Context->userenv->{'number'} if C4::Context->userenv; 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M       C4/Accounts.pm
M       C4/Circulation.pm
M       C4/Reserves.pm
M       Koha/Account.pm
M       t/db_dependent/Accounts.t
.git/rebase-apply/patch:83: trailing whitespace.
         = C4::Context->userenv->{'number'} if C4::Context->userenv; 
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Accounts.t
CONFLICT (content): Merge conflict in t/db_dependent/Accounts.t
Auto-merging Koha/Account.pm
Auto-merging C4/Reserves.pm
Auto-merging C4/Circulation.pm
Auto-merging C4/Accounts.pm
error: Failed to merge in the changes.
Patch failed at 0001 Bug 19066 - Add branchcode to accountlines
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem run git bz apply --continue.
If you would prefer to skip this patch, instead run git bz apply --skip.
To restore the original branch and stop patching run git bz apply --abort.
Bug 19066 - Add branchcode to accountlines

73185 - Bug 19066 - Update Database
73186 - Bug 19066 - Add branchcode to accountlines
73187 - Bug 19066 - Update Schema
73595 - Bug 19066: Follow up to reduce C4::Context references

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-19066---Add-branchcode-to-accountlines-ArAGEA.patch
Comment 22 Kyle M Hall 2018-07-26 13:17:36 UTC
Created attachment 77281 [details] [review]
Bug 19066: Update Database
Comment 23 Kyle M Hall 2018-07-26 13:17:48 UTC
Created attachment 77282 [details] [review]
Bug 19066: Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch
Comment 24 Kyle M Hall 2018-07-26 13:17:52 UTC
Created attachment 77283 [details] [review]
Bug 19066: Update Schema
Comment 25 Cab Vinton 2018-08-30 03:13:38 UTC
Software error in PTFS Sandbox 0:

DBIx::Class::Storage::DBI::_dbh_execute(): Unknown column 'me.branchcode' in 'field list' at /home/koha/src/Koha/Objects.pm line 209
For help, please send mail to the webmaster (webmaster@ptfs-europe.co.uk), giving this error message and the time and date of the error.
Comment 26 Cab Vinton 2018-08-30 03:44:19 UTC
Re-tried & patch applied.
Comment 27 Lisette Scheer 2018-09-15 22:06:18 UTC
Created attachment 78900 [details] [review]
Bug 19066: Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 28 Lisette Scheer 2018-09-15 22:06:22 UTC
Created attachment 78901 [details] [review]
Bug 19066: Update Schema

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 29 Michal Denar 2018-09-16 09:06:39 UTC
Please rebase on master

Error:
CONFLICT (content): Merge conflict in Koha/Schema/Result/Accountline.pm


Thank You


Mike
Comment 30 Tomás Cohen Arazi 2018-09-16 14:32:29 UTC
Please call it library_id, following the latest use.
Comment 31 Kyle M Hall 2018-10-25 12:52:06 UTC
Created attachment 81144 [details] [review]
Bug 19066: Update Database
Comment 32 Kyle M Hall 2018-10-25 12:52:16 UTC
Created attachment 81145 [details] [review]
Bug 19066: Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>
Comment 33 Kyle M Hall 2018-10-25 12:52:20 UTC
Created attachment 81146 [details] [review]
Bug 19066: Update Schema
Comment 34 Jonathan Druart 2018-10-25 20:01:20 UTC
Same as bug 21401.
Comment 35 Josef Moravec 2018-10-26 08:22:01 UTC
Created attachment 81253 [details] [review]
Bug 19066: Update Database

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 36 Josef Moravec 2018-10-26 08:22:10 UTC
Created attachment 81254 [details] [review]
Bug 19066: Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 37 Josef Moravec 2018-10-26 08:22:19 UTC
Created attachment 81255 [details] [review]
Bug 19066: Update Schema

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 38 Jonathan Druart 2018-10-26 11:27:33 UTC
(In reply to Tomás Cohen Arazi from comment #30)
> Please call it library_id, following the latest use.

This has been discussed on bug 21401 comment 14.
Comment 39 Josef Moravec 2018-10-31 07:27:55 UTC
Created attachment 81684 [details] [review]
Bug 19066: (QA follow-up) Move db update to perl
Comment 40 Martin Renvoize 2018-11-02 12:07:05 UTC
Hmm.. I'm working on a push to using Koha::Accounts more.. see the graph dangling off bug 21002 .

Attempting to rebase my work on top of this I can see at least a couple of places where we're missing things here.. I'm going to set to failed qa whilst I add the followups.
Comment 41 Martin Renvoize 2018-11-02 15:09:00 UTC
Koha::Account->add_credit isn't handled
Comment 42 Martin Renvoize 2018-11-02 16:11:09 UTC
Created attachment 81912 [details] [review]
Bug 19066: (QA follow-up) Add handling in add_credit
Comment 43 Martin Renvoize 2018-11-02 16:12:19 UTC
Back to Signed Off.. Tomas, did you fancy taking a look at my follow-up and verifying the add_credit use.. do we need to also account for library_id in the API endpoints.
Comment 44 Tomás Cohen Arazi 2018-11-02 17:51:58 UTC
(In reply to Martin Renvoize from comment #43)
> Back to Signed Off.. Tomas, did you fancy taking a look at my follow-up and
> verifying the add_credit use.. do we need to also account for library_id in
> the API endpoints.

Everything looks correct. But t/db_dependent/Accounts.t is failing.
Comment 45 Kyle M Hall 2018-11-02 18:39:19 UTC
Created attachment 81918 [details] [review]
Bug 19066: (QA follow-up) Remove bad and unecessary test
Comment 46 Tomás Cohen Arazi 2018-11-07 19:53:12 UTC
With Kyle's permission, I'm taking this one. I will complete the tests, so they cover all the changes.
Comment 47 Tomás Cohen Arazi 2018-11-07 20:09:14 UTC Comment hidden (obsolete)
Comment 48 Tomás Cohen Arazi 2018-11-07 20:09:19 UTC Comment hidden (obsolete)
Comment 49 Tomás Cohen Arazi 2018-11-08 14:19:24 UTC
Created attachment 82084 [details] [review]
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee

This patch adds some tests that cover functions changed by this
patchset. A bug in ChargeReserveFee is highlighted.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 50 Tomás Cohen Arazi 2018-11-08 14:19:34 UTC
Created attachment 82086 [details] [review]
Bug 19066: (QA follow-up) Fix ChargeReserveFee

This patch fixes a bug in ChargeReserveFee:

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Reserves.t
=> FAIL: Tests fail because branchcode is not set
- Apply this patch
- Run:
 k$ prove t/db_dependent/Reserves.t
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 51 Tomás Cohen Arazi 2018-11-08 14:19:43 UTC
Created attachment 82087 [details] [review]
Bug 19066: Add tests for Koha::Account->pay

This patch adds tests for Koha::Account->pay, to make sure the
library_id param is used to set the branchcode attribute. The use of
userenv to set branchcode if library_id not passed is removed.
Responsability is left to the callers to pass library_id.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
Comment 52 Martin Renvoize 2018-11-13 20:04:27 UTC
With all the followups this is looking solid to me now.. I'd happily add a second QA stamp to the whole set.
Comment 53 Tomás Cohen Arazi 2018-11-19 12:22:50 UTC
Created attachment 82469 [details] [review]
Bug 19066: Update Database

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 54 Tomás Cohen Arazi 2018-11-19 12:22:58 UTC
Created attachment 82470 [details] [review]
Bug 19066: Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 55 Tomás Cohen Arazi 2018-11-19 12:23:06 UTC
Created attachment 82471 [details] [review]
Bug 19066: Update Schema

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 56 Tomás Cohen Arazi 2018-11-19 12:23:13 UTC
Created attachment 82473 [details] [review]
Bug 19066: (QA follow-up) Move db update to perl
Comment 57 Tomás Cohen Arazi 2018-11-19 12:23:22 UTC
Created attachment 82474 [details] [review]
Bug 19066: (QA follow-up) Add handling in add_credit
Comment 58 Tomás Cohen Arazi 2018-11-19 12:23:28 UTC
Created attachment 82475 [details] [review]
Bug 19066: (QA follow-up) Remove bad and unecessary test
Comment 59 Tomás Cohen Arazi 2018-11-19 12:23:38 UTC
Created attachment 82476 [details] [review]
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee

This patch adds some tests that cover functions changed by this
patchset. A bug in ChargeReserveFee is highlighted.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 60 Tomás Cohen Arazi 2018-11-19 12:23:46 UTC
Created attachment 82477 [details] [review]
Bug 19066: (QA follow-up) Fix ChargeReserveFee

This patch fixes a bug in ChargeReserveFee:

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Reserves.t
=> FAIL: Tests fail because branchcode is not set
- Apply this patch
- Run:
 k$ prove t/db_dependent/Reserves.t
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 61 Tomás Cohen Arazi 2018-11-19 12:23:54 UTC
Created attachment 82478 [details] [review]
Bug 19066: Add tests for Koha::Account->pay

This patch adds tests for Koha::Account->pay, to make sure the
library_id param is used to set the branchcode attribute. The use of
userenv to set branchcode if library_id not passed is removed.
Responsability is left to the callers to pass library_id.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
Comment 62 Martin Renvoize 2018-12-12 08:55:27 UTC
Created attachment 83067 [details] [review]
Bug 19066: Update Database

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 63 Martin Renvoize 2018-12-12 08:55:32 UTC
Created attachment 83068 [details] [review]
Bug 19066: Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 64 Martin Renvoize 2018-12-12 08:55:37 UTC
Created attachment 83069 [details] [review]
Bug 19066: Update Schema

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 65 Martin Renvoize 2018-12-12 08:55:41 UTC
Created attachment 83070 [details] [review]
Bug 19066: (QA follow-up) Move db update to perl
Comment 66 Martin Renvoize 2018-12-12 08:55:45 UTC
Created attachment 83071 [details] [review]
Bug 19066: (QA follow-up) Add handling in add_credit
Comment 67 Martin Renvoize 2018-12-12 08:55:49 UTC
Created attachment 83072 [details] [review]
Bug 19066: (QA follow-up) Remove bad and unecessary test
Comment 68 Martin Renvoize 2018-12-12 08:55:54 UTC
Created attachment 83073 [details] [review]
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee

This patch adds some tests that cover functions changed by this
patchset. A bug in ChargeReserveFee is highlighted.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 69 Martin Renvoize 2018-12-12 08:55:58 UTC
Created attachment 83074 [details] [review]
Bug 19066: (QA follow-up) Fix ChargeReserveFee

This patch fixes a bug in ChargeReserveFee:

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Reserves.t
=> FAIL: Tests fail because branchcode is not set
- Apply this patch
- Run:
 k$ prove t/db_dependent/Reserves.t
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 70 Martin Renvoize 2018-12-12 08:56:04 UTC
Created attachment 83075 [details] [review]
Bug 19066: Add tests for Koha::Account->pay

This patch adds tests for Koha::Account->pay, to make sure the
library_id param is used to set the branchcode attribute. The use of
userenv to set branchcode if library_id not passed is removed.
Responsability is left to the callers to pass library_id.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!
Comment 71 Martin Renvoize 2018-12-12 08:58:43 UTC Comment hidden (obsolete)
Comment 72 Martin Renvoize 2018-12-12 08:58:48 UTC Comment hidden (obsolete)
Comment 73 Martin Renvoize 2018-12-12 08:58:53 UTC Comment hidden (obsolete)
Comment 74 Martin Renvoize 2018-12-12 08:58:58 UTC Comment hidden (obsolete)
Comment 75 Martin Renvoize 2018-12-12 08:59:02 UTC Comment hidden (obsolete)
Comment 76 Martin Renvoize 2018-12-12 08:59:07 UTC Comment hidden (obsolete)
Comment 77 Martin Renvoize 2018-12-12 08:59:12 UTC Comment hidden (obsolete)
Comment 78 Martin Renvoize 2018-12-12 08:59:16 UTC Comment hidden (obsolete)
Comment 79 Martin Renvoize 2018-12-12 08:59:22 UTC Comment hidden (obsolete)
Comment 80 Martin Renvoize 2018-12-12 09:00:47 UTC Comment hidden (obsolete)
Comment 81 Martin Renvoize 2018-12-17 09:42:42 UTC Comment hidden (obsolete)
Comment 82 Martin Renvoize 2018-12-17 11:17:27 UTC Comment hidden (obsolete)
Comment 83 Alex Arnaud 2018-12-27 14:07:27 UTC
Created attachment 83519 [details] [review]
Bug 19066: Update Database

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 84 Alex Arnaud 2018-12-27 14:07:39 UTC
Created attachment 83520 [details] [review]
Bug 19066: Add branchcode to accountlines

For the purposes of statistics, it appears that it would help many
libraries to have branchcode recorded in the accountlines table. For
payments, the field would contain the code for the branch the payment
was made at. For manual invoices, it would be the code of the library
that created the invoice.

Test Plan:
1) Apply this patch set
2) Create and pay some fees
3) Note the branchcode for those fees and payments is set
   to your logged in branch

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Lisette Scheer <lisetteslatah@gmail.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 85 Alex Arnaud 2018-12-27 14:07:51 UTC
Created attachment 83521 [details] [review]
Bug 19066: Update Schema

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 86 Alex Arnaud 2018-12-27 14:08:03 UTC
Created attachment 83522 [details] [review]
Bug 19066: (QA follow-up) Move db update to perl

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 87 Alex Arnaud 2018-12-27 14:08:15 UTC
Created attachment 83523 [details] [review]
Bug 19066: (QA follow-up) Add handling in add_credit

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 88 Alex Arnaud 2018-12-27 14:08:30 UTC
Created attachment 83524 [details] [review]
Bug 19066: (QA follow-up) Remove bad and unecessary test

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 89 Alex Arnaud 2018-12-27 14:08:42 UTC
Created attachment 83525 [details] [review]
Bug 19066: Tests for AddRenewal AddIssuingCharge and ChargeReserveFee

This patch adds some tests that cover functions changed by this
patchset. A bug in ChargeReserveFee is highlighted.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 90 Alex Arnaud 2018-12-27 14:08:54 UTC
Created attachment 83526 [details] [review]
Bug 19066: (QA follow-up) Fix ChargeReserveFee

This patch fixes a bug in ChargeReserveFee:

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Reserves.t
=> FAIL: Tests fail because branchcode is not set
- Apply this patch
- Run:
 k$ prove t/db_dependent/Reserves.t
=> SUCCESS: Tests pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 91 Alex Arnaud 2018-12-27 14:09:06 UTC
Created attachment 83527 [details] [review]
Bug 19066: Add tests for Koha::Account->pay

This patch adds tests for Koha::Account->pay, to make sure the
library_id param is used to set the branchcode attribute. The use of
userenv to set branchcode if library_id not passed is removed.
Responsability is left to the callers to pass library_id.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Koha/Account.t
=> SUCCESS: Tests pass!

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 92 Alex Arnaud 2018-12-27 14:09:18 UTC
Created attachment 83528 [details] [review]
Bug 19066: (QA follow-up) Define foreign key for branchcode

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 93 Alex Arnaud 2018-12-27 14:11:08 UTC
Rebased against master
Comment 94 Nick Clemens 2019-01-03 19:07:12 UTC
Awesome work all!

Pushed to master for 19.05
Comment 95 Nick Clemens 2019-01-03 19:08:02 UTC
Created attachment 83628 [details] [review]
Bug 19066: (RM follow-up) Fix test count and structure error

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 96 Martin Renvoize 2019-01-07 15:42:58 UTC
Enhancement, will not be backported to 18.11.x series