https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18595
Joonas Kylmälä <joonas.kylm...@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #109 from Joonas Kylmälä <joonas.kylm...@helsinki.fi> --- Some things need a bit more work: If no patron email we get error =============================== 1. Add patron without email 2. Go to cgi-bin/koha/opac-messaging.pl and select email checkbox for every messaging setting 3. Observe following error: > Patron has not set email address, cannot use email as message transport at > /usr/share/perl5/Exception/Class/Base.pm line 88 > 1. in Exception::Class::Base::throw at > /usr/share/perl5/Exception/Class/Base.pm line 88 > 2. in Koha::Patron::Message::Preference::_set_message_transport_types at > /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 409 > 3. in Koha::Patron::Message::Preference::message_transport_types at > /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 224 > 4. in Koha::Patron::Message::Preference::set at > /kohadevbox/koha/Koha/Patron/Message/Preference.pm line 247 > 5. in C4::Form::MessagingPreferences::handle_form_action at > /kohadevbox/koha/C4/Form/MessagingPreferences.pm line 110 > 6. in (eval) at /kohadevbox/koha/opac/opac-messaging.pl line 67 After fixing this it should be made sure also that removing email address in a patron which had the messaging preferences removes the messaging preferences (or handles it some proper way). More robust input handling in OPAC needed ========================================= In the opac messaging preferences change with the HTML dev console the value of "Days in advance" to be 50 and submit the request, you will then get following error: > days_in_advance has to be a value between 0-30 for Advance_Notice. at > /usr/share/perl5/Exception/Class/Base.pm line 88 Expected outcome: a nicer result is shown to the user about invalid parameters, we should handle all errors in the code. What do the last two commits do =============================== Bug 18595: Disable digest checkbox when forced on: - This one maybe I don't understand because of the first error caused by missing email. Specifically I don't understand in which situation the digest checkbox should be disabled. Bug 18595: Validate days_in_advance - What does this do exactly? I see it change days_in_advance=NULL but what is the reason behind that? QA tool errors ============== $ koha-qa.pl -v 2 -c 9 FAIL C4/Reserves.pm FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). OK forbidden patterns OK git manipulation OK pod OK pod coverage OK spelling OK valid FAIL circ/returns.pl FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s). OK forbidden patterns OK git manipulation OK pod OK spelling OK valid FAIL koha-tmpl/intranet-tmpl/prog/en/includes/messaging-preference-form.inc OK filters OK forbidden patterns OK git manipulation OK js_in_body OK spelling FAIL tt_valid lines 163, 169 OK valid_template FAIL koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt OK filters OK forbidden patterns OK git manipulation OK js_in_body OK spelling FAIL tt_valid lines 131, 133 OK valid_template For the critic qa errors see the details with for example using $ perlcritic circ/returns.pl $ perlcritic C4/Reserves.pm Not quite sure how to fix the template issues. That's all the issues I spotted. The code review I did didn't find any issues and all the test mentioned in the test plan work. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list Koha-bugs@lists.koha-community.org https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs website : http://www.koha-community.org/ git : http://git.koha-community.org/ bugs : http://bugs.koha-community.org/