[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

--- Comment #111 from Emmi Takkinen <emmi.takkinen at outlook.com> ---
(In reply to Joonas Kylmälä from comment #109)
> 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).
> 
Same error occurs in the staff client if we have email as message transport
type for patron (I'm guessing that error will also be raised if sms number
isn't set but is chosen as message preference). However patron is still saved. 

I'm not sure if automatically removing message preference is a good approach.
Maybe preventing form from sending and error message like 'Patron has email as
message transport type but not email. Either provide valid email or change
patrons messaging preferences.' is enough. Hmm, or error message next to input
field.

> 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.
> 
Again same occurs in the staff client. All in all we need better error handling
with these changes.

> 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?
> 
Lari already commented these. With last one I agree some cleanup might be
needed. First one I understand but I think it's worth checking other options...
Could we e.g. disable checkbox with jQuery?

> 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.
> 
My guess is that if statement isn't allowed to be used inside input tag.

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