Bug 30650

Summary: Add a curbside pickup module
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: new feature    
Priority: P5 - low CC: alissa.desaintlaurent, bdaeuber, black23, caroline.cyr-la-rose, dcook, gmcharlt, hagud, koha, kyle.m.hall, lisettepalouse+koha, magnus, nicolas.legrand, severine.queune, sonia.bouis, tomascohen
Version: unspecifiedKeywords: Manual
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25698
Change sponsored?: Sponsored Patch complexity: Large patch
Documentation contact: Caroline Cyr La Rose Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/617
Text to go in the release notes:
With this module staff and patrons will be available to manage curbside pickups from the OPAC and staff interface. It adds a new configuration page to the administration module which allows each library in an installation to set their own rules and time slots for scheduling pickups. If a library chooses to activate curbside pickups, they will be able to manage all stages to the process from within Koha: scheduling a pickup time, preparing items for pickup and completing pickups. Patrons can indicate that they have arrived at the library for their pickup through the OPAC. Use the CurbsidePickup system preference to enable this module.
Version(s) released in:
22.11.00
Bug Depends on:    
Bug Blocks: 31263, 31266, 32890, 32900, 30963, 30964, 30965, 31261, 31262, 31264, 31265, 32108, 32111, 32785, 32786, 32788, 32883, 32891, 34373    
Attachments: Bug 30650: DB changes
Bug 30650: Koha classes
Bug 30650: Add the CurbsidePickup syspref
Bug 30650: Add new permission manage_curbside_pickups
Bug 30650: Add link from the admin home page
Bug 30650: Admin page
Bug 30650: Adjust atomic update
Bug 30650: Be more flexible with opening slots
Bug 30650: Add some useful modules and tests
Bug 30650: Add circulation page view
Bug 30650: Add dayjs and isSameOfAfter plugin
Bug 30650: Add dayjs to opac
Bug 30650: OPAC view
Bug 30650: Add link from the member toolbar
Bug 30650: Notify the patron when a new curbside pickup is created
Bug 30650: DB - Add policy.enable_waiting_holds_only
Bug 30650: Allow to restrict curbside pickup for waiting holds only
Bug 30650: Add link from the mainpage
Bug 30650: Add link from the circ patron page
Bug 30650: Add filter_by_scheduled_today
Bug 30650: Prevent pickup to be created on holiday
Bug 30650: Split the permission
Bug 30650: Select 'schedule a pickup' if no pickups yet
Bug 30650: Prevent browser crash if no pickup interval set
Bug 30650: Compiled CSS
Bug 30650: DBIC
Bug 30650: DBIC specific
Bug 30650: Display holds waiting at the logged-in library
Bug 30650: Display holds waiting at the logged-in library
Bug 30650: Don't display slots that have are before now
Bug 30650: Add dayjs customParseFormat plugin
Bug 30650: Take dateformat into account
Bug 30650: DB changes
Bug 30650: Koha classes
Bug 30650: Add the CurbsidePickup syspref
Bug 30650: Add new permission manage_curbside_pickups
Bug 30650: Add link from the admin home page
Bug 30650: Admin page
Bug 30650: Adjust atomic update
Bug 30650: Be more flexible with opening slots
Bug 30650: Add some useful modules and tests
Bug 30650: Add circulation page view
Bug 30650: Add dayjs and isSameOfAfter plugin
Bug 30650: Add dayjs to opac
Bug 30650: OPAC view
Bug 30650: Add link from the member toolbar
Bug 30650: Notify the patron when a new curbside pickup is created
Bug 30650: DB - Add policy.enable_waiting_holds_only
Bug 30650: Allow to restrict curbside pickup for waiting holds only
Bug 30650: Add link from the mainpage
Bug 30650: Add link from the circ patron page
Bug 30650: Add filter_by_scheduled_today
Bug 30650: Prevent pickup to be created on holiday
Bug 30650: Split the permission
Bug 30650: Select 'schedule a pickup' if no pickups yet
Bug 30650: Prevent browser crash if no pickup interval set
Bug 30650: Compiled CSS
Bug 30650: DBIC
Bug 30650: DBIC specific
Bug 30650: Display holds waiting at the logged-in library
Bug 30650: Don't display slots that have are before now
Bug 30650: Add dayjs customParseFormat plugin
Bug 30650: Take dateformat into account
Bug 30650: Display the link in the admin even if the pref is off
Bug 30650: DB changes
Bug 30650: Koha classes
Bug 30650: Add the CurbsidePickup syspref
Bug 30650: Add new permission manage_curbside_pickups
Bug 30650: Add link from the admin home page
Bug 30650: Admin page
Bug 30650: Adjust atomic update
Bug 30650: Be more flexible with opening slots
Bug 30650: Add some useful modules and tests
Bug 30650: Add circulation page view
Bug 30650: Add dayjs and isSameOfAfter plugin
Bug 30650: Add dayjs to opac
Bug 30650: OPAC view
Bug 30650: Add link from the member toolbar
Bug 30650: Notify the patron when a new curbside pickup is created
Bug 30650: DB - Add policy.enable_waiting_holds_only
Bug 30650: Allow to restrict curbside pickup for waiting holds only
Bug 30650: Add link from the mainpage
Bug 30650: Add link from the circ patron page
Bug 30650: Add filter_by_scheduled_today
Bug 30650: Prevent pickup to be created on holiday
Bug 30650: Split the permission
Bug 30650: Select 'schedule a pickup' if no pickups yet
Bug 30650: Prevent browser crash if no pickup interval set
Bug 30650: DBIC
Bug 30650: DBIC specific
Bug 30650: Display holds waiting at the logged-in library
Bug 30650: Don't display slots that have are before now
Bug 30650: Add dayjs customParseFormat plugin
Bug 30650: Take dateformat into account
Bug 30650: Display the link in the admin even if the pref is off
Bug 30650: Fix some typos
Bug 30650: Add dayjs license to the about page
Bug 30650: Add info to the trash icon
Bug 30650: Add link to the circ nav bar
Bug 30650: Add the sidebar to the circ view
Bug 30650: Split yml on several lines to help translation
Bug 30650: Add message on the admin page if the pref is off
Bug 30650: DB changes
Bug 30650: Koha classes
Bug 30650: Add the CurbsidePickup syspref
Bug 30650: Add new permission manage_curbside_pickups
Bug 30650: Add link from the admin home page
Bug 30650: Admin page
Bug 30650: Adjust atomic update
Bug 30650: Be more flexible with opening slots
Bug 30650: Add some useful modules and tests
Bug 30650: Add circulation page view
Bug 30650: Add dayjs and isSameOfAfter plugin
Bug 30650: Add dayjs to opac
Bug 30650: OPAC view
Bug 30650: Add link from the member toolbar
Bug 30650: Notify the patron when a new curbside pickup is created
Bug 30650: DB - Add policy.enable_waiting_holds_only
Bug 30650: Allow to restrict curbside pickup for waiting holds only
Bug 30650: Add link from the mainpage
Bug 30650: Add link from the circ patron page
Bug 30650: Add filter_by_scheduled_today
Bug 30650: Prevent pickup to be created on holiday
Bug 30650: Split the permission
Bug 30650: Select 'schedule a pickup' if no pickups yet
Bug 30650: Prevent browser crash if no pickup interval set
Bug 30650: DBIC
Bug 30650: DBIC specific
Bug 30650: Display holds waiting at the logged-in library
Bug 30650: Don't display slots that have are before now
Bug 30650: Add dayjs customParseFormat plugin
Bug 30650: Take dateformat into account
Bug 30650: Display the link in the admin even if the pref is off
Bug 30650: Fix some typos
Bug 30650: Add dayjs license to the about page
Bug 30650: Add info to the trash icon
Bug 30650: Add link to the circ nav bar
Bug 30650: Add the sidebar to the circ view
Bug 30650: Split yml on several lines to help translation
Bug 30650: Add message on the admin page if the pref is off
Bug 30650: Add missing name to notice added by database update
Bug 30650: (QA follow-up) Terminology: location - library
Bug 30650: (QA follow-up) Some higher-level methods usage
Bug 30650: Add missing koha_object[s]_class methods
Bug 30650: (QA follow-up) Fix TINYINT fields discrepancy on upgrade

Description Jonathan Druart 2022-04-29 07:32:00 UTC
The plugin is great and people would like to see it part of Koha.

https://github.com/bywatersolutions/koha-plugin-curbside-pickup
Comment 1 Jonathan Druart 2022-04-29 07:34:46 UTC
If you are using the plugin, let me know if you have any questions or enhancement requests.
Comment 2 Hugo Agud 2022-04-29 07:43:53 UTC
Great idea!!!

suggestions:

a) Some libraries has not continues attention, for example in lunch time has not desktop service? now you can set up opening and closing time, good idea add the change for adding closes period
Comment 3 Michal Denar 2022-04-29 08:53:21 UTC
Great idea,
bBut there are some bugs or suggestions for extensions that would be great to incorporate into the final functionality. Like https://github.com/bywatersolutions/koha-plugin-curbside-pickup/issues/27.
Comment 4 Lisette Scheer 2022-05-03 15:31:56 UTC
As a current user, here are some of the issues listed on the GitHub that would be great to have fixed:
-limit ability for patrons to schedule pickups to only patrons with waiting holds (optionally as some libraries may use the plugin for prints or other non-item pickups) 
-Allow 2 (or more) pickup windows a day (maybe there is a lunch break etc) 
-Linking the information about curbside to checkouts for statistics 
-Allowing staff to add/edit note after reservation is made
-Optionally allow scheduling only until X hours before an appointment (preferably each library should be able to set this along with hours etc)
-Additionally allow to limit scheduling out further than x number of days (we'd like to let people schedule 1-2 weeks in advance max, but currently you could schedule out years)
-Pickups showing up on both "staged & ready" and "delivered" tabs when marked as delivered from "staged & ready"
-Option to remove "patron has arrived" button by library (preferably each library should be able to set this along with hours etc)
-Holidays on Calendar can still have appointments scheduled 

Lisette
Comment 5 Jonathan Druart 2022-05-10 10:05:34 UTC
(In reply to Lisette Scheer from comment #4)

Thanks for your input Lisette, that's very useful.

> As a current user, here are some of the issues listed on the GitHub that
> would be great to have fixed:
> -limit ability for patrons to schedule pickups to only patrons with waiting
> holds (optionally as some libraries may use the plugin for prints or other
> non-item pickups) 

Will be done.

> -Allow 2 (or more) pickup windows a day (maybe there is a lunch break etc) 

Will be done (infinite number of pickup windows!)

> -Linking the information about curbside to checkouts for statistics

Do you have an idea about how to implement that already?

> -Allowing staff to add/edit note after reservation is made
> -Optionally allow scheduling only until X hours before an appointment
> (preferably each library should be able to set this along with hours etc)
> -Additionally allow to limit scheduling out further than x number of days
> (we'd like to let people schedule 1-2 weeks in advance max, but currently
> you could schedule out years)

Won't be done, at least not on this bug report.

> -Pickups showing up on both "staged & ready" and "delivered" tabs when
> marked as delivered from "staged & ready"

I don't recreate this problem.

> -Option to remove "patron has arrived" button by library (preferably each
> library should be able to set this along with hours etc)

Shouldn't be too hard to implement, but won't be done in the first iteration.

> -Holidays on Calendar can still have appointments scheduled 

Not done yet, but I should definitely implement that here.

It would be helpful if you could open follow-up bug reports linked to this one. If I have some time left when this first set is integrated, I will work on the follow-up bug reports which got the more votes.
Comment 6 Jonathan Druart 2022-05-10 12:21:34 UTC
Current state of the development can be found at https://gitlab.com/joubu/Koha/-/commits/bug_30650

It can already be tested if you want to provide feedback.

What has been done in this patch set:
- Deal with installations using the existing plugin (upgrade the DB schema and migrate their data)
- Add a new syspref (CurbsidePickup) and a new permission (manage_curbside_pickups)
- Add an administration page to setup the configuration: admin/curbside_pickup.pl
- Add a circulation page to manage the existing pickups, and create new one
- Add a new OPAC view "your curbside pickups" to let patron manage their pickups, and create new ones
- Add link from the "member" toolbar

Improvements compared to the plugin:
- Ability to create several pickup windows per day
- Better display of the pickup times (not in a dropdown list)
- Ability to disable pickups for patrons without waiting holds
- Display pickups on the patron circulation page
- Display pickups of the library on the homepage
- Prevent pickup to be created on a holiday
- Better error handling (exceptions)
- Unit tests
Comment 8 Jonathan Druart 2022-05-10 12:43:10 UTC
All credit goes to the authors of the existing Curbside Pickup plugin, thanks to them, and to Kyle especially!
Comment 9 Jonathan Druart 2022-06-27 13:24:42 UTC
Created attachment 136584 [details] [review]
Bug 30650: DB changes

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 10 Jonathan Druart 2022-06-27 13:24:47 UTC
Created attachment 136585 [details] [review]
Bug 30650: Koha classes

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 11 Jonathan Druart 2022-06-27 13:24:52 UTC
Created attachment 136586 [details] [review]
Bug 30650: Add the CurbsidePickup syspref

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 12 Jonathan Druart 2022-06-27 13:24:57 UTC
Created attachment 136587 [details] [review]
Bug 30650: Add new permission manage_curbside_pickups

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 13 Jonathan Druart 2022-06-27 13:25:02 UTC
Created attachment 136588 [details] [review]
Bug 30650: Add link from the admin home page

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 14 Jonathan Druart 2022-06-27 13:25:07 UTC
Created attachment 136589 [details] [review]
Bug 30650: Admin page

This patch adds a new page in the administration area to manage curbside
pickups.

Test plan:
1. Enable the new CurbsidePickup syspref
2. Go to Administration > Curbside pickups
=> If the logged-in user has the manage_curbside_pickups permission you
should see the administration view
3. Enable the feature for some libraries and configure it.
 a. You must define a "pickup interval" and a "maximum patrons per interval"
values.
 b. If "Patron-scheduled pickup" is enabled, patrons will be able to
schedule curbside pickups from the OPAC
 c. If "Enable for waiting holds only" is enabled, only patron with waiting
holds will be allowed to schedule curbside pickups
 d. In the bottom part of the form you can define the slots available for
curbside pickups.
 e. Save

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 15 Jonathan Druart 2022-06-27 13:25:12 UTC
Created attachment 136590 [details] [review]
Bug 30650: Adjust atomic update

Take into account existing installs using the plugins

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 16 Jonathan Druart 2022-06-27 13:25:18 UTC
Created attachment 136591 [details] [review]
Bug 30650: Be more flexible with opening slots

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 17 Jonathan Druart 2022-06-27 13:25:23 UTC
Created attachment 136592 [details] [review]
Bug 30650: Add some useful modules and tests

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 18 Jonathan Druart 2022-06-27 13:25:37 UTC
Created attachment 136593 [details] [review]
Bug 30650: Add circulation page view

This is the main commit message.

A plugin already exists to manage curbside pickups. This new
enhancehemnt is suggesting an implementation that is ready to be integrated
into Koha core in order to provide the feature out-of-the-box.

What has been done in this patch set:
- Deal with installations using the existing plugin (upgrade the DB schema and migrate their data)
- Add a new syspref (CurbsidePickup) and two new permissions:
  * parameters.manage_curbside_pickups
  * circulate.manage_curbside_pickups
- Add an administration page to setup the configuration: admin/curbside_pickup.pl
- Add a circulation page to manage the existing pickups, and create new one
- Add a new OPAC view "your curbside pickups" to let patron manage their pickups, and create new ones
- Add link from the "member" toolbar

Improvements compared to the plugin:
- Ability to create several pickup windows per day
- Better display of the pickup times (not in a dropdown list)
- Ability to disable pickups for patrons without waiting holds
- Display pickups on the patron circulation page
- Display pickups of the library on the homepage
- Prevent pickup to be created on a holiday
- Better error handling (exceptions)
- Unit tests

More improvements are already planned, see related bug reports.

Test plan:
After you setup the feature correctly from the administration view, you
will be able to use the schedule curbside pickups from the staff
interface, and from the OPAC interface if you selected "patron-scheduled
pickup"
A. Staff interface
1. Go to Circulation > Curbside pickups
=> If the logged-in user has the circulate.manage_curbside_pickups
permission you will be able to create and manage curbside pickups
2. Go to a patron detail page and click the "Schedule pickup" button in
the toolbar
3. If the patron has waiting holds and you selected "Enable for waiting holds only",
of if you didn't select the option, you will be able to select a pickup
date and slots to create a pickup.
4. Confirm that you cannot create more pickups per slot than what you
defined in the curbside pickup configuration for this library
5. Confirm that you cannot create a pickup if the feature is disabled
for the library
6. Notice that you can mark the pickup as "stage and ready", then
"patron is outside" and finally "delivered today". You can also rollback
the change
7. Notice that once the pickup has been marked as delivered, the
item has been checked out and that a new notice has been generated (if
the patron has "Hold_Filled" in their messaging preferences
8. Confirm that the information about current pickups is displayed on
the circulation page of the patron

B. OPAC interface
1. Create a new curbside pickup from the OPAC
2. Confirm that the same limitations as from the staff interface are in
effect (waiting holds, number of patron per slots, etc.)
3. Confirm that you can cancel a pickup and alert staff of you arrival
4. Confirm that you cannot cancel a pickup that has been delivered
already

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 19 Jonathan Druart 2022-06-27 13:25:44 UTC
Created attachment 136594 [details] [review]
Bug 30650: Add dayjs and isSameOfAfter plugin

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 20 Jonathan Druart 2022-06-27 13:25:49 UTC
Created attachment 136595 [details] [review]
Bug 30650: Add dayjs to opac

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 21 Jonathan Druart 2022-06-27 13:25:55 UTC
Created attachment 136596 [details] [review]
Bug 30650: OPAC view

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 22 Jonathan Druart 2022-06-27 13:26:00 UTC
Created attachment 136597 [details] [review]
Bug 30650: Add link from the member toolbar

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 23 Jonathan Druart 2022-06-27 13:26:05 UTC
Created attachment 136598 [details] [review]
Bug 30650: Notify the patron when a new curbside pickup is created

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 24 Jonathan Druart 2022-06-27 13:26:10 UTC
Created attachment 136599 [details] [review]
Bug 30650: DB - Add policy.enable_waiting_holds_only

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 25 Jonathan Druart 2022-06-27 13:26:15 UTC
Created attachment 136600 [details] [review]
Bug 30650: Allow to restrict curbside pickup for waiting holds only

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 26 Jonathan Druart 2022-06-27 13:26:20 UTC
Created attachment 136601 [details] [review]
Bug 30650: Add link from the mainpage

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 27 Jonathan Druart 2022-06-27 13:26:25 UTC
Created attachment 136602 [details] [review]
Bug 30650: Add link from the circ patron page

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 28 Jonathan Druart 2022-06-27 13:26:30 UTC
Created attachment 136603 [details] [review]
Bug 30650: Add filter_by_scheduled_today

A bit of cleaning

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 29 Jonathan Druart 2022-06-27 13:26:35 UTC
Created attachment 136604 [details] [review]
Bug 30650: Prevent pickup to be created on holiday

We could improve this more and add disable the holiday from the date
picker widget, but it's out of the scope here.

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 30 Jonathan Druart 2022-06-27 13:26:40 UTC
Created attachment 136605 [details] [review]
Bug 30650: Split the permission

We actually want one permission to manage the administration page, and
one for the circulation module.
Comment 31 Jonathan Druart 2022-06-27 13:26:44 UTC
Created attachment 136606 [details] [review]
Bug 30650: Select 'schedule a pickup' if no pickups yet
Comment 32 Jonathan Druart 2022-06-27 13:26:50 UTC
Created attachment 136607 [details] [review]
Bug 30650: Prevent browser crash if no pickup interval set

Don't get stuck in an infinite loop
Comment 33 Jonathan Druart 2022-06-27 13:26:56 UTC
Created attachment 136608 [details] [review]
Bug 30650: Compiled CSS

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 34 Jonathan Druart 2022-06-27 13:27:02 UTC
Created attachment 136609 [details] [review]
Bug 30650: DBIC
Comment 35 Jonathan Druart 2022-06-27 13:27:07 UTC
Created attachment 136610 [details] [review]
Bug 30650: DBIC specific

Sponsored-by: Association KohaLa - https://koha-fr.org/
Comment 36 Jonathan Druart 2022-06-27 13:28:00 UTC
This is ready for testing, see test plans in patches "Bug 30650: Admin page" and "Bug 30650: Add circulation page view"
Comment 37 Jonathan Druart 2022-07-04 15:13:10 UTC
Created attachment 137095 [details] [review]
Bug 30650: Display holds waiting at the logged-in library

Not those waiting at the library of the logged-in user
Comment 38 Jonathan Druart 2022-07-04 15:39:41 UTC
Created attachment 137100 [details] [review]
Bug 30650: Display holds waiting at the logged-in library

Not those waiting at the library of the logged-in user
Comment 39 Jonathan Druart 2022-07-06 07:57:26 UTC
Created attachment 137199 [details] [review]
Bug 30650: Don't display slots that have are before now
Comment 40 Jonathan Druart 2022-07-06 14:10:12 UTC
Created attachment 137211 [details] [review]
Bug 30650: Add dayjs customParseFormat plugin
Comment 41 Jonathan Druart 2022-07-06 14:10:17 UTC
Created attachment 137212 [details] [review]
Bug 30650: Take dateformat into account

dayjs init was not passing the dateformat.
We really need to clean the date handling with the following two bugs:
  Bug 30310 - Replace Moment.js with Day.js
and
  Bug 30718 - Use flatpickr's altInput option everywhere

But it's outside the scope of this bug report.
Comment 42 Koha Team University Lyon 3 2022-07-07 09:36:46 UTC
Hi,
I've tested on a sandbox and everything is as excepted.
- the two circulation permissions are OK
- Curbside pickup is possible only for branches where it has been defined
- If you choose to "Enable for waiting holds only", you can't choose a slot for a pickup if the patron doesn't have available hold. But it's possible if you don't check this in the setting
- If you choose "Enable patrons to schedule their own curbside" on some branches, they are displayed in the OPAC in the user account. If the patron didn't have any available holds and that it is asked for one branches, this one is displayed and disabled. If no branch is set for enable patrons to schedule pickup, the tab doesn't appear in the user account
- The slot for the days, the hours and the intevals are well displayed
- The checking of the number of patrons alowed for the same slot is ok
- the change of status are ok and you can rollback is wanted
- the item is correctly checked out when it is marked as delivered
- A notice is send to the patron is he has a primary mail defined and if the choice "email" is checked for hold filled in the Patron messaging preferences
- the curbisde pickup are displayed on the checjout page for the patron
- If a patron click on "Alert staff of your arrival", the curbside pickup switch on the "patron is arrived" tab in the intranet after 1 or 2 mn

Thanks joubu :)

Sonia
Comment 43 Jonathan Druart 2022-07-07 13:00:42 UTC
Created attachment 137277 [details] [review]
Bug 30650: DB changes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 44 Jonathan Druart 2022-07-07 13:00:49 UTC
Created attachment 137278 [details] [review]
Bug 30650: Koha classes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 45 Jonathan Druart 2022-07-07 13:00:55 UTC
Created attachment 137279 [details] [review]
Bug 30650: Add the CurbsidePickup syspref

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 46 Jonathan Druart 2022-07-07 13:01:01 UTC
Created attachment 137280 [details] [review]
Bug 30650: Add new permission manage_curbside_pickups

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 47 Jonathan Druart 2022-07-07 13:01:06 UTC
Created attachment 137281 [details] [review]
Bug 30650: Add link from the admin home page

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 48 Jonathan Druart 2022-07-07 13:01:12 UTC
Created attachment 137282 [details] [review]
Bug 30650: Admin page

This patch adds a new page in the administration area to manage curbside
pickups.

Test plan:
1. Enable the new CurbsidePickup syspref
2. Go to Administration > Curbside pickups
=> If the logged-in user has the manage_curbside_pickups permission you
should see the administration view
3. Enable the feature for some libraries and configure it.
 a. You must define a "pickup interval" and a "maximum patrons per interval"
values.
 b. If "Patron-scheduled pickup" is enabled, patrons will be able to
schedule curbside pickups from the OPAC
 c. If "Enable for waiting holds only" is enabled, only patron with waiting
holds will be allowed to schedule curbside pickups
 d. In the bottom part of the form you can define the slots available for
curbside pickups.
 e. Save

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 49 Jonathan Druart 2022-07-07 13:01:18 UTC
Created attachment 137283 [details] [review]
Bug 30650: Adjust atomic update

Take into account existing installs using the plugins

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 50 Jonathan Druart 2022-07-07 13:01:24 UTC
Created attachment 137284 [details] [review]
Bug 30650: Be more flexible with opening slots

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 51 Jonathan Druart 2022-07-07 13:01:30 UTC
Created attachment 137285 [details] [review]
Bug 30650: Add some useful modules and tests

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 52 Jonathan Druart 2022-07-07 13:01:37 UTC
Created attachment 137286 [details] [review]
Bug 30650: Add circulation page view

This is the main commit message.

A plugin already exists to manage curbside pickups. This new
enhancehemnt is suggesting an implementation that is ready to be integrated
into Koha core in order to provide the feature out-of-the-box.

What has been done in this patch set:
- Deal with installations using the existing plugin (upgrade the DB schema and migrate their data)
- Add a new syspref (CurbsidePickup) and two new permissions:
  * parameters.manage_curbside_pickups
  * circulate.manage_curbside_pickups
- Add an administration page to setup the configuration: admin/curbside_pickup.pl
- Add a circulation page to manage the existing pickups, and create new one
- Add a new OPAC view "your curbside pickups" to let patron manage their pickups, and create new ones
- Add link from the "member" toolbar

Improvements compared to the plugin:
- Ability to create several pickup windows per day
- Better display of the pickup times (not in a dropdown list)
- Ability to disable pickups for patrons without waiting holds
- Display pickups on the patron circulation page
- Display pickups of the library on the homepage
- Prevent pickup to be created on a holiday
- Better error handling (exceptions)
- Unit tests

More improvements are already planned, see related bug reports.

Test plan:
After you setup the feature correctly from the administration view, you
will be able to use the schedule curbside pickups from the staff
interface, and from the OPAC interface if you selected "patron-scheduled
pickup"
A. Staff interface
1. Go to Circulation > Curbside pickups
=> If the logged-in user has the circulate.manage_curbside_pickups
permission you will be able to create and manage curbside pickups
2. Go to a patron detail page and click the "Schedule pickup" button in
the toolbar
3. If the patron has waiting holds and you selected "Enable for waiting holds only",
of if you didn't select the option, you will be able to select a pickup
date and slots to create a pickup.
4. Confirm that you cannot create more pickups per slot than what you
defined in the curbside pickup configuration for this library
5. Confirm that you cannot create a pickup if the feature is disabled
for the library
6. Notice that you can mark the pickup as "stage and ready", then
"patron is outside" and finally "delivered today". You can also rollback
the change
7. Notice that once the pickup has been marked as delivered, the
item has been checked out and that a new notice has been generated (if
the patron has "Hold_Filled" in their messaging preferences
8. Confirm that the information about current pickups is displayed on
the circulation page of the patron

B. OPAC interface
1. Create a new curbside pickup from the OPAC
2. Confirm that the same limitations as from the staff interface are in
effect (waiting holds, number of patron per slots, etc.)
3. Confirm that you can cancel a pickup and alert staff of you arrival
4. Confirm that you cannot cancel a pickup that has been delivered
already

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 53 Jonathan Druart 2022-07-07 13:01:43 UTC
Created attachment 137287 [details] [review]
Bug 30650: Add dayjs and isSameOfAfter plugin

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 54 Jonathan Druart 2022-07-07 13:01:50 UTC
Created attachment 137288 [details] [review]
Bug 30650: Add dayjs to opac

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 55 Jonathan Druart 2022-07-07 13:01:56 UTC
Created attachment 137289 [details] [review]
Bug 30650: OPAC view

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 56 Jonathan Druart 2022-07-07 13:02:02 UTC
Created attachment 137290 [details] [review]
Bug 30650: Add link from the member toolbar

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 57 Jonathan Druart 2022-07-07 13:02:07 UTC
Created attachment 137291 [details] [review]
Bug 30650: Notify the patron when a new curbside pickup is created

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 58 Jonathan Druart 2022-07-07 13:02:13 UTC
Created attachment 137292 [details] [review]
Bug 30650: DB - Add policy.enable_waiting_holds_only

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 59 Jonathan Druart 2022-07-07 13:02:19 UTC
Created attachment 137293 [details] [review]
Bug 30650: Allow to restrict curbside pickup for waiting holds only

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 60 Jonathan Druart 2022-07-07 13:02:25 UTC
Created attachment 137294 [details] [review]
Bug 30650: Add link from the mainpage

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 61 Jonathan Druart 2022-07-07 13:02:30 UTC
Created attachment 137295 [details] [review]
Bug 30650: Add link from the circ patron page

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 62 Jonathan Druart 2022-07-07 13:02:36 UTC
Created attachment 137296 [details] [review]
Bug 30650: Add filter_by_scheduled_today

A bit of cleaning

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 63 Jonathan Druart 2022-07-07 13:02:42 UTC
Created attachment 137297 [details] [review]
Bug 30650: Prevent pickup to be created on holiday

We could improve this more and add disable the holiday from the date
picker widget, but it's out of the scope here.

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 64 Jonathan Druart 2022-07-07 13:02:47 UTC
Created attachment 137298 [details] [review]
Bug 30650: Split the permission

We actually want one permission to manage the administration page, and
one for the circulation module.

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 65 Jonathan Druart 2022-07-07 13:02:53 UTC
Created attachment 137299 [details] [review]
Bug 30650: Select 'schedule a pickup' if no pickups yet

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 66 Jonathan Druart 2022-07-07 13:02:58 UTC
Created attachment 137300 [details] [review]
Bug 30650: Prevent browser crash if no pickup interval set

Don't get stuck in an infinite loop

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 67 Jonathan Druart 2022-07-07 13:03:05 UTC
Created attachment 137301 [details] [review]
Bug 30650: Compiled CSS

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 68 Jonathan Druart 2022-07-07 13:03:11 UTC
Created attachment 137302 [details] [review]
Bug 30650: DBIC

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 69 Jonathan Druart 2022-07-07 13:03:17 UTC
Created attachment 137303 [details] [review]
Bug 30650: DBIC specific

Note for QA - a false positive is raised by the QA tools:

"""
The new column curbside_pickup_opening_slots.day is using TINYINT(1) as
type but is not defined as boolean in the schema file
(Koha/Schema/Result/CurbsidePickupOpeningSlot.pm)
"""

=> It's not a boolean!

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 70 Jonathan Druart 2022-07-07 13:03:23 UTC
Created attachment 137304 [details] [review]
Bug 30650: Display holds waiting at the logged-in library

Not those waiting at the library of the logged-in user

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 71 Jonathan Druart 2022-07-07 13:03:28 UTC
Created attachment 137305 [details] [review]
Bug 30650: Don't display slots that have are before now

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 72 Jonathan Druart 2022-07-07 13:03:34 UTC
Created attachment 137306 [details] [review]
Bug 30650: Add dayjs customParseFormat plugin

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 73 Jonathan Druart 2022-07-07 13:03:40 UTC
Created attachment 137307 [details] [review]
Bug 30650: Take dateformat into account

dayjs init was not passing the dateformat.
We really need to clean the date handling with the following two bugs:
  Bug 30310 - Replace Moment.js with Day.js
and
  Bug 30718 - Use flatpickr's altInput option everywhere

But it's outside the scope of this bug report.

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 74 Jonathan Druart 2022-07-29 09:22:02 UTC
Created attachment 138261 [details] [review]
Bug 30650: Display the link in the admin even if the pref is off

The link in the sidebar was displayed already. The idea is to let admins
configure the module before enabling it for users.
Comment 75 Jonathan Druart 2022-07-29 09:31:23 UTC
Created attachment 138262 [details] [review]
Bug 30650: DB changes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 76 Jonathan Druart 2022-07-29 09:31:31 UTC
Created attachment 138263 [details] [review]
Bug 30650: Koha classes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 77 Jonathan Druart 2022-07-29 09:31:37 UTC
Created attachment 138264 [details] [review]
Bug 30650: Add the CurbsidePickup syspref

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 78 Jonathan Druart 2022-07-29 09:31:43 UTC
Created attachment 138265 [details] [review]
Bug 30650: Add new permission manage_curbside_pickups

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 79 Jonathan Druart 2022-07-29 09:31:49 UTC
Created attachment 138266 [details] [review]
Bug 30650: Add link from the admin home page

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 80 Jonathan Druart 2022-07-29 09:31:56 UTC
Created attachment 138267 [details] [review]
Bug 30650: Admin page

This patch adds a new page in the administration area to manage curbside
pickups.

Test plan:
1. Enable the new CurbsidePickup syspref
2. Go to Administration > Curbside pickups
=> If the logged-in user has the manage_curbside_pickups permission you
should see the administration view
3. Enable the feature for some libraries and configure it.
 a. You must define a "pickup interval" and a "maximum patrons per interval"
values.
 b. If "Patron-scheduled pickup" is enabled, patrons will be able to
schedule curbside pickups from the OPAC
 c. If "Enable for waiting holds only" is enabled, only patron with waiting
holds will be allowed to schedule curbside pickups
 d. In the bottom part of the form you can define the slots available for
curbside pickups.
 e. Save

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 81 Jonathan Druart 2022-07-29 09:32:02 UTC
Created attachment 138268 [details] [review]
Bug 30650: Adjust atomic update

Take into account existing installs using the plugins

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 82 Jonathan Druart 2022-07-29 09:32:08 UTC
Created attachment 138269 [details] [review]
Bug 30650: Be more flexible with opening slots

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 83 Jonathan Druart 2022-07-29 09:32:14 UTC
Created attachment 138270 [details] [review]
Bug 30650: Add some useful modules and tests

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 84 Jonathan Druart 2022-07-29 09:32:19 UTC
Created attachment 138271 [details] [review]
Bug 30650: Add circulation page view

This is the main commit message.

A plugin already exists to manage curbside pickups. This new
enhancehemnt is suggesting an implementation that is ready to be integrated
into Koha core in order to provide the feature out-of-the-box.

What has been done in this patch set:
- Deal with installations using the existing plugin (upgrade the DB schema and migrate their data)
- Add a new syspref (CurbsidePickup) and two new permissions:
  * parameters.manage_curbside_pickups
  * circulate.manage_curbside_pickups
- Add an administration page to setup the configuration: admin/curbside_pickup.pl
- Add a circulation page to manage the existing pickups, and create new one
- Add a new OPAC view "your curbside pickups" to let patron manage their pickups, and create new ones
- Add link from the "member" toolbar

Improvements compared to the plugin:
- Ability to create several pickup windows per day
- Better display of the pickup times (not in a dropdown list)
- Ability to disable pickups for patrons without waiting holds
- Display pickups on the patron circulation page
- Display pickups of the library on the homepage
- Prevent pickup to be created on a holiday
- Better error handling (exceptions)
- Unit tests

More improvements are already planned, see related bug reports.

Test plan:
After you setup the feature correctly from the administration view, you
will be able to use the schedule curbside pickups from the staff
interface, and from the OPAC interface if you selected "patron-scheduled
pickup"
A. Staff interface
1. Go to Circulation > Curbside pickups
=> If the logged-in user has the circulate.manage_curbside_pickups
permission you will be able to create and manage curbside pickups
2. Go to a patron detail page and click the "Schedule pickup" button in
the toolbar
3. If the patron has waiting holds and you selected "Enable for waiting holds only",
of if you didn't select the option, you will be able to select a pickup
date and slots to create a pickup.
4. Confirm that you cannot create more pickups per slot than what you
defined in the curbside pickup configuration for this library
5. Confirm that you cannot create a pickup if the feature is disabled
for the library
6. Notice that you can mark the pickup as "stage and ready", then
"patron is outside" and finally "delivered today". You can also rollback
the change
7. Notice that once the pickup has been marked as delivered, the
item has been checked out and that a new notice has been generated (if
the patron has "Hold_Filled" in their messaging preferences
8. Confirm that the information about current pickups is displayed on
the circulation page of the patron

B. OPAC interface
1. Create a new curbside pickup from the OPAC
2. Confirm that the same limitations as from the staff interface are in
effect (waiting holds, number of patron per slots, etc.)
3. Confirm that you can cancel a pickup and alert staff of you arrival
4. Confirm that you cannot cancel a pickup that has been delivered
already

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 85 Jonathan Druart 2022-07-29 09:32:24 UTC
Created attachment 138272 [details] [review]
Bug 30650: Add dayjs and isSameOfAfter plugin

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 86 Jonathan Druart 2022-07-29 09:32:30 UTC
Created attachment 138273 [details] [review]
Bug 30650: Add dayjs to opac

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 87 Jonathan Druart 2022-07-29 09:32:35 UTC
Created attachment 138274 [details] [review]
Bug 30650: OPAC view

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 88 Jonathan Druart 2022-07-29 09:32:40 UTC
Created attachment 138275 [details] [review]
Bug 30650: Add link from the member toolbar

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 89 Jonathan Druart 2022-07-29 09:32:46 UTC
Created attachment 138276 [details] [review]
Bug 30650: Notify the patron when a new curbside pickup is created

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 90 Jonathan Druart 2022-07-29 09:32:52 UTC
Created attachment 138277 [details] [review]
Bug 30650: DB - Add policy.enable_waiting_holds_only

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 91 Jonathan Druart 2022-07-29 09:32:58 UTC
Created attachment 138278 [details] [review]
Bug 30650: Allow to restrict curbside pickup for waiting holds only

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 92 Jonathan Druart 2022-07-29 09:33:04 UTC
Created attachment 138279 [details] [review]
Bug 30650: Add link from the mainpage

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 93 Jonathan Druart 2022-07-29 09:33:09 UTC
Created attachment 138280 [details] [review]
Bug 30650: Add link from the circ patron page

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 94 Jonathan Druart 2022-07-29 09:33:15 UTC
Created attachment 138281 [details] [review]
Bug 30650: Add filter_by_scheduled_today

A bit of cleaning

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 95 Jonathan Druart 2022-07-29 09:33:20 UTC
Created attachment 138282 [details] [review]
Bug 30650: Prevent pickup to be created on holiday

We could improve this more and add disable the holiday from the date
picker widget, but it's out of the scope here.

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 96 Jonathan Druart 2022-07-29 09:33:26 UTC
Created attachment 138283 [details] [review]
Bug 30650: Split the permission

We actually want one permission to manage the administration page, and
one for the circulation module.

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 97 Jonathan Druart 2022-07-29 09:33:31 UTC
Created attachment 138284 [details] [review]
Bug 30650: Select 'schedule a pickup' if no pickups yet

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 98 Jonathan Druart 2022-07-29 09:33:37 UTC
Created attachment 138285 [details] [review]
Bug 30650: Prevent browser crash if no pickup interval set

Don't get stuck in an infinite loop

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 99 Jonathan Druart 2022-07-29 09:33:42 UTC
Created attachment 138286 [details] [review]
Bug 30650: DBIC

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 100 Jonathan Druart 2022-07-29 09:33:48 UTC
Created attachment 138287 [details] [review]
Bug 30650: DBIC specific

Note for QA - a false positive is raised by the QA tools:

"""
The new column curbside_pickup_opening_slots.day is using TINYINT(1) as
type but is not defined as boolean in the schema file
(Koha/Schema/Result/CurbsidePickupOpeningSlot.pm)
"""

=> It's not a boolean!

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 101 Jonathan Druart 2022-07-29 09:33:53 UTC
Created attachment 138288 [details] [review]
Bug 30650: Display holds waiting at the logged-in library

Not those waiting at the library of the logged-in user

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 102 Jonathan Druart 2022-07-29 09:33:59 UTC
Created attachment 138289 [details] [review]
Bug 30650: Don't display slots that have are before now

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 103 Jonathan Druart 2022-07-29 09:34:05 UTC
Created attachment 138290 [details] [review]
Bug 30650: Add dayjs customParseFormat plugin

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 104 Jonathan Druart 2022-07-29 09:34:11 UTC
Created attachment 138291 [details] [review]
Bug 30650: Take dateformat into account

dayjs init was not passing the dateformat.
We really need to clean the date handling with the following two bugs:
  Bug 30310 - Replace Moment.js with Day.js
and
  Bug 30718 - Use flatpickr's altInput option everywhere

But it's outside the scope of this bug report.

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>
Comment 105 Jonathan Druart 2022-07-29 09:34:17 UTC
Created attachment 138292 [details] [review]
Bug 30650: Display the link in the admin even if the pref is off

The link in the sidebar was displayed already. The idea is to let admins
configure the module before enabling it for users.
Comment 106 Jonathan Druart 2022-07-29 09:35:42 UTC
There was a typo 'Cubside' in the exception name, and on the UI.
Comment 107 Katrin Fischer 2022-07-29 10:00:48 UTC
o) Tests are failing, but possibly just a typo/later change causing it:

    #   Failed test 'Notice correctly generated'
    #   at t/db_dependent/Koha/CurbsidePickups.t line 299.
    #          got: 'You have schedule a curbside pickup for Te9oDc'
    #     expected: 'You have schedule a curbside pickup for Te9oDc.'
    # Looks like you failed 1 test of 2.


Other finds from code review:

1) Tiny typos

* Curbside pickups are not enabled for your location. = library
* Pickup Date/Time = capitalization
* ... it is an holiday = it is a holiday. (multiple)
* Deliver Date/Time = capitalization
* There is no waiting holds for this patron at this location. = There are no... at this library
* No pickup time define for this day. = defined (multiple)
* You don't have waiting holds at this location = library

2) New JS libraries

There are no license statements for these, as we only add the minified version:

dayjs.min.js
isSameOrAfter.js
customParseFormat.js

Should they be listed on the about page?

Please check!

I'll keep testing on GUI side.
Comment 108 Katrin Fischer 2022-07-29 13:48:04 UTC
From testing in the GUI:

3) Curbside pickup configuration

The trashcan icon behind the times is missing a space before it. It also doesn't have an alt text/tool tip when hovering.

4) Circulation start page

a) With CircSidebar enabled, the curbside pickup entry is missing from the sidebar.
b) Also the curbside configuration page doesn't show the sidebar.
c) Capitalization: Curbside Pickup = Curbside pickup

5) Schedule pickup

a) When scheduling a pickup the calendar widget shows future days greyed out and allows to select dates in the past. (blocker)
b) The button remains greyed out until you pick a date with pickup times available. I think a tool tip would be very helpful here. (not blocker)

5) New notice NEW_CURBSIDE_PICKUP

a) Tiny typo: You have schedule a curbside pickup for [% branch.branchname %] => ... scheduled (normal)
b) Can we break the content text into 3 entries in the .yml file for new installations (one per sentence) to ease translation a bit? (normal)

6) Koha start page

a) The start page for new pickups reads: New curbside pickups 1. We should add a : here: 
New curbside pickups: 1 
This makes it also more consistent with the other entries (looked at suggestions and patron modifications). (trivial)


7) OPAC view

As this is the patron side of things, I worry more about that than about other stuff. I have some issues here:

a) The alert link will only be functional, when the pickup has been "staged & ready". But there is no clue in the GUI about this and the patron might be in front of the library without being able to see what's going on. 
b) Once the pickup has been "marked as delivered" it still remains in the list with no clue that it has been picked up already. It would be nicer to display, that it's already been delivered. Note: only today's and future pickups will show in the list.

Maybe this could be a separate bug as well to get some feedback, but it feels like we should give the patrons more information there.

X) Possible enhancements

These are supposed to go into new wish list bugs. But maybe you could look through them quickly to check if I have missed/misunderstood something?

a) Curbside pickup configuration page: The weekday pull down and display should follow the CalendarFirstDayofWeek system preference.

b) Show a note on top of the Curbside pickup configuration page, if the Curbside pickup module is not enabled (similar to what we do for the Transport cost matrix administration page)

c) Improve GUI for pickup up pickup times in staff interface: The times are a little hard to select/hard to read. I think it would be nice to make this look more like in the OPAC. 

d) Scheduling a pickup from the curbside pickups page only will only work with the cardnumber. It would be nicer to use the standard patron search for this.

e) Add logging for curbside pickup location actions

f) Have different permission descriptions (might require to change the permission code) to make it clearer what the difference between each of the permissions is.

h) Make curbside pickup confirmation its own messaging preference or adapt messaging preference description to include curbside pickups when the pref is enabled.
Comment 109 Jonathan Druart 2022-07-29 15:09:44 UTC
Created attachment 138319 [details] [review]
Bug 30650: Fix some typos

* Curbside pickups are not enabled for your location. = library
* Pickup Date/Time = capitalization
* ... it is an holiday = it is a holiday. (multiple)
* Deliver Date/Time = capitalization
* There is no waiting holds for this patron at this location. = There are no... at this library
* No pickup time define for this day. = defined (multiple)
* You don't have waiting holds at this location = library
Comment 110 Jonathan Druart 2022-07-29 15:09:50 UTC
Created attachment 138320 [details] [review]
Bug 30650: Add dayjs license to the about page
Comment 111 Jonathan Druart 2022-07-29 15:09:58 UTC
Created attachment 138321 [details] [review]
Bug 30650: Add info to the trash icon

And add a missing space between the time and icon
Comment 112 Jonathan Druart 2022-07-29 15:10:05 UTC
Created attachment 138322 [details] [review]
Bug 30650: Add link to the circ nav bar
Comment 113 Jonathan Druart 2022-07-29 15:10:12 UTC
Created attachment 138323 [details] [review]
Bug 30650: Add the sidebar to the circ view
Comment 114 Jonathan Druart 2022-07-29 15:10:19 UTC
Created attachment 138324 [details] [review]
Bug 30650: Split yml on several lines to help translation
Comment 115 Jonathan Druart 2022-07-29 15:10:25 UTC
Created attachment 138325 [details] [review]
Bug 30650: Add message on the admin page if the pref is off
Comment 116 Jonathan Druart 2022-07-29 15:13:13 UTC
(In reply to Katrin Fischer from comment #107)

Thanks a lot, Katrin, for you great QA session :)

> o) Tests are failing, but possibly just a typo/later change causing it:
> 
>     #   Failed test 'Notice correctly generated'
>     #   at t/db_dependent/Koha/CurbsidePickups.t line 299.
>     #          got: 'You have schedule a curbside pickup for Te9oDc'
>     #     expected: 'You have schedule a curbside pickup for Te9oDc.'
>     # Looks like you failed 1 test of 2.
> 
> 
> Other finds from code review:
> 
> 1) Tiny typos
> 
> * Curbside pickups are not enabled for your location. = library
> * Pickup Date/Time = capitalization
> * ... it is an holiday = it is a holiday. (multiple)
> * Deliver Date/Time = capitalization
> * There is no waiting holds for this patron at this location. = There are
> no... at this library
> * No pickup time define for this day. = defined (multiple)
> * You don't have waiting holds at this location = library

Done in patch "Fix some typos"

> 2) New JS libraries
> 
> There are no license statements for these, as we only add the minified
> version:
> 
> dayjs.min.js
> isSameOrAfter.js
> customParseFormat.js
> 
> Should they be listed on the about page?

Yes, done in "Add dayjs license to the about page"

(In reply to Katrin Fischer from comment #108)
> From testing in the GUI:
> 
> 3) Curbside pickup configuration
> 
> The trashcan icon behind the times is missing a space before it. It also
> doesn't have an alt text/tool tip when hovering.

Done in "Add info to the trash icon"

> 4) Circulation start page
> 
> a) With CircSidebar enabled, the curbside pickup entry is missing from the
> sidebar.

Done in "Add link to the circ nav bar"

> b) Also the curbside configuration page doesn't show the sidebar.

"Add the sidebar to the circ view"

> c) Capitalization: Curbside Pickup = Curbside pickup

Done in "Fix some typos"

> 5) Schedule pickup
> 
> a) When scheduling a pickup the calendar widget shows future days greyed out
> and allows to select dates in the past. (blocker)

I don't recreate that. All dates can be selected using the widget, but dates in the past won't display slots.
This is the correct behaviour. We could improve and "disable" dates in the past or even dates without slots, but that's for another bug report.

Opened bug 31261 and bug 31262.

> b) The button remains greyed out until you pick a date with pickup times
> available. I think a tool tip would be very helpful here. (not blocker)

I think I understand what you mean: you have to remove the focus from the widget to be able to select a slot.
I've tried to fix it already but didn't manage to find a correct solution.

> 5) New notice NEW_CURBSIDE_PICKUP
> 
> a) Tiny typo: You have schedule a curbside pickup for [% branch.branchname
> %] => ... scheduled (normal)

Fixed earlier.

> b) Can we break the content text into 3 entries in the .yml file for new
> installations (one per sentence) to ease translation a bit? (normal)

"Split yml on several lines to help translation"

> 6) Koha start page
> 
> a) The start page for new pickups reads: New curbside pickups 1. We should
> add a : here: 
> New curbside pickups: 1 
> This makes it also more consistent with the other entries (looked at
> suggestions and patron modifications). (trivial)

"Fix some typos"

> 7) OPAC view
> 
> As this is the patron side of things, I worry more about that than about
> other stuff. I have some issues here:
> 
> a) The alert link will only be functional, when the pickup has been "staged
> & ready". But there is no clue in the GUI about this and the patron might be
> in front of the library without being able to see what's going on.

I would like this to be discussed on a separate bug report, it was the previous (plugin) behaviour.
See bug 31263.

> b) Once the pickup has been "marked as delivered" it still remains in the
> list with no clue that it has been picked up already. It would be nicer to
> display, that it's already been delivered. Note: only today's and future
> pickups will show in the list.
> 
> Maybe this could be a separate bug as well to get some feedback, but it
> feels like we should give the patrons more information there.

What do you suggest? Do you want a separate column to display the status?

> X) Possible enhancements
> 
> These are supposed to go into new wish list bugs. But maybe you could look
> through them quickly to check if I have missed/misunderstood something?
> 
> a) Curbside pickup configuration page: The weekday pull down and display
> should follow the CalendarFirstDayofWeek system preference.

Why do we accept all the day of the week in this syspref?!
It should, indeed, but not trivial. Opening a separate bug and not sure it will be fixed. That seems a lot of work for not much to gain. Maybe first keeping only Monday, Saturday and Sunday...

...ha ... bug 12137 comment 16...

Opened bug 31264!

> b) Show a note on top of the Curbside pickup configuration page, if the
> Curbside pickup module is not enabled (similar to what we do for the
> Transport cost matrix administration page)

Done in "Add message on the admin page if the pref is off"

> c) Improve GUI for pickup up pickup times in staff interface: The times are
> a little hard to select/hard to read. I think it would be nice to make this
> look more like in the OPAC. 

Opened a separate bug, I would like to ask Owen his opinion what's best here. We could improve both I think.
The plugin was displaying the list in a dropdown list, so it's already a nice improvement how it is now ;)

Opened bug 31265 for discussion.

> d) Scheduling a pickup from the curbside pickups page only will only work
> with the cardnumber. It would be nicer to use the standard patron search for
> this.

Bug 30965 is waiting for you PQA stamp :)

> e) Add logging for curbside pickup location actions

Can you open a bug report with the detail of what you have in mind exactly?

> f) Have different permission descriptions (might require to change the
> permission code) to make it clearer what the difference between each of the
> permissions is.

Is that blocker? :D
If not done here it won't be done. On the other hand I don't see how it could be a problem (the module code is different).

> h) Make curbside pickup confirmation its own messaging preference or adapt
> messaging preference description to include curbside pickups when the pref
> is enabled.

Yes, that is definitely something we should do. I've copied the plugin's behaviour but it's not obvious.
Opened bug 31266.

We will need to prioritize what we want first :)

I will pick the related bugs with the highest number of votes.
Comment 117 Katrin Fischer 2022-07-29 15:34:30 UTC
*** Bug 25698 has been marked as a duplicate of this bug. ***
Comment 118 Katrin Fischer 2022-07-29 16:14:17 UTC
> > 0) Tests are failing, but possibly just a typo/later change causing it:

Tests pass now.

> > 1) Tiny typos

Fixed another "location" to library in a follow-up.

> > 2) New JS libraries
> Yes, done in "Add dayjs license to the about page"

Ok!


> > 3) Curbside pickup configuration
> > 
> > The trashcan icon behind the times is missing a space before it. It also
> > doesn't have an alt text/tool tip when hovering.
> 
> Done in "Add info to the trash icon"

Much better, thanks!


> > 4) Circulation start page
> > 
> > a) With CircSidebar enabled, the curbside pickup entry is missing from the
> > sidebar.
> Done in "Add link to the circ nav bar"

OK!

> > b) Also the curbside configuration page doesn't show the sidebar.
> 
> "Add the sidebar to the circ view"

OK!

> > c) Capitalization: Curbside Pickup = Curbside pickup

OK!

> > 5) Schedule pickup
> > 
> > a) When scheduling a pickup the calendar widget shows future days greyed out
> > and allows to select dates in the past. (blocker)
> 
> I don't recreate that. All dates can be selected using the widget, but dates
> in the past won't display slots.
> This is the correct behaviour. We could improve and "disable" dates in the
> past or even dates without slots, but that's for another bug report.
> 
> Opened bug 31261 and bug 31262.

Talked about it on IRC and agreeing with the approach.

> > b) The button remains greyed out until you pick a date with pickup times
> > available. I think a tool tip would be very helpful here. (not blocker)
> 
> I think I understand what you mean: you have to remove the focus from the
> widget to be able to select a slot.
> I've tried to fix it already but didn't manage to find a correct solution.

Not blocker for me.

> > 5) New notice NEW_CURBSIDE_PICKUP
> > 
> > a) Tiny typo: You have schedule a curbside pickup for [% branch.branchname
> > %] => ... scheduled (normal)
> 
> Fixed earlier.

OK! Fixed another small glitch: letter.name is set in .yml but not in the update. See follow-up.

> > b) Can we break the content text into 3 entries in the .yml file for new
> > installations (one per sentence) to ease translation a bit? (normal)
> 
> "Split yml on several lines to help translation"

OK, thanks!

> 
> > 6) Koha start page
> > 
> > a) The start page for new pickups reads: New curbside pickups 1. We should
> > add a : here: 

OK!
 
> > 7) OPAC view
> > 
> > As this is the patron side of things, I worry more about that than about
> > other stuff. I have some issues here:
> > 
> > a) The alert link will only be functional, when the pickup has been "staged
> > & ready". But there is no clue in the GUI about this and the patron might be
> > in front of the library without being able to see what's going on.
> 
> I would like this to be discussed on a separate bug report, it was the
> previous (plugin) behaviour.
> See bug 31263.

Suggested the same a bit further below, fine by me!


> > b) Once the pickup has been "marked as delivered" it still remains in the
> > list with no clue that it has been picked up already. It would be nicer to
> > display, that it's already been delivered. Note: only today's and future
> > pickups will show in the list.
> > 
> > Maybe this could be a separate bug as well to get some feedback, but it
> > feels like we should give the patrons more information there.
> 
> What do you suggest? Do you want a separate column to display the status?

I think a status column would be nice, but happy to move to a separate bug.
 
> > X) Possible enhancements
> > 
> > These are supposed to go into new wish list bugs. But maybe you could look
> > through them quickly to check if I have missed/misunderstood something?
> > 
> > a) Curbside pickup configuration page: The weekday pull down and display
> > should follow the CalendarFirstDayofWeek system preference.
> 
> Why do we accept all the day of the week in this syspref?!
> It should, indeed, but not trivial. Opening a separate bug and not sure it
> will be fixed. That seems a lot of work for not much to gain. Maybe first
> keeping only Monday, Saturday and Sunday...
> 
> ...ha ... bug 12137 comment 16...
> 
> Opened bug 31264!

Thx for opening the bugs already :) I agree with your past comment, just wishing we could have Monday first to match with the calendars.


> > b) Show a note on top of the Curbside pickup configuration page, if the
> > Curbside pickup module is not enabled (similar to what we do for the
> > Transport cost matrix administration page)
> 
> Done in "Add message on the admin page if the pref is off"

OK! Already off the wish list then :)

> > c) Improve GUI for pickup up pickup times in staff interface: The times are
> > a little hard to select/hard to read. I think it would be nice to make this
> > look more like in the OPAC. 
> 
> Opened a separate bug, I would like to ask Owen his opinion what's best
> here. We could improve both I think.
> The plugin was displaying the list in a dropdown list, so it's already a
> nice improvement how it is now ;)
> Opened bug 31265 for discussion.

Thx for the bug!


> > d) Scheduling a pickup from the curbside pickups page only will only work
> > with the cardnumber. It would be nicer to use the standard patron search for
> > this.

> Bug 30965 is waiting for you PQA stamp :)

I'll look at the other related bugs, once we are done here. Would be nice if it was already pushed as this makes testing a little easier.


> > e) Add logging for curbside pickup location actions
> 
> Can you open a bug report with the detail of what you have in mind exactly?

I'll think about it a bit more. Maybe it's not an issue, but it seemed like a little missing piece.

> > f) Have different permission descriptions (might require to change the
> > permission code) to make it clearer what the difference between each of the
> > permissions is.
> 
> Is that blocker? :D
> If not done here it won't be done. On the other hand I don't see how it
> could be a problem (the module code is different).

Maybe a non-issue, we can wait and see with this one.

> > h) Make curbside pickup confirmation its own messaging preference or adapt
> > messaging preference description to include curbside pickups when the pref
> > is enabled.
> 
> Yes, that is definitely something we should do. I've copied the plugin's
> behaviour but it's not obvious.
> Opened bug 31266.

Thx!

> I will pick the related bugs with the highest number of votes.

Go and vote people! :)
Comment 119 Katrin Fischer 2022-07-29 16:17:52 UTC
Created attachment 138330 [details] [review]
Bug 30650: DB changes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 120 Katrin Fischer 2022-07-29 16:17:57 UTC
Created attachment 138331 [details] [review]
Bug 30650: Koha classes

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 121 Katrin Fischer 2022-07-29 16:18:02 UTC
Created attachment 138332 [details] [review]
Bug 30650: Add the CurbsidePickup syspref

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 122 Katrin Fischer 2022-07-29 16:18:08 UTC
Created attachment 138333 [details] [review]
Bug 30650: Add new permission manage_curbside_pickups

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 123 Katrin Fischer 2022-07-29 16:18:13 UTC
Created attachment 138334 [details] [review]
Bug 30650: Add link from the admin home page

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 124 Katrin Fischer 2022-07-29 16:18:18 UTC
Created attachment 138335 [details] [review]
Bug 30650: Admin page

This patch adds a new page in the administration area to manage curbside
pickups.

Test plan:
1. Enable the new CurbsidePickup syspref
2. Go to Administration > Curbside pickups
=> If the logged-in user has the manage_curbside_pickups permission you
should see the administration view
3. Enable the feature for some libraries and configure it.
 a. You must define a "pickup interval" and a "maximum patrons per interval"
values.
 b. If "Patron-scheduled pickup" is enabled, patrons will be able to
schedule curbside pickups from the OPAC
 c. If "Enable for waiting holds only" is enabled, only patron with waiting
holds will be allowed to schedule curbside pickups
 d. In the bottom part of the form you can define the slots available for
curbside pickups.
 e. Save

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 125 Katrin Fischer 2022-07-29 16:18:23 UTC
Created attachment 138336 [details] [review]
Bug 30650: Adjust atomic update

Take into account existing installs using the plugins

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 126 Katrin Fischer 2022-07-29 16:18:29 UTC
Created attachment 138337 [details] [review]
Bug 30650: Be more flexible with opening slots

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 127 Katrin Fischer 2022-07-29 16:18:35 UTC
Created attachment 138338 [details] [review]
Bug 30650: Add some useful modules and tests

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 128 Katrin Fischer 2022-07-29 16:18:40 UTC
Created attachment 138339 [details] [review]
Bug 30650: Add circulation page view

This is the main commit message.

A plugin already exists to manage curbside pickups. This new
enhancehemnt is suggesting an implementation that is ready to be integrated
into Koha core in order to provide the feature out-of-the-box.

What has been done in this patch set:
- Deal with installations using the existing plugin (upgrade the DB schema and migrate their data)
- Add a new syspref (CurbsidePickup) and two new permissions:
  * parameters.manage_curbside_pickups
  * circulate.manage_curbside_pickups
- Add an administration page to setup the configuration: admin/curbside_pickup.pl
- Add a circulation page to manage the existing pickups, and create new one
- Add a new OPAC view "your curbside pickups" to let patron manage their pickups, and create new ones
- Add link from the "member" toolbar

Improvements compared to the plugin:
- Ability to create several pickup windows per day
- Better display of the pickup times (not in a dropdown list)
- Ability to disable pickups for patrons without waiting holds
- Display pickups on the patron circulation page
- Display pickups of the library on the homepage
- Prevent pickup to be created on a holiday
- Better error handling (exceptions)
- Unit tests

More improvements are already planned, see related bug reports.

Test plan:
After you setup the feature correctly from the administration view, you
will be able to use the schedule curbside pickups from the staff
interface, and from the OPAC interface if you selected "patron-scheduled
pickup"
A. Staff interface
1. Go to Circulation > Curbside pickups
=> If the logged-in user has the circulate.manage_curbside_pickups
permission you will be able to create and manage curbside pickups
2. Go to a patron detail page and click the "Schedule pickup" button in
the toolbar
3. If the patron has waiting holds and you selected "Enable for waiting holds only",
of if you didn't select the option, you will be able to select a pickup
date and slots to create a pickup.
4. Confirm that you cannot create more pickups per slot than what you
defined in the curbside pickup configuration for this library
5. Confirm that you cannot create a pickup if the feature is disabled
for the library
6. Notice that you can mark the pickup as "stage and ready", then
"patron is outside" and finally "delivered today". You can also rollback
the change
7. Notice that once the pickup has been marked as delivered, the
item has been checked out and that a new notice has been generated (if
the patron has "Hold_Filled" in their messaging preferences
8. Confirm that the information about current pickups is displayed on
the circulation page of the patron

B. OPAC interface
1. Create a new curbside pickup from the OPAC
2. Confirm that the same limitations as from the staff interface are in
effect (waiting holds, number of patron per slots, etc.)
3. Confirm that you can cancel a pickup and alert staff of you arrival
4. Confirm that you cannot cancel a pickup that has been delivered
already

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 129 Katrin Fischer 2022-07-29 16:18:45 UTC
Created attachment 138340 [details] [review]
Bug 30650: Add dayjs and isSameOfAfter plugin

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 130 Katrin Fischer 2022-07-29 16:18:51 UTC
Created attachment 138341 [details] [review]
Bug 30650: Add dayjs to opac

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 131 Katrin Fischer 2022-07-29 16:18:57 UTC
Created attachment 138342 [details] [review]
Bug 30650: OPAC view

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 132 Katrin Fischer 2022-07-29 16:19:03 UTC
Created attachment 138343 [details] [review]
Bug 30650: Add link from the member toolbar

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 133 Katrin Fischer 2022-07-29 16:19:08 UTC
Created attachment 138344 [details] [review]
Bug 30650: Notify the patron when a new curbside pickup is created

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 134 Katrin Fischer 2022-07-29 16:19:13 UTC
Created attachment 138345 [details] [review]
Bug 30650: DB - Add policy.enable_waiting_holds_only

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 135 Katrin Fischer 2022-07-29 16:19:19 UTC
Created attachment 138346 [details] [review]
Bug 30650: Allow to restrict curbside pickup for waiting holds only

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 136 Katrin Fischer 2022-07-29 16:19:24 UTC
Created attachment 138347 [details] [review]
Bug 30650: Add link from the mainpage

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 137 Katrin Fischer 2022-07-29 16:19:30 UTC
Created attachment 138348 [details] [review]
Bug 30650: Add link from the circ patron page

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 138 Katrin Fischer 2022-07-29 16:19:35 UTC
Created attachment 138349 [details] [review]
Bug 30650: Add filter_by_scheduled_today

A bit of cleaning

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 139 Katrin Fischer 2022-07-29 16:19:41 UTC
Created attachment 138350 [details] [review]
Bug 30650: Prevent pickup to be created on holiday

We could improve this more and add disable the holiday from the date
picker widget, but it's out of the scope here.

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 140 Katrin Fischer 2022-07-29 16:19:46 UTC
Created attachment 138351 [details] [review]
Bug 30650: Split the permission

We actually want one permission to manage the administration page, and
one for the circulation module.

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 141 Katrin Fischer 2022-07-29 16:19:53 UTC
Created attachment 138352 [details] [review]
Bug 30650: Select 'schedule a pickup' if no pickups yet

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 142 Katrin Fischer 2022-07-29 16:19:59 UTC
Created attachment 138353 [details] [review]
Bug 30650: Prevent browser crash if no pickup interval set

Don't get stuck in an infinite loop

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 143 Katrin Fischer 2022-07-29 16:20:05 UTC
Created attachment 138354 [details] [review]
Bug 30650: DBIC

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 144 Katrin Fischer 2022-07-29 16:20:11 UTC
Created attachment 138355 [details] [review]
Bug 30650: DBIC specific

Note for QA - a false positive is raised by the QA tools:

"""
The new column curbside_pickup_opening_slots.day is using TINYINT(1) as
type but is not defined as boolean in the schema file
(Koha/Schema/Result/CurbsidePickupOpeningSlot.pm)
"""

=> It's not a boolean!

Sponsored-by: Association KohaLa - https://koha-fr.org/

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 145 Katrin Fischer 2022-07-29 16:20:16 UTC
Created attachment 138356 [details] [review]
Bug 30650: Display holds waiting at the logged-in library

Not those waiting at the library of the logged-in user

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 146 Katrin Fischer 2022-07-29 16:20:22 UTC
Created attachment 138357 [details] [review]
Bug 30650: Don't display slots that have are before now

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 147 Katrin Fischer 2022-07-29 16:20:27 UTC
Created attachment 138358 [details] [review]
Bug 30650: Add dayjs customParseFormat plugin

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 148 Katrin Fischer 2022-07-29 16:20:33 UTC
Created attachment 138359 [details] [review]
Bug 30650: Take dateformat into account

dayjs init was not passing the dateformat.
We really need to clean the date handling with the following two bugs:
  Bug 30310 - Replace Moment.js with Day.js
and
  Bug 30718 - Use flatpickr's altInput option everywhere

But it's outside the scope of this bug report.

Signed-off-by: Koha Team University Lyon 3 <koha@univ-lyon3.fr>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 149 Katrin Fischer 2022-07-29 16:20:38 UTC
Created attachment 138360 [details] [review]
Bug 30650: Display the link in the admin even if the pref is off

The link in the sidebar was displayed already. The idea is to let admins
configure the module before enabling it for users.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 150 Katrin Fischer 2022-07-29 16:20:44 UTC
Created attachment 138361 [details] [review]
Bug 30650: Fix some typos

* Curbside pickups are not enabled for your location. = library
* Pickup Date/Time = capitalization
* ... it is an holiday = it is a holiday. (multiple)
* Deliver Date/Time = capitalization
* There is no waiting holds for this patron at this location. = There are no... at this library
* No pickup time define for this day. = defined (multiple)
* You don't have waiting holds at this location = library

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 151 Katrin Fischer 2022-07-29 16:20:49 UTC
Created attachment 138362 [details] [review]
Bug 30650: Add dayjs license to the about page

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 152 Katrin Fischer 2022-07-29 16:20:55 UTC
Created attachment 138363 [details] [review]
Bug 30650: Add info to the trash icon

And add a missing space between the time and icon

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 153 Katrin Fischer 2022-07-29 16:21:01 UTC
Created attachment 138364 [details] [review]
Bug 30650: Add link to the circ nav bar

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 154 Katrin Fischer 2022-07-29 16:21:07 UTC
Created attachment 138365 [details] [review]
Bug 30650: Add the sidebar to the circ view

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 155 Katrin Fischer 2022-07-29 16:21:12 UTC
Created attachment 138366 [details] [review]
Bug 30650: Split yml on several lines to help translation

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 156 Katrin Fischer 2022-07-29 16:21:17 UTC
Created attachment 138367 [details] [review]
Bug 30650: Add message on the admin page if the pref is off

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 157 Katrin Fischer 2022-07-29 16:21:23 UTC
Created attachment 138368 [details] [review]
Bug 30650: Add missing name to notice added by database update

The notice was missing the name, which gives you an error when
you try to save changes in Koha.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 158 Katrin Fischer 2022-07-29 16:21:28 UTC
Created attachment 138369 [details] [review]
Bug 30650: (QA follow-up) Terminology: location - library

Updates one remaining occurrence of "at this location" meaning
"at this library".

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 159 Tomás Cohen Arazi 2022-07-29 18:39:36 UTC
Created attachment 138375 [details] [review]
Bug 30650: (QA follow-up) Some higher-level methods usage

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 160 Tomás Cohen Arazi 2022-07-29 18:49:17 UTC
Great job, Jonathan.

- We could use some more unit tests
- Some of this could've been squashed, specially the typos, etc.
- ^^ I don't care much about the above, this is great
Comment 161 Tomás Cohen Arazi 2022-07-29 18:51:28 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 162 Katrin Fischer 2022-07-30 12:53:15 UTC
It looks like we have a failing test now?
not ok 264 - Module Koha::CurbsidePickupPolicys should be defined
Comment 163 Jonathan Druart 2022-07-31 05:57:47 UTC
Created attachment 138401 [details] [review]
Bug 30650: Add missing koha_object[s]_class methods

To fix TestBuilder.t
        #   at t/db_dependent/TestBuilder.t line 453.
        #          got: 'Can't locate Koha/CurbsidePickupPolicys.pm
Comment 164 Jonathan Druart 2022-07-31 06:54:05 UTC
(In reply to Tomás Cohen Arazi from comment #160)
> Great job, Jonathan.
> 
> - We could use some more unit tests

Which missing tests did you identify?

> - Some of this could've been squashed, specially the typos, etc.

Yes, they should have been. I submitted to easy the QA review, but they could have been squashed before push for sure.
Comment 165 Tomás Cohen Arazi 2022-08-01 12:12:15 UTC
Follow-up pushed to master. Thanks!
Comment 166 Tomás Cohen Arazi 2022-08-04 12:51:06 UTC
Created attachment 138610 [details] [review]
Bug 30650: (QA follow-up) Fix TINYINT fields discrepancy on upgrade

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>