Bug 35977 - Display current date in hold starts on when placing a hold in the OPAC
Summary: Display current date in hold starts on when placing a hold in the OPAC
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Martin Renvoize (ashimema)
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on: 36341
Blocks:
  Show dependency treegraph
 
Reported: 2024-02-01 13:55 UTC by Mirjam Vantieghem
Modified: 2024-11-20 13:21 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.06,23.05.12
Circulation function:


Attachments
Bug 35977: Set holddatefrom default to today (2.50 KB, patch)
2024-02-07 15:22 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35977: Set holddatefrom default to today (2.56 KB, patch)
2024-02-08 15:36 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 35977: Set holddatefrom default to today (2.61 KB, patch)
2024-02-13 18:31 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35977: (follow-up) Perltidy whitespace fixes (810 bytes, patch)
2024-02-13 18:31 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35977: (follow-up) Perltidy whitespace fixes (868 bytes, patch)
2024-04-01 17:25 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35977: Set holddatefrom default to today (2.67 KB, patch)
2024-04-01 17:43 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35977: (follow-up) Perltidy whitespace fixes (868 bytes, patch)
2024-04-01 17:43 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35977: Set holddatefrom default to today (3.15 KB, patch)
2024-04-25 12:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35977: Nuke value if setting required (1.30 KB, patch)
2024-04-25 12:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35977: (follow-up) Use KohaDate instead of raw (2.29 KB, patch)
2024-04-29 10:26 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 35977: (follow-up) Cleaner working approach (4.77 KB, patch)
2024-04-30 15:16 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mirjam Vantieghem 2024-02-01 13:55:49 UTC
This is related to bug #15652.

If you enable AllowHoldDateInFuture (and its OPAC variant), you get
an additional "Hold starts on date". It is possible to select the current date from the datepicker in the OPAC. If no date is selected, the default is the current date.
However, it is not clear to the patron that the current date is selected by default, as the field is empty if no date has been selected from the datepicker.
This caused confusion among some of our patrons.
Therefore we use a custom patch to have the current date shown in the field "Hold starts on date".

What Marcel de Rooy wrote in the related bug, also applies here:
"Although it does not really make any difference if you leave the startdate
empty or put the current date in it, we concluded that it still makes
a psychological difference."
Comment 1 Martin Renvoize (ashimema) 2024-02-07 15:22:22 UTC
Created attachment 161814 [details] [review]
Bug 35977: Set holddatefrom default to today

This patch sets the holddatefrom date to today by default on the opac.
Comment 2 ByWater Sandboxes 2024-02-08 15:36:44 UTC
Created attachment 161928 [details] [review]
Bug 35977: Set holddatefrom default to today

This patch sets the holddatefrom date to today by default on the opac.

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Comment 3 Owen Leonard 2024-02-13 18:31:50 UTC
Created attachment 162125 [details] [review]
Bug 35977: Set holddatefrom default to today

This patch sets the holddatefrom date to today by default on the opac.

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Owen Leonard 2024-02-13 18:31:52 UTC
Created attachment 162126 [details] [review]
Bug 35977: (follow-up) Perltidy whitespace fixes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Marcel de Rooy 2024-03-15 09:51:31 UTC
(In reply to Mirjam Vantieghem from comment #0)
> What Marcel de Rooy wrote in the related bug, also applies here:
> "Although it does not really make any difference if you leave the startdate
> empty or put the current date in it, we concluded that it still makes
> a psychological difference."

Nice quote :) 2016 already..
Comment 6 Marcel de Rooy 2024-03-15 09:57:07 UTC
Still confused though about another "psychological effect". If the field is prefilled, the user might be inclined to go to the next field that he should still fill. If it is blank but required, (s)he is forced to pay a bit more attention?

We do use the field to decide when to fetch a book from our shelves for the reading room. So a more accurate start date is of importance. Makes it a harder choice somehow.
Comment 7 Marcel de Rooy 2024-03-15 09:58:16 UTC
We seem to have a regression here btw. Since it seems possible now to enter past dates in the start date field.
Comment 8 Martin Renvoize (ashimema) 2024-03-18 11:19:26 UTC
Hmm, I don't think the regression comes from here.. I just tested on master and you can select a past date there too without this patch... :(.

I'll look into it however as that certainly is a regression somewhere.
Comment 9 Martin Renvoize (ashimema) 2024-03-18 11:40:41 UTC
Bug 36341 - "Hold starts on date" should be limited to future dates

The above fixes the pre-existing lack of futuredate definitions.
Comment 10 Nick Clemens (kidclamp) 2024-04-01 17:25:35 UTC Comment hidden (obsolete)
Comment 11 Nick Clemens (kidclamp) 2024-04-01 17:26:53 UTC
(In reply to Marcel de Rooy from comment #6)
> Still confused though about another "psychological effect". If the field is
> prefilled, the user might be inclined to go to the next field that he should
> still fill. If it is blank but required, (s)he is forced to pay a bit more
> attention?

Passing QA, but highlighting for RM - I think in this case defaulting to today is more obvious for general purposes.
IMHO, you can use a bit of jQuery to clear the field since you have a unique use case
Comment 12 Nick Clemens (kidclamp) 2024-04-01 17:43:05 UTC
Created attachment 164220 [details] [review]
Bug 35977: Set holddatefrom default to today

This patch sets the holddatefrom date to today by default on the opac.

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Nick Clemens (kidclamp) 2024-04-01 17:43:07 UTC
Created attachment 164221 [details] [review]
Bug 35977: (follow-up) Perltidy whitespace fixes

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 14 Nick Clemens (kidclamp) 2024-04-01 17:43:38 UTC
This will also need rebased on 36341 - but it broke the date population when I tried
Comment 15 Katrin Fischer 2024-04-01 18:55:48 UTC
(In reply to Nick Clemens from comment #11)
> (In reply to Marcel de Rooy from comment #6)
> > Still confused though about another "psychological effect". If the field is
> > prefilled, the user might be inclined to go to the next field that he should
> > still fill. If it is blank but required, (s)he is forced to pay a bit more
> > attention?
> 
> Passing QA, but highlighting for RM - I think in this case defaulting to
> today is more obvious for general purposes.
> IMHO, you can use a bit of jQuery to clear the field since you have a unique
> use case

Hm. 

I think with OPACMandatoryHoldDates set to have the start date as mandatory. This is the standard behavior for required fields as it forces the user to make an active choice, otherwise the preference basically has no effect.

A lot can be solved with JS, but I think we need consistent behavior as the default. And JS is not as reachable or easy to a lot of libraries as it is for some of us. Especially with the new date widget it can get tricky.
Comment 16 Martin Renvoize (ashimema) 2024-04-25 12:40:13 UTC
Created attachment 165535 [details] [review]
Bug 35977: Set holddatefrom default to today

This patch sets the holddatefrom date to today by default on the opac.

Signed-off-by: Barbara Johnson <barbara.johnson@bedfordtx.gov>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 17 Martin Renvoize (ashimema) 2024-04-25 12:40:16 UTC
Created attachment 165536 [details] [review]
Bug 35977: Nuke value if setting required
Comment 18 Katrin Fischer 2024-04-26 11:27:44 UTC
I am sorry, but this breaks the datepickers on the OPAC reserve page totally for me now :(

The input shows:
2024-04-26T1

JS error:
Uncaught ReferenceError: Date_from_syspref is not defined

I was a little mean and set the DateFormat to the German DD.MM.YYYY.
Comment 19 Katrin Fischer 2024-04-26 12:41:29 UTC
... as I just accidentally pushed this - could we get a quick follow-up?
Comment 20 Martin Renvoize (ashimema) 2024-04-26 13:09:06 UTC
I'm on it.
Comment 21 Martin Renvoize (ashimema) 2024-04-29 10:26:53 UTC
Created attachment 165725 [details] [review]
Bug 35977: (follow-up) Use KohaDate instead of raw

This patch updates the value passed to the holdfrom input to use
KohaDate formatting instead of Raw. The 'futureinclusive' logic now
being triggered in the flatpickr instantiation expects the localised
date format as apposed to the raw one.

However, I can't get the flatpickr and input to pick up the passed date
properly now :(.
Comment 22 Martin Renvoize (ashimema) 2024-04-29 10:29:06 UTC
I'm stuck.. I can't get the value to be properly picked up by flatpickr.
Comment 23 Martin Renvoize (ashimema) 2024-04-30 15:16:59 UTC
Created attachment 165910 [details] [review]
Bug 35977: (follow-up) Cleaner working approach

This patch removes the Date_from_syspref recently added and replaces it
with the correct parseDate call as per bug 35559. We also clean up the
code around setting the input value in the first place and use iso which
is what the rest of flatpickr expects and now is handled correctly in
futuredate pickers.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 24 Martin Renvoize (ashimema) 2024-04-30 15:18:23 UTC
Thanks to Nick for helping me wrap my head around this and Emily for the clean code in bug 35559 which lead to the fix.

This should be ready to go again now Katrin.
Comment 25 Katrin Fischer 2024-05-02 10:41:23 UTC
Follow-up will be pushed to main.
Comment 26 Katrin Fischer 2024-05-02 11:20:00 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 27 Fridolin Somers 2024-05-23 15:31:10 UTC
Depends on Bug 36341 that added  function Date_from_syspref()
Comment 28 Fridolin Somers 2024-05-23 15:31:32 UTC
Pushed to 23.11.x for 23.11.06
Comment 29 Lucas Gass (lukeg) 2024-05-28 20:37:30 UTC
Backported to 23.05.x for upcoming 23.05.12
Comment 30 Marcel de Rooy 2024-11-20 13:21:05 UTC
This patch seems to resolve issues with an empty flatpickr on opac-reserve using a mobile phone (Android) on 22.11 too. Note that I have OPACMandatoryHoldDates but did not incorporate the change of the second patch.