Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Neither node.vid, nor entity_test_rev.revision_id is defined as revisionable. This is IMO a bug.
Also, the setRevisionable
method is not on any interface.
Proposed resolution
Enforce in EntityManager much like we enforce default_langcode. For now, we do this only for BaseFieldDefinition instances but that should be enough.
We do not resolve the interface in this issue.
Comment | File | Size | Author |
---|---|---|---|
#19 | 2469533_19.patch | 5.06 KB | chx |
Comments
Comment #1
fagoyes, revision id should be revisionable as it's stored by revision. I remember I already filed an issue for that, but I cannot find it right now :/
Regarding the missing setters in field definition interface: Most things just need read access, but sometimes we have a need a setters. But this is a general problem and does not only apply for revisionable. We should stick with what we have here to keep things consistent and solve this over at #2346329: hook_entity_base_field_info_alter() and hook_entity_bundle_field_info_alter() are documented to get a parameter that doesn't implement an interface with setter methods.
Comment #2
chx CreditAttribution: chx at MongoDB commentedComment #3
chx CreditAttribution: chx at MongoDB commentedWrong patch.
Comment #6
chx CreditAttribution: chx at MongoDB commentedThat's fun: we have a logicexception if the revision_id is revisionable...
Comment #7
chx CreditAttribution: chx at MongoDB commentedComment #10
chx CreditAttribution: chx at MongoDB commentedThanks @plach for pointing me to the right place.
Comment #11
chx CreditAttribution: chx at MongoDB commentedComment #12
plachLooks good
Comment #13
jibranIt's a bug we need tests here I think.
Comment #14
webchickAgreed. We don't want to accidentally make it not revisionable again.
Comment #15
plachThere is plenty of implicit test coverage if we just turn revisionability to false on the definition.
Comment #16
jibranSo can we have fail patch?
Comment #17
chx CreditAttribution: chx at MongoDB commentedComment #19
chx CreditAttribution: chx at MongoDB commentedComment #21
jibran@chx: https://twitter.com/webchick/status/573274036093902848 :D
Comment #22
chx CreditAttribution: chx at MongoDB commentedI will when it makes sense. Not yet.
Comment #35
quietone CreditAttribution: quietone at PreviousNext commentedI asked about this in #bugsmash and acbramley and catch agree this is a duplicate of #2975307: The revision ID field should be flagged as revisionable and moving credit.