Split out from #2957425: Allow the inline creation of non-reusable Custom Blocks in the layout builder

Problem/Motivation

#2957425: Allow the inline creation of non-reusable Custom Blocks in the layout builder will allow placing of custom non-reusable blocks from within the Layout Builder. These blocks should not be available anywhere outside the layout they are placed within. They should be deleted when the parent entity is deleted. Any non-reusable block that is in a previous revision should not be deleted until the entity using the layout is deleted so that if the entity is reverted to that revision the block will be there.

Because a single entity could have 100's(thousands?) of revisions there may be hundreds of individual non-reusable blocks in previous revisions of the entity when it is deleted.

Trying to delete hundreds of block entities may cause performance issues when deleting the parent entity.

Proposed resolution

Introduce an EntityUsage service. The default implementation of this service will be DatabaseBackendEntityUsage. This usage tracking will be similar to FileUsage service except the entity being tracked could be any entity. Once all the uses of any entity have been removed the usage will be zero. This will allow modules to determine which entities had uses that were removed as opposed to entities that will never tracked.

This service will not delete entities that are no longer used. It will be up to the module using the entity tracking service to delete the entities.

Remaining tasks

Write the patch.

User interface changes

None

API changes

The new service

Data model changes

None

CommentFileSizeAuthor
#5 2976366-5.patch21.23 KBtedbow
#5 interdiff-2-5.txt1.96 KBtedbow
#2 2976366-2.patch21.2 KBtedbow

Comments

tedbow created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Active » Needs review
StatusFileSize
new 2976366-2.patch21.2 KB
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

This is a potentially dicey proposition because "usage" is such an ill-defined concept. File usage got deprecated because it was difficult/impossible to correctly track file usage in CKeditor. Once Media integrates with CKeditor, for example, how can we prevent a similar set of problems from arising? How can we guarantee proper usage tracking? This might be beside the point, but I suggest we bring the framework managers into this issue so that we can cull any lessons learned from the file usage debacle into this new patch/service.

johnwebdev’s picture

johnwebdev commented

#3 Sure, that would be really helpful. +1

This patch would break the contrib. module https://www.drupal.org/project/entity_usage btw
Not sure how Drupal core usually handles these things.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Issue tags: +Needs framework manager review
Related issues: +#2821423: Dealing with unexpected file deletion due to incorrect file usage, +#2835840: Track media usage and present it to the site builder (in the media library, media view, on media deletion confirmation, etc.)
StatusFileSize
new interdiff-2-5.txt1.96 KB
new 2976366-5.patch21.23 KB
  1. #3 yes I think framework managers should look at this, adding the tag.

    I don't see that file usage service actually got deprecated yet. There is #2104229: Deprecate file_usage() but that was only because it was just a call to a service.

    Looks like deprecated the idea of file usage entirely is being discussed as 1 solution to #2821423: Dealing with unexpected file deletion due to incorrect file usage.

    Once Media integrates with CKeditor, for example, how can we prevent a similar set of problems from arising?

    What is provided in the patch is simple usage tracking table it does not delete any entities. I think any code that would use the service would have to be reviewed to determine if could track entities without data lose, proper usage tracking. If it could not then it should not be allowed.

    A few other ideas

    1. Not make a generic entity usage system at all but to add nonreusable_block_content.usage service to Layout Builder(or block_content if ends up providing the functionality. In the inline block usage example in #2957425: Allow the inline creation of non-reusable Custom Blocks in the layout builder these are strictly *non-reusable* so the idea that we need to track their usage anywhere in the system might be overkill. Any module that tried to reuse these block in other ways is going against the express intent of the new code. These inline *non-reusable* blocks are *owned* in a real sense by the layout for the entity they are placed in. We only need to track them because there could hundreds referenced in a single entity because we could have hundreds of revision each with their own *non-reusable* blocks. So I think we could get a timeout if we tried to delete them in hook_entity_delete.

      This is a very different use case from file usage where you want to be able to reuse the same file in multiple ways across the site.

    2. Another option would be to add the class files but have every module create their own service which would provide a different table name argument(already an option) so the tracking would be separated out. You could actually do this with 1) just pass a different table name to nonreusable_block_content.usage

      3. \

    3. Third option, I like this best now would be to do 1) but with a @todo migrate to the new generic entity usage tracking system if and when it is created and is compatible. Just seems weird to make the generic entity tracking when our first use case is expressly for for entity that should not be reused.
  2. Re #4 thanks for pointing this out. I changed the name of the table to core_entity_usage.
    I thought about using system_entity_usage because the table is defined by the system module but this is only because you can't provide tables outside of a module. The service that uses the table is outside of the system module.
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Status: Needs review » Closed (outdated)
Issue tags: -Needs framework manager review
Parent issue: #2957425: Allow the inline creation of non-reusable Custom Blocks in the layout builder »

I talked this over with @samuel.mortenson and we both agreed that because inline blocks are very unique case for entity tracking, in that they will only ever have 1 parent, we should create generic entity tracking for this.

Update #2957425: Allow the inline creation of non-reusable Custom Blocks in the layout builder with simple usage tracking for inline blocks.