Bug 15775

Summary: Show message on OPAC summary if holds are blocked due to fines
Product: Koha Reporter: Aleisha Amohia <aleisha>
Component: OPACAssignee: Aleisha Amohia <aleisha>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: 1joynelson, caroline.cyr-la-rose, jonathan.druart, katrin.fischer, lucas, martin.renvoize, veron
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15374
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24771
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/425
Text to go in the release notes:
This adds a note to the patron account in the OPAC if the user is over the maxoutstanding fines limit and can no longer place holds.
Version(s) released in:
20.05.00, 19.11.05
Attachments: Bug 15775 - User should know when they are blocked from checkouts on OPAC
Bug 15775 - User should know when they are blocked from checkouts on OPAC
Bug 15775 - User should know when they are blocked from checkouts on OPAC
Bug 15775 - User should know when they are blocked from checkouts on OPAC + Translatability fixes for OPAC Summary page
Bug 15775 - User should know when they are blocked from checkouts on OPAC + Translatability fixes for OPAC Summary page
Bug 15775 - User should know when they are blocked from checkouts on OPAC + Translatability fixes for OPAC Summary page
Bug 15775 - User should know when they are blocked from checkouts on OPAC + Translatability fixes for OPAC Summary page
Bug 15775: [FOLLOWUP] fixing merge conflicts and using Price plugin for formatting
Bug 15775: Show message on OPAC summary if checkouts blocked due to fines
Bug 15775: Show message on OPAC summary if checkouts blocked due to fines
Bug 15775: Show message on OPAC summary if holds are blocked due to fines
Bug 15775: Change wording slightly

Description Aleisha Amohia 2016-02-09 20:02:48 UTC
When a user has been blocked from checkouts in the staff client for having too many fines, they are not told about being blocked until they try to place a hold on an item in the OPAC. They should have a message about it on the summary page which they'll see as soon as they log in.
Comment 1 Aleisha Amohia 2016-02-10 00:02:26 UTC Comment hidden (obsolete)
Comment 2 Aleisha Amohia 2016-02-10 00:12:04 UTC Comment hidden (obsolete)
Comment 3 Marc Véron 2016-02-11 13:57:09 UTC
Hi Aleisha,

If I apply this patch on top of Bug 15374, I get merge conflicts.
Comment 4 Aleisha Amohia 2016-02-11 20:07:28 UTC Comment hidden (obsolete)
Comment 5 Aleisha Amohia 2016-02-11 20:08:43 UTC
(In reply to Marc Véron from comment #3)
> Hi Aleisha,
> 
> If I apply this patch on top of Bug 15374, I get merge conflicts.

Hi Marc, please try applying on it's own (not on top of Bug 15374). I used the patch from 15374 to write this patch but I realise now it probably doesn't need to be applied on top of the other one.
Comment 6 Marc Véron 2016-02-12 08:48:01 UTC
Hi Aleisha,

I gave up Bug 15374 because it would have conflicts on top of this Bug. It certainly will make it easier to go on with such enhancements.

Plese feel free to re-use code from 15374 on top of your patch, if applicable.
Comment 7 Aleisha Amohia 2016-02-14 20:14:29 UTC
(In reply to Marc Véron from comment #6)
> Hi Aleisha,
> 
> I gave up Bug 15374 because it would have conflicts on top of this Bug. It
> certainly will make it easier to go on with such enhancements.
> 
> Plese feel free to re-use code from 15374 on top of your patch, if
> applicable.

Hi, thanks Marc!

I think I actually wrote the patch for this on top of your patch from 15374 anyway so we kind of wrote this patch together :)

I will recommit this patch with the commit message from 15374 and edit the bug report since we are now testing for all that translatability stuff too.
Comment 8 Aleisha Amohia 2016-02-14 20:16:12 UTC Comment hidden (obsolete)
Comment 9 Marc Véron 2016-03-23 06:12:19 UTC
Wanted to test, but patch does not apply:

Auto-merging opac/opac-user.pl
CONFLICT (content): Merge conflict in opac/opac-user.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
Failed to merge in the changes.
Comment 10 Aleisha Amohia 2016-03-23 10:02:22 UTC Comment hidden (obsolete)
Comment 11 Marc Véron 2016-03-23 14:03:07 UTC
Tests:
...are OK

Translability tests:
- Log in to OPAC
  (...)
  - with user who has got a circulation message (from checkout page)

The circulation message I added on the checkout page displays a bullet only, but no text. (It displays properly without patch).

Other translatability tests are OK.
Comment 13 Aleisha Amohia 2016-03-23 22:03:56 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2016-03-24 06:51:39 UTC
Created attachment 49516 [details] [review]
Bug 15775 - User should know when they are blocked from checkouts on OPAC + Translatability fixes for OPAC Summary page

EDIT: Fixing merge conflicts
EDIT: Fix from Comment 11 / Comment 12

When a user has been blocked from checkouts in the staff client for having too many fines, they will now see a message about this in the OPAC

To test:
- This patch will be easiest to test if you keep three tabs open - system preferences, a patron detail page, and OPAC summary page
- Set some system preferences:
  - OPACFineNoRenewals = 2
  - noissuescharge = 4
  - OpacRenewalAllowed = Allow
- Set up a patron with fines of $3
- Log in to the OPAC and verify the message from patch from Bug 15374, saying the user cannot renew books. Confirm link to accounts page works.
- Set OpacRenewalAllowed = Don't allow
- Refresh OPAC Summary and confirm the message disappears
- Set OpacRenewalAllowed = Allow
- Give the patron total fines of $5 (add $2)
- Refresh OPAC Summary and verify message appears saying user is BLOCKED from checkouts. Confirm link to accounts page works
- Set OpacRenewalAllowed = Don't allow
- Refresh OPAC Summary and confirm message is still there

Translability tests:
- Log in to OPAC
  - with blocked /debarred user (with and without comment and date)
  - with user who has to much fines
  - with user who has his card marked as lost
  - with user with an account that is about to expire
  - with user with expired account
  - with user with uncertain address information
  - with user who has got a circulation message (from checkout page)
  - with user who has a OPAC note
- Verify that information is correct and displays nicely and that links
  in messages work
- Carefully examine code
- Search for regressions

Sponsored-by: Catalyst IT

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 15 Jonathan Druart 2016-04-18 13:11:41 UTC
Who is the author of this patch?
The author commit is Marc, is it right?
Comment 16 Katrin Fischer 2016-04-27 08:54:00 UTC
Comment on attachment 49516 [details] [review]
Bug 15775 - User should know when they are blocked from checkouts on OPAC + Translatability fixes for OPAC Summary page

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

I really like the use of the Price plugin for formatting - I'd love to see that used everywhere and improved to show the currency symbol in some cases.

Found a small regression in testing (see code comments below):

::: opac/opac-user.pl
@@ +126,5 @@
> +my $checkoutsblockedamount = C4::Context->preference( 'noissuescharge' );
> +$checkoutsblockedamount ||=0;
> +if ( C4::Context->preference( 'noissuescharge' ) && $borr->{amountoutstanding} > $checkoutsblockedamount ) {
> +    $borr->{'flagged'} = 1;
> +    $canrenew = 0;

I think this change is not correct. 
noissuecharge should have no effect on renewals, as we have OPACFineNoRenewals to do that.
With noissuecharge 5.00, OPACFineNoRenewals 100 and a fine of 5.01 on my account I can now no longer renew items I can renew without the patch. If I set noissuecharge to 5.02 I can renew with the patch applied.

@@ +128,5 @@
> +if ( C4::Context->preference( 'noissuescharge' ) && $borr->{amountoutstanding} > $checkoutsblockedamount ) {
> +    $borr->{'flagged'} = 1;
> +    $canrenew = 0;
> +    $template->param(
> +        checkouts_blocked_amount => sprintf ( '%.02f', $checkoutsblockedamount ),

Why not format the amount in the template like in the other cases?
Comment 17 Marc Véron 2017-05-23 05:45:15 UTC
(In reply to Jonathan Druart from comment #15)
> Who is the author of this patch?
> The author commit is Marc, is it right?

This Bug 15775 is related to Bug 15374
I abandoned 15374 in favour of 15775, and sigend off 15775
See https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15374#c35

No problem for me if authorship is changed to Aleisha.
Comment 18 Aleisha Amohia 2017-05-25 05:38:59 UTC
Created attachment 63710 [details] [review]
Bug 15775: [FOLLOWUP] fixing merge conflicts and using Price plugin for formatting
Comment 19 Caroline Cyr La Rose 2017-09-22 18:58:28 UTC
Hi, I tried applying the patch, but it doesn't seem to work. I get conflicts...

Using index info to reconstruct a base tree...
M	koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
M	opac/opac-user.pl
Falling back to patching base and 3-way merge...
Auto-merging opac/opac-user.pl
CONFLICT (content): Merge conflict in opac/opac-user.pl
Auto-merging koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
Failed to merge in the changes.
Patch failed at 0001 Bug 15775 - User should know when they are blocked from checkouts on OPAC + Translatability fixes for OPAC Summary page
Comment 20 Aleisha Amohia 2020-02-06 22:29:08 UTC
Created attachment 98540 [details] [review]
Bug 15775: Show message on OPAC summary if checkouts blocked due to fines

To test:
1) Set maxoutstanding syspref to 5
2) Create a manual invoice for a borrower for $6
3) Log in to the OPAC as this borrower
4) Go to opac-user.pl, your summary page
5) Confirm there is a warning box explaining that the borrower is
blocked from checkouts and holds due to fines
6) Back on the staff side, pay some or all of the borrower's fines so
they are no longer over the max outstanding amount
7) Refresh the summary page in the OPAC and confirm the warning is gone

Sponsored-by: Catalyst IT
Comment 21 Aleisha Amohia 2020-02-06 22:30:19 UTC
This patch had gotten a little messy and looks like much of the content of the patch has now been dealt with in master, so I've written a new patch to deal simply with the bug title. Ready for testing.
Comment 22 David Nind 2020-02-07 01:51:13 UTC
Created attachment 98545 [details] [review]
Bug 15775: Show message on OPAC summary if checkouts blocked due to fines

To test:
1) Set maxoutstanding syspref to 5
2) Create a manual invoice for a borrower for $6
3) Log in to the OPAC as this borrower
4) Go to opac-user.pl, your summary page
5) Confirm there is a warning box explaining that the borrower is
blocked from checkouts and holds due to fines
6) Back on the staff side, pay some or all of the borrower's fines so
they are no longer over the max outstanding amount
7) Refresh the summary page in the OPAC and confirm the warning is gone

Sponsored-by: Catalyst IT

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 Katrin Fischer 2020-02-29 22:42:55 UTC
We got 3 different prefs for OPAC renewal, checkouts and holds. This patch uses maxoutstanding = holds. Changed the wording of the warning slightly to reflect this. I could imagine doing warns for the other 2 prefs might be nice as well.
Comment 24 Katrin Fischer 2020-02-29 22:43:08 UTC
Created attachment 99840 [details] [review]
Bug 15775: Show message on OPAC summary if holds are blocked due to fines

To test:
1) Set maxoutstanding syspref to 5
2) Create a manual invoice for a borrower for $6
3) Log in to the OPAC as this borrower
4) Go to opac-user.pl, your summary page
5) Confirm there is a warning box explaining that the borrower is
blocked from holds due to fines
6) Back on the staff side, pay some or all of the borrower's fines so
they are no longer over the max outstanding amount
7) Refresh the summary page in the OPAC and confirm the warning is gone

Sponsored-by: Catalyst IT

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Katrin Fischer 2020-02-29 22:43:12 UTC
Created attachment 99841 [details] [review]
Bug 15775: Change wording slightly

maxoutstanding only blocks holds, for checkouts we got
noissuechage.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2020-03-15 14:19:28 UTC
It looks like I missed changing status here.
Comment 27 Martin Renvoize 2020-03-19 09:23:32 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 28 Joy Nelson 2020-04-03 20:24:04 UTC
backported to 19.11.x for 19.11.05
Comment 29 Lucas Gass 2020-04-13 17:21:33 UTC
not backporting enhancement to 19.05.x