[Koha-bugs] [Bug 12026] Shibboleth auto-provisioning

bugzilla-daemon at bugs.koha-community.org bugzilla-daemon at bugs.koha-community.org
Thu Mar 23 12:17:51 CET 2017


https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12026

Marcel de Rooy <m.de.rooy at rijksmuseum.nl> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Signed Off                  |Passed QA

--- Comment #66 from Marcel de Rooy <m.de.rooy at rijksmuseum.nl> ---
QA Comment:
Code looks good to me.
This smaller change has mostly been code reviewed by me. I tested some
constructs on itself, but I do not have a shibboleth server.
Since most QAers do not and it has been tested by Martin and Mirko, I take the
liberty to pass QA on it now.
But not without a few remarks :)

Koha::Database->new()->schema()->resultset('Borrower')
      ->find( { $config->{matchpoint} => $match } );
Strictly speaking, you should use find with primary keys/unique constraints.
It might fallback to search here depending on your matchpoint (userid has a
unique constraint).

=head2 _autocreate
  my ( $retval, $retcard, $retuserid ) = _autocreate( $config, $match );
Given a database handle [...]
=> Where is that database handle? It got removed after the second patch..
Fixed in a followup.

checkpw_shib
The change to this routine should normally be accompanied by changes in the
tests.
Perhaps you should add autocreate => 0 explicitly to mockedConfig in this test.
I wonder if testing autocreate => 1 should be done better in a db_dependent
test.
Since this patch goes such a long way, I do not want to block it now. But
please add a small test on a new report.

Passed QA

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


More information about the Koha-bugs mailing list