Bug 23048 - Hide non-pickup branches from hold modification select
Summary: Hide non-pickup branches from hold modification select
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: paxed
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-05 08:59 UTC by paxed
Modified: 2019-08-06 14:02 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.03


Attachments
Hide non-pickup branches from hold modification select (2.24 KB, patch)
2019-06-05 09:04 UTC, paxed
Details | Diff | Splinter Review
Bug 23048: Hide non-pickup branches from hold modification select (2.28 KB, patch)
2019-06-07 18:42 UTC, Bin Wen
Details | Diff | Splinter Review
Bug 23048: Hide non-pickup branches from hold modification select (3.35 KB, patch)
2019-06-27 11:10 UTC, paxed
Details | Diff | Splinter Review
Bug 23048: Hide non-pickup branches from hold modification select (3.40 KB, patch)
2019-07-11 05:57 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23048: Hide non-pickup branches from hold modification select (3.50 KB, patch)
2019-07-19 09:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description paxed 2019-06-05 08:59:29 UTC
When modifying holds in staff client via reserve/request.pl or
the holds-tab in members/moremember.pl, the pickup library dropdown
lists libraries not marked for pickup. Hide those branches.
Comment 1 paxed 2019-06-05 09:04:05 UTC
Created attachment 90322 [details] [review]
Hide non-pickup branches from hold modification select

When modifying holds in staff client via reserve/request.pl or
the holds-tab in members/moremember.pl, the pickup library dropdown
lists libraries not marked for pickup. Hide those branches.

Test plan:
1) Create a branch that is marked as a non-pickup location.
2) Create a hold or a patron.
3) Go to members/moremember.pl for that patron, check the Holds-tab
   Pickup library -drop down. It lists the non-pickup branch.
4) Go to reserve/request.pl for that biblio. The Existing hold-table
   Pickup library -drop down lists the non-pickup branch.
5) Apply patch.
6) Repeat 3 & 4, drop down no longer lists the non-pickup branch.
Comment 2 Bin Wen 2019-06-07 18:42:35 UTC
Created attachment 90429 [details] [review]
Bug 23048: Hide non-pickup branches from hold modification select

When modifying holds in staff client via reserve/request.pl or
the holds-tab in members/moremember.pl, the pickup library dropdown
lists libraries not marked for pickup. Hide those branches.

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>
Signed-off-by: Bin Wen <bin.wen@inlibro.com>
Comment 3 paxed 2019-06-13 07:15:52 UTC
Actually, I'll withdraw the patch - when a reserve has a non-pickup library as a pickup location, it doesn't work correctly.

(Perhaps the reserve was created before the library was marked as non-pickup-location)
Comment 4 paxed 2019-06-27 11:10:52 UTC
Created attachment 91048 [details] [review]
Bug 23048: Hide non-pickup branches from hold modification select

When modifying holds in staff client via reserve/request.pl or
the holds-tab in members/moremember.pl, the pickup library dropdown
lists libraries not marked for pickup. Hide those branches,
unless the branch is marked as a pick-up branch for already
existing hold.
    
The reason for showing a non-pickup branch for the holds created
before the branch was marked as non-pickup: If the non-pickup branch
wasn't shown, the branch in those holds would get set to the first
branch in the select if the hold was modified.
    
Test plan:
1) Create a hold for a patron, to be picked up from a certain branch.
2) Edit the branch and another branch so they are no longer
   pickup locations.
3) Go to members/moremember.pl for that patron, check the Holds-tab
   Pickup library -drop down. It lists both of the non-pickup branches.
4) Go to reserve/request.pl for that biblio. The hold-table
   Pickup library -drop down lists both of the non-pickup branches.
5) Apply patch.
6) Repeat 3 & 4, drop down no longer lists the non-pickup branch, but
   should still list the branch where the hold was marked to be picked
   from.
Comment 5 Katrin Fischer 2019-07-10 09:28:13 UTC
Would this be the same as bug 7534, but for staff?

Bug 7534 - New OPACAllowUserToChooseBranch setting for only showing libraries allowing holds
Comment 6 Katrin Fischer 2019-07-11 05:57:53 UTC
Created attachment 91453 [details] [review]
Bug 23048: Hide non-pickup branches from hold modification select

When modifying holds in staff client via reserve/request.pl or
the holds-tab in members/moremember.pl, the pickup library dropdown
lists libraries not marked for pickup. Hide those branches,
unless the branch is marked as a pick-up branch for already
existing hold.

The reason for showing a non-pickup branch for the holds created
before the branch was marked as non-pickup: If the non-pickup branch
wasn't shown, the branch in those holds would get set to the first
branch in the select if the hold was modified.

Test plan:
1) Create a hold for a patron, to be picked up from a certain branch.
2) Edit the branch and another branch so they are no longer
   pickup locations.
3) Go to members/moremember.pl for that patron, check the Holds-tab
   Pickup library -drop down. It lists both of the non-pickup branches.
4) Go to reserve/request.pl for that biblio. The hold-table
   Pickup library -drop down lists both of the non-pickup branches.
5) Apply patch.
6) Repeat 3 & 4, drop down no longer lists the non-pickup branch, but
   should still list the branch where the hold was marked to be picked
   from.

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Katrin Fischer 2019-07-11 05:58:52 UTC
Makes the behaviour more consistent and also takes care of existing holds. +1
Comment 8 Marcel de Rooy 2019-07-19 09:32:37 UTC
Created attachment 91623 [details] [review]
Bug 23048: Hide non-pickup branches from hold modification select

When modifying holds in staff client via reserve/request.pl or
the holds-tab in members/moremember.pl, the pickup library dropdown
lists libraries not marked for pickup. Hide those branches,
unless the branch is marked as a pick-up branch for already
existing hold.

The reason for showing a non-pickup branch for the holds created
before the branch was marked as non-pickup: If the non-pickup branch
wasn't shown, the branch in those holds would get set to the first
branch in the select if the hold was modified.

Test plan:
1) Create a hold for a patron, to be picked up from a certain branch.
2) Edit the branch and another branch so they are no longer
   pickup locations.
3) Go to members/moremember.pl for that patron, check the Holds-tab
   Pickup library -drop down. It lists both of the non-pickup branches.
4) Go to reserve/request.pl for that biblio. The hold-table
   Pickup library -drop down lists both of the non-pickup branches.
5) Apply patch.
6) Repeat 3 & 4, drop down no longer lists the non-pickup branch, but
   should still list the branch where the hold was marked to be picked
   from.

Signed-off-by: Pasi Kallinen <pasi.kallinen@koha-suomi.fi>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Martin Renvoize 2019-07-19 12:51:35 UTC
Ideally, I would like to have seen a selenium regression test for this, but I'm not going to let it prevent push this cycle.  Just a heads up that I may start getting more strict with these as the cycle progresses.
Comment 10 Martin Renvoize 2019-07-19 12:56:55 UTC
Nice work!

Pushed to master for 19.11.00
Comment 11 Fridolin SOMERS 2019-08-06 14:02:56 UTC
Pushed to 19.05.x for 19.05.03