Bug 13763 - Renew feature does not check for the BarcodeInputFilter option
Summary: Renew feature does not check for the BarcodeInputFilter option
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low trivial (vote)
Assignee: Hayley Mapley
QA Contact: Josef Moravec
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-25 21:19 UTC by jaker@mvwsd.org
Modified: 2019-10-14 19:59 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 13763: Added check for filter syspref in renew.pl (1.60 KB, patch)
2019-03-14 05:08 UTC, Hayley Mapley
Details | Diff | Splinter Review
Bug 13763: Added check for filter syspref in renew.pl (1.65 KB, patch)
2019-03-19 14:18 UTC, Bin Wen
Details | Diff | Splinter Review
Bug 13763: Added check for filter syspref in renew.pl (1.70 KB, patch)
2019-03-20 15:54 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 13763: (follow-up) Strip leading whitespace characters from input barcode (1.03 KB, patch)
2019-03-20 15:54 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description jaker@mvwsd.org 2015-02-25 21:19:39 UTC
I noticed that the renew feature under circulation does not check to see if the itemBarcodeInputFilter is being used. Adding the following line directly after it assigns the barcode variable fixes this issue.

$barcode = barcodedecode($barcode) if( $barcode && C4::Context->preference('itemBarcodeInputFilter'));
Comment 1 Owen Leonard 2016-08-10 15:03:38 UTC
Still valid in master.
Comment 2 Hayley Mapley 2019-03-14 05:08:46 UTC
Created attachment 86594 [details] [review]
Bug 13763: Added check for filter syspref in renew.pl

In Circulation->Renew, the renew feature does not check the system
preference itemBarcodeFilter to see whether the barcode needs to be
converted. This patch fixes this, adding a check for the system pref and
applying it accordingly to convert the barcode if it is needed.

Test plan:
1) In Circulation->Renew, enter a barcode that you know exists in the
catalogue but add extra spaces, breaking up the barcode. Note that this
fails with a warning that says 'No item matches this barcode'

2) In system Preferences, set the itemBarcodeFilter pref to Remove
spaces and repeat step1. Note that the result is the same as in step 1

3) Apply the patch

4) Repeat step 1, and note that the filter removes the spaces and the
item is renewed (or gives an error that indicates it recognises the
barcode)

Sponsored-by: Catalyst IT
Comment 3 Bin Wen 2019-03-15 20:44:59 UTC
After applying the patch, it is still invalid
Comment 4 Bin Wen 2019-03-15 20:45:55 UTC
After applying the patch, it is still invalid
Comment 5 Hayley Mapley 2019-03-17 20:04:54 UTC
(In reply to Bin from comment #4)
> After applying the patch, it is still invalid

Hi Bin,

Can you please describe what you did, and what was the result?
Comment 6 Bin Wen 2019-03-18 12:26:29 UTC
(In reply to Hayley Mapley from comment #5)
> (In reply to Bin from comment #4)
> > After applying the patch, it is still invalid
> 
> Hi Bin,
> 
> Can you please describe what you did, and what was the result?

I followed the test plan:

Test plan:
1) In Circulation->Renew, enter a barcode that you know exists in the
catalogue but add extra spaces, breaking up the barcode. Note that this
fails with a warning that says 'No item matches this barcode'

2) In system Preferences, set the itemBarcodeFilter pref to Remove
spaces and repeat step1. Note that the result is the same as in step 1

3) Apply the patch

but still failed with a warning that said 'No item matches this barcode'
Comment 7 Hayley Mapley 2019-03-18 22:26:14 UTC
(In reply to Bin from comment #6)
> (In reply to Hayley Mapley from comment #5)
> > (In reply to Bin from comment #4)
> > > After applying the patch, it is still invalid
> > 
> > Hi Bin,
> > 
> > Can you please describe what you did, and what was the result?
> 
> I followed the test plan:
> 
> Test plan:
> 1) In Circulation->Renew, enter a barcode that you know exists in the
> catalogue but add extra spaces, breaking up the barcode. Note that this
> fails with a warning that says 'No item matches this barcode'
> 
> 2) In system Preferences, set the itemBarcodeFilter pref to Remove
> spaces and repeat step1. Note that the result is the same as in step 1
> 
> 3) Apply the patch
> 
> but still failed with a warning that said 'No item matches this barcode'

Thanks for testing! 
It still works for me, would you mind verifying that after applying the patch you restarted memcached and plack (I use restart_all) and that the barcode you are trying to test actually exists? For example, my test barcode was TEST11111 and I entered it as 'TE ST 11 111' with the system preference set to 'Remove spaces from'. This worked for me, I'm just trying to understand why it hasn't worked for you.
Comment 8 Bin Wen 2019-03-19 14:16:38 UTC
Sorry, I found that I made a wrong step in my understanding. There is no problem with the patch and test plan, which worked well!
Comment 9 Bin Wen 2019-03-19 14:18:56 UTC
Created attachment 86745 [details] [review]
Bug 13763: Added check for filter syspref in renew.pl

In Circulation->Renew, the renew feature does not check the system
preference itemBarcodeFilter to see whether the barcode needs to be
converted. This patch fixes this, adding a check for the system pref and
applying it accordingly to convert the barcode if it is needed.

Test plan:
1) In Circulation->Renew, enter a barcode that you know exists in the
catalogue but add extra spaces, breaking up the barcode. Note that this
fails with a warning that says 'No item matches this barcode'

2) In system Preferences, set the itemBarcodeInputFilter pref to Remove
spaces and repeat step1. Note that the result is the same as in step 1

3) Apply the patch

4) Repeat step 1, and note that the filter removes the spaces and the
item is renewed (or gives an error that indicates it recognises the
barcode)

Sponsored-by: Catalyst IT
Signed-off-by: Bin Wen <bin.wen@inlibro.com>
Comment 10 Hayley Mapley 2019-03-19 18:40:21 UTC
(In reply to Bin from comment #9)
> Created attachment 86745 [details] [review] [review]
> Bug 13763: Added check for filter syspref in renew.pl
> 
> In Circulation->Renew, the renew feature does not check the system
> preference itemBarcodeFilter to see whether the barcode needs to be
> converted. This patch fixes this, adding a check for the system pref and
> applying it accordingly to convert the barcode if it is needed.
> 
> Test plan:
> 1) In Circulation->Renew, enter a barcode that you know exists in the
> catalogue but add extra spaces, breaking up the barcode. Note that this
> fails with a warning that says 'No item matches this barcode'
> 
> 2) In system Preferences, set the itemBarcodeInputFilter pref to Remove
> spaces and repeat step1. Note that the result is the same as in step 1
> 
> 3) Apply the patch
> 
> 4) Repeat step 1, and note that the filter removes the spaces and the
> item is renewed (or gives an error that indicates it recognises the
> barcode)
> 
> Sponsored-by: Catalyst IT
> Signed-off-by: Bin Wen <bin.wen@inlibro.com>

Thanks again for testing! I'll change the status to Signed Off so it'll get picked up by the QA team :)
Comment 11 Josef Moravec 2019-03-20 15:54:45 UTC
Created attachment 86818 [details] [review]
Bug 13763: Added check for filter syspref in renew.pl

In Circulation->Renew, the renew feature does not check the system
preference itemBarcodeFilter to see whether the barcode needs to be
converted. This patch fixes this, adding a check for the system pref and
applying it accordingly to convert the barcode if it is needed.

Test plan:
1) In Circulation->Renew, enter a barcode that you know exists in the
catalogue but add extra spaces, breaking up the barcode. Note that this
fails with a warning that says 'No item matches this barcode'

2) In system Preferences, set the itemBarcodeInputFilter pref to Remove
spaces and repeat step1. Note that the result is the same as in step 1

3) Apply the patch

4) Repeat step 1, and note that the filter removes the spaces and the
item is renewed (or gives an error that indicates it recognises the
barcode)

Sponsored-by: Catalyst IT
Signed-off-by: Bin Wen <bin.wen@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 12 Josef Moravec 2019-03-20 15:54:48 UTC
Created attachment 86819 [details] [review]
Bug 13763: (follow-up) Strip leading whitespace characters from input barcode

this makes the work with barcodes from input consistant on checking out,
     checking in and renewing

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Nick Clemens 2019-03-21 18:36:19 UTC
Awesome work all!

Pushed to master for 19.05
Comment 14 Martin Renvoize 2019-03-25 14:38:06 UTC
Pushed to 18.11.x for 18.11.04
Comment 15 Lucas Gass 2019-04-01 15:39:21 UTC
backported to 18.05.x for 18.05.12