Created attachment 57222 [details] [review] Bug 17558: Fix adding manager id to patron messages in store method Test plan: prove t/db_dependent/Koha/Patron/Messages.t
Created attachment 57223 [details] [review] Bug 17558: Add more tests
Hi, I got an error with koha-qa script FAIL t/db_dependent/Koha/Patron/Messages.t OK critic OK forbidden patterns OK git manipulation OK pod OK spelling FAIL valid Use of uninitialized value $_[0] in lc Don't know the cause.
Strange, my koha-qa says it's OK...
Same, they pass for me, try and update your QA script repo.
This is a(In reply to Bernardo Gonzalez Kriegel from comment #3) > Hi, I got an error with koha-qa script > > FAIL t/db_dependent/Koha/Patron/Messages.t > OK critic > OK forbidden patterns > OK git manipulation > OK pod > OK spelling > FAIL valid > Use of uninitialized value $_[0] in lc > > Don't know the cause. This is a problem from old Cache.pm in Debian and maybe in some version of Ubuntu The old module 2.04 >sub Canonicalize_Expiration_Time { > my $timespec = lc($_[0]) > or return undef; > > my $time; And the update module 2.11: >sub Canonicalize_Expiration_Time { > my $timespec; > > my $timespec_param = shift(@_); > if (! $timespec_param) > { > return undef; > } > $timespec = lc($timespec_param); > > my $time; Regards
Created attachment 57430 [details] [review] [SIGNED-OFF]Bug 17558: Fix adding manager id to patron messages in store method Test plan: prove t/db_dependent/Koha/Patron/Messages.t Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised. Pass OK
Created attachment 57431 [details] [review] [SIGNED-OFF]Bug 17558: Add more tests Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> All tests pass successfuly
my $patron = $builder->build( { source => 'Borrower', values => { branchcode => $library->{branchcode} } } ); +my $patron_2 = Koha::Patrons->search->next; Wouldn't it be more consistent to use TestBuilder for both patrons. This is confusing and might give unexpected results depending on patrons present?
Created attachment 57455 [details] [review] Bug 17558: QA followup - use TestBuilder for creating patron
(In reply to Marcel de Rooy from comment #9) > my $patron = $builder->build( { source => 'Borrower', values => { > branchcode => $library->{branchcode} } } ); > +my $patron_2 = Koha::Patrons->search->next; > > Wouldn't it be more consistent to use TestBuilder for both patrons. This is > confusing and might give unexpected results depending on patrons present? You are right, I've attached a followup patch
Created attachment 57456 [details] [review] [SIGNED-OFF]Bug 17558: QA followup - use TestBuilder for creating patron Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Created attachment 57457 [details] [review] Bug 17558: Fix adding manager id to patron messages in store method Test plan: prove t/db_dependent/Koha/Patron/Messages.t Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Works as advertised. Pass OK Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 57458 [details] [review] Bug 17558: Add more tests Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> All tests pass successfuly Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 57459 [details] [review] Bug 17558: QA followup - use TestBuilder for creating patron Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 16.11, thanks Josef!
(In reply to Kyle M Hall from comment #16) > Pushed to master for 16.11, thanks Josef! blocked by DB changes, passing for 16.05