Bug 2454

Summary: Improve display of credits on pay fines tab
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: CirculationAssignee: Alex Buckley <alexbuckley>
Status: Signed Off --- QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: P3 CC: alexbuckley, clackman, dominic.pichette, felix.hemme, jonathan.druart, katrin.fischer, koha.sekjal, marjorie.barry-vila, mtompset, veron
Version: master   
Hardware: PC   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6427
Whiteboard:
Change sponsored?: --- Patch complexity: ---
Bot Control: --- When did the bot last check this:
Who signed the patch off: Text to go in the release notes:
Attachments: manual credits on pay fines tab
credit showing on pay page
Bug 2545 - Removed credits being shown on the Pay fines tab.
Screenshot example of pay fines tab after patches for this bug are applied
Bug 2454 - Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"
Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"
Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"
[SIGNED OFF] Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"
Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"
Bug 2454 - Removed the display of NotifyID and Level values on Pay fines page
Bug 2454 - Removed the display of NotifyID and Level values on Pay fines page

Description Chris Cormack 2010-05-21 00:51:22 UTC


---- Reported by nengard@gmail.com 2008-08-04 05:56:29 ----

When you view the Pay Fines tab on the patron record, manual credits appear on the list - they shouldn't be there.  See sceenshot.

CHAT:

[08:46]  <nengard> rch - why show the credit line on the 'pay fines' tab at all??  http://git.koha.org/cgi-bin/gitweb.cgi?p=Koha;a=commit;h=dd8bc5076865912e8885a0d87f18a229d680d343 
[08:50]  <rch> nengard: good question.  i can't think of a good answer.
[08:51]  <nengard> rch hehe :)
[08:51]  <nengard> you want me to put an enhancement in?
[08:51]  <rch> nengard: sure



---- Additional Comments From nengard@gmail.com 2008-08-04 05:56:47 ----

Created an attachment
manual credits on pay fines tab





---- Additional Comments From nengard@gmail.com 2009-04-19 18:13:42 ----

Ryan, something to think about while re-writing accounts & fines.



---- Additional Comments From nengard@gmail.com 2009-12-20 21:19:47 ----

Created an attachment
credit showing on pay page

This is still a problem in the most recent version of HEAD.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:51 UTC  ---

This bug was previously known as _bug_ 2454 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2454
Imported an attachment (id=672)
Imported an attachment (id=673)

Actual time not defined. Setting to 0.0
CC member bchurch@ptfs.com does not have an account here
CC member rch@liblime.com does not have an account here
The original submitter of attachment 672 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.
The original submitter of attachment 673 [details] is unknown.
   Reassigning to the person who moved it here: chris@bigballofwax.co.nz.

Comment 1 Katrin Fischer 2013-04-13 18:53:47 UTC
The credit still shows on the current master branch.
Comment 2 Alex Buckley 2017-04-01 01:00:05 UTC
Created attachment 61764 [details] [review]
Bug 2545 - Removed credits being shown on the Pay fines tab.

Now instead of showing a whole row for each credit there is a credit
subtotal shown in order to make it clear what the users total
outstanding fine is
Comment 3 Alex Buckley 2017-04-01 01:09:49 UTC
Created attachment 61765 [details]
Screenshot example of pay fines tab after patches for this bug are applied

Test plan for patch "Bug 2545 - Removed credits being shown on the Pay fines tab":

1. From the Koha staff interface home page navigate to a Patron page

2. Create a fine for the patron (if one does not already exist) by clicking on the 'Fines' tab on the left hand side of the page and then creating a fine in the 'Create manual invoice' page

3. Create a credit for the patron in the 'Create manual credit' page

4. Then go to the 'Pay fines' tab and notice that there is a row for the fine and another for the credit

5. Apply the patch attached to this bug

6. Refresh the page

7. Notice there is no row for credit (as this attachment screenshot example shows) instead above the 'Total due' row there is a 'Credit subtotal' row with the amount of credit.
Comment 4 Felix Hemme 2017-04-11 07:45:55 UTC
With this patch applied, you can not pay or write off the manual credit. Furthermore, information entered the fields 'barcode', 'description' and 'note' is no longer displayed.
Comment 5 Katrin Fischer 2017-04-11 21:46:54 UTC
Hi Felix, if there is a problem like the ones you describe, it's totally ok to set 'failed qa' - 'in discussion' is more when more opinions are needed or when there is disagreement about the implementation.
Comment 6 Alex Buckley 2017-04-17 05:27:15 UTC
Created attachment 62194 [details] [review]
Bug 2454 - Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"

These specific values have been removed because this bug report requires
the manual credit to no longer be shown on the 'Pay fines' tab.

In response to comment 4 (thankyou for noticing that Felix) I have redone this patch so users can now successfully pay or write off manual credits because the
manual credit values although not being displayed are still being
submitted to pay.pl.

Test plan:
1. From the Koha staff interface go the Patron area and select a patron
page

2. Create a fine of $10 for the patron by clicking on the 'Fines' tab and
creating a fine in the 'Create manual invoice' tab

3. Create a credit of $5 for the patron in the 'Create manual credit'
page

4. Then go to the 'Pay fines' tab and notice there is a row for both the fine and
credit both with values in the account type, Notify ID, and level boxes
of the table

5. Select "write off all" and 'OK' in the confirmation box

6. Youre redirected to Account tab, return to 'pay fines' tab and notice no
fines or credits are displayed

7. Repeat steps 2, 3, 4 and then click "Pay amount", confirm it, then navigate back
to the 'Pay fines' tab and notice although the fine and credit are
displayed the 'total due' value is 0.00

8. Apply the patch attached to this bug report

9. Repeat steps 2, and 3.

10. Navigate to 'Pay fines' tab and notice the fine has values
in account type, Notify ID, and level and the credit does not. Additionally
credit has the description value of 'Credit'

11. Repeat steps 5 and 6 and as with before this patch was applied
notice the 'Pay fines' tab does not show any fines or credits

12. Repeat step 7 and again notice the 'Total due' value is 0.00
Comment 7 Katrin Fischer 2017-05-01 14:36:50 UTC
I have to say that the credits system is kind of a mystery to me, as it seems... broken? But the display is better with these patches.
Just a little concerned about those lines:

+        [% ELSE %]
+            Credit
+        [% END %]

Why not check for accounttype = 'C' instead?
Comment 8 Alex Buckley 2017-05-02 11:48:51 UTC
Created attachment 62953 [details] [review]
Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"

These specific values have been removed because this bug report requires
the manual credit to no longer be shown on the 'Pay fines' tab.

In response to comment 4 (thankyou for noticing that Felix) I have redone this patch so users can now successfully pay or write off manual credits because the
manual credit values although not being displayed are still being
submitted to pay.pl.

Test plan:
1. From the Koha staff interface go the Patron area and select a patron
page

2. Create a fine of $10 for the patron by clicking on the 'Fines' tab and
creating a fine in the 'Create manual invoice' tab

3. Create a credit of $5 for the patron in the 'Create manual credit'
page

4. Then go to the 'Pay fines' tab and notice there is a row for both the fine and
credit both with values in the account type, Notify ID, and level boxes
of the table

5. Select "write off all" and 'OK' in the confirmation box

6. Youre redirected to Account tab, return to 'pay fines' tab and notice no
fines or credits are displayed

7. Repeat steps 2, 3, 4 and then click "Pay amount", confirm it, then navigate back
to the 'Pay fines' tab and notice although the fine and credit are
displayed the 'total due' value is 0.00

8. Apply the patch attached to this bug report

9. Repeat steps 2, and 3.

10. Navigate to 'Pay fines' tab and notice the fine has values
in account type, Notify ID, and level and the credit does not. Additionally
credit has the description value of 'Credit'

11. Repeat steps 5 and 6 and as with before this patch was applied
notice the 'Pay fines' tab does not show any fines or credits

12. Repeat step 7 and again notice the 'Total due' value is 0.00
Comment 9 Katrin Fischer 2017-05-02 18:11:13 UTC
Hi Alex,
sorry, looking at this with fresher eyes today maybe. Why remove the account type column from the display? I suggest keeping it. Notify id and level are no longer in use - I am in favor of removing them as you did.
Comment 10 Alex Buckley 2017-05-02 20:56:45 UTC
Created attachment 62991 [details] [review]
Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"

Reimplemented accounttype column but kept the notifyid a
nd level columns

These specific values have been removed because this bug report requires
the manual credit to no longer be shown on the 'Pay fines' tab.

In response to comment 4 (thankyou for noticing that Felix) I have redone this patch so users can now successfully pay or write off manual credits because the
manual credit values although not being displayed are still being
submitted to pay.pl.

Test plan:
1. From the Koha staff interface go the Patron area and select a patron
page

2. Create a fine of $10 for the patron by clicking on the 'Fines' tab and
creating a fine in the 'Create manual invoice' tab

3. Create a credit of $5 for the patron in the 'Create manual credit'
page

4. Then go to the 'Pay fines' tab and notice there is a row for both the fine and
credit both with values in the account type, Notify ID, and level boxes
of the table

5. Select "write off all" and 'OK' in the confirmation box

6. Youre redirected to Account tab, return to 'pay fines' tab and notice no
fines or credits are displayed

7. Repeat steps 2, 3, 4 and then click "Pay amount", confirm it, then navigate back
to the 'Pay fines' tab and notice although the fine and credit are
displayed the 'total due' value is 0.00

8. Apply the patch attached to this bug report

9. Repeat steps 2, and 3.

10. Navigate to 'Pay fines' tab and notice the fine has values
in account type, Notify ID, and level and the credit does not. Additionally
credit has the description value of 'Credit'

11. Repeat steps 5 and 6 and as with before this patch was applied
notice the 'Pay fines' tab does not show any fines or credits

12. Repeat step 7 and again notice the 'Total due' value is 0.00

Bug 2454 - Reimplemented accounttype column but kept the notifyid and level columns
Comment 11 Katrin Fischer 2017-05-02 21:26:32 UTC
Created attachment 62994 [details] [review]
[SIGNED OFF] Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"

Reimplemented accounttype column but kept the notifyid a
nd level columns

These specific values have been removed because this bug report requires
the manual credit to no longer be shown on the 'Pay fines' tab.

In response to comment 4 (thankyou for noticing that Felix) I have redone this patch so users can now successfully pay or write off manual credits because the
manual credit values although not being displayed are still being
submitted to pay.pl.

Test plan:
1. From the Koha staff interface go the Patron area and select a patron
page

2. Create a fine of $10 for the patron by clicking on the 'Fines' tab and
creating a fine in the 'Create manual invoice' tab

3. Create a credit of $5 for the patron in the 'Create manual credit'
page

4. Then go to the 'Pay fines' tab and notice there is a row for both the fine and
credit both with values in the account type, Notify ID, and level boxes
of the table

5. Select "write off all" and 'OK' in the confirmation box

6. Youre redirected to Account tab, return to 'pay fines' tab and notice no
fines or credits are displayed

7. Repeat steps 2, 3, 4 and then click "Pay amount", confirm it, then navigate back
to the 'Pay fines' tab and notice although the fine and credit are
displayed the 'total due' value is 0.00

8. Apply the patch attached to this bug report

9. Repeat steps 2, and 3.

10. Navigate to 'Pay fines' tab and notice the fine has values
in account type, Notify ID, and level and the credit does not. Additionally
credit has the description value of 'Credit'

11. Repeat steps 5 and 6 and as with before this patch was applied
notice the 'Pay fines' tab does not show any fines or credits

12. Repeat step 7 and again notice the 'Total due' value is 0.00

Bug 2454 - Reimplemented accounttype column but kept the notifyid and level columns

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Nick Clemens 2017-10-24 13:57:30 UTC
Created attachment 68470 [details] [review]
Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"

Reimplemented accounttype column but kept the notifyid a
nd level columns

These specific values have been removed because this bug report requires
the manual credit to no longer be shown on the 'Pay fines' tab.

In response to comment 4 (thankyou for noticing that Felix) I have redone this patch so users can now successfully pay or write off manual credits because the
manual credit values although not being displayed are still being
submitted to pay.pl.

Test plan:
1. From the Koha staff interface go the Patron area and select a patron
page

2. Create a fine of $10 for the patron by clicking on the 'Fines' tab and
creating a fine in the 'Create manual invoice' tab

3. Create a credit of $5 for the patron in the 'Create manual credit'
page

4. Then go to the 'Pay fines' tab and notice there is a row for both the fine and
credit both with values in the account type, Notify ID, and level boxes
of the table

5. Select "write off all" and 'OK' in the confirmation box

6. Youre redirected to Account tab, return to 'pay fines' tab and notice no
fines or credits are displayed

7. Repeat steps 2, 3, 4 and then click "Pay amount", confirm it, then navigate back
to the 'Pay fines' tab and notice although the fine and credit are
displayed the 'total due' value is 0.00

8. Apply the patch attached to this bug report

9. Repeat steps 2, and 3.

10. Navigate to 'Pay fines' tab and notice the fine has values
in account type, Notify ID, and level and the credit does not. Additionally
credit has the description value of 'Credit'

11. Repeat steps 5 and 6 and as with before this patch was applied
notice the 'Pay fines' tab does not show any fines or credits

12. Repeat step 7 and again notice the 'Total due' value is 0.00

Bug 2454 - Reimplemented accounttype column but kept the notifyid and level columns

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Jonathan Druart 2017-10-27 14:13:15 UTC
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt

Please do NOT modify the indentation when you do not modify the line or create a new block. That will avoid this kind of unnecessary rebases.
Comment 14 M. Tompsett 2017-10-30 13:43:37 UTC
(In reply to Jonathan Druart from comment #13)
> CONFLICT (content): Merge conflict in
> koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
> 
> Please do NOT modify the indentation when you do not modify the line or
> create a new block. That will avoid this kind of unnecessary rebases.

"If you want to reindent large amounts of existing code in connection with a bugfix or enhancement, you must do so in a separate patch from the one with code changes (unless the reindent is required due to the addition or removal of an indentation level (for example, an additional if added around a block)."
https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL6:_Indentation
Comment 15 M. Tompsett 2017-10-30 13:56:18 UTC
Comment on attachment 62994 [details] [review]
[SIGNED OFF] Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"

The second one is the rebase of this.
Comment 16 M. Tompsett 2017-10-30 14:17:30 UTC
Comment on attachment 68470 [details] [review]
Bug 2454 - Amended patch: Removed the display of accountype, Notify ID, Level values for manual credit in pay.tt and replaced the description with string "Credit"

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

::: koha-tmpl/intranet-tmpl/prog/en/modules/members/pay.tt
@@ -132,5 @@
> -          [% CASE 'HE' %]Hold waiting too long
> -          [% CASE 'Rent' %]Rental fee
> -          [% CASE 'FOR' %]Forgiven
> -          [% CASE 'LR' %]Lost item fee refund
> -          [% CASE 'PF' %]Processing fee

This is why you put white space changes in a different file. I believe this line got removed incorrectly.
Comment 17 Alex Buckley 2017-11-12 02:44:24 UTC
Created attachment 69098 [details] [review]
Bug 2454 - Removed the display of NotifyID and Level values on Pay fines page

Based on tester feedback this patch has been redone so as to remove
changes of indentation to lines not added or modified by this patch.

Test plan:
1. From the Koha staff interface go the Patron area and select a patron
page

2. Create a fine of $10 for the patron by clicking on the 'Fines' tab
and creating a fine in the 'Create manual invoice' tab

3. Create a credit of $5 for the patron in the 'Create manual credit'
page

4. Then go to the 'Pay fines' tab and notice there is a row for both the
fine and credit both with values in the account type, Notify ID, and level boxes
of the table

5. Select "write off all" and 'OK' in the confirmation box

6. Youre redirected to Account tab, return to 'pay fines' tab and notice
no fines or credits are displayed

7. Repeat steps 2, 3, 4 and then click "Pay amount", confirm it, then
navigate back to the 'Pay fines' tab and notice although the fine and credit are
displayed the 'total due' value is 0.00

8. Apply the patch attached to this bug report

9. Repeat steps 2, and 3.

10. Navigate to 'Pay fines' tab and notice the fine has values
in account type, Notify ID, and level and the credit does not.
Additionally credit has the description value of 'Credit'

11. Repeat steps 5 and 6 and as with before this patch was applied
notice the 'Pay fines' tab does not show any fines or credits

12. Repeat step 7 and again notice the 'Total due' value is 0.00

Sponsored-By: Catalyst IT
Comment 18 M. Tompsett 2017-11-12 13:53:49 UTC
(In reply to M. Tompsett from comment #16)
> > -          [% CASE 'PF' %]Processing fee
> 
> This is why you put white space changes in a different file. I believe this
> line got removed incorrectly.

You still cut Processing Fee.
Comment 19 Alex Buckley 2017-11-15 22:36:15 UTC
Sorry I should have obsoleted the first patch which removed the 'processing fee'. I have done so now.
Comment 20 Dominic Pichette 2017-11-17 21:33:22 UTC
Created attachment 69210 [details] [review]
Bug 2454 - Removed the display of NotifyID and Level values on Pay fines page

Based on tester feedback this patch has been redone so as to remove
changes of indentation to lines not added or modified by this patch.

Test plan:
1. From the Koha staff interface go the Patron area and select a patron
page

2. Create a fine of $10 for the patron by clicking on the 'Fines' tab
and creating a fine in the 'Create manual invoice' tab

3. Create a credit of $5 for the patron in the 'Create manual credit'
page

4. Then go to the 'Pay fines' tab and notice there is a row for both the
fine and credit both with values in the account type, Notify ID, and level boxes
of the table

5. Select "write off all" and 'OK' in the confirmation box

6. Youre redirected to Account tab, return to 'pay fines' tab and notice
no fines or credits are displayed

7. Repeat steps 2, 3, 4 and then click "Pay amount", confirm it, then
navigate back to the 'Pay fines' tab and notice although the fine and credit are
displayed the 'total due' value is 0.00

8. Apply the patch attached to this bug report

9. Repeat steps 2, and 3.

10. Navigate to 'Pay fines' tab and notice the fine has values
in account type, Notify ID, and level and the credit does not.
Additionally credit has the description value of 'Credit'

11. Repeat steps 5 and 6 and as with before this patch was applied
notice the 'Pay fines' tab does not show any fines or credits

12. Repeat step 7 and again notice the 'Total due' value is 0.00

Sponsored-By: Catalyst IT
Signed-off-by: Dominic Pichette <dominic@inlibro.com>