https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22818
--- Comment #71 from Martin Renvoize <[email protected]> --- (In reply to Katrin Fischer from comment #50) > I am still working on this, but adding some first notes and questions: > > 1) QA test tools > > FAIL Koha/MessageAttributes.pm > FAIL forbidden patterns > forbidden pattern: Incorrect license statement (using postal > address), may > be a false positive if the file is coming from outside Koha (bug 24545) > (line 16) > > FAIL Koha/MessageAttribute.pm > FAIL forbidden patterns > forbidden pattern: Incorrect license statement (using postal > address), may > be a false positive if the file is coming from outside Koha (bug 24545) > (line 16) Fixed > > 2) Unit tests > > While tests pass, they produce some non-good looking output: > > kohadev-koha@kohadevbox:/home/vagrant/kohaclone$ prove > t/db_dependent/Illrequests.t > t/db_dependent/Illrequests.t .. 7/12 Use of uninitialized value > $staff_to_send in pattern match (m//) at > /home/vagrant/kohaclone/Koha/Illrequest.pm line 1522. > > Koha::Illrequest::send_staff_notice(Koha::Illrequest=HASH(0x55b06c17ba40), > "ILL_REQUEST_CANCEL") called at /home/vagrant/kohaclone/Koha/Illrequest.pm > line 288 > Koha::Illrequest::status(Koha::Illrequest=HASH(0x55b06c17ba40), > "CANCREQ") > called at t/db_dependent/Illrequests.t line 788 > main::__ANON__() called at /usr/share/perl5/Test/Builder.pm line 310 > eval {...} called at /usr/share/perl5/Test/Builder.pm line 310 > Test::Builder::subtest(Test::Builder=HASH(0x55b06bac3bf8), "Helpers", > CODE(0x55b063112be8)) called at /usr/share/perl5/Test/More.pm line 807 > Test::More::subtest("Helpers", CODE(0x55b063112be8)) called at > t/db_dependent/Illrequests.t line 934 > t/db_dependent/Illrequests.t .. ok > All tests successful. > Files=1, Tests=12, 32 wallclock secs ( 0.09 usr 0.03 sys + 21.40 cusr 6.88 > csys = 28.40 CPU) > Result: PASS Fixed > 3) Code Review > > 3.1) Email addressing > > I feel like here is a general issue here in that we will have problems with > spam protection if the mail server is not in the same domain as the > branchillemail given. > > Is the the new ILL staff email is to be used as the sender or the recipient > of the ILL emails to staff? It looks like it's actually used as both? As the > pref only reads "to", maybe we could just keep the current from and it would > resolve a lot of the possible issues with spam. > > I think we need to at least make use of $library->inbound_email_address in a > couple of cases replacing the branchemail here: > > + my $from = $branch->branchillemail || $branch->branchemail; > > get_staff_to_address seems ok in that regard, but send_staff_notice > from_adress does not. > Reply-to and From are the spam critical ones. > > Use of Koha::Email is removed? Fixed > 3.2) Translatability > > The "N/A" here are problematic. I think it would be better to just create an > empty string? With TT libraries can still resolve that to another text. > > + substitute => { > + ill_bib_title => $title ? $title->value : 'N/A', > + ill_bib_author => $author ? $author->value : 'N/A', Fixed > 3.3) Sample letters > > I need to take a closer look for typos and such, but the sample letters need > to also be added to the new sample .yml and to the translated installers. Converted to TT syntax and added to sampl_notices.yml > 3.4) Sysprefs.sql > > New prefs are in the wrong alphabetic spot :) FIxed > 3.5) Notices & slips > > The module in the letters summary table is not translatable yet and shows as > "ill". Was a copy/paste error.. also fixed > ------------- > > Most of those are not bad ones, only 3.1 Email addressing worries me a fair > bit. Clearing up intentions and intended behaviour would be a good first step > here. -- 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/
