[Koha-bugs] [Bug 18595] Move C4::Members::Messaging to Koha namespace

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

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

Joonas Kylmälä <joonas.kylmala at helsinki.fi> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Needs Signoff               |Failed QA

--- Comment #109 from Joonas Kylmälä <joonas.kylmala at 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.