Offline circ : You now can upload all offline files from the Firefox extension. Once all circ desks have uploaded the file, the librarian can apply all of them, sorted by date. This avoid the problem of someone issuing an item on desk A, returning it on desk B. Before this improvement, if desk B uploaded the file before A, the return was applied before the issue, resulting in the items reamining issued.
Created attachment 3315 [details] [review] Proposed Patch
I'm confused about this one - the patch applies fine. I installed the Firefox KOCT https://addons.mozilla.org/en-US/firefox/addon/koct/ I set it up and my log shows that checkouts have been recorded, but when I try to send them to Koha nothing happens - nothing I can tell at least. I need a test case and/or a tutorial on how this works and how to test this patch. Nicole
Nicole, do you have configured your extension correctly (last tab of the extension, where you define you koha url & librarian login & password) ? when you upload the datas, the last line is updated from "local" to "uploaded" (or something like that)
I do have it set up with my super librarian login and the URL of my Koha staff client - but I do not see any note about it being uploaded.
Nicole, the table pending_offline_operations that must be here seems not to be on the database ! Does it work better after a: CREATE TABLE `pending_offline_operations` ( `operationid` int(11) NOT NULL AUTO_INCREMENT, `userid` varchar(30) NOT NULL, `branchcode` varchar(10) NOT NULL, `timestamp` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP, `action` varchar(10) NOT NULL, `barcode` varchar(20) NOT NULL, `cardnumber` varchar(16) DEFAULT NULL, PRIMARY KEY (`operationid`) ) ENGINE=MyISAM AUTO_INCREMENT=2 DEFAULT CHARSET=latin1
Created attachment 3400 [details] FF4 released today The plugin isn't compatible with FF 4 - so I haven't tried adding the database info because FF has disabled the plugin until it's updated to work with the newest version of the browser. Nicole
The table pending_offline_operations is not part of the patch and not part of my new 3.4 database. Marking as Failed QA.
(In reply to comment #7) > Created attachment 3400 [details] > FF4 released today > > The plugin isn't compatible with FF 4 - so I haven't tried adding the database > info because FF has disabled the plugin until it's updated to work with the > newest version of the browser. Plugin updated, now compatible with FF4
It still says last updated in February ... was that when it was fixed? https://addons.mozilla.org/en-US/firefox/addon/koct/
Also, that page links to the old koha sites - can it be updated?
(In reply to comment #10) > It still says last updated in February ... was that when it was fixed? > > https://addons.mozilla.org/en-US/firefox/addon/koct/ in fact, the fix was just to say to mozilla.org that it was OK with FF4. No fix in KOCT itself
(In reply to comment #11) > Also, that page links to the old koha sites - can it be updated? OK, will ask Claire immediatly for that
some of our customers are now live with FF4 and this plugin, switching back to "needs signoff" Notice: 3 customers reported "the plugin has disappeared". It just because, by default, on FF4, the plugin bar at the bottom is hidden. ctrl-/ to have it displayed !
The necessary database structure is still not part of this patch, and needs to be added, as Katrin mentioned earlier. Marking as Failed QA on those grounds.
Created attachment 4661 [details] [review] proposed patch (updated) patch highly updated : * added kohastructure & updatedatabase * fixed forgotten subs in Circulation.pm * link on circulation-home.pl added tested with koct 0.2.2 successfully
Created attachment 4861 [details] [review] Rebased patch with version set correctly Change version to 3.05.00.XXX in kohaversion.pl and updatedatabase.pl
Created attachment 4862 [details] [review] Rebased patch with version set correctly Forgot to make obsolete previous patch.
I'm trying to test this and failing because I don't understand how the offline circ addon communicates with Koha. The wiki page (http://wiki.koha-community.org/wiki/Offline_circulation_firefox_plugin) shows screenshots of the KOCT which don't match the current plugin: The current plugin offers two buttons under the log tab, "Commit to Koha" and "Apply directly." Clicking either one of these doesn't appear to do anything. The wiki page also shows a screenshot of a page in Koha which doesn't appear to exist in master. The "Process Pending Offline Operations" page doesn't exist as far as I can tell. I'm guessing this is in a Biblibre branch? Is this why the KOCT doesn't do anything?
Created attachment 5681 [details] [review] rebased patch vith version set correctly in previous patch, there was a kohaversion that should not have been here. rebased against today master
Created attachment 5683 [details] [review] proposed patch new patch, fixing owen comment (on IRC) about no message in case of an invalid barcode
Updating Version : This ENH will be for Koha 3.8
Bug versionned for master. entries will be made against rel_3_8 once the patch has been applied (see thread about that on koha-devel yesterday)
Created attachment 6540 [details] [review] [PATCH] [SIGNED-OFF] Bug 5877 : Offline circulation improvements : upload all files, apply at once Offline circ : You now can upload all offline files from the Firefox extension. Once all circ desks have uploaded the file, the librarian can apply all of them, sorted by date. This avoid the problem of someone issuing an item on desk A, returning it on desk B. Before this improvement, if desk B uploaded the file before A, the return was applied before the issue, resulting in the items reamining issued. Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com>
Before starting QA, I would say that a patch of this size needs external signoff. From the comments I understand that several tested, but nobody signed off until now..
Marcel, it seems Sophie made a mistake when signing off the patch: i'm the author of the patch (see obsoletes), Sophie just signed it off. I don't know why she appears as the author ! going back to signed off for QA, thx
(In reply to comment #26) > Marcel, it seems Sophie made a mistake when signing off the patch: i'm the > author of the patch (see obsoletes), Sophie just signed it off. I don't know > why she appears as the author ! > > going back to signed off for QA, thx Well, Sophie could correct that easily, I suppose. My earlier question was actually: a patch of this size would preferably not have patch writer and patch signer from the same company.. I already asked the list and QA team to think about a status for that situation like Needs additional signoff or so, but no response so far.. Don't think that In Discussion is a good alternative now.
Nicole, Liz & owen, one of you already has tried to test this patch, if you want to give it another try...
Paul, I'd like to hear my questions in comment 19 addressed before I test again.
(In reply to comment #29) > Paul, I'd like to hear my questions in comment 19 addressed before I test > again. wiki page updated, with new screenshots & updated documentation. Feel free to fix typos ;-) Also please check that you have a pending_offline_operations table (should be added by the updatedatase), it "uploading" circ information can also fail because of that !
I have installed the plugin and I can't get it to log my actions at all - I haven't tested this patch yet because I can't get to that step. Also, my screens don't look like the ones on the Wiki - will attach my screens here. Nicole
Created attachment 7218 [details] log My log stays empty no matter what I scan and save on the previous tabs.
(In reply to comment #32) > Created attachment 7218 [details] > log > > My log stays empty no matter what I scan and save on the previous tabs. Are you using MacOS ? this plugin works for all our customers that are under windows. Works also under linux, I think it's untested on MacOS-X
Bingo! I'm on Mac and it does not work. Nicole
(In reply to comment #34) > Bingo! I'm on Mac and it does not work. And, for the record: bingo bingo, now it works, and Nicole made nothing specific :(
Paul, the screenshot on the wiki page shows a calendar for choosing "expected return" date in the KOCT addon. Is that something you've developed for the addon which isn't in the official version?
Created attachment 7250 [details] [review] Follow-up fix Internationalization fix: Offline circulation operations in Circulation.pm return English strings for display in the template. These strings can't be translated, so we must check their values in the JavaScript and return a translatable string based on their values. Interface change: Switching to text links for checking/unchecking checkboxes since that is more consistent with other areas in Koha.
(In reply to comment #36) > Paul, the screenshot on the wiki page shows a calendar for choosing "expected > return" date in the KOCT addon. Is that something you've developed for the > addon which isn't in the official version? nope, I don't know where this come from. The KOCT we deploy for our customers is the one that is at https://addons.mozilla.org/en-US/firefox/addon/koct/ (I can't ask Jean André anymore about early code, as he's not more working for us. Maybe it was an early-code screenshot, I really don't know)
I would like to see an external signoff on a patch of this size. Sophie signed it off, while Paul apparently should be the author. Could someone outside Biblibre signoff please? I see that several people tested this one already, but did not.. Changing status to reflect this request.
Created attachment 7618 [details] [review] [SIGNED-OFF] Bug 5877 : Offline circulation improvements : upload all files, apply at once Offline circ : You now can upload all offline files from the Firefox extension. Once all circ desks have uploaded the file, the librarian can apply all of them, sorted by date. This avoid the problem of someone issuing an item on desk A, returning it on desk B. Before this improvement, if desk B uploaded the file before A, the return was applied before the issue, resulting in the items reamining issued. Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
The script devel/misc/testbugzillapatches.pl has detected that this bug does not apply anymore, so i'm marking this bug as "does not apply". The script is supposed to handle conflicts related to updatedatabase, but no other cases of merge conflict. So the conflict may be trivial to fix. If it happens that the patch attached does apply cleanly, please send me an email, there's something wrong in the script. It's possible, as it's a recent script. If the patch applies does not apply as I think, please rebase and resubmit, thanks
Created attachment 7715 [details] [review] Bug 5877 Follow-up for Offline circulation improvements Internationalization fix: Offline circulation operations in Circulation.pm return English strings for display in the template. These strings can't be translated, so we must check their values in the JavaScript and return a translatable string based on their values. Interface change: Switching to text links for checking/unchecking checkboxes since that is more consistent with other areas in Koha. Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Created attachment 7716 [details] [review] Bug 5877 : Offline circulation improvements : upload all files, apply at once Offline circ : You now can upload all offline files from the Firefox extension. Once all circ desks have uploaded the file, the librarian can apply all of them, sorted by date. This avoid the problem of someone issuing an item on desk A, returning it on desk B. Before this improvement, if desk B uploaded the file before A, the return was applied before the issue, resulting in the items reamining issued. Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 7717 [details] [review] Bug 5877 : Offline circulation improvements : upload all files, apply at once Offline circ : You now can upload all offline files from the Firefox extension. Once all circ desks have uploaded the file, the librarian can apply all of them, sorted by date. This avoid the problem of someone issuing an item on desk A, returning it on desk B. Before this improvement, if desk B uploaded the file before A, the return was applied before the issue, resulting in the items reamining issued. Signed-off-by: Sophie Meynieux <sophie.meynieux@biblibre.com> Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 7718 [details] [review] [PASSED_QA]Bug 5877 Follow-up for Offline circulation improvements Internationalization fix: Offline circulation operations in Circulation.pm return English strings for display in the template. These strings can't be translated, so we must check their values in the JavaScript and return a translatable string based on their values. Interface change: Switching to text links for checking/unchecking checkboxes since that is more consistent with other areas in Koha. Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
OK, after checking why the script devel/misc/testbugzillapatches.pl has detected this patch does not apply, I've discovered that it was just because the 2 follow-up was appearing before the main patch. So: * i've rebased on master, everything is OK, i just managed the updatedatabase * signed-off owen follow-up * re-uploaded the 2 patches, in the correct order * we're back to signed-off / waiting for QA. Thanks ian/marcel for your QA !
QA Comment (first patch): Looks good in general, just a few points need attention. C4/Circulation.pm: Please adjust or send a follow up for AddOfflineOperation: The insert statement does not mention the field names. If the table structure is changed some day, this kind of statement will fail. offline_circ/list.pl, offline_circ/process.pl, offline_circ/service.pl: Address of Free Software Foundation is not correct. Please add use strict and use warnings. Please confirm too if tests do not show any warnings in your logfile. Second patch looks good. Marked as Passed QA in attachment name.
Created attachment 7812 [details] [review] Bug 5877 QA follow-up * removing tabs as indenter * adding fieldnames in INSERT clause * updating FSF address * removing ` in SQL table creation (mysql-ism) * use strict & use warning added & no error in logs checked Note that process_koc.pl and updatedatabase are not related to this bug, but the FSF address was wrong, I fixed it as well
(In reply to comment #47) > QA Comment (first patch): Looks good in general, just a few points need > attention. > > C4/Circulation.pm: > Please adjust or send a follow up for AddOfflineOperation: > The insert statement does not mention the field names. If the table structure > is changed some day, this kind of statement will fail. good catch ! follow-up coming. > offline_circ/list.pl, offline_circ/process.pl, offline_circ/service.pl: > Address of Free Software Foundation is not correct. good catch too. I also have fixed the FSF address ... in updatedatabase, as it was wrong here too ! I also have fixed some tabs (replaced by 4 spaces) and removed the ` in the table creation. > Please add use strict and use warnings. same thing : in the follow-up i'll attach now. they don't issue any warning. switching to "signed off", then changes are QA changes that should have no effect on the feature itself
When I apply 7716 followed by 7718 and then the latest one (7812), the third patch fails. What am I doing wrong here? Please check.
After dealing with the updatedatabase.pl conflict in attachment 7716 [details] [review] the other two patches apply fine for me. I used "git am -i3u" on the first patch and "git bz apply" for the others.
After fixing the first, the third still gives: Applying: Bug 5877 QA follow-up error: patch failed: C4/Circulation.pm:3037 error: C4/Circulation.pm: patch does not apply error: patch failed: offline_circ/list.pl:41 error: offline_circ/list.pl: patch does not apply error: patch failed: offline_circ/service.pl:30 error: offline_circ/service.pl: patch does not apply Using index info to reconstruct a base tree... error: patch failed: C4/Circulation.pm:3037 error: C4/Circulation.pm: patch does not apply error: patch failed: offline_circ/list.pl:41 error: offline_circ/list.pl: patch does not apply error: patch failed: offline_circ/service.pl:30 error: offline_circ/service.pl: patch does not apply Did you hand edit your patch? It does not apply to blobs recorded in its index. Cannot fall back to three-way merge.
(In reply to comment #51) > After dealing with the updatedatabase.pl conflict in attachment 7716 [details] [review] the other > two patches apply fine for me. I used "git am -i3u" on the first patch and "git > bz apply" for the others. Are we on the same commit? 1a5afaa26d374144d1ebde6bcf9c69b97c048a5d
(In reply to comment #53) > Are we on the same commit? 1a5afaa26d374144d1ebde6bcf9c69b97c048a5d Yes
Forget my latest comments: problems with vim..
Created attachment 7960 [details] [review] Bug 5877 : Offline circulation improvements : upload all files, apply at once Simple rebase.
So, everything applied fine now; vim was causing problems yesterday. The last followup patch resolves the earlier QA comments (and even more!) So the three patches together satisy QA now. Changing status.
patches where not in the right order (follow-up before main patch), but I deal with that. patch pushed.
There have been no further reports of problems so I am marking this bug resolved.