Currently in core there are only two revisionable entity types, Node and BlockContent. There are other issues (such as #2705433: Node should implement RevisionLogInterface) which look at making these implement RevisionLongInterface, because extending RevisionableContentEntityBase could be difficult or API breaking.
This issue will look at converting the following entity types to be revisionable and extend RevisionableContentEntityBase:
- Comment
- Feed (actual feed information)
- File
- Item (Feed item)
- MenuLinkContent
- Message (Contact form message)
- Shortcut
- Term
This first depends on #2707255: All content entities should inherit baseFieldDefinitions to make sure the relevant base fields are added once these entity types are updated.
Note: We are not planning to make the User entity revisionable. Users are a special case in Drupal as they can be both content, and the account that you authenticate against. This means that making them revisionable could result in more complications than are in scope of this issue.
To be done
- Figure out how the data migration should work
Background
Multiversion module currently makes all content entities revisionable by a series of hooks. It however does not make use of RevisionableContentEntityBase.
Comment | File | Size | Author |
---|---|---|---|
#42 | interdiff.txt | 5 KB | timmillwood |
#42 | all_content_entities-2705389-42.patch | 36.88 KB | timmillwood |
Comments
Comment #3
timmillwoodThis doesn't work because many entities baseFieldDefinitions method doesn't inherit fields from the parent, and therefore doesn't get a revision_id field added. Upon fixing that I got the following error:
As I said in the original post, this is no mean feat.
Comment #4
timmillwoodBlocked by #2707255: All content entities should inherit baseFieldDefinitions.
Comment #5
dixon_Comment #6
daffie CreditAttribution: daffie commented#2707255: All content entities should inherit baseFieldDefinitions has landed.
Comment #7
timmillwoodBlocked by #2721313: Upgrade path between revisionable / non-revisionable entities
Comment #8
dixon_Comment #9
timmillwoodThis patch makes add a revision key to all content entities and switches them to use RevisionableContentEntityBase.
It also makes a couple of the fields on Shortcut revisionable.
Comment #11
timmillwoodFixing a bunch of tests.
Comment #13
timmillwoodFixing a bunch more tests.
Comment #15
timmillwoodSwitching Users to being non-revisionable.
Comment #16
timmillwoodComment #18
jojototh CreditAttribution: jojototh at Pfizer, Inc. commentedComment #19
timmillwoodAdding a proof of concept upgrade path solution.
\Drupal\system\EntitySchemaUpdater
service uses the storage schema to generate missing revision tables, andentityDefinitionUpdateManager
to add missing revision fields.Only an update hook for comment module has been added so far, I also need to look at how to copy the data from the non-revision tables.
Comment #20
timmillwoodComment #21
timmillwoodComment #24
timmillwoodWorking towards more passing tests.
Comment #26
timmillwoodMaking fields revisionable seems to fix a load of issues, testing here to see how many.
Comment #28
markdorisonComment #29
timmillwoodVery much a work on progress patch this time with some hacky "fixes" as I try to understand the bugs / issues we're facing.
Comment #31
timmillwoodComment #33
timmillwoodAdding revision_created, revision_user, and revision_log_message fields in update hook.
Also adding making all translatable fields revisionable.
Comment #35
timmillwoodOne thing I can't understand is I am not touching
Node
orBlockContent
, but they are playing a part of theAfter all updates ran, entity schema is up to date.
failure.Comment #36
timmillwoodJust wanted to upload my latest developments.
Comment #38
timmillwoodWell that was full of fail!
Comment #39
timmillwoodThink I'm making progress now.
This patch adds a load of debug stuff that won't be needed in the final version, but just want to show my working.
Comment #42
timmillwoodFixing due to core moving
MenuLinkTreeTest
Comment #44
timmillwoodSwitching this back to postponed. Going to try and get just an upgrade path service working in #2721313: Upgrade path between revisionable / non-revisionable entities, then go back to making things revisionable.
Comment #45
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI know this issue is postponed, but just linking in a related discussion from #2745619: [policy, no patch] Which core entities get revisions?.
In #2745619-2: [policy, no patch] Which core entities get revisions?, the proposal is for comments and files to not be revisionable.
In #2745619-3: [policy, no patch] Which core entities get revisions?, I asked if Items need to be independently revisionable. Or if instead,
fid
needs to be converted from anentity_reference
to anentity_revision_reference
. We don't have the latter in core yet, though perhaps this is a use case that would require it.Comment #48
catchWhen we convert taxonomy terms we're going to have to deal with the issue of taxonomy hierarchy (both the vocabulary drag and drop UI and the parent field on term forms). See the equivalent issue for book module #2858431: Book storage and UI is not revision aware, changes to drafts leak into live site.
Comment #49
amateescu CreditAttribution: amateescu for Pfizer, Inc. commented@catch, like your proposed solutions from #2858431: Book storage and UI is not revision aware, changes to drafts leak into live site, we could choose to do things in two steps for taxonomy terms as well:
1) leave the 'parent' field as non-revisionable in the initial coversion patch/issue and make sure that a draft term can not change its parent
2) work out the complexity of getting entity reference revisions in core :)
Comment #50
timmillwoodThis issue should really depend on #2825973: Introduce a EditorialContentEntityBase class for revisionable and publishable entity types.
Comment #51
timmillwoodMaybe not all content entities.
Comment #52
catch@amateescu I'm not sure how entity reference revisions would work for term hierarchy - if revision A references term 2 and revision B references term 3, the issue is that the reference itself is not versioned with the referencing entity, not with versions of the referenced entity. The main issue with {term_hierarchy} and versioning is that it's not an entity reference at all. We should possibly open a new issue for this, but it'd be PP-4 or PP-5 or something...
Comment #53
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedWe now have dedicated issues for each entity type conversion, so I'm going to close this one out as a duplicate.
#2880149: Convert taxonomy terms to be revisionable
#2880151: Convert shortcuts to be revisionable and publishable
#2880152: Convert custom menu links to be revisionable
#2880154: Convert comments to be revisionable
@catch, The number you were looking for at the time was more like PP-8, but now it got to a more manageable 3 :) Also, I mentioned your concern from #48 in the dedicated issue.