|
Description
Kyle M Hall
2012-06-10 09:51:11 UTC
Created attachment 10065 [details] [review] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. The primary advantage to the Firefox offline cirulation plugin when compared to the offline circulation desktop application, is the ability to add offline circulation actions to a queue so that multiple machines running offline circ can have their circ actions combined and ordered chronologically before being executed. This commit adds the ability to put actions from uploaded KOC files into this queue. In this way, both the FF plugina and the desktop application can be run side by side with no ill effects. Created attachment 10189 [details] [review] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. The primary advantage to the Firefox offline cirulation plugin when compared to the offline circulation desktop application, is the ability to add offline circulation actions to a queue so that multiple machines running offline circ can have their circ actions combined and ordered chronologically before being executed. This commit adds the ability to put actions from uploaded KOC files into this queue. In this way, both the FF plugina and the desktop application can be run side by side with no ill effects. Created attachment 10221 [details] [review] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. The primary advantage to the Firefox offline cirulation plugin when compared to the offline circulation desktop application, is the ability to add offline circulation actions to a queue so that multiple machines running offline circ can have their circ actions combined and ordered chronologically before being executed. This commit adds the ability to put actions from uploaded KOC files into this queue. In this way, both the FF plugina and the desktop application can be run side by side with no ill effects. I ordered a Biblibre sandbox to test this patch. It appears that the patch did not apply. The extra links do not appear on the Circulation page. The email from the sandbox creator included the following: The patches attached to bugzilla 8220 have been applied, and the result is <h1>Something went wrong !</h1>Applying: Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly.<br/> Using index info to reconstruct a base tree...<br/> Falling back to patching base and 3-way merge...<br/> Auto-merging C4/Circulation.pm<br/> Auto-merging installer/data/mysql/updatedatabase.pl<br/> CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl<br/> Failed to merge in the changes.<br/> Patch failed at 0001 Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly.<br/> When you have resolved this problem run git am --resolved.<br/> If you would prefer to skip this patch, instead run git am --skip.<br/> To restore the original branch and stop patching run git am --abort.<br/> Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly.<br/> <br/> Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly.<br/> Apply? [yn] <br/> Patch left in /tmp/Bug-8220---Allow-koc-uploads-to-go-to-process-queu-W6oPhU.patch<br/> Unable to sign off. Created attachment 10587 [details] [review] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. The primary advantage to the Firefox offline cirulation plugin when compared to the offline circulation desktop application, is the ability to add offline circulation actions to a queue so that multiple machines running offline circ can have their circ actions combined and ordered chronologically before being executed. This commit adds the ability to put actions from uploaded KOC files into this queue. In this way, both the FF plugina and the desktop application can be run side by side with no ill effects. Patch rebased and merge conflicts fixed Patch tested with a sandbox, by Bob Birchall <bob@calyx.net.au> Created attachment 11350 [details] [review] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. The primary advantage to the Firefox offline cirulation plugin when compared to the offline circulation desktop application, is the ability to add offline circulation actions to a queue so that multiple machines running offline circ can have their circ actions combined and ordered chronologically before being executed. This commit adds the ability to put actions from uploaded KOC files into this queue. In this way, both the FF plugina and the desktop application can be run side by side with no ill effects. Signed-off-by: Bob Birchall <bob@calyx.net.au> Test 1: Created two Koc files. Loaded both to Koha. Was able to manage the processing of the files OK. Test 2: Used KOC utility to record an issue of an item to a patron. Saved the KOC file as file A. Closed KOC. Opened KOC again, recorded the return of the same item. Saved the KOC file as file B. Returned to Koha, uploaded file B first, then file A. Processed the queue. Transactions processed in correct sequence (by timestamp). All good. Signed off. Patch contains merge markers. Created attachment 12310 [details] [review] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. The primary advantage to the Firefox offline cirulation plugin when compared to the offline circulation desktop application, is the ability to add offline circulation actions to a queue so that multiple machines running offline circ can have their circ actions combined and ordered chronologically before being executed. This commit adds the ability to put actions from uploaded KOC files into this queue. In this way, both the FF plugina and the desktop application can be run side by side with no ill effects. Signed-off-by: Bob Birchall <bob@calyx.net.au> Are you sure this is an enhancement? It sounds to me like it fixes a bug. Without the patch, anyone using both the standalone and FF offline circ modules will end up putting their system in an indeterminate state. (In reply to comment #12) > Are you sure this is an enhancement? It sounds to me like it fixes a bug. > Without the patch, anyone using both the standalone and FF offline circ > modules will end up putting their system in an indeterminate state. I think that's a non-sense to use both systems. Either you want the advanced features of the standalone, at the price of local install on your system, or you want the trivial to install & maintain at the price of no local copy of patrons & other adv features. So, yes I think it's an ENH Hm, I think the problem is different. As you said, the client version offers some advanced features, but you run into problems when you use it at more than one computer. So the patch makes this work (as it does work for the plugin). I think it fixes a problem people might not be aware of using the client version. I see no reason to limit libraries to one version or the other. Why put up an artificial limit? QA comment: * passes koha-qa.pl, except for a strange valid FAIL Subroutine ProcessOfflinePayment redefined * this patch introduces a new sub in C4/Circulation.pm, that has no unit test. This is a requirement for 3.12 failing QA (I haven't tested the patch yet, the code looks correct) Created attachment 14014 [details] [review] Bug 8220 - QA Followup - Unit Test Thank you for the unit tests, Kyle! :) Kyle, I would be happy to QA this patch, but I need a .koc file to test it, could you please attach one ? Created attachment 14452 [details]
Test KOC File
That would be great! I've attached a KOC file that checks out barcodes 1, 2, 3, 4 and 5 to borrower cardnumber 123456789, and returns barcodes 6, 7, 8 and 9. If you want to change the borrower cardnumber and/or the barcodes, you should be able to just open the file in any text editor and alter it to your liking. (In reply to comment #19) > Kyle, I would be happy to QA this patch, but I need a .koc file to test it, > could you please attach one ? Comment on attachment 12310 [details] [review] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. Review of attachment 12310 [details] [review]: ----------------------------------------------------------------- Hi Kyle, I started testing with some code review to see what you have done here - I found some smaller issues. A follow-up resolving them would be good. ::: C4/Circulation.pm @@ +3334,5 @@ > + my $amount = $operation->{amount}; > + > + recordpayment( $borrower->{borrowernumber}, $amount ); > + > + return "Success." Untranslatable string. ::: installer/data/mysql/updatedatabase.pl @@ +5830,5 @@ > +$DBversion = "3.09.00.XXX"; > +if (C4::Context->preference("Version") < TransformToNum($DBversion)) { > + $dbh->do("ALTER TABLE `pending_offline_operations` CHANGE `barcode` `barcode` VARCHAR( 20 ) CHARACTER SET utf8 COLLATE utf8_general_ci NULL DEFAULT NULL"); > + $dbh->do("ALTER TABLE `pending_offline_operations` ADD `amount` DECIMAL( 28, 6 ) NULL DEFAULT NULL"); > + print "Upgrade to $DBversion done (redefine the field branchcode as PRIMARY KEY of branches)\n"; Wrong text in update message. ::: koha-tmpl/intranet-tmpl/prog/en/modules/offline_circ/enqueue_koc.tt @@ +17,5 @@ > + > +[% FOREACH message IN messages %] > + [% IF ( message.message ) %] > + [% IF ( message.ERROR_file_version ) %] > + <div class="dialog alert"><p>Warning: This file is version [% message.upload_version %], but I only know how to import version [% message.current_version %]. I'll try my best.</p> Missing </div> ::: offline_circ/enqueue_koc.pl @@ +18,5 @@ > +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > +# > + > +use strict; > +use warnings; Maybe better use Modern::Perl? Created attachment 16156 [details] [review] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. The primary advantage to the Firefox offline cirulation plugin when compared to the offline circulation desktop application, is the ability to add offline circulation actions to a queue so that multiple machines running offline circ can have their circ actions combined and ordered chronologically before being executed. This commit adds the ability to put actions from uploaded KOC files into this queue. In this way, both the FF plugina and the desktop application can be run side by side with no ill effects. Signed-off-by: Bob Birchall <bob@calyx.net.au> Created attachment 16157 [details] [review] Bug 8220 - QA Followup - Unit Test Attaching rebased patches. Created attachment 16230 [details] [review] [PASSED QA] Bug 8220 - Allow koc uploads to go to process queue instead of being applied directly. The primary advantage to the Firefox offline cirulation plugin when compared to the offline circulation desktop application, is the ability to add offline circulation actions to a queue so that multiple machines running offline circ can have their circ actions combined and ordered chronologically before being executed. This commit adds the ability to put actions from uploaded KOC files into this queue. In this way, both the FF plugina and the desktop application can be run side by side with no ill effects. Signed-off-by: Bob Birchall <bob@calyx.net.au> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 16232 [details] [review] [PASSED QA] Bug 8220 - QA Followup - Unit Test Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> All tests and qa script pass. Tests done: - created a .koc file with return, issue and fine payments. - queued that file into Koha - created some transactions using the Firefox plugin - queued that into Koha - processed files and checked outcome was ok This patch has been pushed to master. I did not test this since I have a Mac and no VMs suitable for testing the KOC application at the moment, but as it is non-core and has been well-tested by Katrin and Bob I am pushing it as-is. A nice follow-up bug might remove the extra variable(s) in the pl file (our $dbh, possibly others). |