Bug 13913 - Renewal error message in OPAC is confusing
Summary: Renewal error message in OPAC is confusing
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-26 11:26 UTC by Katrin Fischer
Modified: 2017-07-31 11:16 UTC (History)
5 users (show)

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


Attachments
screenshot of opac error message (35.28 KB, image/png)
2015-03-26 11:26 UTC, Katrin Fischer
Details
Bug 13913 - Renewal error message in OPAC is confusing (3.53 KB, patch)
2017-05-04 14:04 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13913 - Renewal error message in OPAC is confusing (3.59 KB, patch)
2017-05-10 09:00 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 13913 - Renewal error message in OPAC is confusing (3.68 KB, patch)
2017-06-02 08:46 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2015-03-26 11:26:20 UTC
Created attachment 37266 [details]
screenshot of opac error message

To test:
- checkout a few items
- make sure you can't renew yet for some other reason than max limit of holds reached
   - no renewal before in circ rules
   - a hold on the item
- click renewa all in opac patron account

The shown message reads:
Please note: Your loan renewal failed because of the following reason(s): You have renewed this item the maximum number of times allowed. 

The given reason why the renewal failed is not correct.
Comment 1 Katrin Fischer 2015-04-13 10:30:51 UTC
Hm retesting this, I now see different results - maybe we are only missing an error message for the 'too early'? It seems to list all messages that occured one after the other.

For the 4 items on my patron account it reads now:

Please note: Your loan renewal failed because of the following reason(s): This item is on hold for another patron. You have renewed this item the maximum number of times allowed. This item is on hold for another patron. 

And this is correct:
1 on hold
2 not renewable
3 on hold
4 renewed

But it's still very confusing - wouldn't it make more sense to say: Not all items could be renewed, please check the table of checkouts carefully?
Comment 2 Katrin Fischer 2017-03-06 09:17:59 UTC
We have another report on this.

Currently, all reasons why a renewal failed are listed as unformatted text and it's very hard to see, which message applies to which checkout. We shoudl fix this to show the specific reasons with the issues and only a general message on top.
Comment 3 Owen Leonard 2017-05-04 14:04:22 UTC
Created attachment 63105 [details] [review]
Bug 13913 - Renewal error message in OPAC is confusing

This patch adds some formatting to the error message a patron receives
when there are renewal failures in the OPAC.

This is pretty much the least which could be done to address this
problem. However, I don't think the issue can be fixed without
re-thinking how renewals are processed. Sending error messages back to
opac-user.pl via URL parameter isn't flexible enough.

To test, apply the patch and attempt to renew multiple items in the OPAC
which cannot be renewed for some reason, for instance because they have
been renewed too many times. The error messages should appear in a list
rather than strung together in one long block of text.
Comment 4 Owen Leonard 2017-05-04 14:05:20 UTC
I do NOT think my patch fixes this problem. It simply addresses a cosmetic issue and improves the situation until a real fix is made.
Comment 5 Josef Moravec 2017-05-10 09:00:58 UTC
Created attachment 63321 [details] [review]
[SIGNED-OFF] Bug 13913 - Renewal error message in OPAC is confusing

This patch adds some formatting to the error message a patron receives
when there are renewal failures in the OPAC.

This is pretty much the least which could be done to address this
problem. However, I don't think the issue can be fixed without
re-thinking how renewals are processed. Sending error messages back to
opac-user.pl via URL parameter isn't flexible enough.

To test, apply the patch and attempt to renew multiple items in the OPAC
which cannot be renewed for some reason, for instance because they have
been renewed too many times. The error messages should appear in a list
rather than strung together in one long block of text.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 6 Marcel de Rooy 2017-05-19 09:14:33 UTC
Looks good to me.

Cannot pass this in string freeze. The string is now in the po file with a lot of %s's. And with html tags in between they will become separate translations.
Comment 7 Marcel de Rooy 2017-06-02 08:46:41 UTC
Created attachment 63922 [details] [review]
Bug 13913 - Renewal error message in OPAC is confusing

This patch adds some formatting to the error message a patron receives
when there are renewal failures in the OPAC.

This is pretty much the least which could be done to address this
problem. However, I don't think the issue can be fixed without
re-thinking how renewals are processed. Sending error messages back to
opac-user.pl via URL parameter isn't flexible enough.

To test, apply the patch and attempt to renew multiple items in the OPAC
which cannot be renewed for some reason, for instance because they have
been renewed too many times. The error messages should appear in a list
rather than strung together in one long block of text.

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Jonathan Druart 2017-06-05 14:56:12 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 9 Fridolin SOMERS 2017-06-09 14:18:19 UTC
Pushed to 17.05.x, will be in 17.05.01
Comment 10 Katrin Fischer 2017-06-11 22:44:58 UTC
This patch has been pushed to 16.11.x and will be in 16.11.09.

Changed borrower to patron in the last message as it was the case before this patch in 16.11.x.
Comment 11 Mason James 2017-07-31 11:16:27 UTC
Pushed to 16.05.x, for 16.05.15 release