News

I hope we get the stable version as soon as possible.

Just now tested this beta version which throws the error `Could not save content without created_by!` when using the command `integrity/run`; ### Error Stack ``` #0 /home/USER/SITE/protected/vendor/yiisoft/yii2/db/ActiveRecord.php(600): humhub\modules\content\models\Content->beforeSave() #1 /home/USER/SITE/protected/vendor/yiisoft/yii2/db/ActiveRecord.php(570): yii\db\ActiveRecord->insertInternal() #2 /home/USER/SITE/protected/vendor/yiisoft/yii2/db/BaseActiveRecord.php(676): yii\db\ActiveRecord->insert() #3 /home/USER/SITE/protected/humhub/modules/content/models/Content.php(1026): yii\db\BaseActiveRecord->save() #4 /home/USER/SITE/protected/humhub/modules/content/components/ContentActiveRecord.php(486): humhub\modules\content\models\Content->softDelete() #5 /home/USER/SITE/protected/humhub/modules/post/Events.php(32): humhub\modules\content\components\ContentActiveRecord->delete() #6 [internal function]: humhub\modules\post\Events::onIntegrityCheck() #7 /home/USER/SITE/protected/vendor/yiisoft/yii2/base/Event.php(312): call_user_func() #8 /home/USER/SITE/protected/vendor/yiisoft/yii2/base/Component.php(642): yii\base\Event::trigger() #9 /home/USER/SITE/protected/humhub/commands/IntegrityController.php(39): yii\base\Component->trigger() #10 [internal function]: humhub\commands\IntegrityController->actionRun() #11 /home/USER/SITE/protected/vendor/yiisoft/yii2/base/InlineAction.php(57): call_user_func_array() #12 /home/USER/SITE/protected/vendor/yiisoft/yii2/base/Controller.php(178): yii\base\InlineAction->runWithParams() #13 /home/USER/SITE/protected/vendor/yiisoft/yii2/console/Controller.php(180): yii\base\Controller->runAction() #14 /home/USER/SITE/protected/vendor/yiisoft/yii2/base/Module.php(552): yii\console\Controller->runAction() #15 /home/USER/SITE/protected/vendor/yiisoft/yii2/console/Application.php(180): yii\base\Module->runAction() #16 /home/USER/SITE/protected/vendor/yiisoft/yii2/console/Application.php(147): yii\console\Application->runAction() #17 /home/USER/SITE/protected/vendor/yiisoft/yii2/base/Application.php(384): yii\console\Application->handleRequest() #18 /home/USER/SITE/protected/yii(29): yii\base\Application->run() #19 {main} ``` To fix this for my case I replaced the `beforeSave` with the master branch and the command worked as expected for deleting broken content; [https://github.com/humhub/humhub/blob/master/protected/humhub/modules/content/models/Content.php#L188-L218](https://github.com/humhub/humhub/blob/master/protected/humhub/modules/content/models/Content.php#L188-L218) | Q | A | | - | - | | HumHub version | 1.14-beta.2 | | PHP version | 8.1.16 | | Operating system | CloudLinux/CentOS |

The [module migration guide](https://docs.humhub.org/docs/develop/modules-migrate/#version-114) states that as of version 1.14 the “new [content soft delete](https://docs.humhub.org/docs/develop/content#delete-content) mechanism” should be used. If that’s the case, why was it opted to introduce the `softDelete()`, which should be the new default, rather than the `hardDelete()`, to explicitly permanently delete content? Would it not have been better to replace `delete()` with `softDelete()` under the hood, and introduce a `hardDelete()` if someone insists in the old mechanism? Because now one need to implement the `softDelete` all over the place, breaking the backwards-compatibility of the module (since the function does not exist in old version or distinguishing between versions <1.14 and >= 1.14), while potentially few `hardDeletes` are actually required? I’m sure there it makes sense but I currently don’t see it. :wink:

Yes we have implemented this behavior that way: [https://github.com/humhub/humhub/blob/develop/protected/humhub/modules/content/components/ContentActiveRecord.php#L484-L487](https://github.com/humhub/humhub/blob/develop/protected/humhub/modules/content/components/ContentActiveRecord.php#L484-L487) ``` $post->delete(); // Soft Delete # or $post->content->softDelete(); ```

Thank you. So on the `ContentActiveRecord` the following is true: ``` ContentActiveRecord::delete() // soft delete ContentActiveRecord::hardDelete() // hard delete ``` Whereas on the `Content` it is different: ``` Content::delete() // hard delete Content::softDelete() // soft delete ``` Yet in both cases, * a **hard delete** means:\ really deleting both records from `content` table, as well as the the `ContentActiveRecord::tableName()`, as the `afterDelete()` method of one class would delete the record of the other class too. * a **soft delete** means:\ not deleting any of the above records, but just setting the field `status` to `Content::STATE_DELETED`. Also, `beforeDelete()` is not called and hence the deleting cannot be nilled. Neither is the `afterDelete()` called. Therefore also `BaseActiveRecord::EVENT_BEFORE_DELETE` and `BaseActiveRecord::EVENT_AFTER_DELETE` do not fire on a soft delete. Instead one would have to listen to `Content::EVENT_SOFT_DELETE`. * in addition, **both hard and soft delete** really delete the related records in `activity` and `notification`. I would expect, that * `delete()` means the same thing, both on the `ContentActiveRecord` as well as on the `Content`, specifically as hard deleting one of them, means also deleting the the other. * *soft delete* would be restorable, including activities and notifications. * soft delete would still call the `EVENT_BEFORE_DELETE` and `EVENT_AFTER_DELETE` events, as “logically” the record is gone. Maybe an additional parameter to the given events could be added, stating *soft* or *hard*. I do think the side-effects of this change is massive. But may it be that I don’t entirely understand the concept of the *soft delete*?

[Martin Rüegg](mention:bde05d5e-edf4-4a43-a9ff-f2c1a41ba4f6 "/u/metaworx/") It really makes sense to adjust the `delete` methods in the `Content` model as well, so that they behave the same as in the `ContentActiveRecord`. I’ll created another issue for this. However: * That the activities/notifications are also deleted with a SoftDelete is currently intended. But we could improve this in the future releases. * The events `EVENT_AFTER/BEFORE_DELETE` come from the Yii Framework and I would not like to modify them. They are also often used for cleanups (e.g. All comments of a content). So we need to be careful with this event.

Btw: I am personally not affected by these changes. I’m just pointing to potential compatibility issues. I guess streamlining the function names and what they do is a great approach. Particularly as relying on the `delete()` function, rather than the `softDelete()` makes it much easier for module developers to keep backwards-compatibility upright. I also understand that you don’t want to fire `EVENT_AFTER/BEFORE_DELETE` events, as they indicate something different. However, I would suggest to rename the current `EVENT_SOFT_DELETE` to `EVENT_AFTER_SOFT_DELETE`. And also introduce a `EVENT_BEFORE_SOFT_DELETE` allowing to set `$isValid=false` in order to abort the soft delete as [with the original event](https://www.yiiframework.com/doc/api/2.0/yii-db-baseactiverecord#EVENT_BEFORE_DELETE-detail). Furthermore I’d suggest to update the [module migration guide](https://docs.humhub.org/docs/develop/modules-migrate/#version-114) stating about the “change” of these events.

Thanks for the feedback. These are valid points and help to improve the feature. We’ll try to integrate some of this into the release.