Null pointer in the Editorial Reminder if the user has blocked notifications

[nwoodward]

Describe the bug
We see several errors in the OJS logs about a null pointer in EditorialReminder.php, and it's due to the $notification being null. In the handle() method below the $notification variable returned from $notificationManager->createNotification() could potentially be null. So when it's used in the allowUnsubscribe() method of $mailable it throws a null pointer error.

pkp-lib/jobs/email/EditorialReminder.php

Lines 143 to 162 in 09388ce

	$notificationManager = new NotificationManager();
	$notification = $notificationManager->createNotification(
	Application::get()->getRequest(),
	$this->editorId,
	Notification::NOTIFICATION_TYPE_EDITORIAL_REMINDER,
	$this->contextId
	);
	$mailable = new MailablesEditorialReminder($context);
	$emailTemplate = Repo::emailTemplate()->getByKey($context->getId(), $mailable::getEmailTemplateKey());
	$mailable
	->setOutstandingTasks($outstanding, $submissions, $submissionIds->count())
	->from($context->getContactEmail(), $context->getLocalizedName(Locale::getLocale()))
	->recipients([$editor])
	->subject($emailTemplate->getLocalizedData('subject'))
	->body($emailTemplate->getLocalizedData('body'))
	->allowUnsubscribe($notification);
	Mail::send($mailable);

This happens when the user has blocked notifications. In the createNotification() method of PKPNotificationOperationManager there's a check at the beginning to see if the user exists and has blocked notifications, in which case null is returned.

pkp-lib/classes/notification/PKPNotificationOperationManager.php

Lines 143 to 167 in 09388ce

	public function createNotification(PKPRequest $request, ?int $userId = null, ?int $notificationType = null, ?int $contextId = Application::SITE_CONTEXT_ID, ?int $assocType = null, ?int $assocId = null, int $level = Notification::NOTIFICATION_LEVEL_NORMAL, ?array $params = null): ?Notification
	{
	if ($userId && in_array($notificationType, $this->getUserBlockedNotifications($userId, $contextId))) {
	return null;
	}
	$notification = Notification::create([
	'userId' => $userId,
	'type' => $notificationType,
	'contextId' => $contextId,
	'assocType' => $assocType,
	'assocId' => $assocId,
	'level' => $level,
	'dateCreated' => Carbon::now()
	]);
	if ($params) {
	$notificationSettingsDao = DAORegistry::getDAO('NotificationSettingsDAO'); /** @var NotificationSettingsDAO $notificationSettingsDao */
	foreach ($params as $name => $value) {
	$notificationSettingsDao->updateNotificationSetting($notification->id, $name, $value);
	}
	}
	return $notification;
	}

I don't understand the code well enough to say if the fix should be a null check on $notification after it gets created in EditorialReminder.php, and if so then return from handle(). Or if $mailable should still be called without the allowUnsubscribe($notification) method.

What application are you using?
OJS 3.4.0-5

[touhidurabir]

Fix is applied at https://github.com/pkp/pkp-lib/pull/10332/files#diff-83d61d197c61d91e16b32cc318edb9f2c3ba21032384d2337d75ce12c7189fdfR173-R177 for issue #10214 .