Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dev/php8 - PHP8 Fixes #1467

Merged
merged 4 commits into from
Jul 15, 2023
Merged

Conversation

Monica-Wood
Copy link

Small PHP8 Fixes

@@ -1469,7 +1469,7 @@ public static function importMetadata($po_request, $ps_source, $ps_importer, $ps

if (isset($pa_options['sendMail']) && $pa_options['sendMail']) {
if ($vs_email = trim($po_request->user->get('email'))) {
$info = $t_importer->getInfoForLastImport();
$info = $t_importer?->getInfoForLastImport();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are you trying to do here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting this error when calling process-task-queue with the -r flag, when there are stuck processes to restart.

PHP Fatal error:  Uncaught Error: Call to a member function getInfoForLastImport() on null in /srv/providence/app/lib/BatchProcessor.php:1472
Stack trace:
#0 /srv/providence/app/lib/Plugins/TaskQueueHandlers/metadataImport.php(106): BatchProcessor::importMetadata()
#1 /srv/providence/app/lib/TaskQueue.php(328): WLPlugTaskQueueHandlermetadataImport->process()
#2 /srv/providence/app/lib/Utils/CLIUtils/Cron.php(46): TaskQueue->processQueue()
#3 /srv/providence/support/bin/caUtils(172): CLIUtils::process_task_queue()
#4 {main}
  thrown in /srv/providence/app/lib/BatchProcessor.php on line 1472

I am not sure if $t_importer is meant to be not null in this situation, but this allows the getInfoForLastImport to be called without an error even if $t_importer is null

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue with this is the null safe operator is only available in PHP 8, and we'd like to keep support for PHP 7.4 for a while, even though it's EOL. Maybe we should just abandon that idea, but if we don't then this should be done using and if()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in new commit. I have tested that this still sends emails correctly with a normal metadata import.

@collectiveaccess collectiveaccess merged commit 1b8a242 into collectiveaccess:dev/php8 Jul 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants