Merge pull request #9680 from hypeJunction/notifications-rel-delete

fix(notifications): users are again unsubscribed when friendship and …

engine/classes/Elgg/EventsService.php

@@ -17,6 +17,16 @@ class EventsService extends \Elgg\HooksRegistrationService {
 	const OPTION_DEPRECATION_MESSAGE = 'deprecation_message';
 	const OPTION_DEPRECATION_VERSION = 'deprecation_version';
 
+	/**
+	 * {@inheritdoc}
+	 */
+	public function registerHandler($name, $type, $callback, $priority = 500) {
+		if (in_array($type, ['member', 'friend', 'member_of_site', 'attached']) && in_array($name, ['create', 'update', 'delete'])) {
+			$this->logger->error("'$name, $type' event is no longer triggered. Update your event registration to use '$name, relationship'");
+		}
+		return parent::registerHandler($name, $type, $callback, $priority);
+	}
+
 	/**
 	 * Triggers an Elgg event.
 	 * 

mod/notifications/start.php

@@ -20,8 +20,7 @@ function notifications_plugin_init() {
 	elgg_unextend_view('forms/account/settings', 'core/settings/account/notifications');
 
 	// update notifications based on relationships changing
-	elgg_register_event_handler('delete', 'member', 'notifications_relationship_remove');
-	elgg_register_event_handler('delete', 'friend', 'notifications_relationship_remove');
+	elgg_register_event_handler('delete', 'relationship', 'notifications_relationship_remove');
 
 	// update notifications when new friend or access collection membership
 	elgg_register_event_handler('create', 'relationship', 'notifications_update_friend_notify');
@@ -30,6 +29,9 @@ function notifications_plugin_init() {
 	$actions_base = __DIR__ . '/actions';
 	elgg_register_action("notificationsettings/save", "$actions_base/save.php");
 	elgg_register_action("notificationsettings/groupsave", "$actions_base/groupsave.php");
+
+	// register unit tests
+	elgg_register_plugin_hook_handler('unit_test', 'system', 'notifications_register_tests');
 }
 
 /**
@@ -106,19 +108,20 @@ function notifications_plugin_pagesetup() {
 /**
  * Update notifications when a relationship is deleted
  *
- * @param string $event
- * @param string $object_type
- * @param object $relationship
+ * @param string            $event        "delete"
+ * @param string            $object_type  "relationship"
+ * @param \ElggRelationship $relationship Relationship obj
+ * @return void
  */
 function notifications_relationship_remove($event, $object_type, $relationship) {
-	$NOTIFICATION_HANDLERS = _elgg_services()->notifications->getMethodsAsDeprecatedGlobal();
-
-	$user_guid = $relationship->guid_one;
-	$object_guid = $relationship->guid_two;
-
-	// loop through all notification types
-	foreach($NOTIFICATION_HANDLERS as $method => $foo) {
-		remove_entity_relationship($user_guid, "notify{$method}", $object_guid);
+
+	if (!in_array($relationship->relationship, ['member', 'friend'])) {
+		return;
+	}
+	
+	$methods = array_keys(_elgg_services()->notifications->getMethodsAsDeprecatedGlobal());
+	foreach($methods as $method) {
+		elgg_remove_subscription($relationship->guid_one, $method, $relationship->guid_two);
 	}
 }
 
@@ -211,3 +214,16 @@ function notifications_update_collection_notify($event, $object_type, $returnval
 		}
 	}
 }
+
+/**
+ * Register unit tests
+ * 
+ * @param string   $hook  "unit_test"
+ * @param string   $type  "system"
+ * @param string[] $tests Tests
+ * @return string[]
+ */
+function notifications_register_tests($hook, $type, $tests) {
+	$tests[] = __DIR__ . '/tests/ElggNotificationsPluginUnitTest.php';
+	return $tests;
+}

mod/notifications/tests/ElggNotificationsPluginUnitTest.php

@@ -0,0 +1,74 @@
+<?php
+
+/**
+ * Notifications plugin tests
+ */
+class ElggNotificationsPluginUnitTest extends ElggCoreUnitTest {
+
+	/**
+	 * @var ElggUser
+	 */
+	private $user1;
+
+	/**
+	 * @var ElggUser
+	 */
+	private $user2;
+
+	/**
+	 * @var ElggUser
+	 */
+	private $group;
+
+	public function setUp() {
+
+		elgg_register_notification_method('test');
+
+		$this->user1 = new ElggUser();
+		$this->user1->username = 'test1';
+		$this->user1->save();
+
+		$this->user2 = new ElggUser();
+		$this->user2->username = 'test2';
+		$this->user2->save();
+
+		$this->group = new ElggGroup();
+		$this->group->save();
+	}
+
+	public function tearDown() {
+
+		elgg_unregister_notification_method('test');
+
+		$this->user1->delete();
+		$this->user2->delete();
+		$this->group->delete();
+	}
+
+	public function testFriendSubscriptionRemovedWhenFriendRelationshipDeleted() {
+
+		$this->user1->addFriend($this->user2->guid);
+		$this->assertTrue($this->user1->isFriendsWith($this->user2->guid));
+
+		elgg_add_subscription($this->user1->guid, 'test', $this->user2->guid);
+		$this->assertIsA(check_entity_relationship($this->user1->guid, 'notifytest', $this->user2->guid), ElggRelationship::class);
+
+		$this->user1->removeFriend($this->user2->guid);
+		$this->assertFalse($this->user1->isFriendsWith($this->user2->guid));
+		$this->assertFalse(check_entity_relationship($this->user1->guid, 'notifytest', $this->user2->guid));
+	}
+
+	public function testGroupSubscriptionRemovedWhenMemberRelationshipRemoved() {
+
+		$this->group->join($this->user1);
+		$this->assertTrue($this->group->isMember($this->user1));
+
+		elgg_add_subscription($this->user1->guid, 'test', $this->group->guid);
+		$this->assertIsA(check_entity_relationship($this->user1->guid, 'notifytest', $this->group->guid), ElggRelationship::class);
+
+		$this->group->leave($this->user1);
+		$this->assertFalse($this->group->isMember($this->user1));
+		$this->assertFalse(check_entity_relationship($this->user1->guid, 'notifytest', $this->user2->guid));
+	}
+
+}