Move entity revision, content translation, validation and field methods to ContentEntityInterface

All I did was remove the views test's expectations for 'setNewRevision' and 'isDefaultRevision', since they aren't on the interface it's testing anymore.

+++ b/core/lib/Drupal/Core/Entity/BundleableInterface.php
@@ -0,0 +1,32 @@
+ /**
+ * Provides methods for an entity to support bundles.
+ */
+interface BundleableInterface {
+  public function bundle();
...
+  public function isTranslatable();
+}

I think this interface makes no sense. We should just move bundle() to ContentEntityInterface and isTranslatable() to TranslatableInterface.

The other two interfaces are great though! :)

Patch looks good to me.

+++ b/core/lib/Drupal/Core/Entity/BundleableInterface.php
@@ -0,0 +1,32 @@
+  /**
+   * Returns the translation support status.
+   *
+   * @return bool
+   *   TRUE if the entity bundle has translation support enabled.
+   */

This sounds like it should not be here at all. Anyway, that's another issue.

I think this interface makes no sense. We should just move bundle() to ContentEntityInterface and isTranslatable() to TranslatableInterface.

As discussed with tim, I think we should either keep BundleableInterface or keep it in EntityInterface. If it's baked into ContentEntityInterface you cannot decide to start using bundles for something else later on without breaking APIs. Any yes, use-cases for that could come up as we do not know what functionality will be provided to bundles (e.g. language module already does stuff with bundles).

Then my vote goes to keeping it in EntityInterface.

Ok, here's a reroll.

- Moved bundle() back, moved isTranslatable() to TranslatableInterface
- Added abstract ContentEntityBase and moved related properties and methods there. This currently extends from EntityNG, this means that it needs to be duplicated in MenuLink right now.
- also moved preSaveRevision() to RevisionableInterface.
- Changed content_translation to check for the ContentEntityInterface. We discussed if it should be that or TranslatableInterface, going to this now as it's *content*_translation.
- that also requires us to make users content.

Let's see if this works :)

Overall this is looking good, we will need to make sure that type hints are the most specific wherever possibile as otherwise IDE's code completion might not show the content entity methods. Btw, I think I said @eff referring to entities with revision support as "Revisable", which seems to be more common, at least in Google result pages. I don't have a strong preference though.

Ok,so EntityInterface needs a language() methods. Which is fine, as all entities have a language, also config entities, but they're not translatable( they just get the current translation magically injected from inside :p)

Added language() to views UI, removed revision test code form ConfigEntityTest.

Just a re-roll for now, haven't been able to reproduce the upgrade path fails, maybe this fixes it?

Patch would help.

Most of the fails are because I broke the entity form controller.

Maybe there should be a subclass of the form controller that is specific for config entities, or one for the other way round, but this at lest fixed the first test.

This should fix the translation failures.

This is the patch from #17 with the interdiff from #19.

Thanks @plach!

This one should fix the latest failures. The problem is that the EntityBCDecorator is not implementin the ContentEntityInterface, which I believe it should. Anyway, since I'm not sure and I'd like to avoid breaking more stuff, I just provided the minimal fix needed to make tests pass.

#22: interfaces-2004244-22.patch queued for re-testing.

@tim.plunkett also moved the complex data interface down to ContentEntityInterface. I basically agree with that, but might require non-trivial changes, and this seems already a big enough step for a single issue, we agreed on tackling that in a follow-up issue.

Discussed with Dries, effulgentsia and xjm. Raising to critical as this is necessary in order to remove the BC layer from content entities.

#27: entity-2004244-27.patch queued for re-testing.

Re-roll.

Just experimenting a bit, ignore this for now.

More experimenting and re-roll.

This should fix most tests, no idea yet what's up with the upgrade path tests.

Re-roll.

Trying to make sense of entity form controllers, more EntityNG removals/replacements.

Isn't that a great interdiff ;) That's what you get when renaming classes with search and replace.

#42: entity-config-ng-interfaces-2004244-42.patch queued for re-testing.

Re-roll.

Forgot to update NodeFormController.

Re-roll and some additional changes.

The main part of this patch is moving methods and interfaces that are currently on EntityInterface to either ConfigEntityInterface or ContentEntityInterface. That means that the revision/content translation/validation/field API's are only available for content enties.

Will write this down in more details in the issue summary, currently, this results in the following class structure:

See also the attached version that only contains the base interfaces including their methods.

Additionally, I'm moving everything that's related to translation and validation from the default EntityFormController to a ContentEntityFormController.

Patch missing.

Ups.

1. +++ b/core/lib/Drupal/Core/Entity/ContentEntityBase.php
   @@ -168,6 +181,142 @@ public function __construct(array $values, $entity_type, $bundle = FALSE, $trans
   ...
   +   * Implements \Drupal\Core\TypedData\TypedDataInterface::setValue().
   ...
   +   * Implements \Drupal\Core\TypedData\TypedDataInterface::getName().
   ...
   +   * Implements \Drupal\Core\TypedData\TypedDataInterface::getRoot().
   ...
   +   * Implements \Drupal\Core\TypedData\TypedDataInterface::getPropertyPath().
   ...
   +   * Implements \Drupal\Core\TypedData\TypedDataInterface::getParent().
   ...
   +   * Implements \Drupal\Core\TypedData\TypedDataInterface::setContext().
   
   +++ b/core/lib/Drupal/Core/Entity/ContentEntityFormController.php
   @@ -0,0 +1,162 @@
   +   * Overrides EntityFormController::form().
   ...
   +   * Implements \Drupal\Core\Entity\EntityFormControllerInterface::validate().
   ...
   +   * Implements \Drupal\Core\Entity\EntityFormControllerInterface::getFormLangcode().
   ...
   +   * Implements \Drupal\Core\Entity\EntityFormControllerInterface::isDefaultFormLangcode().
   ...
   +   * Overrides EntityFormController::buildEntity().
   
   +++ b/core/modules/menu_link/lib/Drupal/menu_link/Entity/MenuLink.php
   @@ -271,6 +272,70 @@ class MenuLink extends Entity implements \ArrayAccess, MenuLinkInterface {
   +   * Implements \Drupal\Core\TypedData\TranslatableInterface::language().
   

   {@inheritdoc}

2. +++ b/core/lib/Drupal/Core/Entity/ContentEntityFormController.php
   @@ -0,0 +1,162 @@
   + * Contains \Drupal\Core\Entity\EntityFormControllerNG.
   

   ContentEntityFormController

3. +++ b/core/lib/Drupal/Core/Entity/Entity.php
   @@ -587,58 +335,17 @@ public static function postLoad(EntityStorageControllerInterface $storage_contro
   -   * {@inheritdoc}
   +   * Implements \Drupal\Core\Entity\EntityInterface::getBCEntity().
   ...
   -   * {@inheritdoc}
   +   * Implements \Drupal\Core\Entity\EntityInterface::getNGEntity().
   

   {@inheritdoc}. Can we add @todo here why we still need this? I really don't know.

4. +++ b/core/lib/Drupal/Core/Entity/EntityBCDecorator.php
   @@ -135,9 +135,9 @@ public function &__get($name) {
   +      // always keys/ default language values with Language::LANGCODE_DEFAULT
   
   @@ -168,9 +168,9 @@ public function __set($name, $value) {
   +        // values/ with Language::LANGCODE_DEFAULT while field API expects them
   

   extra / in the comment.

5. +++ b/core/modules/field/field.deprecated.inc
   @@ -822,11 +822,7 @@ function field_attach_load_revision($entity_type, $entities, $options = array())
   +function field_attach_form_validate(ContentEntityInterface $entity, $form, &$form_state, array $options = array()) {
   

   $form and $form_state should be typhinted as array.

Updating title as per #50.

Will need a reroll after #2056405: Let field types provide their support for "default values" and associated input UI - patch over there changed FieldInstanceEditForm::getFieldItem() to getFieldItem*s*(), because we now also call methods that live on the Field ('list_class') object.

Re-roll, not sure if this is valid, we'll see :)

@jibran: Most methods are just copied/moved around from one class to another, not finished with that yet. Will get to those sooner or later :)

One of the remaining question marks for me are the set() and get() methods. ConfigEntityBase() already implments something that has nothing with the actual definition of it (It's supposed to return a FieldInterface object), there's no way to define it in a way that works for both of them but EntityFormController (and possibly others places too), rely on it.

@@ -7,9 +7,39 @@
+  /**
+   * Marks the translation identified by the given language code as existing.
+   *
+   * @todo Remove this as soon as translation metadata have been converted to
+   *    regular fields.
+   *
+   * @param string $langcode
+   *   The language code identifying the translation to be initialized.
+   */
+  public function initTranslation($langcode);
+

Can we add a @deprecated here and move the @todo below the @param?

@@ -351,6 +243,11 @@ public function getTranslationLanguages($include_default = TRUE) {
+  public function getTranslation($langcode) { }

Why are we restoring this on Entity?

@@ -587,58 +340,26 @@ public static function postLoad(EntityStorageControllerInterface $storage_contro
+   * Implements \Drupal\Core\TypedData\ComplexDataInterface::getIterator().

{@inheritdoc}

@@ -587,58 +340,26 @@ public static function postLoad(EntityStorageControllerInterface $storage_contro
+    // @todo: Replace by EntityNG implementation once all entity types have been

This should refer to ContentEntityBase.

@@ -165,9 +162,15 @@ public function form(array $form, array &$form_state) {
+        '#value' => $langcode,

What's wrong with the ternary? ;)

@@ -376,28 +333,24 @@ public function getFormLangcode(array $form_state) {
+    if (!empty($langcode))  {
+      return $langcode;
+    }
+    else {
+      // Fall back to the current language of the entity.
+      return $entity->language()->id;
+    }

If we don't support entity translations, then the form language should always match the entity language, otherwise we would be editing something that we cannot save.

@@ -5,11 +5,13 @@
-use Drupal\Core\Language\Language;
-use Drupal\Core\Session\AccountInterface;
+use Drupal\Core\Entity\ContentEntityInterface;
 use Drupal\Core\Entity\EntityFormControllerInterface;
 use Drupal\Core\Entity\EntityInterface;
-use Drupal\Core\Entity\EntityNG;
+use Drupal\Core\Entity\ContentEntityBase;
+use Drupal\Core\Language\Language;
+use Drupal\Core\Session\AccountInterface;
+use Drupal\Core\TypedData\TranslatableInterface;
 

ContentEntityBase should be the first one here.

#2077313: Remove unecessary BC decorator code from edit.module should hopefully get rid of the edit related test exception. The other one might be fixed by @plach's suggestion or might might be an invalid test now.

Tagging for rocketship focus

#57: entity-config-ng-interfaces-2004244-57.patch queued for re-testing.

@plach (If you update dreditor, it will automatically number your reviews)

1. AFAIK, we usually don't add @deprecated if we don't have a replacement to point to. Moved the @todo down for now.

2. I had to keep getTranslation() on Entity because there are some places that use it to identify Non-NG entities (it's not one if it doesn't return something), see interdiff. Replaced with an interface check instead and also removed BC code there, can't get that far now. Yes, this patch is getting big, might be able to split it up if necessary. This might explode in a few places now. Let's hope not too many. Also removed getTranslationLanguages() and a todo on language(), that will stay, possibly needs a cache (99% of our language() calls are language()->id, that's stupid, we need a langcode() method.

3. Haven't found that specific Implements, but yes, there are a few left. Will clean them up.

4. set() and get() need some sort of solution, leaving the EntityNG there as sort of a reminder. This probably needs to move down to config, content entities have a completely different implementation. But the default form controller uses it, move that to a config form controller? Not sure :(

5. Nothing wrong with the ternary. That was temporarily more complex as it also contained interface checks. That got moved down, restored the original code here.

6. Ok, changed to a single line that return the entity language. yep, another language()->id ;)

7. Not changed yet, those uses were automatically renamed by phpstorm, not really looking forward to search all of them and put the min the right order :)

Also fixed the broken installer, which ws due to edit.module and removed some more code in ViewUI.

we need a langcode() method.

Totally. And we probably need that also for getTranslationLanguages(): in most cases we instantiate language objects just to throw them away in the consumer code.

re: langcode() method
Funny, #2078625: Field/FieldItem value objects should carry their language is precisely discussing why the method added to Field/FieldItem returns a langcode rather than a Language.
Although, we also need a setter over there, so it's currently setLangcode() / getLangcode(). If we could get consistent names on Entity and Field...

#64: entity-config-ng-interfaces-2004244-64.patch queued for re-testing.

Re-roll.

Ah, forgot the interface check on the delete hooks. And, I don't want to know why the hell we *delete* menu links during installation :)

Wrong interdiff.

#72: entity-config-ng-interfaces-2004244-72.patch queued for re-testing.

Another re-roll.

Ups, reverted too much there, removed the getUntranslated() call there.

This fixes the new confirm forms, not yet sure about the other fails.

This fixes the tests, the field tests were because the bc decorator, that is still used in a few places, wasn't implementing the content entity interface. And the entity translation form test fail was actually valid.

Two things left to clean up as far as I know:

- DatabaseStorageControllerNG: We now have a pretty weird class hierarchy of FieldableEntityStorageControllerBase > DatabaseStorageController > DatabaseStorageController, but with this, only CotentEntities/NG entities can actually have fields. FieldableEntityStorageControllerBase should really be a trait, then we could remove all field stuff from DatabaseStorageController but that doesn't work now :(

- The set/get stuff, see above.

Conflicted with a remove unused variable patch.. not sure it's the right phase for those :(

with this, only CotentEntities/NG entities can actually have fields

Hm, which other kind of entities would we want to have fields ?

1. +++ b/core/includes/entity.api.php
   @@ -171,9 +171,7 @@ function hook_entity_info_alter(&$entity_info) {
   -  // @todo Remove the check for EntityNG once all entity types have been
   -  //   converted to it.
   -  if (!isset($entity->foo) && ($entity instanceof \Drupal\Core\Entity\EntityNG)) {
   +  if ($entity instanceof \Drupal\Core\Entity\ContentEntityBase && !$entity->foo->value) {
        $entity->foo->value = 'some_initial_value';
   

   Whay are we checking a base class?

2. +++ b/core/lib/Drupal/Core/Entity/ContentEntityBase.php
   @@ -168,6 +181,142 @@ public function __construct(array $values, $entity_type, $bundle = FALSE, $trans
   +  /**
   +   * Implements \Drupal\Core\TypedData\TypedDataInterface::setValue().
   +   */
   +  public function setValue($value, $notify = TRUE) {
   

   Lots more {@inheritdoc}s.

3. +++ b/core/lib/Drupal/Core/Entity/ContentEntityFormController.php
   @@ -0,0 +1,167 @@
   +/**
   + * @file
   + * Contains \Drupal\Core\Entity\EntityFormControllerNG.
   + */
   

   No it doesn't :)

4. +++ b/core/lib/Drupal/Core/Entity/EntityStorageControllerBase.php
   @@ -257,6 +207,10 @@ protected function invokeHook($hook, EntityInterface $entity) {
   +    // Only act on content entities.
   +    if (!($entity instanceof ContentEntityInterface)) {
   +      return;
   +    }
   

   Seems like we should be throwing an InvalidArgumentException for some of these.

5. +++ b/core/modules/contact/lib/Drupal/contact/Entity/Message.php
   @@ -36,7 +36,7 @@
   -class Message extends EntityNG implements MessageInterface {
   +class Message extends ContentEntityBase implements MessageInterface {
   
   +++ b/core/modules/user/lib/Drupal/user/Entity/User.php
   @@ -47,7 +47,7 @@
   -class User extends EntityNG implements UserInterface {
   +class User extends ContentEntityBase implements UserInterface {
   

   Yay, now these implement ContentEntityInterface.

6. +++ b/core/modules/content_translation/lib/Drupal/content_translation/Tests/ContentTranslationUITest.php
   @@ -260,7 +260,7 @@ protected function getFormSubmitAction(EntityInterface $entity) {
   -    return $entity instanceof EntityNG ? $entity->getTranslation($langcode) : $entity;
   +    return $entity instanceof ContentEntityBase ? $entity->getTranslation($langcode) : $entity;
      }
   
   @@ -279,7 +279,7 @@ protected function getTranslation(EntityInterface $entity, $langcode) {
   -    if (($translation instanceof EntityInterface) && !($translation instanceof EntityNG) && !($translation instanceof EntityBCDecorator)) {
   +    if (($translation instanceof EntityInterface) && !($translation instanceof ContentEntityBase) && !($translation instanceof EntityBCDecorator)) {
   

   Why are we checking base classes?

Overall this is awesome.

I would prefer if ContentEntityInterface didn't extend TranslatableInterface, and RevisionableInterface to make things a bit more composable, but I get it. It would be nice for code check for them specifically when it can.

It seems like ContentEntityInterface now means FieldableEntityInterface :(

Re-roll after EntityBCDecorator went to where it belongs...

Switched to instanceof the interfaces, some @inheritdocs.

Still left to do: make sense of get()/set(), storage controller and write an issue summary, see also #1983554-79: Remove BC-mode from EntityNG .

That should fix those failures.

- Also attempted to fix get()/set(), moved the methods down to ConfigEntityBase and defined them like that on ConfigEntityInterface. That means that EntityFormController will only for config entities, so I expect editing menu links to fail..
- Also found an additional method to move down to ContentEntityBase (isDefaultRevision()), that was overlooked before.

FieldableEntityStorageControllerBase should really be a trait, then we could remove all field stuff from DatabaseStorageController but that doesn't work now :(

Yes - that's why have been suggesting/discussing with chx to keep its code in a separate helper class, so you can easily add it in (internally) when you want it.

I did not have a detailed look at the patch yet, but some first thoughts:

+++ b/core/modules/path/path.module
@@ -254,7 +255,7 @@ function path_entity_insert(EntityInterface $entity) {
-  if ($entity->getPropertyDefinition('path')) {
+  if ($entity instanceof ContentEntityInterface && $entity->getPropertyDefinition('path')) {

This looks like we've got do do quite some checking here - is it really necessary to drop ComplexDataInterface from EntityInterface? I'd prefer to simplify & keep it such that we could at least rely on the general way to access fields being there and save us from quite some checks like that.

So - to what this boils down is that we are back to with to property terminology for config entities. I guess there is no way around that anymore, so imho we really have discuss and settle on "what's an entity property & field" now.

E.g., I could see it working as:
- Entities consists of properties, which can be (but do not have to be) fields
- Content entities can only have fields as properties (different to config entities)
- Implement ComplexDataInterface with entity properties also (could be 'type' => 'any' for now)

Then, we need to figure out how to deal with get() and ComplexDataInterface wording in general, such that it does not impose not fitting terminology on objects (properties on content entities).

Let's see if this fixes menu links.

Overriding the menu link form controller for now. Also inlined a simplified, non-fieldable version of entity_form_submit_build_entity() as that was the only place left that called it. See @todo's there. We need to discuss what config entities need specifically when being edited (config context?), so a config entity form controller (base?) makes sense, but would make this patch considerably bigger.

Fixing a new confirm form.

#95: entity-config-ng-interfaces-2004244-95.patch queued for re-testing.

Minor doc issues. Feel free to discard anyone :).

1. +++ b/core/lib/Drupal/Core/Entity/EntityFormController.php
   @@ -400,32 +355,36 @@ public function buildEntity(array $form, array &$form_state) {
   +    // Invoke all specified builders for copying form values to entity properties.
   

   80 lines.

2. +++ b/core/lib/Drupal/Core/Entity/EntityManager.php
   @@ -477,6 +477,10 @@ public function getFieldDefinitions($entity_type, $bundle = NULL) {
   +        // @todo: This shouldn't be called...
   

   comment needs update.

3. +++ b/core/modules/menu_link/lib/Drupal/menu_link/MenuLinkFormController.php
   @@ -252,6 +252,34 @@ public function validate(array $form, array &$form_state) {
   +    // @todo: Remove this when menu links are content entities.
   

   issue link will help.

This patch looks great!!! I didn't go through it line by line but overall this is one of the nicest patches I've seen in a while. (Although BC-removal was pretty sweet as well.) I only found one superficial nitpick:

+++ b/core/modules/field/lib/Drupal/field/Plugin/field/field_type/LegacyConfigFieldItem.php
@@ -49,7 +49,8 @@ public function isEmpty() {
-    // The previous hook was never called on an empty item, but EntityNG always
+    // The previous hook was never called on an empty item, but
+    // ContentEntityBase always
     // creates a FieldItem element for an empty field.

Very weird wrapping here.

Re-rolled, thanks for the reviews.

- Fixed comments, added link to menu link issue.
- Removed that this shouldn't be called thing. Let's see if something still calls it, shouldn't. Also cleaned up some interface checks and type hints while looking for possible calls.

@fago:

This looks like we've got do do quite some checking here - is it really necessary to drop ComplexDataInterface from EntityInterface? I'd prefer to simplify & keep it such that we could at least rely on the general way to access fields being there and save us from quite some checks like that.

A number of hook_entity_*() implementations have to check the interface now, correct. But previously, they simply relied on the fact that config entities didn't implement isTranslatable()/getPropertyDefinitions()/getPropertyDefinition(). Conceptually, that check was always there. Also, while other examples are valid hook implementations, that example should really be a method in the field item class and there's an issue to fix that.
I was thinking about introducing hook_contententity_*() hooks, given that they are so different.

I don't think it makes sense to keep ComplexDataInterface on EntityInterface. It's a lie. The only method that config entities actually implementation is get()/set() and those violate the contract of ComplexDataInterface by returning raw values and not field objects, so they only work because PHP is too stupid to be able to define return type hints :)

Not yet sure about the field/property stuff. But I didn't really re-introduce that, config entities always continued to use that term (getExportProperties()).

#102: entity-config-ng-interfaces-2004244-102.patch queued for re-testing.

Oh, we can also kill ContentTranslationControllerNG now.

I think we can and should push the clean-up of database storage (NG) to a follow-up issue. It's not directly tied to this and probably a bit bigger.

That's my idea/plan on that topic, which would also work quite well with the current class hierarchy, which consists of just two points:
- Merge DatabaseStorageController and DatabaseStorageControllerNG into FieldableDatabaseStorageController
- Introduce a new SimpleDatabaseStorageController or something like that basically only supports to save plain entity objects, no fields, (we could support revisions, but maybe even leave that out?)

I'll open an issue for this asap if people are ok with doing this in a different issue. I really think we're touching enough here.

Re-roll.

Per our epic discussions today, we concluded that we will do this and try to re-add ComplexDataInterface in a way that a) uses the existing schema definition to return typed data definitions on this and b) do it in a non-intrusive way, so that config entities continue to work as they do right now.

So left to do is a follow-up for the storage controller NG clean-up and an issue summary.

Not sure why the install failed, will look into it. For now, found some more mentions of EntityNG to clean up.

I thin the two new methods are not relevant to config entities, so we should possibly move them down too?

For now, just moved the referencedEntities() implementation down and also simplified/generalized it a bit to not hardcode ->entity. The get() call was also unnecessary because getProperties() already returns the field item lists. It could even just be foreach ($entity as $field_items).

The database storage controller follow-up, already with a initial patch: #2095399: Merge DatabaseStorageController and DatabaseStorageControllerNG

re: referencedEntities() - added a comment in #1605290-205: Enable entity render caching with cache tag support

#108: entity-config-ng-interfaces-2004244-108.patch queued for re-testing.

Re-roll.

#113: entity-config-ng-interfaces-2004244-113.patch queued for re-testing.

Crosspost.

Re-roll after IdentifiableInterface removal.

#118: entity-config-ng-interfaces-2004244-118.patch queued for re-testing.

Another re-roll :)

Usually works better with a patch.

As discussed (see notes), yes ConfigEntities work differently so we have to move on with that. The patch makes what we have already (config entities don't support EntityNG APIs) explicit, so that's good. So as config entities are not bound to have fields only any more (as it was in theory), we can now wire the exist typed data information of config entities from the config schema into it and provide back the metadata we have in place via the usual APIs. However, as ComplexDataInterface needs to be improved in order to solve the conflict on how get() is implemented I agree that it's the best way forward to remove it now, and add it back once we can implement it in a proper fashion (and wiring in the TypedConfig metadata works out).

So here a review of the actual patch:

+++ b/core/lib/Drupal/Core/Entity/EntityInterface.php
@@ -50,6 +48,14 @@ public function uuid();
+   * Returns the default language.
+   *
+   * @return
+   *   The language object.
+   */
+  public function language();

This seems to miss the @return class. Also it does not clarify what the default language is. Then, I'm wondering whether it makes sense on EntityInterface when this isn't translatable? It probably should go into TranslatableInterface?

+++ b/core/modules/aggregator/lib/Drupal/aggregator/Entity/Item.php
@@ -102,7 +102,7 @@ class Item extends EntityNG implements ItemInterface {
-   * Overrides Drupal\Core\Entity\EntityNG::init().
+   *{@inheritdoc}

Missing space.

+++ b/core/modules/content_translation/content_translation.module
@@ -597,7 +597,7 @@ function content_translation_form_alter(array &$form, array &$form_state) {
-    if (!$entity->isNew() && (!isset($translations[$form_langcode]) || count($translations) > 1)) {
+     if (!$entity->isNew() && (!isset($translations[$form_langcode]) || count($translations) > 1)) {

Wrong indentation change.

Else, the patch looks good to me.

Ok, fixed that, improved language() a bit. We discussed that, but will keep it for now on EntityInterface, for example also because menu links need it to.

We'll open an issue to discuss the config entity language override behavior, where we might want to move/copy it to re-define the exact behavior of config entity's language() method.

Yes, I think that works.

I just found something quite minor, else I think this is ready. -> RTBC given bot is still green.

+++ b/core/lib/Drupal/Core/Entity/ContentEntityBase.php
@@ -160,6 +167,153 @@ public function __construct(array $values, $entity_type, $bundle = FALSE, $trans
+    $this->newRevision = $value;
+  }
+  /**
+   * {@inheritdoc}

Missing new line.

Updated issue summary.

Update, the mentioned follow-up about config entity language() and language override behavior has been opened: #2100309: Make ConfigEntityBase::language() match the behavior of EntityNG::language().

Fixed that and also moved over some documentation that I noticed should moved down to content entities now, thanks #2100411: Wrong doc for interface EntityInterface, which I will close as duplicate.

I'm too stupid to upload patches. Was a long week ;)

Back to RTBC as per #126.

+++ b/core/lib/Drupal/Core/Entity/ContentEntityInterface.php
@@ -12,6 +12,19 @@
+ * content entity entity implements the ComplexDataInterface among others, thus
+ * is complex data containing fields as its data properties. The contained

content entity entity

Fixed that.

Thanks!

#133: entity-config-ng-interfaces-2004244-133.patch queued for re-testing.

Re-rolled

Back to RTBC then, thanks.

Back to RTBC then, thanks.

Erm. Whatever that was.

#138: entity-config-ng-interfaces-2004244-138.patch queued for re-testing.

Committed/pushed to 8.x, thanks!

Needs a change notice.

I made some small changes to https://drupal.org/node/1806650 ( to clarify that this only applies to content entities) and to https://drupal.org/node/1818734 (to clarify that they don't support revisions, translations and fields).

I'm working on improving the documentation below https://drupal.org/developing/api/entity.

Where did that tag come from?

Updated issue summary.

FYI: #2134985: Limit the concept of 'bundle' to ContentEntities ?

Related follow-up: #2275659: Separate FieldableEntityInterface out of ContentEntityInterface