[Koha-bugs] [Bug 22343] Add SMTP configuration options to Administration

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

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

Jonathan Druart <jonathan.druart at bugs.koha-community.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|Passed QA                   |Failed QA

--- Comment #101 from Jonathan Druart <jonathan.druart at bugs.koha-community.org> ---
1. If you edit a server without password, there is "****" anyway
I tried to fix it, and only display '****' if the password is defined. But then
found another bug, you cannot remove the password field once it has been filled
already.

2. Escaping/unescaping issues. Fixed in a follow-up patch.

3. Deletion - Why are not you using the DELETE route?
Here you are cheating, I don't think we should GET with "op=delete", the
parameters stay in the url

4. flagsrequired   => { parameters => 1 },
Are you sure it's what we want? Not a dedicated subperm?

5. "SMTP servers Define which SMTP servers to use." is displayed on the admin
home page regardless the logged in user has enough permission (click then get
"Error: You do not have permission to access this page.")

6. 
-    $params->{from} ||= C4::Context->preference('KohaAdminEmailAddress');
+    my $from    = $params->{from}    //
C4::Context->preference('KohaAdminEmailAddress');

Any good reasons? I'd stick with || to catch ""

7. I have tried to cheat and insert twice a default server, on the form edit
the dropdown and select value "" (create it)
I don't get any errors and see in the log "Cannot add or update a child row: a
foreign key constraint fails" for the branch fk
I then noticed you used "*"
In admin/smtp_servers.pl:
 77     catch {
 78         if ( blessed $_ and
$_->isa('Koha::Exceptions::Object::DuplicateID') ) {

else? We are hiding a potential error/exception. We must log it anyway and make
the interface display "something wrong happened"

8. Should not we display the "default config" if no default server is defined?

9. And, finally, is this working? I don't think so.

kohadev-koha at kohadevbox:/kohadevbox/koha$ perl
misc/cronjobs/process_message_queue.pl 
Use of uninitialized value for string at /usr/share/perl5/Email/MIME/Encode.pm
line 66.
Undefined subroutine &Email::Sender::Simple::sendmail called at
/kohadevbox/koha/C4/Letters.pm line 1386.
 at /kohadevbox/koha/C4/Letters.pm line 1405.

% pmvers Email::Sender::Simple
1.300034

If I replace Email::Sender::Simple::sendmail with sendmail (following the POD
of the module), I get:
sending email message to patron: 51 at /kohadevbox/koha/C4/Letters.pm line
1007.
Use of uninitialized value for string at /usr/share/perl5/Email/MIME/Encode.pm
line 66.
Don't know how to handle Koha::Email at
/usr/share/perl5/Email/Sender/Role/CommonSending.pm line 78.
 at /kohadevbox/koha/C4/Letters.pm line 1405.

Proving tests is not a sufficient test plan.

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