Accessing user notification settings from the site throws a fatal error

[Vitaliy-1]

Describe the bug
When 2 or more journals are installed, site settings page becomes accessible to the admin user. That means that the admin user can edit own user profile from the site, without context being available. This leads to a fatal error when trying to view notification settings as they are bound to the context

PHP Fatal error:  Uncaught TypeError: APP\notification\form\NotificationSettingsForm::getNotificationSettingCategories(): Argument #1 ($context) must be of type PKP\context\Context, null given, called in /Users/vitalii/Documents/development/ojs-main/ojs/lib/pkp/classes/notification/form/PKPNotificationSettingsForm.php on line 131 and defined in /Users/vitalii/Documents/development/ojs-main/ojs/classes/notification/form/NotificationSettingsForm.php:27
Stack trace:
#0 /Users/vitalii/Documents/development/ojs-main/ojs/lib/pkp/classes/notification/form/PKPNotificationSettingsForm.php(131): APP\notification\form\NotificationSettingsForm->getNotificationSettingCategories(NULL)
#1 /Users/vitalii/Documents/development/ojs-main/ojs/lib/pkp/controllers/tab/user/ProfileTabHandler.php(342): PKP\notification\form\PKPNotificationSettingsForm->fetch(Object(APP\core\Request))
#2 [internal function]: PKP\controllers\tab\user\ProfileTabHandler->notificationSettings(Array, Object(APP\core\Request))
#3 /Users/vitalii/Documents/development/ojs-main/ojs/lib/pkp/classes/core/PKPRouter.php(465): call_user_func(Array, Array, Object(APP\core\Request))
#4 /Users/vitalii/Documents/development/ojs-main/ojs/lib/pkp/classes/core/PKPComponentRouter.php(291): PKP\core\PKPRouter->_authorizeInitializeAndCallRequest(Array, Object(APP\core\Request), Array)
#5 /Users/vitalii/Documents/development/ojs-main/ojs/lib/pkp/classes/core/Dispatcher.php(163): PKP\core\PKPComponentRouter->route(Object(APP\core\Request))
#6 /Users/vitalii/Documents/development/ojs-main/ojs/lib/pkp/classes/core/PKPApplication.php(379): PKP\core\Dispatcher->dispatch(Object(APP\core\Request))
#7 /Users/vitalii/Documents/development/ojs-main/ojs/index.php(21): PKP\core\PKPApplication->execute()
#8 {main}
  thrown in /Users/vitalii/Documents/development/ojs-main/ojs/classes/notification/form/NotificationSettingsForm.php on line 27

To Reproduce
Steps to reproduce the behavior:

1. OJS instance should have at least 2 journals installed
2. Log in as an admin user, go to the Administration -> Site settings
3. In the user navigation, click on the Edit Profile
4. Click on the Notifications tab
5. See the error

What application are you using?
OJS main branch. Stable branch also needs testing

Additional information

As it affects only users with an admin role and we don't have site-level notification settings, I'd probably just avoid loading notifications settings in this case, when context isn't available. Another option could be to display the settings for each journal, the same way as it's done for Roles.

PRs regarding issue reopening

• pkp/pkp-lib#8592 Updated field nullability to match the migration I85… #10068
• Bugfix stable 3 4 0 8592 fix notification subscription settings integrity ojs#4321

[NateWr]

Confirmed on main. I tested on stable-3_3_0 and could not reproduce it.

[NateWr]

Looking at stable-3_3_0, it looks like it is possible to save notification settings at the site level using CONTEXT_ID_NONE (0). And it looks like it's possible to request notifications blocked at the site level with PKPNotificationOperationManager::getUserBlockedNotifications(). However, I don't see any notifications created without a context, so the site-level setting is never used.

We now have a foreign key on the notification_subscription_settings table that references the context id, so such values will be invalid in 3.4.

It is only possible to access the user profile at the site level for users who have roles in more than one journal. So if the user lands on this page, it is because they have roles in more than one journal. I think there are two things to do:

1. When viewing the user profile at the site-level, replace the form in the Notifications tab with a list of links to the user's profile in each context they are registered in. Like this:

Manage notifications for each journal you are registered with.

• Journal of Public Knowledge
• Another Journal About Stuff

2. Write a pre-flight check to ensure there are no entries in notification_subscription_settings with invalid context_id.

@asmecher just want to run this plan by you. Does this sound right to you?

[asmecher]

I would also be game to have the notification_subscription_settings.context_id column be nullable in the database -- that's the equivalent to CONTEXT_ID_NONE in SQL-land that doesn't break referential integrity with a made-up ID of 0. But we're talking about hypotheticals, since we don't have a current use case for CONTEXT_ID_NONE in this case (and the notification toolset will eventually get a rewrite), so I'm OK either way.

[NateWr]

That was much easier, so I've done that. The context column in notification_subscription_settings is now nullable, which fixes this bug and allows subscription opt-outs to be saved at the site-wide level.

PRs:
#8600
pkp/ojs#3738
pkp/omp#1317
pkp/ops#453

[NateWr]

Thanks all, merged to main.

[jonasraoni]

I'll reopen this one, the migration to setup the field with null was added (I8592_SiteNotificationSubscriptions), but the migration that creates the table is still the same.

Therefore, the migration will need to be re-run at > 3.4.

[asmecher]

@jonasraoni, actually, could you open a new issue for the new part of this? Since the initial fix already went out in 3.4.0, I'd rather not change the milestone on this to 3.5.0 and lose that history.

[jonasraoni]

I was thinking about it, so I'll proceed 😁