When trying to create a new entity module using entity operations, I encountered a situation. My entity had only one bundle / entity type, but it was named differently to the core entity. When trying to add a new instance of my entity (on the add form), I would encounter a fatal error from common.inc and the entity_extract_ids function.

As the code stands at the moment, if there is only one bundle, the module automatically assumes the name of the bundle is the same as that of the entity and does not check the name of the bundle and goes straight to to the entity_ui_add_form. We need to add some code that will check the name of the bundle if the bundle entity key value is set. If it is not then the standard behaviour can be kept.

I have discussed this issue with Joachim at some length on IRC so filing this issue as suggested by him.

CommentFileSizeAuthor
#6 entity_operations-add-form-single-bundle-2233507-6.patch1.56 KBjeebsuk

Comments

joachim’s picture

joachim commented

Thanks for filing :)

The problem is in EntityOperationsOperationAdd::build().

      // Special case: single-bundled entity. Go straight to the form.
      if (count($entity_info['bundles']) == 1) {
        $entity = entity_create($entity_type, array());
        return entity_ui_get_form($entity_type, $entity, 'add');
      }

I've made the stupid assumption that with only one bundle, there's no bundle key on the entity. Hence the proto-entity is broken in your case.

What we need here is something like:

    if (!empty($entity_info['entity keys']['bundle'])) {
      $bundle_key = $entity_info['entity keys']['bundle'];

Then you need to use array_pop() to get the machine name of the only bundle, and put that into a $values array for entity_create().

jeebsuk’s picture

jeebsuk commented

What I've got which seems to work is the following:

      // Special case: single-bundled entity. Sometimes go straight to the form.
      if (count($entity_info['bundles']) == 1) {
        //in some instances with one bundle, the name of the bundle may not be the same as the name of the entity so we need to handle this scenario.
        if (isset($entity_info['entity keys']['bundle'])) {
          $bundle_field_name = $entity_info['entity keys']['bundle'];
          $entity = entity_create($entity_type, array());
          //there is only one bundle in the bundles array so we can grab the key of the first array element, which is the name of the bundle.
          $entity->{$bundle_field_name} = key($entity_info['bundles']);
          return entity_ui_get_form($entity_type, $entity, 'add');
        }
        else {
          //use default bundle - go straight to the form
          $entity = entity_create($entity_type, array());
          return entity_ui_get_form($entity_type, $entity, 'add');
        }

      }

See what you think to that - if you think that looks OK I'll build a patch.

joachim’s picture

joachim commented

That's the right approach. What I'd to to aid legibility though is have less stuff going on in a conditional block, so:

$values = array();
if (** there is a bundle key) {
 // figure out the bundle key.
// figure out the bundle name
 $values[bundle key] = bundle name
}

entity = entity_create($entity_type, $values);
// ... same as before

So rather than 2 separate cases, it's 'do something special first, then do something common'.

jeebsuk’s picture

jeebsuk commented

Second go (which works):

      // Special case: single-bundled entity. 
      if (count($entity_info['bundles']) == 1) {
        //in some instances with one bundle, the name of the bundle may not be the same as the name of the entity so we need to handle this scenario.
        if (isset($entity_info['entity keys']['bundle'])) {
          $bundle_field_name = $entity_info['entity keys']['bundle'];
          //there is only one bundle in the bundles array so we can grab the key of the first array element, which is the name of the bundle.
          $values[$bundle_field_name] = key($entity_info['bundles']);
        }

        //create our entity - with the additional bundle information if appropriate and then load the form
        $entity = entity_create($entity_type, $values);
        return entity_ui_get_form($entity_type, $entity, 'add');

      }
joachim’s picture

joachim commented

Looks good. A few small things:

- you need to initialize $values to an empty array() before the if(){}, otherwise it'll be undeclared if you don't go into the if()
- let's say $bundle_key = $entity_info['entity keys']['bundle']; as that's the variable name used elsewhere in the module code. Consistency helps readability.
- comments need to be full sentences with capital and full stop and wrapped to 80 characters

Can you make a patch?

jeebsuk’s picture

jeebsuk commented
StatusFileSize
new entity_operations-add-form-single-bundle-2233507-6.patch1.56 KB

This is the first time I've created a patch so apologies if it's not right - but it seems to be based on various ones I've had to apply.

jeebsuk’s picture

jeebsuk commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: entity_operations-add-form-single-bundle-2233507-6.patch, failed testing.

joachim’s picture

joachim commented

Argh, the tests were failing on their own.

I've fixed the generic form tests. Not sure why the Views plugin test is failing, as for me it passes locally. Let's re-test and see what happens...

joachim’s picture

joachim commented
Status: Needs work » Needs review

6: entity_operations-add-form-single-bundle-2233507-6.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 6: entity_operations-add-form-single-bundle-2233507-6.patch, failed testing.

joachim’s picture

joachim commented
Status: Needs work » Fixed

Tests all pass for me locally, so committing.

Thanks for the patch!

  • Commit d12d000 on 7.x-1.x authored by JeebsUK, committed by joachim: 
    Issue #2233507 by JeebsUK: Fixed error in Add handler form when a single...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.