This is due to Bug 29087, see comment 39
Created attachment 172680 [details] [review] Bug 38148: Check value of holdallowed circ rule properly To test: 1. Search for a record on the OPAC. Notice there is no link to place a hold. 2. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and notice it is empty 3. Apply patch and restart services 4. Search for a record on the OPAC. Confirm you can see a link to place a hold. 5. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and confirm your hold shows in the list. Sponsored-by: Whanganui District Council
So Bug 29087 was the cause of being unable to place OPAC holds and the holds to pull list being empty for this library, and the fix does solve it for them... but I'm not totally sure what combination of their catalogue and circ rules caused it to happen to them. In short, I don't know how to write a test plan for this. I've updated the bug report accordingly and will attach a new test plan.
Created attachment 172681 [details] [review] Bug 38148: Check value of holdallowed circ rule properly It was noted on Bug 29087 that the value of circulation rules needs to be checked differently, so this follow-up addresses that. To test: 1. Apply patch and restart services 2. Search for a record on the OPAC. Confirm you can see a link to place a hold. 3. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and confirm your hold shows in the list. 4. Confirm tests still pass t/db_dependent/Koha/Holds.t Sponsored-by: Whanganui District Council
Created attachment 172929 [details] [review] Bug 38148: Check value of holdallowed circ rule properly It was noted on Bug 29087 that the value of circulation rules needs to be checked differently, so this follow-up addresses that. To test: 1. Apply patch and restart services 2. Search for a record on the OPAC. Confirm you can see a link to place a hold. 3. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and confirm your hold shows in the list. 4. Confirm tests still pass t/db_dependent/Koha/Holds.t Sponsored-by: Whanganui District Council Signed-off-by: David Nind <david@davidnind.com>
Hi guys, thanks for helping here, we're now also experiencing this regression in 23.11.09. At first glance this patch looks good, but it's causing the following to FAIL: prove t/db_dependent/Koha/Items.t I'm adding others involved in bug 29087 to pitch in if they have the chance / are available to.
(In reply to Aleisha Amohia from comment #2) > In short, I don't know how to write > a test plan for this. I've updated the bug report accordingly and will > attach a new test plan. On fresh k-t-d: 1) Search for 'music' on OPAC 2) Notice records have the 'place hold' link 3) Visit circulation rules on Staff: <staff_url>/cgi-bin/koha/admin/smart-rules.pl 4) On 'Default checkout, hold and return policy' , under 'Hold policy' pick 'From any library'. Hit 'Save'. 5) Repeat 1). Notice the 'place hold' link is gone.
Created attachment 173125 [details] [review] Bug 38148: Revert back to get_effective_rule And update the first conditional logic. This patch aims to maintain the logic behind the first patch, but restore get_effective_rule. get_effective_rule_value is using cache and I believe that is the reason why Items.t fail when using that. Test plan: 1) Search for 'music' on OPAC 2) Notice records have the 'place hold' link 3) Visit circulation rules on Staff: <staff_url>/cgi-bin/koha/admin/smart-rules.pl 4) On 'Default checkout, hold and return policy' , under 'Hold policy' pick 'From any library'. Hit 'Save'. 5) Repeat 1). Notice the 'place hold' link is gone. 6) Apply patch. restart plack. Repeat 1). Notice the 'place hold' is shown again Run: $ prove t/db_dependent/Koha/Items.t
Hi all, asking for someone to take a look here when possible as this bug is causing issues for a lot of our user base. I've submitted a follow-up with an updated test plan which aims to keep the fix introduced by the original patch but updated to not break tests.
Created attachment 173141 [details] [review] Bug 38148: Check value of holdallowed circ rule properly It was noted on Bug 29087 that the value of circulation rules needs to be checked differently, so this follow-up addresses that. To test: 1. Apply patch and restart services 2. Search for a record on the OPAC. Confirm you can see a link to place a hold. 3. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and confirm your hold shows in the list. 4. Confirm tests still pass t/db_dependent/Koha/Holds.t Sponsored-by: Whanganui District Council Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: andrewa <andrew.auld@ptfs-europe.com>
Created attachment 173142 [details] [review] Bug 38148: Revert back to get_effective_rule And update the first conditional logic. This patch aims to maintain the logic behind the first patch, but restore get_effective_rule. get_effective_rule_value is using cache and I believe that is the reason why Items.t fail when using that. Test plan: 1) Search for 'music' on OPAC 2) Notice records have the 'place hold' link 3) Visit circulation rules on Staff: <staff_url>/cgi-bin/koha/admin/smart-rules.pl 4) On 'Default checkout, hold and return policy' , under 'Hold policy' pick 'From any library'. Hit 'Save'. 5) Repeat 1). Notice the 'place hold' link is gone. 6) Apply patch. restart plack. Repeat 1). Notice the 'place hold' is shown again Run: $ prove t/db_dependent/Koha/Items.t Signed-off-by: andrewa <andrew.auld@ptfs-europe.com>
Created attachment 173143 [details] [review] Bug 38148: Check value of holdallowed circ rule properly It was noted on Bug 29087 that the value of circulation rules needs to be checked differently, so this follow-up addresses that. To test: 1. Apply patch and restart services 2. Search for a record on the OPAC. Confirm you can see a link to place a hold. 3. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and confirm your hold shows in the list. 4. Confirm tests still pass t/db_dependent/Koha/Holds.t Sponsored-by: Whanganui District Council Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: andrewa <andrew.auld@ptfs-europe.com>
Created attachment 173144 [details] [review] Bug 38148: Revert back to get_effective_rule And update the first conditional logic. This patch aims to maintain the logic behind the first patch, but restore get_effective_rule. get_effective_rule_value is using cache and I believe that is the reason why Items.t fail when using that. Test plan: 1) Search for 'music' on OPAC 2) Notice records have the 'place hold' link 3) Visit circulation rules on Staff: <staff_url>/cgi-bin/koha/admin/smart-rules.pl 4) On 'Default checkout, hold and return policy' , under 'Hold policy' pick 'From any library'. Hit 'Save'. 5) Repeat 1). Notice the 'place hold' link is gone. 6) Apply patch. restart plack. Repeat 1). Notice the 'place hold' is shown again Run: $ prove t/db_dependent/Koha/Items.t Signed-off-by: andrewa <andrew.auld@ptfs-europe.com>
Great job all! Test plan worked like a breeze.
Tested in both a sandbox and a ktd.
Created attachment 173169 [details] [review] Bug 38148: Check value of holdallowed circ rule properly It was noted on Bug 29087 that the value of circulation rules needs to be checked differently, so this follow-up addresses that. To test: 1. Apply patch and restart services 2. Search for a record on the OPAC. Confirm you can see a link to place a hold. 3. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and confirm your hold shows in the list. 4. Confirm tests still pass t/db_dependent/Koha/Holds.t Sponsored-by: Whanganui District Council Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: andrewa <andrew.auld@ptfs-europe.com> Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Created attachment 173170 [details] [review] Bug 38148: Revert back to get_effective_rule And update the first conditional logic. This patch aims to maintain the logic behind the first patch, but restore get_effective_rule. get_effective_rule_value is using cache and I believe that is the reason why Items.t fail when using that. Test plan: 1) Search for 'music' on OPAC 2) Notice records have the 'place hold' link 3) Visit circulation rules on Staff: <staff_url>/cgi-bin/koha/admin/smart-rules.pl 4) On 'Default checkout, hold and return policy' , under 'Hold policy' pick 'From any library'. Hit 'Save'. 5) Repeat 1). Notice the 'place hold' link is gone. 6) Apply patch. restart plack. Repeat 1). Notice the 'place hold' is shown again Run: $ prove t/db_dependent/Koha/Items.t Signed-off-by: andrewa <andrew.auld@ptfs-europe.com> Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk>
Created attachment 173183 [details] [review] Bug 38148: Unit tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173184 [details] [review] Bug 38148: Check value of holdallowed circ rule properly It was noted on Bug 29087 that the value of circulation rules needs to be checked differently, so this follow-up addresses that. To test: 1. Apply patch and restart services 2. Search for a record on the OPAC. Confirm you can see a link to place a hold. 3. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and confirm your hold shows in the list. 4. Confirm tests still pass t/db_dependent/Koha/Holds.t Sponsored-by: Whanganui District Council Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: andrewa <andrew.auld@ptfs-europe.com> Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk> Bug 38148: Revert back to get_effective_rule And update the first conditional logic. This patch aims to maintain the logic behind the first patch, but restore get_effective_rule. get_effective_rule_value is using cache and I believe that is the reason why Items.t fail when using that. Test plan: 1) Search for 'music' on OPAC 2) Notice records have the 'place hold' link 3) Visit circulation rules on Staff: <staff_url>/cgi-bin/koha/admin/smart-rules.pl 4) On 'Default checkout, hold and return policy' , under 'Hold policy' pick 'From any library'. Hit 'Save'. 5) Repeat 1). Notice the 'place hold' link is gone. 6) Apply patch. restart plack. Repeat 1). Notice the 'place hold' is shown again Run: $ prove t/db_dependent/Koha/Items.t Squashed during QA - wnc Signed-off-by: andrewa <andrew.auld@ptfs-europe.com> Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173185 [details] [review] Bug 38148: (QA follow-up) Code comments Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
I squashed the two existing patches - I added test to confirm that an all allowed rule doesn't break, they fail before and pass after these. I added an optional patch with comments if RM finds them useful
Created attachment 173205 [details] [review] Bug 38148: Unit tests Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173206 [details] [review] Bug 38148: Check value of holdallowed circ rule properly It was noted on Bug 29087 that the value of circulation rules needs to be checked differently, so this follow-up addresses that. To test: 1. Apply patch and restart services 2. Search for a record on the OPAC. Confirm you can see a link to place a hold. 3. Search for this record on the staff interface and place a hold. Go to Circulation -> Holds to pull and confirm your hold shows in the list. 4. Confirm tests still pass t/db_dependent/Koha/Holds.t Sponsored-by: Whanganui District Council Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: andrewa <andrew.auld@ptfs-europe.com> Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173207 [details] [review] Bug 38148: Revert back to get_effective_rule And update the first conditional logic. This patch aims to maintain the logic behind the first patch, but restore get_effective_rule. get_effective_rule_value is using cache and I believe that is the reason why Items.t fail when using that. Test plan: 1) Search for 'music' on OPAC 2) Notice records have the 'place hold' link 3) Visit circulation rules on Staff: <staff_url>/cgi-bin/koha/admin/smart-rules.pl 4) On 'Default checkout, hold and return policy' , under 'Hold policy' pick 'From any library'. Hit 'Save'. 5) Repeat 1). Notice the 'place hold' link is gone. 6) Apply patch. restart plack. Repeat 1). Notice the 'place hold' is shown again Run: $ prove t/db_dependent/Koha/Items.t Signed-off-by: andrewa <andrew.auld@ptfs-europe.com> Signed-off-by: Ray Delahunty <r.delahunty@arts.ac.uk> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 173208 [details] [review] Bug 38148: (QA follow-up) Code comments Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Thanks for the quick work here!
Pushed for 24.11! Well done everyone, thank you!
Backport to 24.05????
(In reply to Christopher Brannon from comment #27) > Backport to 24.05???? Yes please, and 23.11
Backported to 24.05.x for upcoming 24.05.05
Pushed to 23.11.x for 23.11.10
Not backporting to 23.05.x unless requested
(In reply to Wainui Witika-Park from comment #31) > Not backporting to 23.05.x unless requested Should not be needed. I believe this was caused by bug 29087 (not in 23.05.x).
*** Bug 38134 has been marked as a duplicate of this bug. ***