Bug 30108 - Allow making hold dates required
Summary: Allow making hold dates required
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Marcel de Rooy
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-02-15 12:38 UTC by Marcel de Rooy
Modified: 2023-06-08 22:26 UTC (History)
5 users (show)

See Also:
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 (2.47 KB, patch)
2022-02-17 13:41 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30108: Preference description (1.74 KB, patch)
2022-02-17 13:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30108: Allow making hold dates required (5.47 KB, patch)
2022-02-17 13:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30108: (follow-up) Add required indication, differentiate alert (3.25 KB, patch)
2022-03-04 10:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 30108: Database revision, adding new preference (2.49 KB, patch)
2022-03-04 15:11 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30108: Preference description (1.75 KB, patch)
2022-03-04 15:11 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30108: Allow making hold dates required (5.48 KB, patch)
2022-03-04 15:12 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30108: (follow-up) Add required indication, differentiate alert (3.26 KB, patch)
2022-03-04 15:12 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 30108: Database revision, adding new preference (2.55 KB, patch)
2022-04-20 12:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30108: Preference description (1.81 KB, patch)
2022-04-20 12:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30108: Allow making hold dates required (5.48 KB, patch)
2022-04-20 12:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30108: (follow-up) Add required indication, differentiate alert (3.26 KB, patch)
2022-04-20 12:26 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30108: (follow-up) Make require label translatable (1.70 KB, patch)
2022-05-02 21:06 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.