Created attachment 31471 [details] Screen Shot from 3.14 When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16.
Created attachment 31472 [details] Screen Shot from 3.16
Created attachment 31581 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen Bug 12892 - Holds Waiting: not showing from check out screen TEST PLAN: 1. Create a reservation for a Borrower with pickup branch 2. Check-in the corresponding Item in the pickup branch and confirm the hold for the Borrower 3. Go to circ/circulation.pl for the Borrower and verify that the reservation is "Waiting for pickup" 4. In the Borrower -> Check Out -tab (circ/circulation.pl) on the right of the big "Checking out to borrowername" should be a notification about holds ready for pickup.
I am not able to apply this patch. ================================== Bug 12892 - Holds Waiting: not showing from check out screen 31581 - Bug 12892 - Holds Waiting: not showing from check out screen Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 12892 - Holds Waiting: not showing from check out screen fatal: sha1 information is lacking or useless (Koha/Reserves.pm). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Bug 12892 - Holds Waiting: not showing from check out screen When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-12892---Holds-Waiting-not-showing-from-check-o-g6Guwy.patch ==================================
Hi Olli-Antti, please submit your patches against master if the problem exists there too, this will make it much easier to get them tested and integrated. I have verifed this problem also appears on master.
Thank you Katrin and Jesse. This feature needs the Koha/Reserves.pm -file to be introduced to the git which happens in Bug 12917
It prolly still fails to merge. I'll take a look into this when I have more time ;)
I tried with the dependency - it didn't apply yesterday.
Created attachment 31600 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized.
There seems to be some urgency to this issue. Due to that, I've submitted an alternate solution. I hope that's ok! (In reply to Olli-Antti Kivilahti from comment #6) > It prolly still fails to merge. I'll take a look into this when I have more > time ;)
Created attachment 31601 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized.
No problem, just sharing my code in hopes of it being helpful. I also like your solution :)
Created attachment 31603 [details] [review] Bug 12892 - Unit Tests
Created attachment 31604 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized.
Created attachment 31605 [details] [review] Bug 12892 - Unit Tests
Created attachment 31607 [details] [review] [SIGNED-OFF] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 31608 [details] [review] Bug 12892 [QA Followup] - Encode printed vars as UTF8
Hm, not sure if the last patch needs to be undone when we push bug 11944 - creating a link.
This patch uses 2 "new" technical concepts: - new Koha::Schema::ResultSet namespace - use of Koha::Schema in pl file They don't have been voted by the Koha dev community. Note sure they should be introduced to fix a major bug. I let someone else QA this patch but, IMO, it should not be pushed as it.
I can understand Jonathan's point and not sure how to proceed here - we got a dev meeting tomorrow - should we discuss this?
Created attachment 31670 [details] Bug12892_multiple_waitingholds_utf8_in_branchname.png Found some issues with multiple waitingholds and diacritics in branchname
Created attachment 31671 [details] [review] Bug 12892 [QA Followup] - Encode printed vars as UTF8 --amended to add missing [% USE EncodeUTF8 %]
Created attachment 31672 [details] [review] Bug 12892 [QA Followup] - Encode printed vars as UTF8 --amended to add missing [% USE EncodeUTF8 %] and fixing branch encoding
I also think it sucks super badly to put [% VALUE | $EncodeUTF8 | html %] to the templates. It is so wrong. As if the tempates aren't cluttered enough.
Created attachment 31962 [details] [review] Bug 12892 - Add Koha::Object, Koha::Objects and unit tests This patch adds the base classes upon which we can build new Koha classes to give us smarter, simpler code. Test Plan: 1) Apply this patch 2) prove t/Object.t t/db_dependent/Object.t t/db_dependent/Objects.t
Created attachment 31963 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized.
Comment on attachment 31962 [details] [review] Bug 12892 - Add Koha::Object, Koha::Objects and unit tests Moving the base class patch to it's own bug. Bug 13019.
Applying: Bug 12892 - Holds Waiting: not showing from check out screen Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Reserves.t CONFLICT (content): Merge conflict in t/db_dependent/Reserves.t Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt Auto-merging circ/circulation.pl Failed to merge in the changes. Patch failed at 0001 Bug 12892 - Holds Waiting: not showing from check out screen When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-12892---Holds-Waiting-not-showing-from-check-o-xSl9aQ.patch
Did you apply the dependencies first?
SMH, no I got caught up reading through the discussion and forgot to go back up and note them. I can't apply 11518 so can't apply 11703 and so can't apply this, will retreat for simpler bugs
Created attachment 32010 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized.
Created attachment 32017 [details] [review] [SIGNED OFF] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized. Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
There is an analogous problem on check-in. See bug 13033.
Created attachment 32044 [details] [review] [SIGNED OFF] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized. Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 32057 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized. Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 32066 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized. Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Created attachment 32069 [details] [review] Bug 12892 - Holds Waiting: not showing from check out screen When a patron has holds waiting to be pick up, they used to show in the check out screen. These notifications are no longer showing in 3.16. Test Plan: 1) Apply this patch 2) Create a hold, set to waiting 3) Browse to circulation.pl for that patron 4) Note you see the list of waiting holds 5) Switch you logged in branch to a different branch 6) Note the "Waiting at" line is no longer emphasized. Signed-off-by: Nick Clemens <nick@quecheelibrary.org>
Since this is a major one, it could be good to have a fix not using bug 13019.
*** Bug 13117 has been marked as a duplicate of this bug. ***
I have been playing around allot with bug 13019 lately and my general feeling is that it's pretty workable already and a viable route to using proper objects in Koha. Whilst it has allot of further enhancements required to make it 'really' useful, it's in a good state as is for a first step. I still feel these objects kinda of lock away allot of the advantages of dbic behind an abstraction that sometimes detracts rather than enhances; but with our debates going on and on I'de rather see Koha moving forward than stagnating in debate territory. My real feeling after all this is that we're doing way too much logical work in our .pl's.. if it's logic based at all, it really belongs in a .pm where it can be fully tested ;-) ! With the above in mind, I would be happy to pass QA on this bug.. and sign off on bug 13019 on which it relies. Is anyone watching this conversation able to also take a look at 13019 with either signoff or QA in mind as I obviously can't do both roles ;)
We urgently need to get bug 13019 moving in order to resolve this - please?
This feels like an evil bug stuck in a discussion about architecture - could we have a small and easy fix bringing the old functionality back and figure out the other stuff separately?
Created attachment 33222 [details] [review] Bug 12892 : Alternate fix to restore the deleted code This doesn't depend on anything other patch. 1. Create a reservation for a Borrower with pickup branch 2. Check-in the corresponding Item in the pickup branch and confirm the hold for the Borrower 3. Go to circ/circulation.pl for the Borrower and verify that the reservation is "Waiting for pickup" 4. In the Borrower -> Check Out -tab (circ/circulation.pl) on the right of the big "Checking out to borrowername" should be a notification about holds ready for pickup.
I have attached an alternate patch, which restores the code that was deleted when the ajax checkouts were added. This is not dependent on other patches.
Created attachment 33225 [details] [review] [SIGNED OFF] Bug 12892 : Alternate fix to restore the deleted code This doesn't depend on anything other patch. 1. Create a reservation for a Borrower with pickup branch 2. Check-in the corresponding Item in the pickup branch and confirm the hold for the Borrower 3. Go to circ/circulation.pl for the Borrower and verify that the reservation is "Waiting for pickup" 4. In the Borrower -> Check Out -tab (circ/circulation.pl) on the right of the big "Checking out to borrowername" should be a notification about holds ready for pickup. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Thanks for stepping into the breach Chris... alternate patch works as expected for me.. signing off.
Created attachment 33236 [details] [review] Bug 12892 : Alternate fix to restore the deleted code This doesn't depend on anything other patch. 1. Create a reservation for a Borrower with pickup branch 2. Check-in the corresponding Item in the pickup branch and confirm the hold for the Borrower 3. Go to circ/circulation.pl for the Borrower and verify that the reservation is "Waiting for pickup" 4. In the Borrower -> Check Out -tab (circ/circulation.pl) on the right of the big "Checking out to borrowername" should be a notification about holds ready for pickup. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Rafal Kopaczka <rkk0@poczta.onet.pl>
This patch also fix bug 12971, and maybe some other bugs.
(In reply to Rafal Kopaczka from comment #47) > This patch also fix bug 12971, and maybe some other bugs. Nope, sorry this does not fix that issue. But, still this is good patch :)
Created attachment 33275 [details] [review] [PASSED QA] Bug 12892 : Alternate fix to restore the deleted code This doesn't depend on anything other patch. 1. Create a reservation for a Borrower with pickup branch 2. Check-in the corresponding Item in the pickup branch and confirm the hold for the Borrower 3. Go to circ/circulation.pl for the Borrower and verify that the reservation is "Waiting for pickup" 4. In the Borrower -> Check Out -tab (circ/circulation.pl) on the right of the big "Checking out to borrowername" should be a notification about holds ready for pickup. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Rafal Kopaczka <rkk0@poczta.onet.pl> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes all tests and QA script, restores the old code and behaviour.
Patch pushed to master. Thanks Chris!
Pushed to 3.16.x, will be in 3.16.7
I set to pushed to stable since its in 3.16.x