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.
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?
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.
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.
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.
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>
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.
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>
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.01
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.
Pushed to 16.05.x, for 16.05.15 release