Bug 27340 - We should introduce Koha::PickupLocation(s)
Summary: We should introduce Koha::PickupLocation(s)
Status: ASSIGNED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-05 13:17 UTC by Tomás Cohen Arazi
Modified: 2024-04-01 14:00 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2021-01-05 13:17:57 UTC
Looking at the codebase, a pickup location is a combination of a branch/library, and now also a 'desk'. So it would make sense to have a special class for them.
Comment 1 Tomás Cohen Arazi 2021-01-05 13:18:35 UTC
I assign it to Martin to have his thoughts and discuss this.
Comment 2 Martin Renvoize 2021-01-12 06:50:03 UTC
Hmm, I agree with the general idea here, though I'm struggling a little at this moment to identify the use cases.. I suppose as an embedded object from the hold/item.
Comment 3 Donna 2023-01-17 18:33:56 UTC
Some of our partners would love to see desks as part of the pickup location.  For instance, in a library there may be a main circ desk, a children's desk on another floor, and a drive up window. Or, self-serve holds shelves and a drive up window. If a patron wants to pick up their items at the drive up window, it would be great to have that as a "desk" instead of relying on the patron and staff adding/reading holds notes.
Comment 4 Katrin Fischer 2023-01-17 21:19:46 UTC
(In reply to Donna from comment #3)
> Some of our partners would love to see desks as part of the pickup location.
> For instance, in a library there may be a main circ desk, a children's desk
> on another floor, and a drive up window. Or, self-serve holds shelves and a
> drive up window. If a patron wants to pick up their items at the drive up
> window, it would be great to have that as a "desk" instead of relying on the
> patron and staff adding/reading holds notes.

Hi Donna, as this is more a bug about architecture, you might want to file separate bugs for how the desks should present in the GUI better. I think at the moment the desk will only be assigned when a hold is triggered (for the desk you are checking things in). I was thinking it would be nice if the user could pick the desk on the hold request form - but should we move this to a new bug?
Comment 5 Tomás Cohen Arazi 2024-03-07 14:44:36 UTC
(In reply to Katrin Fischer from comment #4)
> (In reply to Donna from comment #3)
> > Some of our partners would love to see desks as part of the pickup location.
> > For instance, in a library there may be a main circ desk, a children's desk
> > on another floor, and a drive up window. Or, self-serve holds shelves and a
> > drive up window. If a patron wants to pick up their items at the drive up
> > window, it would be great to have that as a "desk" instead of relying on the
> > patron and staff adding/reading holds notes.
> 
> Hi Donna, as this is more a bug about architecture, you might want to file
> separate bugs for how the desks should present in the GUI better. I think at
> the moment the desk will only be assigned when a hold is triggered (for the
> desk you are checking things in). I was thinking it would be nice if the
> user could pick the desk on the hold request form - but should we move this
> to a new bug?

I think this is the right place to discuss if this addition would be useful.
Comment 6 Katrin Fischer 2024-03-07 14:49:32 UTC
Hm, not sure if there is misunderstanding or not.

You can already make the desk part of the hold request, but it only happens automatically when checking it in for pick-up. I think what Donna and I would like is be able to assign the desk in other worksflow steps: when placing the hold in the OPAC, when placing the hold in the staff interface, when editing the hold in the staff interface... so also at the beginning not the end.

I understood this was mostly about creating a new module and assumed we should discuss workflow separately. I think the need for the module might already be there without additions to the existing feature.
Comment 7 Tomás Cohen Arazi 2024-03-07 14:58:09 UTC
(In reply to Katrin Fischer from comment #6)
> Hm, not sure if there is misunderstanding or not.
> 
> You can already make the desk part of the hold request, but it only happens
> automatically when checking it in for pick-up. I think what Donna and I
> would like is be able to assign the desk in other worksflow steps: when
> placing the hold in the OPAC, when placing the hold in the staff interface,
> when editing the hold in the staff interface... so also at the beginning not
> the end.
> 
> I understood this was mostly about creating a new module and assumed we
> should discuss workflow separately. I think the need for the module might
> already be there without additions to the existing feature.

I found the need for a specific class for representing pickup locations when making holds pickup locations dropdown API-driven. I didn't go as far as I would loved because of the little feedback I got at that time.

I'm happy to discuss possible use cases now there's interest :-D

I believe libraries should be able to define whatever they want to call their pickup locations, for example. So as a starting point, I'd say a pickup location should contain the following columns:

pickup_location_id (PK)
library_id
desk_id
description
public_description

That way, defined pickup locations will be displayed to end users. The 'branches.pickup_location' flag could be migrated to entries in this table.
Comment 8 Katrin Fischer 2024-04-01 13:04:25 UTC
IIUC this would change what we understand as a pick-up location. Currently we understand the pick-up location as the library the item is waiting at. We'd like to add the desk to this as another optional factor.

> pickup_location_id (PK)
> library_id
> desk_id
> description
> public_description

My feeling is that we don't need a new table here as we already have all the information in "desks", especially since they are already tied to a library. I don't feel like we need to be able to reuse desks at different libraries, the current implementation defining them as a "place" within a library feels correct and sufficient. 

I am not sure a separate description is strictly needed either - we could imagine 2 possible implementations in the interface:

* Have 2 separate pull-downs: select the library first, then see a list of available desks for it.
* Have a combined pull-down of library + desk name.
Comment 9 Tomás Cohen Arazi 2024-04-01 13:12:53 UTC
(In reply to Katrin Fischer from comment #8)
> IIUC this would change what we understand as a pick-up location. Currently
> we understand the pick-up location as the library the item is waiting at.
> We'd like to add the desk to this as another optional factor.
> 
> > pickup_location_id (PK)
> > library_id
> > desk_id
> > description
> > public_description
> 
> My feeling is that we don't need a new table here as we already have all the
> information in "desks", especially since they are already tied to a library.
> I don't feel like we need to be able to reuse desks at different libraries,
> the current implementation defining them as a "place" within a library feels
> correct and sufficient.

We need a table in which each row represents each allowed combination. Not all combinations are necessarily valid pickup locations.

> I am not sure a separate description is strictly needed either - we could
> imagine 2 possible implementations in the interface:
> 
> * Have 2 separate pull-downs: select the library first, then see a list of
> available desks for it.
> * Have a combined pull-down of library + desk name.

This is overcomplicating it, actually. UI-wise it would be clearer if we just provided a 'Pickup location' pull down. And in some cases you just wouldn't specify a description and it will fall back to the library or 'Library name (desk)'.

On the other hand, whenever we add endpoints for things that are not really mapped to a table with other linked tables, we end up generating weird controller code that is buggy and too complex :-D

Happy to keep the discussion open, I'm not really attached to any solution. I just think our current approach to pickup locations is too limited on the API front.
Comment 10 Katrin Fischer 2024-04-01 13:29:00 UTC
(In reply to Tomás Cohen Arazi from comment #9)
> (In reply to Katrin Fischer from comment #8)
> > IIUC this would change what we understand as a pick-up location. Currently
> > we understand the pick-up location as the library the item is waiting at.
> > We'd like to add the desk to this as another optional factor.
> > 
> > > pickup_location_id (PK)
> > > library_id
> > > desk_id
> > > description
> > > public_description
> > 
> > My feeling is that we don't need a new table here as we already have all the
> > information in "desks", especially since they are already tied to a library.
> > I don't feel like we need to be able to reuse desks at different libraries,
> > the current implementation defining them as a "place" within a library feels
> > correct and sufficient.
> 
> We need a table in which each row represents each allowed combination. Not
> all combinations are necessarily valid pickup locations.

I think that's where we might misunderstand each other.  

Which combinations are you referring to?

Library A - Desk A is a fixed combination. 
Library B - Desk A would be invalid. 

But there is a need to have desks optional, so maybe:

Library A
Library B - Desk A
Library B - Desk B

I am still confused by the need for a new table unless we completely restructure, but maybe that is the plan?

Option A:
* Remove the "is pick-up location" from the library configuration page (seems logical to not have multiple spots to maintain this information)
* Add the new table
* Possible problems: migration for existing libraries, additional setup requirements for making holds functional, possible conflicts with pickup-location related settings in circulation conditions (needs a closer look)
* Possible advantages: It might be easier to implement something like "certain item types can only be picked up at certain desks" later on?

Option B:
* Keep checkbox "is pick-up location" on library configuration page
* Add checkbox "available for pick-up" to desks
* Possible problems: might limit future enhancements... (maybe I am getting there)

> > I am not sure a separate description is strictly needed either - we could
> > imagine 2 possible implementations in the interface:
> > 
> > * Have 2 separate pull-downs: select the library first, then see a list of
> > available desks for it.
> > * Have a combined pull-down of library + desk name.
> 
> This is overcomplicating it, actually. UI-wise it would be clearer if we
> just provided a 'Pickup location' pull down. And in some cases you just
> wouldn't specify a description and it will fall back to the library or
> 'Library name (desk)'.

OK, it might depend on the numbers of options. Adding desks could make the list significantly longer for consortia, but we can still group by library with your data model for UI if needed.

> On the other hand, whenever we add endpoints for things that are not really
> mapped to a table with other linked tables, we end up generating weird
> controller code that is buggy and too complex :-D
> 
> Happy to keep the discussion open, I'm not really attached to any solution.
> I just think our current approach to pickup locations is too limited on the
> API front.

I have no idea about the API front, but it might be limiting our workflows too :)
Comment 11 Tomás Cohen Arazi 2024-04-01 13:36:50 UTC
(In reply to Katrin Fischer from comment #10)
> I am still confused by the need for a new table unless we completely
> restructure, but maybe that is the plan?
> 
> Option A:
> * Remove the "is pick-up location" from the library configuration page
> (seems logical to not have multiple spots to maintain this information)
> * Add the new table
> * Possible problems: migration for existing libraries, additional setup
> requirements for making holds functional, possible conflicts with
> pickup-location related settings in circulation conditions (needs a closer
> look)
> * Possible advantages: It might be easier to implement something like
> "certain item types can only be picked up at certain desks" later on?

You got it. The plan is to perform a cleanup in the area. At least the goal.
I don't think the 'migration' during the upgrade would be an issue.

I do agree having a very long list, though searchable, might not be that convenient.
Comment 12 Katrin Fischer 2024-04-01 13:49:28 UTC
I agree, the migration should not be an issue right now, especially before we add more 'desks' functionality.

Another complication I expect: Patrons might only be able to choose the library pick-up location, while staff can assign desks. This would fit the current workflow where a desk can only be assigned on checkin of an on hold item. 

Expectation: libraries might require different behavior for OPAC and staff interface: Patron can't select a desk, only the library.
Comment 13 Tomás Cohen Arazi 2024-04-01 14:00:18 UTC
(In reply to Katrin Fischer from comment #12)
> I agree, the migration should not be an issue right now, especially before
> we add more 'desks' functionality.
> 
> Another complication I expect: Patrons might only be able to choose the
> library pick-up location, while staff can assign desks. This would fit the
> current workflow where a desk can only be assigned on checkin of an on hold
> item.
> 
> Expectation: libraries might require different behavior for OPAC and staff
> interface: Patron can't select a desk, only the library.

Good point. Seems we could find ways to solve it in the code. But worth thinking. Thanks!