Merge pull request Elgg#14669 from jeabakker/6x-features

feat(relationships): trigger :before and :after events for relations

docs/guides/events-list.rst

@@ -337,13 +337,11 @@ User events
 Relationship events
 ===================
 
-**create, relationship**
-    Triggered after a relationship has been created. Returning false deletes
-    the relationship that was just created.
+**create, relationship** |sequence|
+    Triggered during the creation of a relationship.
 
-**delete, relationship**
-    Triggered before a relationship is deleted. Return false to prevent it
-    from being deleted.
+**delete, relationship** |sequence|
+    Triggered during the deletion of a relationship.
 
 **join, group**
     Triggered after the user ``$params['user']`` has joined the group ``$params['group']``.

engine/classes/Elgg/Database/RelationshipsTable.php

@@ -65,25 +65,22 @@ public function get(int $id): ?\ElggRelationship {
 	/**
 	 * Delete a relationship by its ID
 	 *
-	 * @param int  $id         Relationship ID
-	 * @param bool $call_event Call the delete event before deleting
+	 * @param int $id Relationship ID
 	 *
 	 * @return bool
 	 */
-	public function delete(int $id, bool $call_event = true): bool {
+	public function delete(int $id): bool {
 		$relationship = $this->get($id);
 		if (!$relationship instanceof \ElggRelationship) {
 			return false;
 		}
 
-		if ($call_event && !$this->events->trigger('delete', 'relationship', $relationship)) {
-			return false;
-		}
-
-		$delete = Delete::fromTable(self::TABLE_NAME);
-		$delete->where($delete->compare('id', '=', $id, ELGG_VALUE_ID));
-
-		return (bool) $this->db->deleteData($delete);
+		return $this->events->triggerSequence('delete', 'relationship', $relationship, function() use ($id) {
+			$delete = Delete::fromTable(self::TABLE_NAME);
+			$delete->where($delete->compare('id', '=', $id, ELGG_VALUE_ID));
+
+			return (bool) $this->db->deleteData($delete);
+		});
 	}
 
 	/**
@@ -92,63 +89,63 @@ public function delete(int $id, bool $call_event = true): bool {
 	 * This function lets you make the statement "$guid_one is a $relationship of $guid_two". In the statement,
 	 * $guid_one is the subject of the relationship, $guid_two is the target, and $relationship is the type.
 	 *
-	 * @param int    $guid_one     GUID of the subject entity of the relationship
-	 * @param string $relationship Type of the relationship
-	 * @param int    $guid_two     GUID of the target entity of the relationship
-	 * @param bool   $return_id    Return the ID instead of bool?
+	 * @param \ElggRelationship $relationship the relationship to create
+	 * @param bool              $return_id    Return the ID instead of bool?
 	 *
 	 * @return bool|int
 	 * @throws LengthException
 	 */
-	public function add(int $guid_one, string $relationship, int $guid_two, bool $return_id = false): bool|int {
-		if (strlen($relationship) > self::RELATIONSHIP_COLUMN_LENGTH) {
+	public function add(\ElggRelationship $relationship, bool $return_id = false): bool|int {
+		if (strlen($relationship->relationship) > self::RELATIONSHIP_COLUMN_LENGTH) {
 			throw new LengthException('Relationship name cannot be longer than ' . self::RELATIONSHIP_COLUMN_LENGTH);
 		}
 
 		// Check for duplicates
 		// note: escape $relationship after this call, we don't want to double-escape
-		if ($this->check($guid_one, $relationship, $guid_two)) {
+		if ($this->check($relationship->guid_one, $relationship->relationship, $relationship->guid_two)) {
 			return false;
 		}
 
 		// Check if the related entities exist
-		if (!$this->entities->exists($guid_one) || !$this->entities->exists($guid_two)) {
+		if (!$this->entities->exists($relationship->guid_one) || !$this->entities->exists($relationship->guid_two)) {
 			// one or both of the guids doesn't exist
 			return false;
 		}
 
-		$insert = Insert::intoTable(self::TABLE_NAME);
-		$insert->values([
-			'guid_one' => $insert->param($guid_one, ELGG_VALUE_GUID),
-			'relationship' => $insert->param($relationship, ELGG_VALUE_STRING),
-			'guid_two' => $insert->param($guid_two, ELGG_VALUE_GUID),
-			'time_created' => $insert->param($this->getCurrentTime()->getTimestamp(), ELGG_VALUE_TIMESTAMP),
-		]);
+		$id = 0;
 
-		try {
-			$id = $this->db->insertData($insert);
-			if (!$id) {
-				return false;
-			}
-		} catch (DatabaseException $e) {
-			$prev = $e->getPrevious();
-			if ($prev instanceof UniqueConstraintViolationException) {
-				// duplicate key error see https://github.com/Elgg/Elgg/issues/9179
-				return false;
+		$result = $this->events->triggerSequence('create', 'relationship', $relationship, function (\ElggRelationship $relationship) use (&$id) {
+			$insert = Insert::intoTable(self::TABLE_NAME);
+			$insert->values([
+				'guid_one' => $insert->param($relationship->guid_one, ELGG_VALUE_GUID),
+				'relationship' => $insert->param($relationship->relationship, ELGG_VALUE_STRING),
+				'guid_two' => $insert->param($relationship->guid_two, ELGG_VALUE_GUID),
+				'time_created' => $insert->param($this->getCurrentTime()->getTimestamp(), ELGG_VALUE_TIMESTAMP),
+			]);
+
+			try {
+				$id = $this->db->insertData($insert);
+				if (!$id) {
+					return false;
+				}
+			} catch (DatabaseException $e) {
+				$prev = $e->getPrevious();
+				if ($prev instanceof UniqueConstraintViolationException) {
+					// duplicate key error see https://github.com/Elgg/Elgg/issues/9179
+					return false;
+				}
+
+				throw $e;
 			}
 
-			throw $e;
-		}
-
-		$obj = $this->get($id);
-
-		$result = $this->events->trigger('create', 'relationship', $obj);
+			return true;
+		});
+
 		if (!$result) {
-			$this->delete($id, false);
 			return false;
 		}
-
-		return $return_id ? $obj->id : true;
+		
+		return $return_id ? $id : true;
 	}
 
 	/**
@@ -302,6 +299,10 @@ protected function removeAllWithEvents(int $guid, string $relationship = '', boo
 
 		/* @var $rel \ElggRelationship */
 		foreach ($relationships as $rel) {
+			if (!$this->events->triggerBefore('delete', 'relationship', $rel)) {
+				continue;
+			}
+
 			if (!$this->events->trigger('delete', 'relationship', $rel)) {
 				continue;
 			}
@@ -322,6 +323,15 @@ protected function removeAllWithEvents(int $guid, string $relationship = '', boo
 			$this->db->deleteData($delete);
 		}
 
+		/* @var $rel \ElggRelationship */
+		foreach ($relationships as $rel) {
+			if (!in_array($rel->id, $remove_ids)) {
+				continue;
+			}
+
+			$this->events->triggerAfter('delete', 'relationship', $rel);
+		}
+
 		return true;
 	}
 

engine/classes/Elgg/Notifications/SubscriptionsService.php

@@ -195,7 +195,12 @@ public function addSubscription(int $user_guid, string $method, int $target_guid
 
 		$rel[] = $method;
 
-		return $this->relationshipsTable->add($user_guid, implode(':', $rel), $target_guid);
+		$relationship = new \ElggRelationship();
+		$relationship->guid_one = $user_guid;
+		$relationship->relationship = implode(':', $rel);
+		$relationship->guid_two = $target_guid;
+
+		return $relationship->save();
 	}
 
 	/**
@@ -443,7 +448,7 @@ function(QueryBuilder $qb, $main_alias) use ($methods, $type, $subtype, $action)
 	/**
 	 * Mute notifications about events affecting the target
 	 *
-	 * @param int $user_guid   The GUID of the user to mute notifcations for
+	 * @param int $user_guid   The GUID of the user to mute notifications for
 	 * @param int $target_guid The GUID of the entity to for which to mute notifications
 	 *
 	 * @return bool
@@ -452,7 +457,12 @@ public function muteNotifications(int $user_guid, int $target_guid): bool {
 		// remove all current subscriptions
 		$this->removeSubscriptions($user_guid, $target_guid);
 
-		return $this->relationshipsTable->add($user_guid, self::MUTE_NOTIFICATIONS_RELATIONSHIP, $target_guid);
+		$rel = new \ElggRelationship();
+		$rel->guid_one = $user_guid;
+		$rel->relationship = self::MUTE_NOTIFICATIONS_RELATIONSHIP;
+		$rel->guid_two = $target_guid;
+
+		return $rel->save();
 	}
 
 	/**

engine/classes/ElggEntity.php

@@ -550,7 +550,12 @@ public function setVolatileData(string $name, $value): void {
 	 * @throws \Elgg\Exceptions\LengthException
 	 */
 	public function addRelationship(int $guid_two, string $relationship): bool {
-		return _elgg_services()->relationshipsTable->add($this->guid, $relationship, $guid_two);
+		$rel = new \ElggRelationship();
+		$rel->guid_one = $this->guid;
+		$rel->relationship = $relationship;
+		$rel->guid_two = $guid_two;
+
+		return $rel->save();
 	}
 
 	/**

engine/classes/ElggPlugin.php

@@ -1282,9 +1282,9 @@ protected function setStatus(bool $active): bool {
 
 		$site = elgg_get_site_entity();
 		if ($active) {
-			$result = _elgg_services()->relationshipsTable->add($this->guid, 'active_plugin', $site->guid);
+			$result = $this->addRelationship($site->guid, 'active_plugin');
 		} else {
-			$result = _elgg_services()->relationshipsTable->remove($this->guid, 'active_plugin', $site->guid);
+			$result = $this->removeRelationship($site->guid, 'active_plugin');
 		}
 
 		if ($result) {

engine/classes/ElggRelationship.php

@@ -36,36 +36,34 @@ class ElggRelationship extends \ElggData {
 	 * Holds the original (persisted) attribute values that have been changed but not yet saved.
 	 * @var array
 	 */
-	protected $orig_attributes = [];
+	protected array $orig_attributes = [];
 
 	/**
 	 * Create a relationship object
 	 *
-	 * @param \stdClass $row Database row
+	 * @param null|\stdClass $row Database row
 	 */
-	public function __construct(\stdClass $row) {
+	public function __construct(\stdClass $row = null) {
 		$this->initializeAttributes();
 
-		foreach ((array) $row as $key => $value) {
-			if (!in_array($key, static::PRIMARY_ATTR_NAMES)) {
-				// don't set arbitrary attributes that aren't supported
-				continue;
+		if (!empty($row)) {
+			foreach ((array) $row as $key => $value) {
+				if (!in_array($key, static::PRIMARY_ATTR_NAMES)) {
+					// don't set arbitrary attributes that aren't supported
+					continue;
+				}
+
+				if (in_array($key, static::INTEGER_ATTR_NAMES)) {
+					$value = (int) $value;
+				}
+
+				$this->attributes[$key] = $value;
 			}
-
-			if (in_array($key, static::INTEGER_ATTR_NAMES)) {
-				$value = (int) $value;
-			}
-
-			$this->attributes[$key] = $value;
 		}
 	}
 
 	/**
-	 * (non-PHPdoc)
-	 *
-	 * @see \ElggData::initializeAttributes()
-	 *
-	 * @return void
+	 * {@inheritdoc}
 	 */
 	protected function initializeAttributes() {
 		parent::initializeAttributes();
@@ -81,9 +79,10 @@ protected function initializeAttributes() {
 	 *
 	 * @param string $name  Name
 	 * @param mixed  $value Value
+	 *
 	 * @return void
 	 */
-	public function __set($name, $value) {
+	public function __set(string $name, mixed $value): void {
 		if (in_array($name, static::INTEGER_ATTR_NAMES) && isset($value) && !is_int($value)) {
 			// make sure the new value is an int for the int columns
 			$value = (int) $value;
@@ -116,9 +115,10 @@ public function __set($name, $value) {
 	 * Get an attribute of the relationship
 	 *
 	 * @param string $name Name
+	 *
 	 * @return mixed
 	 */
-	public function __get($name) {
+	public function __get(string $name): mixed {
 		if (array_key_exists($name, $this->attributes)) {
 			return $this->attributes[$name];
 		}
@@ -127,10 +127,10 @@ public function __get($name) {
 	}
 
 	/**
-	 * {@inheritDoc}
+	 * {@inheritdoc}
 	 */
 	public function save(): bool {
-		if (empty($this->orig_attributes)) {
+		if ($this->id > 0 && empty($this->orig_attributes)) {
 			// nothing has changed
 			return true;
 		}
@@ -139,18 +139,13 @@ public function save(): bool {
 			_elgg_services()->relationshipsTable->delete($this->id);
 		}
 
-		$id = _elgg_services()->relationshipsTable->add(
-			$this->guid_one,
-			$this->relationship,
-			$this->guid_two,
-			true
-		);
-
+		$id = _elgg_services()->relationshipsTable->add($this, true);
 		if ($id === false) {
 			return false;
 		}
 
 		$this->attributes['id'] = $id;
+		$this->attributes['time_created'] = _elgg_services()->relationshipsTable->getCurrentTime()->getTimestamp();
 
 		return true;
 	}

engine/tests/classes/Elgg/Mocks/Database/RelationshipsTable.php

@@ -37,15 +37,15 @@ class RelationshipsTable extends DbRelationshipsTable {
 	/**
 	 * {@inheritdoc}
 	 */
-	public function add(int $guid_one, string $relationship, int $guid_two, bool $return_id = false): bool|int {
+	public function add(\ElggRelationship $relationship, bool $return_id = false): bool|int {
 		// Check for duplicates
 		// note: escape $relationship after this call, we don't want to double-escape
-		if ($this->check($guid_one, $relationship, $guid_two)) {
+		if ($this->check($relationship->guid_one, $relationship->relationship, $relationship->guid_two)) {
 			return false;
 		}
 
 		// Check if the related entities exist
-		if (!$this->entities->exists($guid_one) || !$this->entities->exists($guid_two)) {
+		if (!$this->entities->exists($relationship->guid_one) || !$this->entities->exists($relationship->guid_two)) {
 			// one or both of the guids doesn't exist
 			return false;
 		}
@@ -58,17 +58,17 @@ public function add(int $guid_one, string $relationship, int $guid_two, bool $re
 
 		$row = (object) [
 			'id' => $id,
-			'guid_one' => (int) $guid_one,
-			'guid_two' => (int) $guid_two,
-			'relationship' => $relationship,
+			'guid_one' => $relationship->guid_one,
+			'guid_two' => $relationship->guid_two,
+			'relationship' => $relationship->relationship,
 			'time_created' => $this->getCurrentTime()->getTimestamp(),
 		];
 
 		$this->rows[$id] = $row;
 
 		$this->addQuerySpecs($row);
 
-		$result = parent::add($row->guid_one, $row->relationship, $row->guid_two, $return_id);
+		$result = parent::add($relationship, $return_id);
 
 		// reset the time
 		$this->resetCurrentTime();