Bug 30108

Summary: Allow making hold dates required
Product: Koha Reporter: Marcel de Rooy <m.de.rooy>
Component: Hold requestsAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, gmcharlt, kyle, lucas, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00
Attachments: Bug 30108: Database revision, adding new preference
Bug 30108: Preference description
Bug 30108: Allow making hold dates required
Bug 30108: (follow-up) Add required indication, differentiate alert
Bug 30108: Database revision, adding new preference
Bug 30108: Preference description
Bug 30108: Allow making hold dates required
Bug 30108: (follow-up) Add required indication, differentiate alert
Bug 30108: Database revision, adding new preference
Bug 30108: Preference description
Bug 30108: Allow making hold dates required
Bug 30108: (follow-up) Add required indication, differentiate alert
Bug 30108: (follow-up) Make require label translatable

Description Marcel de Rooy 2022-02-15 12:38:32 UTC
Make it possible that the two hold dates on opac-reserve (starts on and not needed after) are mandatory. Enable OPACAllowHoldDateInFuture to see the first one.
A new pref could include the various (4) combinations.
Comment 1 Marcel de Rooy 2022-02-17 13:41:59 UTC
Created attachment 130745 [details] [review]
Bug 30108: Database revision, adding new preference

Test plan:
Run upgrade or new install.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2022-02-17 13:42:03 UTC
Created attachment 130746 [details] [review]
Bug 30108: Preference description

Test plan:
Check the preferences form, tab Circulation, tab Hold policy.
Look for OPACMandatoryHoldDates.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2022-02-17 13:42:07 UTC
Created attachment 130747 [details] [review]
Bug 30108: Allow making hold dates required

Side note: Template still referred to class holddateto while not
being used any longer. Replaced this occurrence by futuredate.

Test plan:
[1] Disable OPACAllowHoldDateInFuture.
[2] Set pref OPACMandatoryHoldDates to "end date".
[3] Verify that the end date is mandatory on opac-reserve.
[4] Set pref OPACMandatoryHoldDates to "start date".
[5] Verify that no dates are required.
[6] Enable OPACAllowHoldDateInFuture.
[7] Verify that start dates are required.
[8] Set pref OPACMandatoryHoldDates to "no dates".
[9] Verify that no dates are required again.

Bonus:
Check that more options box is opened when a date still is required.
In case of item level being forced, verify that more options is opened
regardless of pref setting.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Lucas Gass 2022-02-23 23:08:40 UTC
Everything works as excepted but I think a few small things can be done to make the user experience easier:

1. On other OPAC pages (self-reg, suggestions ) we add a required div that makes it clear that the field is required: 

<div class="required_label required">Required</div>

That would be nice so that users can see what is required.


2. The alert message 'Date should be filled.' is generic. We should tell the user exactly which field needs attention. 

The "Hold starts on date" field is required
The "Hold not needed after" field is required
Comment 5 Owen Leonard 2022-03-03 17:32:17 UTC
Marking Failed QA based on Lucas's comment.
Comment 6 Marcel de Rooy 2022-03-04 10:51:07 UTC
Created attachment 131372 [details] [review]
Bug 30108: (follow-up) Add required indication, differentiate alert

As asked on comment4:
[1] Add a required div as a visible hint when applicable.
[2] Differentiate alert for hold start and end date.

Test plan:
See former patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2022-03-04 10:51:49 UTC
(In reply to Lucas Gass from comment #4)
> Everything works as excepted but I think a few small things can be done to
> make the user experience easier:
> 
> 1. On other OPAC pages (self-reg, suggestions ) we add a required div that
> makes it clear that the field is required: 
> 
> <div class="required_label required">Required</div>
> 
> That would be nice so that users can see what is required.
> 
> 
> 2. The alert message 'Date should be filled.' is generic. We should tell the
> user exactly which field needs attention. 
> 
> The "Hold starts on date" field is required
> The "Hold not needed after" field is required

Thx Lucas for your testing and observations. Hope they are fixed now.
Comment 8 Lucas Gass 2022-03-04 15:11:56 UTC
Created attachment 131376 [details] [review]
Bug 30108: Database revision, adding new preference

Test plan:
Run upgrade or new install.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 9 Lucas Gass 2022-03-04 15:11:59 UTC
Created attachment 131377 [details] [review]
Bug 30108: Preference description

Test plan:
Check the preferences form, tab Circulation, tab Hold policy.
Look for OPACMandatoryHoldDates.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 10 Lucas Gass 2022-03-04 15:12:03 UTC
Created attachment 131378 [details] [review]
Bug 30108: Allow making hold dates required

Side note: Template still referred to class holddateto while not
being used any longer. Replaced this occurrence by futuredate.

Test plan:
[1] Disable OPACAllowHoldDateInFuture.
[2] Set pref OPACMandatoryHoldDates to "end date".
[3] Verify that the end date is mandatory on opac-reserve.
[4] Set pref OPACMandatoryHoldDates to "start date".
[5] Verify that no dates are required.
[6] Enable OPACAllowHoldDateInFuture.
[7] Verify that start dates are required.
[8] Set pref OPACMandatoryHoldDates to "no dates".
[9] Verify that no dates are required again.

Bonus:
Check that more options box is opened when a date still is required.
In case of item level being forced, verify that more options is opened
regardless of pref setting.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 11 Lucas Gass 2022-03-04 15:12:06 UTC
Created attachment 131379 [details] [review]
Bug 30108: (follow-up) Add required indication, differentiate alert

As asked on comment4:
[1] Add a required div as a visible hint when applicable.
[2] Differentiate alert for hold start and end date.

Test plan:
See former patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 12 Tomás Cohen Arazi 2022-04-20 12:25:40 UTC
Created attachment 133462 [details] [review]
Bug 30108: Database revision, adding new preference

Test plan:
Run upgrade or new install.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Tomás Cohen Arazi 2022-04-20 12:25:46 UTC
Created attachment 133463 [details] [review]
Bug 30108: Preference description

Test plan:
Check the preferences form, tab Circulation, tab Hold policy.
Look for OPACMandatoryHoldDates.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 14 Tomás Cohen Arazi 2022-04-20 12:25:52 UTC
Created attachment 133464 [details] [review]
Bug 30108: Allow making hold dates required

Side note: Template still referred to class holddateto while not
being used any longer. Replaced this occurrence by futuredate.

Test plan:
[1] Disable OPACAllowHoldDateInFuture.
[2] Set pref OPACMandatoryHoldDates to "end date".
[3] Verify that the end date is mandatory on opac-reserve.
[4] Set pref OPACMandatoryHoldDates to "start date".
[5] Verify that no dates are required.
[6] Enable OPACAllowHoldDateInFuture.
[7] Verify that start dates are required.
[8] Set pref OPACMandatoryHoldDates to "no dates".
[9] Verify that no dates are required again.

Bonus:
Check that more options box is opened when a date still is required.
In case of item level being forced, verify that more options is opened
regardless of pref setting.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 15 Tomás Cohen Arazi 2022-04-20 12:26:00 UTC
Created attachment 133465 [details] [review]
Bug 30108: (follow-up) Add required indication, differentiate alert

As asked on comment4:
[1] Add a required div as a visible hint when applicable.
[2] Differentiate alert for hold start and end date.

Test plan:
See former patch.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Fridolin Somers 2022-05-02 21:06:11 UTC
Created attachment 134476 [details] [review]
Bug 30108: (follow-up) Make require label translatable
Comment 17 Fridolin Somers 2022-05-02 21:07:11 UTC
Maybe in the future this could be rewritten with JQuery validator like opac-memberentry.tt
Comment 18 Fridolin Somers 2022-05-02 21:24:23 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 20 Katrin Fischer 2022-08-25 09:40:12 UTC
Wrong bug... too many tabs.