Move message notification system to a submodule

Hi
notifications moved to a separate submodule.
Few things to pay attention to:
- not sure what to do with composer dependencies on message and message_notify modules - if we remove them users currently using this module may face issues;
- added update hook to enable a new submodule for all instances.

At some point, we will have to remove dependency from composer.json

@gilmord The hook hook_private_message_notify_exclude doesn't work for me, excluded is always empty. I have created a patch from your PR with changes for the hook functionality.

Hi @Ressinel

So you added "return [1]" to the hook_private_message_notify_exclude and it started working, is that correct?
Patch has 500 lines, can you provide a diff so I can see your changes?

But if this is the only fix - looks like you use this hook the wrong way.
This hook should not return anything, it should modify the variable passed by reference, so you want something like this:
$exclude[] = $some_uid_to_exclude;

Also when you define a hook it should have $exclude argument as a reference.

I tried this scenario:
- implement hook_private_message_notify_exclude in custom module,
- add uids to the $exclude variable
- I can see the added uids in getNotificationRecipients where the hook is invoked

Hi, @gilmord

I used the same scenario as you wrote, but UIDs added to $exclude argument as a reference ($exclude[] = $some_uid_to_exclude;) do not pass to getNotificationRecipients().

Apologize I didn't send the diff earlier. Check it, please:

@@ -39,7 +39,7 @@ new file mode 100644
 index 0000000000000000000000000000000000000000..c3791d1723d75abca93b2ea2b7d6a31c93ec01c4
 --- /dev/null
 +++ b/modules/private_message_notify/private_message_notify.api.php
-@@ -0,0 +1,26 @@
+@@ -0,0 +1,29 @@
 +<?php
 +
 +/**
@@ -60,11 +60,14 @@ index 0000000000000000000000000000000000000000..c3791d1723d75abca93b2ea2b7d6a31c
 + * @param array $exclude
 + *   Array of UIDs to exclude from notifications.
 + *
-+ * @return void
++ * @return array
 + */
 +function hook_private_message_notify_exclude(PrivateMessageInterface $privateMessage, PrivateMessageThreadInterface $thread, array &$exclude) {
 +  // Allow other modules to exclude notifications recipient,
 +  // i.e. - check some "mute" flags.
++  return [
++    1,
++  ];
 +}
 diff --git a/modules/private_message_notify/private_message_notify.info.yml b/modules/private_message_notify/private_message_notify.info.yml
 new file mode 100644
@@ -207,7 +210,7 @@ index 7977b2b5d5bd3b7ec15d861648c942fa39be0bc1..3f3fed96920b790d846d0f3c50393109
 +    $exclude = [];
 +
 +    // Allow other modules to alter notification recipients.
-+    $this->moduleHandler->invokeAll(
++    $exclude = $this->moduleHandler->invokeAll(
 +      'private_message_notify_exclude', [
 +        $message,
 +        $thread,
@@ -223,7 +226,7 @@ index 7977b2b5d5bd3b7ec15d861648c942fa39be0bc1..3f3fed96920b790d846d0f3c50393109
 +      $recipients, static function (AccountInterface $account) use ($exclude) {
 +      // If this user is in the excluded list, filter them from the recipients
 +      // list, so they do not receive the notification.
-+      return !in_array($account, $exclude);
++      return !in_array($account->id(), $exclude);
 +    });
 +  }
 +

Hi again @Ressinel

You are right, nice catch!
$exclude was passed to a hooks in the wrong way. And $account->id() was missing, thanks for that also!

But I still want to keep it a as a reference:

Here is a commit:
https://git.drupalcode.org/project/private_message/-/merge_requests/31/d...

Can you please test if it works for you?
Thanks!

Actually, I am not 100% sure if it should better be a return or variable passed by reference.

Reference allows to alter data, but maybe then the hook should be renamed to "_alter"?
Or we better have two hooks - one to collect excludes and one to alter?

Anyways solution with reference allows us to use it both as a collector and to alter the excludes.

Hi, @gilmord

Yes, with the last changes hook works properly. Thanks!

But I think it would be nice to have the possibility of removing needed $recipients in hook directly instead of adding UIDs. It seems more logical to me.

I'd be happy to merge this but it is a breaking API update.
Let's try to switch the target branch to 3.x

@artem_sylchuk Thank you very much for the merge! This was one of the hot issues at Open Social. Is there any plan to release 3.0 branch?

@navneet0693 the first 3.x release was publised.

Hi, @artem_sylchuk

Thanks for releasing these changes!

But we have faced another issue related to these changes. Take a look at this, please: https://www.drupal.org/project/private_message/issues/3265901