From b30f675bd289699fc03c87fe861a734eaac0cb7d Mon Sep 17 00:00:00 2001 From: Yuriy Bakhtin Date: Mon, 3 Apr 2023 13:53:56 +0400 Subject: [PATCH] Improve SoftDelete implementation (#6214) * Improve SoftDelete implementation * Update CHANGELOG-DEV.md * Fix tests of content soft deletion --- CHANGELOG-DEV.md | 1 + .../activity/helpers/ActivityHelper.php | 9 +- protected/humhub/modules/content/Events.php | 4 +- .../components/ContentActiveRecord.php | 51 +++++++-- .../modules/content/events/ContentEvent.php | 4 +- .../content/interfaces/SoftDeletable.php | 65 +++++++++++ .../humhub/modules/content/models/Content.php | 105 ++++++++++++------ .../models/forms/AdminDeleteContentForm.php | 2 +- .../tests/codeception/unit/ContentTagTest.php | 6 +- .../modules/search/libs/SearchHelper.php | 8 +- 10 files changed, 201 insertions(+), 54 deletions(-) create mode 100644 protected/humhub/modules/content/interfaces/SoftDeletable.php diff --git a/CHANGELOG-DEV.md b/CHANGELOG-DEV.md index d4850f502c..2f0e902a6a 100644 --- a/CHANGELOG-DEV.md +++ b/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) ------------------------------ diff --git a/protected/humhub/modules/activity/helpers/ActivityHelper.php b/protected/humhub/modules/activity/helpers/ActivityHelper.php index b55b4b69e7..42a8de06ff 100644 --- a/protected/humhub/modules/activity/helpers/ActivityHelper.php +++ b/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'); diff --git a/protected/humhub/modules/content/Events.php b/protected/humhub/modules/content/Events.php index 270e01a6d2..88b5e43c76 100644 --- a/protected/humhub/modules/content/Events.php +++ b/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(); } } diff --git a/protected/humhub/modules/content/components/ContentActiveRecord.php b/protected/humhub/modules/content/components/ContentActiveRecord.php index 8488d19b82..62da713f99 100644 --- a/protected/humhub/modules/content/components/ContentActiveRecord.php +++ b/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(); diff --git a/protected/humhub/modules/content/events/ContentEvent.php b/protected/humhub/modules/content/events/ContentEvent.php index 8e1eba5bd8..efe8e238f1 100644 --- a/protected/humhub/modules/content/events/ContentEvent.php +++ b/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; } diff --git a/protected/humhub/modules/content/interfaces/SoftDeletable.php b/protected/humhub/modules/content/interfaces/SoftDeletable.php new file mode 100644 index 0000000000..8708d8a99a --- /dev/null +++ b/protected/humhub/modules/content/interfaces/SoftDeletable.php @@ -0,0 +1,65 @@ +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 [ diff --git a/protected/humhub/modules/content/models/forms/AdminDeleteContentForm.php b/protected/humhub/modules/content/models/forms/AdminDeleteContentForm.php index 9d9070fdad..ee5b5f2307 100644 --- a/protected/humhub/modules/content/models/forms/AdminDeleteContentForm.php +++ b/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 diff --git a/protected/humhub/modules/content/tests/codeception/unit/ContentTagTest.php b/protected/humhub/modules/content/tests/codeception/unit/ContentTagTest.php index 6b92b4d6a0..3244a91c89 100644 --- a/protected/humhub/modules/content/tests/codeception/unit/ContentTagTest.php +++ b/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()); } diff --git a/protected/humhub/modules/search/libs/SearchHelper.php b/protected/humhub/modules/search/libs/SearchHelper.php index 4ccc7116e5..0d02ec8b20 100644 --- a/protected/humhub/modules/search/libs/SearchHelper.php +++ b/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();