[Koha-bugs] [Bug 30650] Convert the Curbside Pickup plugin to Koha core

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30650

Jonathan Druart <jonathan.druart+koha at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Failed QA                   |Signed Off

--- Comment #116 from Jonathan Druart <jonathan.druart+koha at gmail.com> ---
(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.

-- 
You are receiving this mail because:
You are watching all bug changes.