Bug 19489 - Detailed description of charges on Accounting tab
Summary: Detailed description of charges on Accounting tab
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Fines and fees (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Devinim
QA Contact: Josef Moravec
URL:
Whiteboard:
Keywords:
Depends on: 22006
Blocks: 14825 15985 21879
  Show dependency treegraph
 
Reported: 2017-10-18 16:39 UTC by Kyle M Hall
Modified: 2019-06-04 10:43 UTC (History)
10 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 19489 - Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab (14.41 KB, patch)
2018-11-05 14:20 UTC, Devinim
Details | Diff | Splinter Review
(Object method) Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab (13.89 KB, patch)
2018-11-06 14:49 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489 - (Object Method) Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab (13.86 KB, patch)
2018-11-06 15:13 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489 - (Object Method) Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab (1.14 KB, patch)
2018-11-19 16:49 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489 - (Object Method) Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab (16.07 KB, patch)
2018-11-19 17:02 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: Improvements in Unit test and Koha::Account::Line->issue method (2.97 KB, patch)
2018-11-23 11:59 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: Development for fines and Fees Module (12.24 KB, patch)
2018-11-23 13:21 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: Development for fines and Fees Module (12.32 KB, patch)
2018-12-04 15:44 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: (Improvements in Unit test and Koha::Account::Line->issue method) (3.02 KB, patch)
2018-12-12 12:53 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19489: Development for fines and Fees Module (12.38 KB, patch)
2018-12-12 12:53 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19489: Simplify the code a bit (1.23 KB, patch)
2018-12-14 19:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19489: (Improvements in KohaDate) Development for fines and Fees Module (6.08 KB, patch)
2018-12-17 12:49 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: (Improvements in KohaDate) Development for fines and Fees Module (12.33 KB, patch)
2018-12-17 13:21 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: Add the Koha::Account::Line->issue method (3.00 KB, patch)
2018-12-17 15:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19489: Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab (12.62 KB, patch)
2018-12-17 15:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19489: Cover non-existent issue (1.14 KB, patch)
2018-12-17 15:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19489: Add the Koha::Account::Line->issue method (3.03 KB, patch)
2019-01-03 14:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19489: Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab (12.60 KB, patch)
2019-01-03 14:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19489: Cover non-existent issue (1.22 KB, patch)
2019-01-03 14:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 19489: (Rebased) Koha::Account::Line->issue method and Unit test (3.10 KB, patch)
2019-02-18 09:49 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: (Rebased) Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab (13.86 KB, patch)
2019-02-18 09:52 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: Change method name (8.51 KB, patch)
2019-02-19 12:47 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: (follow-up) Fix typos in the tests (2.04 KB, patch)
2019-02-20 07:22 UTC, Devinim
Details | Diff | Splinter Review
Bug 19489: Koha::Account::Line->issue method and Unit test (3.12 KB, patch)
2019-02-25 21:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19489: Detailed Description of charges in Patron accounting (13.79 KB, patch)
2019-02-25 21:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19489: Change method name issue --> checkout (8.53 KB, patch)
2019-02-25 21:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19489: (follow-up) Fix typos in the tests (2.08 KB, patch)
2019-02-25 21:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19489: (QA follow-up) Cache checkout object in variable (982 bytes, patch)
2019-02-25 21:48 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19489: (QA follow-up) Update test to use objects and module methods for creating needed data (3.08 KB, patch)
2019-02-25 21:49 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19489: (QA follow-up) Remove passing issue_id in pay form (2.10 KB, patch)
2019-02-25 21:49 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19489: (QA follow-up) Revert changes in printinvoice (3.66 KB, patch)
2019-02-25 21:49 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19489: Koha::Account::Line->issue method and Unit test (3.20 KB, patch)
2019-02-26 19:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19489: Detailed Description of charges in Patron accounting (13.90 KB, patch)
2019-02-26 19:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19489: Change method name issue --> checkout (8.58 KB, patch)
2019-02-26 19:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19489: (follow-up) Fix typos in the tests (2.16 KB, patch)
2019-02-26 19:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19489: (QA follow-up) Cache checkout object in variable (1.04 KB, patch)
2019-02-26 19:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19489: (QA follow-up) Update test to use objects and module methods for creating needed data (3.16 KB, patch)
2019-02-26 19:12 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19489: (QA follow-up) Remove passing issue_id in pay form (2.18 KB, patch)
2019-02-26 19:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 19489: (QA follow-up) Revert changes in printinvoice (3.73 KB, patch)
2019-02-26 19:13 UTC, Kyle M Hall
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-10-18 16:39:58 UTC
Some libraries would like to see the item barcode, due date, and return date ( if any ) for fines in the accounts tables related to fines ( pay.pl and boraccount.pl ).
Comment 1 Devinim 2018-11-05 14:20:29 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2018-11-06 12:07:48 UTC
Comment on attachment 81956 [details] [review]
Bug 19489 - Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab

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

Everything works nicely in testing, the columns appear as expected.

The method for checkouts itself should be an object method for Koha::Account::Line

It would be nice too if there were some refactoring to simply pass the Line objects and use the fields/method on the the template rather than formatting in a hash in the scripts

::: members/accountline-details.pl
@@ +24,5 @@
>  use C4::Output;
>  use C4::Context;
>  use Koha::Patrons;
> +use Koha::Checkouts;
> +use Koha::Old::Checkouts;

Rather than importing these here we should implement 'checkout' as an object method for the Koha::Account::Line

@@ +67,5 @@
>      my $patron = Koha::Patrons->find( $accountline->borrowernumber );
> +    my $issues;
> +    if ( $accountline->issue_id ) {
> +        $issues = Koha::Checkouts->find($accountline->issue_id );
> +        $issues = Koha::Old::Checkouts->find($accountline->issue_id ) unless $issues;

As mentioned above - this should use an object method from Koha::Account::Line

The plural is also confusing as there should on ly ever be one

$accountline->checkout;

::: members/boraccount.pl
@@ +112,5 @@
> +
> +    if ( $accountline->{issue_id} ) {
> +        my $issues = Koha::Checkouts->find($accountline->{issue_id} );
> +        $issues = Koha::Old::Checkouts->find($accountline->{issue_id} ) unless $issues;
> +        $accountline->{issue}        = $issues;

As before, this should use an object method

::: members/pay.pl
@@ +152,5 @@
>          }
> +        if ( $account_line->{issue_id} ) {
> +            my $issues = Koha::Checkouts->find($account_line->{issue_id} );
> +            $issues = Koha::Old::Checkouts->find($account_line->{issue_id} ) unless $issues;
> +            $account_line->{issue}        = $issues;

Use object method

::: members/printinvoice.pl
@@ +64,5 @@
> +
> +if ( $accountline->{issue_id} ) {
> +    my $issues = Koha::Checkouts->find($accountline->{issue_id} );
> +    $issues = Koha::Old::Checkouts->find($accountline->{issue_id} ) unless $issues;
> +    $accountline->{issue}        = $issues;

Use object method
Comment 3 Devinim 2018-11-06 14:49:03 UTC Comment hidden (obsolete)
Comment 4 Devinim 2018-11-06 15:13:49 UTC Comment hidden (obsolete)
Comment 5 Devinim 2018-11-19 16:49:49 UTC Comment hidden (obsolete)
Comment 6 Devinim 2018-11-19 17:02:07 UTC Comment hidden (obsolete)
Comment 7 Devinim 2018-11-23 11:59:57 UTC Comment hidden (obsolete)
Comment 8 Devinim 2018-11-23 13:21:28 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2018-12-04 13:18:04 UTC
This seems to be working, but the test doesn't pass:

t/db_dependent/Koha/Account/Lines.t .. 1/5
#   Failed test 'No tests run for subtest "item() tests"'
#   at t/db_dependent/Koha/Account/Lines.t line 67.
DBIx::Class::Storage::DBI::_dbh_execute(): Field 'account_balance' doesn't have a default value at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 288
# Looks like your test exited with 255 just after 1.
t/db_dependent/Koha/Account/Lines.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 5/5 subtests

Test Summary Report
-------------------
t/db_dependent/Koha/Account/Lines.t (Wstat: 65280 Tests: 1 Failed: 1)
  Failed test:  1
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 5 tests but ran 1.
Files=1, Tests=1,  3 wallclock secs ( 0.01 usr  0.01 sys +  1.21 cusr  0.52 csys =  1.75 CPU)
Result: FAIL
Comment 10 Devinim 2018-12-04 13:32:33 UTC
Hi Owen,

It is most probably another patch made it this since we've tested before sending patches and unit tests. 

Our developers is looking once again, will change it soon.

Thanks a lot.

Devinim Koha Team
Comment 11 Devinim 2018-12-04 14:42:05 UTC
(In reply to Owen Leonard from comment #9)
> This seems to be working, but the test doesn't pass:
> 
> t/db_dependent/Koha/Account/Lines.t .. 1/5
> #   Failed test 'No tests run for subtest "item() tests"'
> #   at t/db_dependent/Koha/Account/Lines.t line 67.
> DBIx::Class::Storage::DBI::_dbh_execute(): Field 'account_balance' doesn't
> have a default value at /home/vagrant/kohaclone/t/lib/TestBuilder.pm line 288
> # Looks like your test exited with 255 just after 1.
> t/db_dependent/Koha/Account/Lines.t .. Dubious, test returned 255 (wstat
> 65280, 0xff00)
> Failed 5/5 subtests
> 
> Test Summary Report
> -------------------
> t/db_dependent/Koha/Account/Lines.t (Wstat: 65280 Tests: 1 Failed: 1)
>   Failed test:  1
>   Non-zero exit status: 255
>   Parse errors: Bad plan.  You planned 5 tests but ran 1.
> Files=1, Tests=1,  3 wallclock secs ( 0.01 usr  0.01 sys +  1.21 cusr  0.52
> csys =  1.75 CPU)
> Result: FAIL

We applied the test on the latest provisioned kohadevbox environment and we found that there are some changes to pay.tt .
When we changed pay.tt manually we passed dependency test as you may see below.

prove t/db_dependent/Koha/Account/Lines.t

t/db_dependent/Koha/Account/Lines.t .. ok   
All tests successful.
Files=1, Tests=5,  5 wallclock secs ( 0.03 usr  0.01 sys +  3.90 cusr  0.66 csys =  4.60 CPU)
Result: PASS


How did you get your errors?

Thanks,
Comment 12 Devinim 2018-12-04 15:44:59 UTC Comment hidden (obsolete)
Comment 13 Nick Clemens 2018-12-12 12:53:10 UTC
Created attachment 83109 [details] [review]
Bug 19489: (Improvements in Unit test and Koha::Account::Line->issue method)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Nick Clemens 2018-12-12 12:53:14 UTC Comment hidden (obsolete)
Comment 15 Jonathan Druart 2018-12-13 18:59:40 UTC
due_date must be displayed with the as_due_date flag, not with_hours:

[% your_due_date | $KohaDates as_due_date => 1 %]
Comment 16 Jonathan Druart 2018-12-13 19:02:41 UTC
2. As well, the html filter is not needed when already KohaDates filtered.

3. in printinvoice.pl

+$accountline->{'item'} = $accountline->{itemnumber} ? $accountline_object->item : "" ;
+$accountline->{'issue'} = $accountline->{issue_id} ? $accountline_object->issue : "" ;

It read wrong, why not:
  $accountline->{item} = $accountline_object->item || "" ;
  $accountline->{issue} = $accountline_object->issue || "";
?
Comment 17 Devinim 2018-12-14 10:52:52 UTC
(In reply to Jonathan Druart from comment #15)
> due_date must be displayed with the as_due_date flag, not with_hours:
> 
> [% your_due_date | $KohaDates as_due_date => 1 %]

Hi Jonathan,

In order to make code integrity, we've looked at 
members/readingrec.tt 
Line 95 of that file also uses due_date.
Comment 18 Devinim 2018-12-14 11:02:08 UTC
(In reply to Jonathan Druart from comment #16)
> 2. As well, the html filter is not needed when already KohaDates filtered.
> 
> 3. in printinvoice.pl
> 
> +$accountline->{'item'} = $accountline->{itemnumber} ?
> $accountline_object->item : "" ;
> +$accountline->{'issue'} = $accountline->{issue_id} ?
> $accountline_object->issue : "" ;
> 
> It read wrong, why not:
>   $accountline->{item} = $accountline_object->item || "" ;
>   $accountline->{issue} = $accountline_object->issue || "";
> ?

2. As similar to answer 1, some other files also use html filter like
reserve/request.tt
catalogue/issuehistory.tt


3. If you choose Create manual invoice tab and then select a type without using a barcode number (like New Card), since there won't be an itemnumber in accountline table, item object will give an error like (  DBIC result _type isn't of the _type Item at /home/vagrant/kohaclone/members/printinvoice.pl line 60) .
Comment 19 Jonathan Druart 2018-12-14 19:11:58 UTC
(In reply to Devinim from comment #17)
> (In reply to Jonathan Druart from comment #15)
> > due_date must be displayed with the as_due_date flag, not with_hours:
> > 
> > [% your_due_date | $KohaDates as_due_date => 1 %]
> 
> Hi Jonathan,
> 
> In order to make code integrity, we've looked at 
> members/readingrec.tt 
> Line 95 of that file also uses due_date.

It is wrong :)
IIRC due dates must be displayed using the as_due_date flag, to not show the time part if 23:59

(In reply to Devinim from comment #18)
> (In reply to Jonathan Druart from comment #16)
> > 2. As well, the html filter is not needed when already KohaDates filtered.
> 
> 2. As similar to answer 1, some other files also use html filter like
> reserve/request.tt
> catalogue/issuehistory.tt

It is because we removed the html filter when the $KohaDates occurrences were used without parameters. It is not a big deal anyway but we could avoid unnecessary processing removing these filters. I am going to deal with that on a separate bug report.

> > 3. in printinvoice.pl
> > 
> > +$accountline->{'item'} = $accountline->{itemnumber} ?
> > $accountline_object->item : "" ;
> > +$accountline->{'issue'} = $accountline->{issue_id} ?
> > $accountline_object->issue : "" ;
> > 
> > It read wrong, why not:
> >   $accountline->{item} = $accountline_object->item || "" ;
> >   $accountline->{issue} = $accountline_object->issue || "";
> > ?
> >
> 3. If you choose Create manual invoice tab and then select a type without
> using a barcode number (like New Card), since there won't be an itemnumber
> in accountline table, item object will give an error like (  DBIC result
> _type isn't of the _type Item at
> /home/vagrant/kohaclone/members/printinvoice.pl line 60) .

It's a bug, I have opened bug 22006
Comment 20 Jonathan Druart 2018-12-14 19:19:23 UTC Comment hidden (obsolete)
Comment 21 Devinim 2018-12-17 12:49:25 UTC Comment hidden (obsolete)
Comment 22 Devinim 2018-12-17 13:21:39 UTC
Created attachment 83277 [details] [review]
Bug 19489: (Improvements in KohaDate) Development for fines and Fees Module
Comment 23 Jonathan Druart 2018-12-17 15:18:52 UTC
Created attachment 83299 [details] [review]
Bug 19489: Add the Koha::Account::Line->issue method

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 24 Jonathan Druart 2018-12-17 15:18:57 UTC
Created attachment 83300 [details] [review]
Bug 19489: Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab

Test plan:
1) Apply this patch
2) Choose a patron with overdue items applied fines
3) Click Fines from left menu
4) Test the values in barcode, due date and return date in Pay fines and
Account tabs
5) Also test Column visibility, Print and Details buttons for
functioning
Comment 25 Jonathan Druart 2018-12-17 15:19:02 UTC
Created attachment 83301 [details] [review]
Bug 19489: Cover non-existent issue
Comment 26 Jonathan Druart 2018-12-17 15:20:44 UTC
I have updated the commit message, re-added the test plan and submitted a follow-up to cover the non-existing issue case.
Comment 27 Nick Clemens 2019-01-03 14:32:36 UTC
Created attachment 83618 [details] [review]
Bug 19489: Add the Koha::Account::Line->issue method

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 28 Nick Clemens 2019-01-03 14:32:39 UTC
Created attachment 83619 [details] [review]
Bug 19489: Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab

Test plan:
1) Apply this patch
2) Choose a patron with overdue items applied fines
3) Click Fines from left menu
4) Test the values in barcode, due date and return date in Pay fines and
Account tabs
5) Also test Column visibility, Print and Details buttons for
functioning

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 29 Nick Clemens 2019-01-03 14:32:43 UTC
Created attachment 83620 [details] [review]
Bug 19489: Cover non-existent issue

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 30 Nick Clemens 2019-01-03 14:34:08 UTC
Cleared minor conflict when applying (test counts)
Comment 31 Katrin Fischer 2019-02-17 11:38:16 UTC
Hi Devinim, can we please get a rebase? Thx!
Comment 32 Devinim 2019-02-18 09:49:04 UTC
Created attachment 85236 [details] [review]
Bug 19489: (Rebased) Koha::Account::Line->issue method and Unit test

Test Plan:
1) Apply this patch
2) prove t/db_dependent/Koha/Account/Lines.t
Comment 33 Devinim 2019-02-18 09:52:22 UTC
Created attachment 85237 [details] [review]
Bug 19489: (Rebased) Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab

Test plan:
1) Apply this patch
2) Choose a patron with overdue items applied fines
3) Click Fines from left menu
4) Test the values in barcode, due date and return date in Pay fines and
Account tabs
5) Also test Column visibility, Print and Details buttons for
functioning
Comment 34 Katrin Fischer 2019-02-18 10:08:54 UTC
Depending on how difficult the rebase was, it's ok to set back to "Signed off" after rebasing. If you feel it should be tested from scratch (hard rebase), setting to "Needs Signoff" is of course better.

We usually keep the sign-off lines on the patches - it shows that someone spent time on it already. :)
Comment 35 Devinim 2019-02-19 06:35:36 UTC
Hi Katrin,
As you suggested I changed the Signed Off.

Thanks.
Comment 36 Josef Moravec 2019-02-19 07:56:18 UTC
Comment on attachment 85236 [details] [review]
Bug 19489: (Rebased) Koha::Account::Line->issue method and Unit test

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

::: Koha/Account/Line.pm
@@ +55,5 @@
>  }
>  
> +=head3 issue
> +
> +Return the item linked to this account line if exists

item?

@@ +59,5 @@
> +Return the item linked to this account line if exists
> +
> +=cut
> +
> +sub issue {

According to Koha terminology, this should be named "checkout"
Comment 37 Josef Moravec 2019-02-19 08:02:13 UTC
Comment on attachment 85237 [details] [review]
Bug 19489: (Rebased) Development for fines and Fees Module - Detailed Description of charges in Patron Account Tab/Fines Tab

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

::: members/printinvoice.pl
@@ +57,5 @@
>  my $total = $patron->account->balance;
> +my $accountline_object = Koha::Account::Lines->find($accountlines_id);
> +my $accountline = $accountline_object->unblessed;
> +$accountline->{item} = $accountline_object->item || "" ;
> +$accountline->{issue} = $accountline_object->issue || "";

We are still only adding this non-objective code, but as this file is still not fully refactored to using objects it is not blocker from me.
Comment 38 Josef Moravec 2019-02-19 08:02:58 UTC
Failed because of issue method name. It really should be changed
Comment 39 Devinim 2019-02-19 12:47:18 UTC
Created attachment 85275 [details] [review]
Bug 19489: Change method name
Comment 40 Devinim 2019-02-20 07:22:56 UTC
Created attachment 85315 [details] [review]
Bug 19489: (follow-up) Fix typos in the tests
Comment 41 Josef Moravec 2019-02-25 21:48:35 UTC
Created attachment 85668 [details] [review]
Bug 19489: Koha::Account::Line->issue method and Unit test

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 42 Josef Moravec 2019-02-25 21:48:40 UTC
Created attachment 85669 [details] [review]
Bug 19489: Detailed Description of charges in Patron accounting

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 43 Josef Moravec 2019-02-25 21:48:44 UTC
Created attachment 85670 [details] [review]
Bug 19489: Change method name issue --> checkout

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 44 Josef Moravec 2019-02-25 21:48:49 UTC
Created attachment 85671 [details] [review]
Bug 19489: (follow-up) Fix typos in the tests

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 45 Josef Moravec 2019-02-25 21:48:54 UTC
Created attachment 85672 [details] [review]
Bug 19489: (QA follow-up) Cache checkout object in variable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 46 Josef Moravec 2019-02-25 21:49:00 UTC
Created attachment 85673 [details] [review]
Bug 19489: (QA follow-up) Update test to use objects and module methods for creating needed data

Test plan:
prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 47 Josef Moravec 2019-02-25 21:49:05 UTC
Created attachment 85674 [details] [review]
Bug 19489: (QA follow-up) Remove passing issue_id in pay form

This is not needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 48 Josef Moravec 2019-02-25 21:49:11 UTC
Created attachment 85675 [details] [review]
Bug 19489: (QA follow-up) Revert changes in printinvoice

It would be change behaviour, libraries are often using POS printers and
wider table could make a problem for them and break theirs workflow.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 49 Josef Moravec 2019-02-25 21:50:42 UTC
I added some follow-ups and am leaving this in signed off state. Would like to have another QA team member to look here.
Comment 50 Kyle M Hall 2019-02-26 19:12:24 UTC
Created attachment 85731 [details] [review]
Bug 19489: Koha::Account::Line->issue method and Unit test

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 51 Kyle M Hall 2019-02-26 19:12:36 UTC
Created attachment 85732 [details] [review]
Bug 19489: Detailed Description of charges in Patron accounting

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 52 Kyle M Hall 2019-02-26 19:12:41 UTC
Created attachment 85733 [details] [review]
Bug 19489: Change method name issue --> checkout

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 53 Kyle M Hall 2019-02-26 19:12:46 UTC
Created attachment 85734 [details] [review]
Bug 19489: (follow-up) Fix typos in the tests

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 54 Kyle M Hall 2019-02-26 19:12:50 UTC
Created attachment 85735 [details] [review]
Bug 19489: (QA follow-up) Cache checkout object in variable

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 55 Kyle M Hall 2019-02-26 19:12:55 UTC
Created attachment 85736 [details] [review]
Bug 19489: (QA follow-up) Update test to use objects and module methods for creating needed data

Test plan:
prove t/db_dependent/Koha/Account/Lines.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 56 Kyle M Hall 2019-02-26 19:13:00 UTC
Created attachment 85737 [details] [review]
Bug 19489: (QA follow-up) Remove passing issue_id in pay form

This is not needed

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 57 Kyle M Hall 2019-02-26 19:13:06 UTC
Created attachment 85738 [details] [review]
Bug 19489: (QA follow-up) Revert changes in printinvoice

It would be change behaviour, libraries are often using POS printers and
wider table could make a problem for them and break theirs workflow.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 58 Nick Clemens 2019-03-11 13:11:02 UTC
Awesome work all!

Pushed to master for 19.05
Comment 59 Martin Renvoize 2019-03-12 09:40:56 UTC
Enhancement will not be backported to 18.11.x series.