Bug 14428

Summary: Remove C4::Input
Product: Koha Reporter: Mark Tompsett <mtompset>
Component: Architecture, internals, and plumbingAssignee: Mark Tompsett <mtompset>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: minor    
Priority: P5 - low CC: m.de.rooy, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9006
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 9006    
Attachments: Bug 14428: Remove C4::Input
Bug 14428: Remove C4::Input
Bug 14428: Remove C4::Input

Description Mark Tompsett 2015-06-19 19:16:47 UTC 
C4/Input.pm was tweaked to not use dbh directly. This gives us something kind of ugly autoload mess. Fix it. It is a partial revert of 'git show a1fe32d2'.
Comment 1 Mark Tompsett 2015-07-10 04:56:04 UTC 
After discussing on mailing list a little, and with no objections, it seems that C4/Input.pm is beyond its useful days. This bug has been repurposed to remove it, rather than correct the C4::Context->prepare autoload mess.
Comment 2 Mark Tompsett 2015-07-10 05:04:20 UTC 
mtompset@debian:~/kohaclone$ git reset --hard origin/master
HEAD is now at 6e9086f Bug 3206: (QA followup) missing comma on sysprefs.sql
mtompset@debian:~/kohaclone$ git grep C4::Input
C4/Input.pm:package C4::Input; #assumes C4/Input
C4/Input.pm:C4::Input - Miscellaneous sanity checks
C4/Input.pm:  use C4::Input;
acqui/addorderiso2709.pl:use C4::Input;
acqui/basketgroup.pl:use C4::Input;
acqui/neworderempty.pl:use C4::Input;
acqui/neworderempty.pl:use C4::Input;
acqui/neworderempty.pl:use C4::Input;
acqui/uncertainprice.pl:use C4::Input;
admin/aqplan.pl:use C4::Input;
members/memberentry.pl:use C4::Input;
t/Input.t:        use_ok('C4::Input');
mtompset@debian:~/kohaclone$ grep sub C4/Input.pm
sub checkdigit ($;$) {
                my $temp2 = substr($infl,$i,1);
        if ($rem eq substr($infl,8,1)) {
} # sub checkdigit
mtompset@debian:~/kohaclone$ grep checkdigit `find acqui -type f`
mtompset@debian:~/kohaclone$ grep checkdigit `find admin -type f`
mtompset@debian:~/kohaclone$ grep checkdigit `find members -type f`
mtompset@debian:~/kohaclone$ cat t/Input.t
#!/usr/bin/perl
#
# This Koha test module is a stub!
# Add more tests here!!!

use strict;
use warnings;

use Test::More tests => 1;

BEGIN {
        use_ok('C4::Input');
}
Comment 3 Mark Tompsett 2015-07-10 05:19:31 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-07-10 07:25:06 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2015-07-10 08:56:07 UTC 
Created attachment 40913 [details] [review]
Bug 14428: Remove C4::Input

Jonathan Druart agreed that C4::Input is vestigial code that should be removed.
Here is how I checked. First I found where C4::Input was used. Then, I checked
what functions are in the package: just checkdigit. Then, I confirmed that
checkdigit is not used at all in any acquisition, administration, or member
related perl scripts. Lastly, I took a look at our supposed test file for the
package. It was painfully sparse.

As such, this patch removes the test file and the package file, and removes
C4::Input references from these six files:
- acqui/addorderiso2709.pl
- acqui/basketgroup.pl
- acqui/neworderempty.pl
- acqui/uncertainprice.pl
- admin/aqplan.pl
- members/memberentry.pl
NOTE: neworderempty had 3 lines of it?! Didn't anyone see that?!

Here is the output of what I did to confirm this correction:

mtompset@debian:~/kohaclone$ git reset --hard origin/master
HEAD is now at 6e9086f Bug 3206: (QA followup) missing comma on sysprefs.sql
mtompset@debian:~/kohaclone$ git grep C4::Input
C4/Input.pm:package C4::Input; #assumes C4/Input
C4/Input.pm:C4::Input - Miscellaneous sanity checks
C4/Input.pm:  use C4::Input;
acqui/addorderiso2709.pl:use C4::Input;
acqui/basketgroup.pl:use C4::Input;
acqui/neworderempty.pl:use C4::Input;
acqui/neworderempty.pl:use C4::Input;
acqui/neworderempty.pl:use C4::Input;
acqui/uncertainprice.pl:use C4::Input;
admin/aqplan.pl:use C4::Input;
members/memberentry.pl:use C4::Input;
t/Input.t:        use_ok('C4::Input');
mtompset@debian:~/kohaclone$ grep sub C4/Input.pm
sub checkdigit ($;$) {
                my $temp2 = substr($infl,$i,1);
        if ($rem eq substr($infl,8,1)) {
} # sub checkdigit
mtompset@debian:~/kohaclone$ grep checkdigit `find acqui -type f`
mtompset@debian:~/kohaclone$ grep checkdigit `find admin -type f`
mtompset@debian:~/kohaclone$ grep checkdigit `find members -type f`
mtompset@debian:~/kohaclone$ cat t/Input.t

use strict;
use warnings;

use Test::More tests => 1;

BEGIN {
        use_ok('C4::Input');
}

Apply this patch, and the output of git grep C4::Input will be empty.
Run koha qa test tools (kind of overkill)

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2015-07-10 08:58:52 UTC 
Checked enabling the pref checkdigit. This triggers code in C4::Members:fixup_cardnumber that apparently replaced the stuff in C4::Input at some point in time..
Still works :)
Comment 7 Tomás Cohen Arazi (tcohen) 2015-07-10 14:04:13 UTC 
Patch pushed to master.

Thanks Mark!