Bug 20305

Summary: Warnings in tools scripts need to be removed
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: josef.moravec, julian.maurice, maksim.sen, martin.renvoize, roch.damour
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 20017    
Bug Blocks:    
Attachments: Bug 20305: Remove warnings from tools scripts
Bug 20305: Remove warnings from tools scripts
Bug 20305: Remove warnings from tools scripts
Bug 20305: Remove warnings from tools scripts
Bug 20305: Remove warnings from tools scripts

Description Jonathan Druart 2018-02-27 16:36:00 UTC
Since bug 20017 we have now warnings in tools scripts.
Comment 1 Jonathan Druart 2018-02-27 16:38:29 UTC
Created attachment 72273 [details] [review]
Bug 20305: Remove warnings from tools scripts
Comment 2 Roch D'Amour 2018-03-02 20:22:54 UTC
Hi Jonathan
Do you have a test plan for this?
Comment 3 Maksim Sen 2018-03-09 21:29:34 UTC
+1 for test plan
Comment 4 Jonathan Druart 2018-03-23 17:51:44 UTC
Test plan:
- do not apply the patch
- use the "Batch item modification"
=> Notice the warnings "Argument "" isn't numeric in numeric eq (==)"
- apply the patch
- use the "Batch item modification"
=> No more warnings
Comment 5 Mark Tompsett 2018-03-26 06:01:54 UTC
Comment on attachment 72273 [details] [review]
Bug 20305: Remove warnings from tools scripts

Review of attachment 72273 [details] [review]:
-----------------------------------------------------------------

::: tools/batchMod.pl
@@ +315,4 @@
>  my $subfieldsToAllowForBatchmod = C4::Context->preference('SubfieldsToAllowForRestrictedBatchmod');
>  my $allowAllSubfields = (
>      not defined $subfieldsToAllowForBatchmod
> +      or $subfieldsToAllowForBatchmod ne q||

Wrong equality, I believe.
!~ /\S/ might be better?
Comment 6 Jonathan Druart 2018-03-26 14:11:20 UTC
I do not think we do that for other prefs.
Comment 7 Roch D'Amour 2018-03-30 20:44:08 UTC
Created attachment 73504 [details] [review]
Bug 20305: Remove warnings from tools scripts

Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>
Comment 8 Julian Maurice 2018-04-09 12:22:25 UTC
Jonathan,

your patch changes '==' into 'ne'. It should be 'eq' (unless this behavior change is intended but it's not mentioned in the commit message)
Comment 9 Jonathan Druart 2018-04-09 13:51:12 UTC
Created attachment 73868 [details] [review]
Bug 20305: Remove warnings from tools scripts
Comment 10 Jonathan Druart 2018-04-09 13:51:46 UTC
(In reply to Julian Maurice from comment #8)
> Jonathan,
> 
> your patch changes '==' into 'ne'. It should be 'eq' (unless this behavior
> change is intended but it's not mentioned in the commit message)

Erk! Thanks for catching this!
Comment 11 Julian Maurice 2018-04-09 15:02:15 UTC
Created attachment 73876 [details] [review]
Bug 20305: Remove warnings from tools scripts

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Comment 12 Tomás Cohen Arazi 2018-05-04 19:39:03 UTC
Created attachment 75095 [details] [review]
Bug 20305: Remove warnings from tools scripts

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Jonathan Druart 2018-05-07 15:26:06 UTC
Pushed to master for 18.05, thanks to everybody involved!