IBX-8418: Remove draft when trashing its parent or ancestor location #398
Conversation
barw4
force-pushed
the
ibx-8418-deleting-orphaned-drafts
branch
from
a70054e
to
964cd70
Compare
barw4
force-pushed
the
ibx-8418-deleting-orphaned-drafts
branch
from
964cd70
to
d737438
Compare
There was a problem hiding this comment.
Ok, so I was thinking about something completely different during internal sync.
It's possible to create content items without Locations. Maybe in case of removal of a Location, just node assignment should be removed, keeping drafts "dangling"? These drafts should still be visible on Drafts view.
Just an idea. // POV ping @ibexa/php-dev
|
@alongosz That's a cool idea, however, I think there are too many problems with drafts without node assignment. We cannot publish them using UI and I assume some other things will fail for them as well. |
barw4
force-pushed
the
ibx-8418-deleting-orphaned-drafts
branch
from
f2e6dd0
to
f80cb5c
Compare
barw4
changed the title
src/lib/Persistence/Legacy/Content/Location/Gateway/DoctrineDatabase.php
Outdated
Show resolved
Hide resolved
| { | ||
| $subLocations = $this->locationGateway->getChildren($locationId); | ||
| foreach ($subLocations as $subLocation) { | ||
| $this->deleteChildrenDrafts($subLocation['node_id']); |
There was a problem hiding this comment.
A recursive call? Won't this be a massive performance penalty for deep trees?
There was a problem hiding this comment.
Few lines above we have the same situation and it was there for ages 🤷🏻 : https://github.com/ibexa/core/blob/main/src/lib/Persistence/Legacy/Content/TreeHandler.php#L196
Removing it in a single query may not be easy as this table is tree-like with the only information being of a record's parent.
|
barw4
added
Doc needed
|
@ibexa/design-team as we have this PR ready can we have some idea/design on how to inform a user in the Back Office that during content trashing we are deleting all not-yet published drafts? Perhaps an information in the modal (the one with active relations)? |
|
@barw4 The only thing needed to be change is wording on notification. @juskora propose: Dialog 2
|
|
@NataliaBecla We have to change the wording. Imo we have to specifically mention that only not-yet published drafts (so ones without location) will be removed and not only drafts of this Location - but also drafts of sub-locations. Besides, drafts are removed completely, not trashed. Maybe something like: ping @ibexa/documentation |
|
@NataliaBecla Sending this content item to Trash will also delete all drafts of content items that haven’t beed published yet, and belong to the trashed subtree. |
|
@NataliaBecla @juskora information added in ibexa/admin-ui#1321 and https://github.com/ibexa/content-tree/pull/85 |
Related PRs:
ibexa/admin-ui#1321
https://github.com/ibexa/content-tree/pull/85
Description:
Currently, drafts without a location (so not yet published) that are orphaned due to missing ancestor location are forever stuck in the void. They cause multiple issues in different parts of DXP and they are not easily removable.
This PR makes sure that every time a location is trashed drafts under this given location or its child locations are removed.
Documentation:
Probably required