Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tmsimont’s picture

I would also like this feature...

I was looking at the code in CommerceProductInlineEntityFormController::entityForm() and I think the conflict might be coming from the fact that IEF lets you set up an automatic title with the "Auto generate the product title " on the field settings form.

It looks like at some point it was adjusted to specifically react to the Title module...



    // Hide the title field if it is auto-generated.
    if ($this->settings['autogenerate_title']) {
      $entity_form['title']['#required'] = FALSE;
      $entity_form['title']['#access'] = FALSE;
      // Hide the replacement field added by the Title module as well.
      if (module_exists('title')) {
        $title_field = title_field_replacement_info('commerce_product', 'title');
        if ($title_field) {
          $title_field_name = $title_field['field']['field_name'];
          if (isset($entity_form[$title_field_name])) {
            $entity_form[$title_field_name]['#access'] = FALSE;
            $entity_form[$title_field_name]['#required'] = FALSE;
          }
        }
      }
    }

I wonder how hard it would be to put in a similar replacement for the Automatic Entity Label...

tmsimont’s picture

Status: Active » Needs review
FileSize
1.88 KB

The attached patch will allow auto entity label to set and/or hide the title. Note that "Auto generate the product title " on the field settings form will have to be unchecked for this to work.

bennos’s picture

Status: Needs review » Reviewed & tested by the community

Patch from #2 works for commerce products.
Status set to RTBTC. One more review would be great.

But I think we need also support for entity's and nodes.

malberts’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
2.56 KB

Here's a patch expanding #2 (support for Commerce Product) to also support Nodes. I'm not going to try this for Line Item/Term/Entity right now, but the code would be similar in all cases. I think we should expand this patch to get Automatic Entity Label support for all the core IEF Controllers before we commit this. Any thoughts?

malberts’s picture

Sorry, I made a silly omission.

bennos’s picture

works for core entity (nodes).

bojanz’s picture

I like this idea.
I'm going to investigate removing the custom IEF product title generation in favor of this integration.

The Title module specific code was made unnecessary by #1748008: Replaced fields should inherit the legacy field access and I'm going to remove it as soon as Title module gets a new alpha.

mrfelton’s picture

Updated patch against latest dev.

froboy’s picture

FileSize
11.46 KB

I'm testing the patch from #8 and it seems to work correctly, but has a small UI issue:

When adding a new node using the inline form, the title for the new node is not generated until the containing form is previewed or saved, which results in a blank title until that point as seen here:
.

This could be confusing to users.

In node.inline_entity_form.inc adding auto_entitylabel_set_title($entity_form['#entity'], 'node'); to the entityFormSubmit function seems to properly generate the title upon submission of the inline form, but I'm not sure if this is proper or where in the function it should go...

Thoughts?

tmsimont’s picture

I'm not sure what the difference is but in the patch i wrote for commerce line items, the title there reads "Title will be automatically generated on submission" or something like that... I'm not sure what is different in the node entity handler that mrfelton patched

froboy’s picture

It seems like my proposed method (after the image in my post) could be better for both handlers as users would see a live preview of their node title. Would you be willing/able to integrate that into the commerce handler?

Robin van Emden’s picture

Status: Needs review » Reviewed & tested by the community

I encountered this issue as well - combining mrfeltons #8 patch combined with froboy's #9 suggestion worked perfectly.

   ... 

  /**
   * Overrides EntityInlineEntityFormController::entityFormSubmit().
   */
  public function entityFormSubmit(&$entity_form, &$form_state) {
    parent::entityFormSubmit($entity_form, $form_state);

    // line added so auto title shows in node before parent node save
    auto_entitylabel_set_title($entity_form['#entity'], 'node');

    ...

No time to roll a patch adding #9 to #8 just now, very close deadline. Mrfeltons & froboy: thanks for your patches.

Tmsimont: #9 shows the auto title, indeed does seem more user friendly than "Title will be automatically generated on submission" - but maybe there's some disadvantage to #9 I am not immediately aware of?

geek-merlin’s picture

Status: Reviewed & tested by the community » Needs work

it's only rtbc if we have a patch.
#9 must at least be wrapped in something like

if(module_exists('auto_entitylabel')) {
...
froboy’s picture

Rolled both of our patches and the suggestion from #13 into one patch. I don't have anything set up to test the commerce side on and it looks a little more complicated, but this should do it for node titles.

Feel free to test, but this should probably still be "needs work" until someone can do the commerce side.

tmsimont’s picture

I don't think the commerce patch I used in #2 needs any more work. When I used it, I get a temporary title. I think commerce is doing something on its own to generate the temporary title.

froboy’s picture

Status: Needs work » Needs review

Okey, changing status. Who's going to test?

geek-merlin’s picture

Status: Needs review » Needs work

* code looks fine now
* has some trailing whitespace
* does what is says for nodes

did not test for products yet.

i think the code for nodes and products is cut-and-paste and should be factored out to a function.
this way integration of new entities will be simpler and easier to review.
so setting to needswork.

if someone(tm) tests the products integration feel free to rtbc and we might first fix this PITA and do the refactoring in a followup patch.

tmsimont’s picture

Status: Needs work » Needs review
FileSize
8.59 KB
3.84 KB

alex.rutz's comments applied to latest dev version in attached patch.

  1. moved code to function in parent class
  2. removed whitespace error

interdiff attached

tmsimont’s picture

FileSize
4.85 KB

proper interdiff, last one no good

please test inline_entity_form-auto_entity_label-1787838-15.patch against latest dev branch

derekw’s picture

Tested the patch. Had to manually apply the patch to node.inline_entity_form.inc for some reason.

Title/label does autogenerate, i.e. it works when adding nodes.

However if I then click Edit the node, Title field shows up.

tmsimont’s picture

Might that be an issue with Automatic Entity Label?

derekw’s picture

First guess -- it seems to be this code in includes/node.inline_entity_form.inc. If I take this out completely then the title is hidden and auto generates correctly on both new creation and edit. Maybe this is running after auto_entitylabel_form_alter, and overwriting the hidden, not required settings?

$entity_form['title'] = array(
      '#type' => 'textfield',
      '#title' => check_plain($type->title_label),
      '#required' => TRUE,
      '#default_value' => $node->title,
      '#maxlength' => 255,
      // The label might be missing if the Title module has replaced it.
      '#weight' => !empty($extra_fields['title']) ? $extra_fields['title']['weight'] : -5,
    );
tmsimont’s picture

dang so its coming from IEF then -- I'm not familiar with the node controller in IEF.

Would wrapping that code in this work:

if (module_exists('auto_entitylabel')) {
  if (auto_entitylabel_is_needed($entity, $entity_type)) {
    //put that code in here
  }
}

I'm not sure where you'd be able to set $entity and $entity_type but I bet the $entity_form variable would at least have an #entity property.

tmsimont’s picture

oops i meant to include NOT:

if (!module_exists('auto_entitylabel')) {
  if (!auto_entitylabel_is_needed($entity, $entity_type)) {
    //put that code in here
  }
}
mtoscano’s picture

I tested patch #14 and #18: they both works hiding and autogenerating Title/label, but then adding a new node with a normal title, no Automatic Entity Label, via Inline Entity Form they both produce this error
Notice: Undefined property: stdClass::$nid in auto_entitylabel_set_title() (line 185 of /sites/all/modules/auto_entitylabel/auto_entitylabel.module).
and the title is replaced with the node type name.

alesr’s picture

I had the same issue from #25 and found a bug in the patch.

The problem was in entityFormSubmit() function because auto_entitylabel_set_title() was called every time, just by checking if module_exists('auto_entitylabel')

The solution is to add auto_entitylabel_is_needed() verification.

Patch is attached.
(Applied on patch #14, because #15 patch is not well constructed, but it can be easily applied on #15 too) by using those lines:

+    if (module_exists('auto_entitylabel')) {
+      if (auto_entitylabel_is_needed($entity_form['#entity'], 'node')) {
+        // Pre-set the title before we submit so that it's visible.
+        auto_entitylabel_set_title($entity_form['#entity'], 'node');
+      }
+    }
tmsimont’s picture

#15 would work if you go #14, then interdiff.. or try just going straight into #15 -- there are differences there in 15 that are important.

alesr’s picture

No problem. Here is the patch #28 with the fix applied on patch #15.

Dave Reid’s picture

Actually I propose an alternative that doesn't require making any changes to IEF. If entity_autolabel uses hook_field_attach_form() to do it's title processing, then it just *works* with IEF and all possible entity types. I propose we mark this as a duplicate of #1980652: Make auto_entitylabel work with inline_entity_form widget.

Gaelan’s picture

martin74’s picture

After the patch (28) my commerce kickstart is still not generating auto title's

its a basic installation, with addittional catergories and collections ,translating to dutch

Dave Reid’s picture

@Martin_S: Rather than trying the patch here, can you test if using the most recent dev version of auto_entitylabel on your site makes this work for you?

lehmanshaunc’s picture

I just tested this with the 7.x-1.x-dev version dated 2013-May-05 and it worked!

The Node Title was set to the Node ID on the first save.

martin74’s picture

@ Dave Reid
tnx for the reply , doing that now!

Edit
@ Dave Reid , you made my day.. works great!

martin74’s picture

A bit to early cheering.. i get errors on new products ( the name changed item work fine )

Notice: Undefined index: nl in auto_entitylabel_set_title() (regel 246 van /var/www/vhosts/xxx/httpdocs/sites/all/modules/auto_entitylabel/auto_entitylabel.module).

johnstav’s picture

Entity label appears as %AutoEntityLabel% when:
- Add an entity via the inline entity widget
- Save the added entity
- Don't save the parent entity
- Edit the added entity
- Save the added entity again.

martin74’s picture

it doesnt store my auto title in one time if i use the product title in the entity

i need to open the product a 2th time and store the variation and product again.

Dave Reid’s picture

This does need another patch to the auto_entitylabel module to set the label in hook_field_attach_submit(). I've submitted a patch here: https://drupal.org/node/1980652#comment-7609069

lathan’s picture

Rerolled patch does not apply to 7.x-1.x.

$ patch -p1 < inline_entity_form-auto_entity_label-1787838-28.patch
patching file includes/commerce_product.inline_entity_form.inc
Hunk #1 succeeded at 219 (offset -7 lines).
patching file includes/entity.inline_entity_form.inc
Hunk #1 succeeded at 383 (offset 4 lines).
patching file includes/node.inline_entity_form.inc
Hunk #1 succeeded at 65 with fuzz 1 (offset 23 lines).
patch: **** malformed patch at line 81:      $child_form_state = form_state_defaults();
lathan’s picture

missed patch here it is.

vasike’s picture

Component: Code » Documentation
Status: Needs review » Needs work

as Dave Reid said on #38, this was fixed in Automatic Entity Label - #1980652-7: Make auto_entitylabel work with inline_entity_form widget.
my tests confirm that with the last dev of Automatic Entity Label, there's nothing to be done here.
Except the documentation - at least to specify on project page that IEF works with Automatic Entity Label.

davewilly’s picture

Remove

bojanz’s picture

The issue is open, so it hasn't been solved obviously.

milos.kroulik’s picture

Issue summary: View changes
Status: Needs work » Needs review

I have tested IEF 1.5 with AEL 1.x-dev and can confirm, that it works as expected. I don't see any work left, that should be done here.

bojanz’s picture

Status: Needs review » Fixed

Okay, then.
Opened #2181441: Remove product title autogeneration as a followup.

Status: Fixed » Closed (fixed)

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

ayalas’s picture

From my tests the switch to the Automatic Entity Label module does not resolve #2212861: title autogenerate doesn't always copy updated parent entity title to child commerce products.

thomasmurphy’s picture

I've just tested this against the full release and the dev version and it hasn't fixed this issue (core 7.50 / inline_entity_form 1.8 / auto_entitylabel 1.3) ...ah, but (core 7.50 / inline_entity_form 1.8 / auto_entitylabel 1.x-dev) works fine. Just posting this to save people a bit of time.