https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24151 --- Comment #93 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #90)
Looks good to me, some additional comments:
Thanks Marcel.
t/db_dependent/Koha/Pseudonymization.t Please add test descriptions. This is a good habit. If we dont do it now, we will not. # Subtest: Config does not exist ok 2 # Subtest: Koha::Anonymized::Transactions tests ok 3 ok 4 ok 5 ok 6 ok 7 ok 8 ok 9 ok 10 ok 11 ok 12
Indeed, fixed.
$values->{branchcode} = $patron->branchcode; # FIXME Must be removed from the pref options, or FK removed (?) Adding a FIXME !
I have to admit I cannot remember what is the meaning of this FIXME. It seems to make sense to record the patron's library (?)
my $attributes = Koha::Database->new->schema->resultset('PseudonymizedBorrowerAttribute')-
search({transaction_id => $p->id }); So no Koha objects here but DBIx ?
It's only used in test file. In pm file it's using create_related. 97 $self->_result->create_related('pseudonymized_borrower_attributes', $attribute); I don't think it is worth creating a new module only for that purpose.
$(htpasswd -bnBC 10 "" password | tr -d ':\n' | sed 's/$2y/$2a/'); Why do you change $2y to $2a ? If I understand correctly, $2y signals use of a safer updated algorithm.
Good question. I remember I first used some methods from Koha::AuthUtils, where we hardcode $2a. I guess it comes from there and so the replacement should be removed. I will investigate a bit further.
<key>$2a$10$PfdrEBdRcL2MZlEtKueyLegxI6zg735jD07GRnc1bt.N/ZYMvBAB2</key> BAD choice for a new config variable! What is key? Where is it used for? Please add pseudonymize or something ?
We could, but the point of the generic wording was to make it use for something else, if needed. It could be better named "secret_key". -- You are receiving this mail because: You are watching all bug changes.