Bug 14428 - Remove C4::Input
Summary: Remove C4::Input
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Mark Tompsett
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 9006
  Show dependency treegraph
 
Reported: 2015-06-19 19:16 UTC by Mark Tompsett
Modified: 2016-12-05 21:23 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 14428: Remove C4::Input (7.72 KB, patch)
2015-07-10 05:19 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 14428: Remove C4::Input (7.79 KB, patch)
2015-07-10 07:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14428: Remove C4::Input (7.90 KB, patch)
2015-07-10 08:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!