Test plan: Use the koct firefox plugin or a .koc file and make a return with an unknown barcode. Display the Pending offline circulation actions page: offline_circ/list.pl => The page crashes with a software error: Can't call method "biblio" on an undefined value at /home/koha/src/offline_circ/list.pl line 50. Apply the patch, repeat the test plan. => The page shows the list correctly. Click on process. The unknown barcode is marked as Item not found.
Created attachment 69610 [details] [review] Bug 19771: Prevent Pending offline circulation actions page from crashing when there is an unknown barcode Test plan: Use the koct firefox plugin or a .koc file and make a return with an unknown barcode. Display the Pending offline circulation actions page: offline_circ/list.pl => The page crashes with a software error: Can't call method "biblio" on an undefined value at /home/koha/src/offline_circ/list.pl line 50. Apply the patch, repeat the test plan. => The page shows the list correctly. Click on process. The unknown barcode is marked as Item not found.
Is this ready for test? If so, please set the status to Need Signoff.
The page crashes also when a payment is on the list - it goes through all operations, and payment does even have no barcode...
Created attachment 69837 [details] [review] Bug 19771: Fix crashing in pending offline circulation actions list 0) Do not apply the patch 1) Add some checkout / checkin actions on existing barcodes, you can use standalone application, Firefox add-on or built-in offline circulation client 2) Go to Circulation -> Pending offline circulation actions and you will see you actions in the table 3) Add some actions with non-exitent barcodes or a payment 4) Go to Circulation -> Pending offline circulation actions - the page crashes 5) Apply the patch 6) Go to Circulation -> Pending offline circulation actions - the page is working again
I added a counterpatch, could you please test?
Your patch works well, it prevents from crashing on a unknown barcode. However, I couldn't reproduce the crash on a payment.
Comment on attachment 69837 [details] [review] Bug 19771: Fix crashing in pending offline circulation actions list Review of attachment 69837 [details] [review]: ----------------------------------------------------------------- ::: offline_circ/list.pl @@ +50,5 @@ > + if ($item) { > + my $biblio = $item->biblio; > + $_->{'bibliotitle'} = $biblio->title; > + $_->{'biblionumber'} = $biblio->biblionumber; > + } Uses same type of logic as $patron below. Nice.
(In reply to Matthias Meusburger from comment #6) > Your patch works well, it prevents from crashing on a unknown barcode. > > However, I couldn't reproduce the crash on a payment. I will revise test plan so it is explicitly clear. I confirmed it does fail on payments. I am also pleased to announce that FINALLY, I RTFM'd regarding the offline client, and am much happier about it now.
Created attachment 70695 [details] [review] Bug 19771: Fix crashing in pending offline circulation actions list When the barcode is empty or invalid, an "Internal Server Error" is triggered on the kohadev box. TEST PLAN --------- 0) back up your database if you care about it. 1) Run the following commands: git checkout master git pull git checkout -b bug_19771 origin/master git bz apply 19771 -- This should be quite uneventful. Standard promptings. 2) Continue to run the following commands: git checkout master reset_all sudo koha-shell -c bash kohadev ./misc/cronjobs/create_te_koc_db.pl --sqlite3 -- This will create a borrowers.db in your current directory. 3) On the host where you intend on running the koct install it. -- https://sourceforge.net/projects/koha-oc/files/ download and install. 4) get the borrowers.db file to your host where you installed koct 5) point koct at the file you downloaded Database -> Select Borrowers DB File 6) Create a .koc file with the following transactions. check in, check out, check in (bad barcode), pay fines (any non-zero amount). 7) Run the following commands: restart_all -- we want to make sure caching for plack isn't in the way. 8) In the staff client: Home -> Circulation -> Upload offline circulation file (.koc) 9) Choose the file created and click 'Upload file'. 10) Add to offline circulation queue. 11) View pending offline circulation actions -- This should die. Reading /var/log/koha/kohadev/plack-error.log should be the same error as comment #0. However, this was only the bad biblio error. 12) Run the following commands: git checkout bug_19771 restart_all 13) Refresh staff client page. -- it should all come up. 14) Select the bad biblio line, and delete it. 15) Run the following commands: git checkout master restart_all 16) Refresh the staff client page. -- it should die. Same error as comment #0. This confirms the fine payment issue. 17) Run the following commands git checkout bug_19771 restart_all 18) Refresh the staff client page. -- it should all come up. 19) run the koha qa test tools 20) if you backed up your database, restore it. :) Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
> 3) On the host where you intend on running the koct install it. > -- https://sourceforge.net/projects/koha-oc/files/ > download and install. https://koha-community.org/manual/18.05/en/html/05_circulation.html#offline-circ-tool-for-windows-label I didn't read this. This is the newer KOCT. Still, doesn't change the test plan. :)
(In reply to M. Tompsett from comment #8) > (In reply to Matthias Meusburger from comment #6) > > Your patch works well, it prevents from crashing on a unknown barcode. > > > > However, I couldn't reproduce the crash on a payment. > > I will revise test plan so it is explicitly clear. I confirmed it does fail > on payments. I am also pleased to announce that FINALLY, I RTFM'd regarding > the offline client, and am much happier about it now. Thanks Mark, excellent work!
Created attachment 70745 [details] [review] Bug 19771: Fix crashing in pending offline circulation actions list When the barcode is empty or invalid, an "Internal Server Error" is triggered on the kohadev box. TEST PLAN --------- 0) back up your database if you care about it. 1) Run the following commands: git checkout master git pull git checkout -b bug_19771 origin/master git bz apply 19771 -- This should be quite uneventful. Standard promptings. 2) Continue to run the following commands: git checkout master reset_all sudo koha-shell -c bash kohadev ./misc/cronjobs/create_te_koc_db.pl --sqlite3 -- This will create a borrowers.db in your current directory. 3) On the host where you intend on running the koct install it. -- https://sourceforge.net/projects/koha-oc/files/ download and install. 4) get the borrowers.db file to your host where you installed koct 5) point koct at the file you downloaded Database -> Select Borrowers DB File 6) Create a .koc file with the following transactions. check in, check out, check in (bad barcode), pay fines (any non-zero amount). 7) Run the following commands: restart_all -- we want to make sure caching for plack isn't in the way. 8) In the staff client: Home -> Circulation -> Upload offline circulation file (.koc) 9) Choose the file created and click 'Upload file'. 10) Add to offline circulation queue. 11) View pending offline circulation actions -- This should die. Reading /var/log/koha/kohadev/plack-error.log should be the same error as comment #0. However, this was only the bad biblio error. 12) Run the following commands: git checkout bug_19771 restart_all 13) Refresh staff client page. -- it should all come up. 14) Select the bad biblio line, and delete it. 15) Run the following commands: git checkout master restart_all 16) Refresh the staff client page. -- it should die. Same error as comment #0. This confirms the fine payment issue. 17) Run the following commands git checkout bug_19771 restart_all 18) Refresh the staff client page. -- it should all come up. 19) run the koha qa test tools 20) if you backed up your database, restore it. :) Signed-off-by: Mark Tompsett <mtompset@hotmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Pushed to master for 18.05, thanks to everybody involved!
Awesome work all! Pushed to Stable for 17.11.02
Those lines has been modified by Bug 18276. If needed for 17.05.x please rebase.
*** Bug 20394 has been marked as a duplicate of this bug. ***