Improve SoftDelete implementation (#6214)

* Improve SoftDelete implementation

* Update CHANGELOG-DEV.md

* Fix tests of content soft deletion

CHANGELOG-DEV.md

@@ -6,6 +6,7 @@ HumHub Changelog (DEVELOP)
 - Fix #6196: Use class names for default logging targets in default common config
 - Fix #6202: Invite by link is not possible for a user already invited by email
 - Fix #5718: Fix profile field "Country" to use js plugin Select2
+- Enh #6214: Improve SoftDelete implementation
 
 1.14.0-beta.2 (March 28, 2023)
 ------------------------------

protected/humhub/modules/activity/helpers/ActivityHelper.php

@@ -9,8 +9,12 @@ use Yii;
 class ActivityHelper
 {
 
-    public static function deleteActivitiesForRecord(ActiveRecord $record)
+    public static function deleteActivitiesForRecord(?ActiveRecord $record)
     {
+        if ($record === null) {
+            return;
+        }
+
         $pk = $record->getPrimaryKey();
 
         // Check if primary key exists and is not array (multiple pk)
@@ -22,7 +26,8 @@ class ActivityHelper
             ])->each();
 
             foreach ($modelsActivity as $activity) {
-                $activity->delete();
+                /* @var Activity $activity */
+                $activity->hardDelete();
             }
 
             Yii::debug('Deleted activities for ' . get_class($record) . " with PK " . $pk, 'activity');

protected/humhub/modules/content/Events.php

@@ -37,7 +37,7 @@ class Events extends BaseObject
     {
         // Delete user profile content on soft delete
         foreach (Content::findAll(['contentcontainer_id' => $event->user->contentcontainer_id]) as $content) {
-            $content->delete();
+            $content->hardDelete();
         }
     }
 
@@ -50,7 +50,7 @@ class Events extends BaseObject
     {
         $user = $event->sender;
         foreach (Content::findAll(['created_by' => $user->id]) as $content) {
-            $content->delete();
+            $content->hardDelete();
         }
     }
 

protected/humhub/modules/content/components/ContentActiveRecord.php

@@ -8,6 +8,7 @@
 
 namespace humhub\modules\content\components;
 
+use humhub\modules\content\interfaces\SoftDeletable;
 use humhub\modules\content\models\Movable;
 use humhub\modules\content\widgets\stream\StreamEntryWidget;
 use humhub\modules\content\widgets\stream\WallStreamEntryWidget;
@@ -25,6 +26,7 @@ use humhub\components\ActiveRecord;
 use humhub\modules\content\models\Content;
 use humhub\modules\content\interfaces\ContentOwner;
 use yii\base\InvalidConfigException;
+use yii\base\ModelEvent;
 
 /**
  * ContentActiveRecord is the base ActiveRecord [[\yii\db\ActiveRecord]] for Content.
@@ -63,7 +65,7 @@ use yii\base\InvalidConfigException;
  * @property User $owner
  * @author Luke
  */
-class ContentActiveRecord extends ActiveRecord implements ContentOwner, Movable
+class ContentActiveRecord extends ActiveRecord implements ContentOwner, Movable, SoftDeletable
 {
     /**
      * @see StreamEntryWidget
@@ -480,17 +482,51 @@ class ContentActiveRecord extends ActiveRecord implements ContentOwner, Movable
      * Use `hardDelete()` method to delete record immediately.
      *
      * @return bool|int
+     * @inheritdoc
      */
     public function delete()
     {
-        return $this->content->softDelete();
+        return $this->softDelete();
     }
 
     /**
-     * Deletes this content record immediately and permanently
-     *
-     * @return bool
-     * @since 1.14
+     * @inheritdoc
+     */
+    public function beforeSoftDelete(): bool
+    {
+        $event = new ModelEvent();
+        $this->trigger(self::EVENT_BEFORE_SOFT_DELETE, $event);
+
+        return $event->isValid;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function softDelete(): bool
+    {
+        if (!$this->beforeSoftDelete()) {
+            return false;
+        }
+
+        if (!$this->content->softDelete()) {
+            return false;
+        }
+
+        $this->afterSoftDelete();
+        return true;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function afterSoftDelete()
+    {
+        $this->trigger(self::EVENT_AFTER_SOFT_DELETE, new ModelEvent());
+    }
+
+    /**
+     * @inheritdoc
      */
     public function hardDelete(): bool
     {
@@ -498,13 +534,14 @@ class ContentActiveRecord extends ActiveRecord implements ContentOwner, Movable
     }
 
     /**
+     * This method is invoked after HARD deleting a record.
      * @inheritdoc
      */
     public function afterDelete()
     {
         $content = Content::findOne(['object_id' => $this->getPrimaryKey(), 'object_model' => static::getObjectModel()]);
         if ($content !== null) {
-            $content->delete();
+            $content->hardDelete();
         }
 
         parent::afterDelete();

protected/humhub/modules/content/events/ContentEvent.php

@@ -9,9 +9,9 @@
 namespace humhub\modules\content\events;
 
 use humhub\modules\content\models\Content;
-use yii\base\Event;
+use yii\base\ModelEvent;
 
-class ContentEvent extends Event
+class ContentEvent extends ModelEvent
 {
     public Content $content;
 }

protected/humhub/modules/content/interfaces/SoftDeletable.php

@@ -0,0 +1,65 @@
+<?php
+/**
+ * @link https://www.humhub.org/
+ * @copyright Copyright (c) 2023 HumHub GmbH & Co. KG
+ * @license https://www.humhub.com/licences
+ */
+
+namespace humhub\modules\content\interfaces;
+
+/**
+ * Interface for classes which are deletable softly.
+ *
+ * @see \humhub\modules\content\models\Content
+ * @since 1.14
+ */
+interface SoftDeletable
+{
+    /**
+     * @event ModelEvent an event that is triggered before soft deleting a record.
+     * You may set [[ModelEvent::isValid]] to be `false` to stop the deletion.
+     */
+    const EVENT_BEFORE_SOFT_DELETE = 'beforeSoftDelete';
+
+    /**
+     * @event Event an event that is triggered after a record is deleted softly.
+     */
+    const EVENT_AFTER_SOFT_DELETE = 'afterSoftDelete';
+
+    /**
+     * This method is invoked before soft deleting a record.
+     *
+     * The default implementation raises the [[EVENT_BEFORE_SOFT_DELETE]] event.
+     *
+     * @return bool whether the record should be deleted. Defaults to `true`.
+     * @since 1.14
+     */
+    public function beforeSoftDelete(): bool;
+
+    /**
+     * Marks the record as deleted.
+     *
+     * Content which are marked as deleted will not longer returned in queries/stream/search.
+     * A cron job will remove these content permanently.
+     * If installed, such content can also be restored using the `recycle-bin` module.
+     *
+     * @return bool
+     * @since 1.14
+     */
+    public function softDelete(): bool;
+
+    /**
+     * This method is invoked after soft deleting a record.
+     * The default implementation raises the [[EVENT_AFTER_SOFT_DELETE]] event.
+     * @since 1.14
+     */
+    public function afterSoftDelete();
+
+    /**
+     * Deletes this content record immediately and permanently
+     *
+     * @return bool
+     * @since 1.14
+     */
+    public function hardDelete(): bool;
+}

protected/humhub/modules/content/models/Content.php

@@ -21,6 +21,7 @@ use humhub\modules\content\components\ContentContainerModule;
 use humhub\modules\content\events\ContentEvent;
 use humhub\modules\content\events\ContentStateEvent;
 use humhub\modules\content\interfaces\ContentOwner;
+use humhub\modules\content\interfaces\SoftDeletable;
 use humhub\modules\content\live\NewContent;
 use humhub\modules\content\permissions\CreatePrivateContent;
 use humhub\modules\content\permissions\CreatePublicContent;
@@ -80,7 +81,7 @@ use yii\helpers\Url;
  * @mixin GUID
  * @since 0.5
  */
-class Content extends ActiveRecord implements Movable, ContentOwner
+class Content extends ActiveRecord implements Movable, ContentOwner, SoftDeletable
 {
     /**
      * The default stream channel.
@@ -130,11 +131,6 @@ class Content extends ActiveRecord implements Movable, ContentOwner
      */
     public $muteDefaultSocialActivities = false;
 
-    /**
-     * @event Event is used when a Content is soft deleted.
-     */
-    const EVENT_SOFT_DELETE = 'softDelete';
-
     /**
      * @event Event is used when a Content state is changed.
      */
@@ -335,6 +331,18 @@ class Content extends ActiveRecord implements Movable, ContentOwner
             ->about($contentSource)->save();
     }
 
+    /**
+     * Marks this content for deletion (soft delete).
+     * Use `hardDelete()` method to delete a content immediately.
+     *
+     * @return bool
+     * @inheritdoc
+     */
+    public function delete()
+    {
+        return $this->softDelete();
+    }
+
     /**
      * @inheritdoc
      */
@@ -353,6 +361,62 @@ class Content extends ActiveRecord implements Movable, ContentOwner
         parent::afterDelete();
     }
 
+    /**
+     * @inheritdoc
+     */
+    public function beforeSoftDelete(): bool
+    {
+        $event = new ContentEvent(['content' => $this]);
+        $this->trigger(self::EVENT_BEFORE_SOFT_DELETE, $event);
+
+        return $event->isValid;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function softDelete(): bool
+    {
+        if (!$this->beforeSoftDelete()) {
+            return false;
+        }
+
+        ActivityHelper::deleteActivitiesForRecord($this->getModel());
+
+        Notification::deleteAll([
+            'source_class' => get_class($this),
+            'source_pk' => $this->getPrimaryKey(),
+        ]);
+
+        $this->setState(self::STATE_DELETED);
+
+        if (!$this->save()) {
+            return false;
+        }
+
+        $this->afterSoftDelete();
+        return true;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function afterSoftDelete()
+    {
+        $this->trigger(self::EVENT_AFTER_SOFT_DELETE, new ContentEvent(['content' => $this]));
+    }
+
+    /**
+     * Deletes this content immediately and permanently
+     *
+     * @return bool
+     * @since 1.14
+     */
+    public function hardDelete(): bool
+    {
+        return (parent::delete() !== false);
+    }
+
     /**
      * Returns the visibility of the content object
      *
@@ -1002,35 +1066,6 @@ class Content extends ActiveRecord implements Movable, ContentOwner
         return $this->created_at !== $this->updated_at && !empty($this->updated_at) && is_string($this->updated_at);
     }
 
-    /**
-     * Marks the content as deleted.
-     *
-     * Content which are marked as deleted will not longer returned in queries/stream/search.
-     * A cron job will remove these content permanently.
-     * If installed, such content can also be restored using the `recycle-bin` module.
-     *
-     * @return bool
-     * @since 1.14
-     */
-    public function softDelete(): bool
-    {
-        ActivityHelper::deleteActivitiesForRecord($this->getModel());
-
-        Notification::deleteAll([
-            'source_class' => get_class($this),
-            'source_pk' => $this->getPrimaryKey(),
-        ]);
-
-        $this->setState(self::STATE_DELETED);
-
-        if (!$this->save()) {
-            return false;
-        }
-
-        $this->trigger(self::EVENT_SOFT_DELETE, new ContentEvent(['content' => $this]));
-        return true;
-    }
-
     public static function getAllowedStates(): array
     {
         return [

protected/humhub/modules/content/models/forms/AdminDeleteContentForm.php

@@ -59,7 +59,7 @@ class AdminDeleteContentForm extends Model
             return false;
         }
 
-        return (bool) $this->content->softDelete();
+        return $this->content->delete();
     }
 
     public function notify(): bool

protected/humhub/modules/content/tests/codeception/unit/ContentTagTest.php

@@ -179,7 +179,11 @@ class ContentTagTest extends HumHubDbTestCase
         $this->assertEquals(1, ContentTagRelation::find()->count());
 
         $content->delete();
-        $this->assertEquals(0, ContentTagRelation::find()->count());
+        $this->assertEquals(1, Content::find()->where(['id' => 1])->count());
+        $this->assertEquals(0, Content::find()->where(['id' => 1, 'state' => Content::STATE_PUBLISHED])->count());
+
+        $content->hardDelete();
+        $this->assertEquals(0, Content::find()->where(['id' => 1])->count());
 
     }
 

protected/humhub/modules/search/libs/SearchHelper.php

@@ -45,10 +45,10 @@ class SearchHelper extends BaseObject
     /**
      * Queues search index update of an active record
      *
-     * @param ActiveRecord $record
+     * @param ActiveRecord|null $record
      * @return bool
      */
-    public static function queueUpdate(ActiveRecord $record)
+    public static function queueUpdate(?ActiveRecord $record)
     {
         if ($record instanceof Searchable) {
             $pk = $record->getPrimaryKey();
@@ -66,10 +66,10 @@ class SearchHelper extends BaseObject
     /**
      * Queues search index delete of an active record
      *
-     * @param ActiveRecord $record
+     * @param ActiveRecord|null $record
      * @return bool
      */
-    public static function queueDelete(ActiveRecord $record)
+    public static function queueDelete(?ActiveRecord $record)
     {
         if ($record instanceof Searchable) {
             $pk = $record->getPrimaryKey();