[Koha-bugs] [Bug 6022] Auth_with_ldap check if categorycode is valid
bugzilla-daemon at bugs.koha-community.org
bugzilla-daemon at bugs.koha-community.org
Tue Aug 9 17:18:20 CEST 2011
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=6022
Paul Poulain <paul.poulain at biblibre.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |paul.poulain at biblibre.com
Patch Status|Signed Off |Failed QA
--- Comment #4 from Paul Poulain <paul.poulain at biblibre.com> 2011-08-09 15:18:20 UTC ---
QA comment
I have some comments, 2 of them being blockers :
+ # XXX check if categorycode exists, if not, fallback to default from
koha-conf.xml
1 why XXX at the beginning of the comment ? (not blocking, i'm just surprised.
++ for the comment though
+ my $dbh = C4::Context->dbh;
+ my $sth = $dbh->prepare("SELECT categorycode FROM categories WHERE
upper(categorycode) = upper(?)");
2 Why upped in SQL ? It's useless, as SQL don't care with uc/lc or diacritics.
categorycodes are automatically UCed, so I think it would be better to :
* UC the $borrower{'categorycode'}
* do SQL query without upped()
+ $sth->execute( $borrower{'categorycode'} );
+ if ( my $row = $sth->fetchrow_hashref ) {
3 Why an empty IF then ELSE ? UNLESS would have done the job !
+
+ } else {
+ my $default = $mapping{'categorycode'}->{content};
+ warn "Can't find ", $borrower{'categorycode'}, " default to: $default
for ", $borrower{userid};
4 UNCONDITIONAL WARN detected. If you want to add a warn (which is OK), please
use
$DEBUG && warn "blabla...";
Thus the warn will be issued only if you're in DEBUG mode (set in httpd.conf)
+ $borrower{'categorycode'} = $default
+ }
Sorry, but failed QA for reasons 2 and 4 (and 3 should be fixed as well, but I
wouldn't fail QA just for reason 3)
--
Configure bugmail: http://bugs.koha-community.org/bugzilla3/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.
More information about the Koha-bugs
mailing list