[Koha-bugs] [Bug 7067] allow patron self registration via the opac

[bugzilla-daemon at bugs.koha-community.org]

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7067

Jared Camins-Esakov <jcamins at cpbibliography.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |Failed QA

--- Comment #123 from Jared Camins-Esakov <jcamins at cpbibliography.com> ---
There are no unit tests for Koha::Borrower::Modifications, nor for the
AddMember_Opac routine (any other routines added should also have unit tests,
of course). Starting with the 3.12 release cycle, unit tests are required for
code added to the Koha:: and C4:: namespaces.

Also, please use hashrefs rather than hashes as arguments in the Koha::
namespace. I could have sworn you did a follow-up changing all the hash
arguments to hashrefs but I don't see it anywhere, even among the obsolete
patches.

Other notes:
* Do not access the database in BEGIN {} blocks, especially not in the Koha::
namespace.
* Use of C4::SQLHelper from the Koha:: namespace. Calling into the C4::
namespace from Koha:: is not supposed to be done. If that was the only
objection, I would probably push it anyway, at least this time, but arguably I
probably shouldn't.
* Package-level my variables are verboten, since they break persistence, and
replacing "my" with "our" should be done only under extreme duress, and never
in new code (note: you can use our when it's called for by the code, just not
as a workaround for my not working under Plack).

Thank you for including a test plan on the bug. I copied it (and the original
RFC) into the commit message for the first patch. I did not review it for
accuracy yet, since I discovered the lack of unit tests before I got that far,
so you may want to do that.

Also, I noticed a few other issues that would not prevent me pushing this but
you might want to keep in mind for the future:
* The standard for help in command-line scripts is to use pod2usage.
* Object-oriented classes do not export routines and therefore should not use
Exporter. Even procedural classes that do not export any routines should not
use Exporter.
* When creating ->new() subroutines, the following idiom may be useful:
    return bless( { 'verification_token' => $args{'verification_token'}, ... },
$class );

Or even:
    return bless( $args, $class );

I find those two idioms make code easier to read, and certainly save typing,
but they are by no means required.

-- 
You are receiving this mail because:
You are watching all bug changes.