pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
297 stars 443 forks source link

"Create Reviewer" reviewer selection option breaks email template #4547

Closed jmacgreg closed 5 years ago

jmacgreg commented 5 years ago

This occurs when, as an editor, I go to assign a reviewer and instead choose the "Create reviewer" option. The Create New Reviewer modal loads, but the email is blank except for a "{", which I suspect is some sort of underlying code-breakage issue. See attached screenshot.

image

I'm not seeing anything super-relevant in the log, except for possibly the following (and there are a few of these at the same time):

PHP message: PHP Warning: Illegal string offset 'en_US' in /var/home/pkpsandbox/sandbox.url/www/cache/t_compile/2122dd4b4d038d06701b7a5cbc73d21546ea835d_0.app.formtextInput.tpl.php on line 125\n', referer: https://sandbox.url/index.php/publicknowledge/workflow/index/28/3

When I input text and try and save the modal, the modal doesn't save/close and I have to click outside of it. If I refresh the page, the user has been created and assigned, but no email has gone out and I don't see an email/discussion anywhere. There is then a massive set of PHP notices and warnings in the log after attempting that action, a lot of which doesn't look relevant, but I'll paste below in the event that it might be.

jmacgreg commented 5 years ago

OK, in the error log I see:

PHP message: PHP Warning:  strstr() expects parameter 1 to be string, array given in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/classes/mail/MailTemplate.inc.php on line 216
PHP message: PHP Deprecated:  Non-static method PKPRequest::getUser() should not be called statically in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/classes/mail/MailTemplate.inc.php on line 226
PHP message: PHP Deprecated:  Non-static method PKPRequest::_checkThis() should not be called statically in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/classes/core/PKPRequest.inc.php on line 564
PHP message: PHP Warning:  preg_match_all() expects parameter 2 to be string, array given in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/classes/mail/MailTemplate.inc.php on line 305
PHP message: PHP Warning:  preg_match_all() expects parameter 2 to be string, array given in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/classes/mail/MailTemplate.inc.php on line 305
PHP message: PHP Warning:  strip_tags() expects parameter 1 to be string, array given in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/classes/core/PKPString.inc.php on line 405
PHP message: PHP Deprecated:  Non-static method PKPRequest::getRemoteAddr() should not be called statically in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/classes/mail/Mail.inc.php on line 547
PHP message: PHP Deprecated:  Non-static method PKPRequest::_checkThis() should not be called statically in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/classes/core/PKPRequest.inc.php on line 425
PHP message: PHP Warning:  substr() expects parameter 1 to be string, array given in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/lib/vendor/phpmailer/phpmailer/src/PHPMailer.php on line 2148
PHP message: PHP Warning:  explode() expects parameter 2 to be string, array given in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/lib/vendor/phpmailer/phpmailer/src/PHPMailer.php on line 2153
PHP message: PHP Warning:  Invalid argument supplied for foreach() in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/lib/vendor/phpmailer/phpmailer/src/PHPMailer.php on line 2156
PHP message: PHP Notice:  Array to string conversion in /var/home/pkpsandbox/sandbox.url/www/lib/pkp/lib/adodb/adodb.inc.php on line 1021
PHP message: ojs2: DB Error: Unknown column 'Array' in 'field list'
', referer: https://sandbox.url/index.php/publicknowledge/workflow/index/28/3
asmecher commented 5 years ago

Thanks, @jmacgreg -- fixed by the attached commits (one for master, one for stable-3_1_2).

For some reason the Smarty clearAssign function wasn't behaving as expected. I worked around it by simply changing clearAssign statements into assign statements with null values (or false or whatever was appropriate). This cleared out a few redundant lines of code so I don't mind it as a solution. This works now locally (the problem reproduced here as well).