Description
Chad Billman
2016-02-05 19:53:54 UTC
Created attachment 47716 [details] [review] Bug 15752 - Automatically switch patron during circulation With this syspref enabled scanning a patron barcode into the item barcode field during circulation will redirect you to the patron's circulation page. This allows circulation with only a barcode scanner. Testing: - Enable the AutoSwitchPatron syspref - Navigate to a patron's check out screen - Enter a patron barcode into the "Enter item barcode" field Result - Browser should be redirected to the entered patron Hi Chad, Patch works as expected but atomicupdate file should be a *.sql file instead of a *.pl file :-) Fix that and I can retest/signoff Patch does no longer apply. Created attachment 55953 [details] [review] Bug 15752 - Automatically switch patron during circulation With this syspref enabled scanning a patron barcode into the item barcode field during circulation will redirect you to the patron's circulation page. This allows circulation with only a barcode scanner. Testing: - Enable the AutoSwitchPatron syspref - Navigate to a patron's check out screen - Enter a patron barcode into the "Enter item barcode" field Result - Browser should be redirected to the entered patron Easy conflict fixed, atomic .pl moved to .sql Hi Jonathan, With patch applied I can not open the checkout page e.g. from the link "My checkouts" or the link "Check out" at the left hand of a patron detail page. To make it work, I changed the following line: if (C4::Context->preference("AutoSwitchPatron") ) { to: if (C4::Context->preference("AutoSwitchPatron") && $query->param('barcode') ) { With this change, the patch behaves as expected. I'd like to get feedback from others to know if it is a good behaviour to implement before fixing it. I think this could speed up things at the desk, but I have one worry: What happens when there is an overlap between item barcodes and patron cardnumbers? Maybe we should show a warning message in that case, asking if the number was one or the other before proceeding. Hm, if this is to avoid using the mouse to switch to the check-in field - have you seen that there are keyboard shortcuts for this too? From the circulation start page help: - jump to the catalog search with Alt+Q - jump to the checkout with Alt+U (this will not work for Mac user) - jump to the checkin with Alt+R (In reply to Katrin Fischer from comment #8) > I think this could speed up things at the desk, but I have one worry: What > happens when there is an overlap between item barcodes and patron > cardnumbers? > > Maybe we should show a warning message in that case, asking if the number > was one or the other before proceeding. I think in the case of overlapping barcodes the feature should be turned off anyway, as the explanation for syspref AutoSwitchPatron says: "This should not be enabled if you have overlapping patron and book barcodes." If in the future a library with overlapping barcodes wants to have this feature, they can ask for or provide an enhancement. Hi, For autoswitch, in my library we use this JavaScript code (thanks to S.D.): ///////////////// $(document).ready(function () { if (window.location.href.indexOf("cgi-bin/koha/circ/circulation.pl") > -1) if ($("#circ_impossible ul li").html()) { var message = $("#circ_impossible ul li").html(); var terme = message.split(' '); var cb= terme[9]; var n= cb.length;} if (n==5) if (check_numerique(cb) == true) {$("#patronsearch #findborrower").val(cb); $("#patronsearch #autocsubmit").click(); } else {return false;} function check_numerique(input){ return !isNaN(input); } } //////////////////// This present 1 or 2 seconds the usual yellow message : Barcode not found we are satisfied with this solution for 1 year. Is it possible to make the same timing in this patch ? Thanks, Fred Patch tested with a sandbox, by Joel <aloi54@live.fr> Created attachment 57015 [details] [review] Bug 15752 - Automatically switch patron during circulation With this syspref enabled scanning a patron barcode into the item barcode field during circulation will redirect you to the patron's circulation page. This allows circulation with only a barcode scanner. Testing: - Enable the AutoSwitchPatron syspref - Navigate to a patron's check out screen - Enter a patron barcode into the "Enter item barcode" field Result - Browser should be redirected to the entered patron Signed-off-by: Joel <aloi54@live.fr> Should not we add a message box if the switch happened? From the Koha log file: circulation.pl: CGI::param called in list context from package main line 83, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 436., referer: http://pro.kohadev.vm/cgi-bin/koha/circ/circulation-home.pl circulation.pl: Odd number of elements in anonymous hash at /home/vagrant/kohaclone/circ/circulation.pl line 83., referer: http://pro.kohadev.vm/cgi-bin/koha/circ/circulation-home.pl The search should only be done if the barcode field has been filled. Created attachment 66175 [details] [review] Bug 15752 - Automatically switch patron during circulation With this syspref enabled scanning a patron barcode into the item barcode field during circulation will redirect you to the patron's circulation page. This allows circulation with only a barcode scanner. Testing: - Enable the AutoSwitchPatron syspref - Navigate to a patron's check out screen - Enter a patron barcode into the "Enter item barcode" field Result - Browser should be redirected to the entered patron Signed-off-by: Joel <aloi54@live.fr> Created attachment 66176 [details] [review] Bug 15752: QA followup - search for patrons only if param barcode is defined Created attachment 66177 [details] [review] Bug 17572: QA followup - inform user that patron was automatically switched Test plan: 1) Start checking out to some patron - there should be no message about auto switching 2) read the another patrons card - note the patron is switched and there is a message about it at top of the main page content Hi Josef Sorry but failed in the running of QA test tools: OK circ/circulation.pl OK critic OK forbidden patterns OK git manipulation OK pod OK spelling OK valid OK installer/data/mysql/sysprefs.sql OK git manipulation OK semicolon OK sysprefs_order FAIL koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt OK forbidden patterns OK git manipulation OK spelling OK tt_valid FAIL valid_template After that is fixed up I will be happy to test and signoff as the patches worked as described in the test plans Created attachment 67037 [details] [review] Bug 15752 - Automatically switch patron during circulation With this syspref enabled scanning a patron barcode into the item barcode field during circulation will redirect you to the patron's circulation page. This allows circulation with only a barcode scanner. Testing: - Enable the AutoSwitchPatron syspref - Navigate to a patron's check out screen - Enter a patron barcode into the "Enter item barcode" field Result - Browser should be redirected to the entered patron Signed-off-by: Joel <aloi54@live.fr> Created attachment 67038 [details] [review] Bug 15752: QA followup - search for patrons only if param barcode is defined Created attachment 67039 [details] [review] Bug 15752: QA followup - inform user that patron was automatically switched Test plan: 1) Start checking out to some patron - there should be no message about auto switching 2) read the another patrons card - note the patron is switched and there is a message about it at top of the main page content (In reply to Alex Buckley from comment #19) > Hi Josef > > Sorry but failed in the running of QA test tools: > > OK circ/circulation.pl > OK critic > OK forbidden patterns > OK git manipulation > OK pod > OK spelling > OK valid > > OK installer/data/mysql/sysprefs.sql > OK git manipulation > OK semicolon > OK sysprefs_order > > FAIL koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt > OK forbidden patterns > OK git manipulation > OK spelling > OK tt_valid > FAIL valid_template > > > After that is fixed up I will be happy to test and signoff as the patches > worked as described in the test plans Did you have any conflicts applying the patches? For me it passes the QA test tools... Anyway, I rebased the patches on master and fixed commit message on 3rd patch (wrong bug number). Could you try it again? Created attachment 67174 [details] [review] Bug 15752 - Automatically switch patron during circulation With this syspref enabled scanning a patron barcode into the item barcode field during circulation will redirect you to the patron's circulation page. This allows circulation with only a barcode scanner. Testing: - Enable the AutoSwitchPatron syspref - Navigate to a patron's check out screen - Enter a patron barcode into the "Enter item barcode" field Result - Browser should be redirected to the entered patron Signed-off-by: Joel <aloi54@live.fr> Followed test plan, patch works as described Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Created attachment 67175 [details] [review] Bug 15752: QA followup - search for patrons only if param barcode is defined Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Created attachment 67176 [details] [review] Bug 15752: QA followup - inform user that patron was automatically switched Test plan: 1) Start checking out to some patron - there should be no message about auto switching 2) read the another patrons card - note the patron is switched and there is a message about it at top of the main page content Followed test plan, patch worked as described. Passed QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Created attachment 69994 [details] [review] Bug 15752 - Automatically switch patron during circulation With this syspref enabled scanning a patron barcode into the item barcode field during circulation will redirect you to the patron's circulation page. This allows circulation with only a barcode scanner. Testing: - Enable the AutoSwitchPatron syspref - Navigate to a patron's check out screen - Enter a patron barcode into the "Enter item barcode" field Result - Browser should be redirected to the entered patron Signed-off-by: Joel <aloi54@live.fr> Followed test plan, patch works as described Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 69995 [details] [review] Bug 15752: QA followup - search for patrons only if param barcode is defined Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 69996 [details] [review] Bug 15752: QA followup - inform user that patron was automatically switched Test plan: 1) Start checking out to some patron - there should be no message about auto switching 2) read the another patrons card - note the patron is switched and there is a message about it at top of the main page content Followed test plan, patch worked as described. Passed QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 70077 [details] [review] Bug 15752: Remove unecessary redirect We are on circulation.pl, no need to redirect. We should not need to redirect, we are on circulation.pl. Could someone take a look and retest quickly? Created attachment 70085 [details] [review] Bug 15752: Remove unecessary redirect (follow-up) If the borrowernumber is defined in query, it starts to get information about previous borrower and the new one is taken into account fully on second page reload (In reply to Jonathan Druart from comment #31) > We should not need to redirect, we are on circulation.pl. Could someone take > a look and retest quickly? True, but probably not so easy... I tested your patch and added followup - now it is working for me... but I am not sure if it is enough in all situations... Created attachment 71359 [details] [review] Bug 15752: Automatically switch to patron when cardnumber is read during circulation With this syspref enabled scanning a patron barcode into the item barcode field during circulation will redirect you to the patron's circulation page. This allows circulation with only a barcode scanner. Testing: - Enable the AutoSwitchPatron syspref - Navigate to a patron's check out screen - Enter a patron barcode into the "Enter item barcode" field Result - Browser should be redirected to the entered patron Signed-off-by: Joel <aloi54@live.fr> Followed test plan, patch works as described Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 71360 [details] [review] Bug 15752: (follow-up) Search for patrons only if param barcode is defined Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 71361 [details] [review] Bug 15752: (QA follow-up) Inform user that patron was automatically switched Test plan: 1) Start checking out to some patron - there should be no message about auto switching 2) read the another patrons card - note the patron is switched and there is a message about it at top of the main page content Followed test plan, patch worked as described. Passed QA test tool Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 71362 [details] [review] Bug 15752: (QA follow-up) Remove unecessary redirect We are on circulation.pl, no need to redirect. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 71363 [details] [review] Bug 15752: (QA follow-up) Remove unecessary redirect If the borrowernumber is defined in query, it starts to get information about previous borrower and the new one is taken into account fully on second page reload Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Patches pushed to master for 18.05. Congratulations Chad for your first patch in! Awesome work all! Enhancement, not backported to stable. |