Bug 28876 - No renewal before advisory text not wrapped in selector
Summary: No renewal before advisory text not wrapped in selector
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-18 09:37 UTC by Sally
Modified: 2022-12-12 21:23 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement adds more <span>s to the user summary page in the OPAC so that information about the renewal status can be targeted with CSS or JS. It adds each <span> with a "usr-msg" class for general styling and a specific class for each renewal message, for example: <span class="usr-msg no-renew-hold">Not renewable <span class="renewals">(on hold)</span></span> These classes are added: - no-renew-hold: Not renewable (on hold) - no-renew-too-many: Not renewable (too many renewals) - no-renew-unseen: Item must be renewed at the library - no-renew-overdue: Not allowed (overdue) - no-renew-too-late: No longer renewable - auto-renew-fines: Automatic renewal failed, you have unpaid fines - auto-renew-expired: Automatic renewal failed, your account is expired - no-renewal-before: No renewal before [date] - automatic-renewal: Automatic renewal
Version(s) released in:
22.05.00
Circulation function:

Attachments
Bug 28876: No renewal before advisory text not wrapped in selector (6.98 KB, patch)
2021-10-07 16:58 UTC, Owen Leonard 		Details | Diff | Splinter Review
Bug 28876: No renewal before advisory text not wrapped in selector (6.98 KB, patch)
2021-10-07 17:56 UTC, Owen Leonard 		Details | Diff | Splinter Review
Bug 28876: Wrap no renewal before advisory text in selector (7.08 KB, patch)
2021-12-15 10:00 UTC, Jonathan Druart 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Sally 2021-08-18 09:37:33 UTC 
Using 'No renewal before' in the circulation rules is a very useful setting, as it blocks a patron from accidentally renewing too many times in quick succession on the OPAC and using up all of their permitted renewals.

But the setting also adds some advisory text to the OPAC which confuses our patrons.  We would like to be able to hide this text, but it's not wrapped in a selector so we can't target it.   

To replicate:

- Create a circulation rule which includes:  

Renewals allowed:  4
Renewal period:  21
No renewal before:  20

- Check an appropriate item out to an account with a due date of +10 days
- Log into the OPAC with the account
- Renew the item
- Note that the following (example) information appears in the Renew column:

Renewed!
No renewal before 19/08/2021 00:00
(3 of 4 renewals remaining)

- Use Chrome/Firefox dev/inspect tools and note that 'Renewed!' is wrapped in a span class called 'blabel label-success', and '(3 of 4 renewals remaining)' is wrapped in a span class called 'renewals'.

It would be really helpful if a span class could be added to this part of the opac-user.tt template:

[% ELSIF ( ISSUE.too_soon ) %]
 No renewal before [% ISSUE.soonestrenewdate | html %]
Comment 1 Owen Leonard 2021-10-07 16:58:11 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2021-10-07 17:56:02 UTC 
Created attachment 125900 [details] [review]
Bug 28876: No renewal before advisory text not wrapped in selector

This patch adds some more <span>s to the user summary page in the OPAC
so that information about renewal status can be targeted with CSS or JS.
It adds each <span> with a "usr-msg" class for general styling and a
specific class for each renewal message, e.g.

<span class="usr-msg no-renew-hold">Not renewable <span
class="renewals">(on hold)</span></span>

These classes are added:

- no-renew-hold - Not renewable (on hold)
- no-renew-too-many - Not renewable (too many renwals)
- no-renew-unseen - Item must be renewed at the library
- no-renew-overdue - Not allowed (overdue)
- no-renew-too-late - No longer renewable
- auto-renew-fines - Automatic renewal failed, you have unpaid fines
- auto-renew-expired - Automatic renewal failed, your account is expired
- no-renewal-before - No renewal before [date]
- automatic-renewal - Automatic renewal

To test, apply the patch and try to locate or set up a patron to have
multiple checkouts with multiple renewal conditions:

- On hold
- Too many renewals
- Overdues (if  OverduesBlockRenewing is on)

...etc.

In the OPAC, log in and view the table of checkouts on the "your
summary" page. Confirm that messages in the "Renew" column are marked up
correctly.
Comment 3 David Nind 2021-11-08 07:07:36 UTC 
I tested for six of the classes and these all worked as expected:

1. no-renew-hold - Not renewable (on hold) [check out an item with renewals allowed, place an item level hold]

2. no-renew-too-many - Not renewable (too many renwals) [change circulation rules so only one renewal allowed (change 'Renewals allowed (count)' to 1), check out an item, from the patron's account summary in the OPAC select renew and refresh the page]

3. no-renew-unseen - Item must be renewed at the library [set UnseenRenewals to allow, change circulation rules so that Unseen renewals allowed (count) = 1, check out an item, in the OPAC summary renew, message now says 'Item must be renewed at the library.']

4. no-renew-overdue - Not allowed (overdue) [set OverduesBlockRenewing = block renewing for all the patron's items, check out an item with renewals allowed, make check out date before current date]

5. no-renewal-before - No renewal before [date] [set circulation rules: circulation period = 21 and No renewal before = 20; check out an item, in OPAC renew from summary page]

6. automatic-renewal - Automatic renewal [check out an item with renewals] 

I wasn't able to figure out how to test for the remaining classes:
- no-renew-too-late - No longer renewable
- auto-renew-fines - Automatic renewal failed, you have unpaid fines
- auto-renew-expired - Automatic renewal failed, your account is expired
- no-renew-too-late - No longer renewable

Happy to sign off if this is sufficient, otherwise I will leave for someone else who knows how to test the remaining classes.
Comment 4 Fridolin Somers 2021-11-12 07:13:11 UTC 
This will also be easier for translation
Comment 5 Jonathan Druart 2021-12-15 10:00:34 UTC 
Created attachment 128551 [details] [review]
Bug 28876: Wrap no renewal before advisory text in selector

This patch adds some more <span>s to the user summary page in the OPAC
so that information about renewal status can be targeted with CSS or JS.
It adds each <span> with a "usr-msg" class for general styling and a
specific class for each renewal message, e.g.

<span class="usr-msg no-renew-hold">Not renewable <span
class="renewals">(on hold)</span></span>

These classes are added:

- no-renew-hold - Not renewable (on hold)
- no-renew-too-many - Not renewable (too many renwals)
- no-renew-unseen - Item must be renewed at the library
- no-renew-overdue - Not allowed (overdue)
- no-renew-too-late - No longer renewable
- auto-renew-fines - Automatic renewal failed, you have unpaid fines
- auto-renew-expired - Automatic renewal failed, your account is expired
- no-renewal-before - No renewal before [date]
- automatic-renewal - Automatic renewal

To test, apply the patch and try to locate or set up a patron to have
multiple checkouts with multiple renewal conditions:

- On hold
- Too many renewals
- Overdues (if  OverduesBlockRenewing is on)

...etc.

In the OPAC, log in and view the table of checkouts on the "your
summary" page. Confirm that messages in the "Renew" column are marked up
correctly.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
JD amended patch: fix commit title
Comment 6 Jonathan Druart 2021-12-15 10:01:06 UTC 
Skipping QA.
Comment 7 Fridolin Somers 2022-01-04 07:54:17 UTC 
Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑