Add bundle info in the field definition data

In the current state, this feature would help with

• labelling the model correctly (if the same entity type and/or bundle does not exist on the server)
• recreating the full bundle, including label and more (e.g. node bundles have description, publishing options etc.)

A potential complication is that this might not be logically well covered by the FieldDefinition plugin (already in the name there is a mismatch). Perhaps it's in order to create a new entity-focused definition plugin, just like we are already considering for CollectJson/EntityJson. Discussable.

+++ b/src/Controller/CreateMissingFieldsController.php
@@ -71,17 +71,17 @@ class CreateMissingFieldsController extends ControllerBase {
+      $typed_data = $this->getContainerTypedData($collect_model);
+      $bundle_data = $typed_data->getParsedData()['bundle'];

@@ -130,14 +130,30 @@ class CreateMissingFieldsController extends ControllerBase {
+   * @return \Drupal\collect\TypedData\CollectDataInterface
...
+  public function getContainerTypedData(ModelInterface $collect_model) {
+    if ($field_definition_container_id = FieldDefinition::findDefinitionContainer($collect_model)) {
...
+      return $this->typedDataProvider->getTypedData($stored_field_definition_container);

This can return NULL and you don't consider it.
Other than that, looking good.

There is an exception when I try to recreate an entity without first recreating it's content type. I'd say catch that exception (as discussed).

Works as intended.

Btw: I thought you'd just catch the exception, but your solution is definitely nicer. What I had in mind:

CreateEntityController.php

Line 146ff

    try{
      $violations = $entity->validate();
    }
    catch(\Exception $e) {
      drupal_set_message($e->getMessage(), 'error');
      return new RedirectResponse($collect_container->urlInfo()->setAbsolute()->toString());
    }
    if (!empty($violations)) {

Edit: Almost forgot, can you create a (Meta) Issue with the follow-up issues?

Yes, would love to commit this. Here some clarifications about the concepts.

1. +++ b/src/CaptureEntity.php
   @@ -147,12 +147,20 @@ class CaptureEntity {
   +    if ($bundle_entity_type && $bundle_entity_type != 'bundle') {
   ...
   +      $data['bundle'] = $this->serializer->normalize($bundle_storage->load($entity->bundle()));
   
   +++ b/src/Controller/CreateEntityController.php
   @@ -58,10 +70,20 @@ class CreateEntityController extends ControllerBase {
   +      $bundle_entity = $this->entityManager->getStorage($entity_definition->getBundleEntityType())->load($bundle);
   +      if (!$bundle_entity) {
   

   OK the bundle key is thus missing if an entity has only a default bundle. Can these really be properly reconstructed? We need a test case for reconstructing an entity without a bundle.

   Also i think we should additionally persist entity type as key and not match them from the URL.

2. +++ b/src/Controller/CreateEntityController.php
   @@ -58,10 +70,20 @@ class CreateEntityController extends ControllerBase {
        $entity_type = $matches['entity_type'];
   ...
          $bundle = $matches['bundle'];
   

   Then this should not consider matches at all. Instead it should be loaded from the data.

Finally there are lots of lines like this in collect:

    if (!$matches = CollectJson::matchSchemaUri($collect_container->getSchemaUri())) {

When persisting entity type and bundle to the container, we should no more consider the SchemaUri to match things. It should just be used for a lookup address. The original container then contains the actual (entity type and bundle) data. The reason is that we don't have any hardcoded assumptions anymore in the Schema URI except possibly for the auto schema suggestions.

Rebase needed.

Looks good.

Came up with a few review comments. They're mostly DX and UX, nothing super important, and I'm fine with committing without them too.

1. +++ b/src/Controller/CreateEntityController.php
   @@ -51,18 +64,35 @@ class CreateEntityController extends ControllerBase {
   +    if ($typed_data = FieldDefinition::getContainerTypedData($model)) {
   +      $parsed_data = $typed_data->getParsedData();
   +      $entity_type = $parsed_data['entity_type'];
   +      $bundle_data = isset($parsed_data['bundle']) ? $parsed_data['bundle'] : NULL;
   
   +++ b/src/Controller/CreateMissingFieldsController.php
   @@ -71,17 +71,18 @@ class CreateMissingFieldsController extends ControllerBase {
   +      $typed_data = FieldDefinition::getContainerTypedData($collect_model);
   +      $bundle_data = $typed_data->getParsedData()['bundle'];
   

   I think the FieldDefinition plugin should offer the bundle data and entity type ID as properties, just like it does with ['fields']. Then this can be $bundle_data = $typed_data->get('bundle').

2. +++ b/src/Tests/EntityWebTest.php
   @@ -461,11 +467,54 @@ class EntityWebTest extends WebTestBase {
   +    // Create a new user.
   ...
   +    // Capture a user.
   ...
   +    // Remove a user entity.
   ...
   +    // Recreate a captured entity.
   ...
   

   I suppose the point here is to assert that you can recreate an entity whose bundle is not missing? Perhaps that should be stated clearly in a comment.

3. +++ b/src/Controller/CreateEntityController.php
   @@ -51,18 +64,35 @@ class CreateEntityController extends ControllerBase {
   +    if ($typed_data = FieldDefinition::getContainerTypedData($model)) {
   ...
   +    else {
   +      throw new NotFoundHttpException('Container data is not found.');
        }
   

   Please clarify: what container, and why does it have to be found?

4. +++ b/src/Controller/CreateEntityController.php
   @@ -51,18 +64,35 @@ class CreateEntityController extends ControllerBase {
   +      // In case the bundle does not exist in the system, entity reconstruction
   +      // is not possible.
   +      $bundle_entity = $this->entityManager->getStorage($entity_definition->getBundleEntityType())->load($bundle);
   +      if (!$bundle_entity) {
   +        drupal_set_message(t('The entity could not be recreated. Create a missing %bundle bundle to recreate the entity.', ['%bundle' => $bundle]), 'error');
   

   It would be helpful if this error message contained some instructions on how to recreate the bundle. Like "use the 'Create entity' button" or whatever makes the most sense in this context.

The testbot run is passing (although it says "Queued for testing" in the comment) but it failed on DrupalCI. I'm not sure about the error message, "The test did not complete due to a fatal error", what is that about?

Okay, looks good. Committed, with the attached minor change. (ControllerBase has a special take on service injection...)

The change I made.