https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22343
Tomás Cohen Arazi <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED --- Comment #104 from Tomás Cohen Arazi <[email protected]> --- (In reply to Jonathan Druart from comment #101) > 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. I didn't think about deleting the password, good catch. > 2. Escaping/unescaping issues. Fixed in a follow-up patch. Thanks > 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 That's because I based it on branches.pl and didn't want to make this bug dependent on an open discussion on how to use the API. I can anyway make the modal call the DELETE route and refresh the datatable. > 4. flagsrequired => { parameters => 1 }, > Are you sure it's what we want? Not a dedicated subperm? That's actually a good idea, I will probably add it here. Kyle suggested something along the same lines. parameters => smtp_servers? > 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.") Oops. > 6. > - $params->{from} ||= C4::Context->preference('KohaAdminEmailAddress'); > + my $from = $params->{from} // > C4::Context->preference('KohaAdminEmailAddress'); > > Any good reasons? I'd stick with || to catch "" I'll fix it. My opinion is we should (on a separate bug) use Email::Valid against all passed email addresses. But I'd do it on a separate bug because we need to revisit all controller scripts to verify the exception is caught correctly (you guessed, it is not, and there's no eval/try generally). > 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" I'll see what you're talking about and provide a fix. > 8. Should not we display the "default config" if no default server is > defined? Would you display above the datatable? Or just as another row? > 9. And, finally, is this working? I don't think so. > > kohadev-koha@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. I'll check this. This is probably due to an unadvertised change I made. > Proving tests is not a sufficient test plan. -- You are receiving this mail because: You are watching all bug changes. _______________________________________________ Koha-bugs mailing list [email protected] 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/
