I've just pushed my first two commits:
http://drupalcode.org/sandbox/rszrama/1181848.git/commitdiff/47580ae
http://drupalcode.org/sandbox/rszrama/1181848.git/commitdiff/e34f141

Notes:
1) The product forms are implemented from scratch.
This allowed easy implementation and working Field API integration, without the disliked subform dependency or hacks needed to embed whole forms.
The product forms know they are being embedded, and have $parents to consult.
Originally, I found myself altering out most of the original form. The context is different enough to justify separate implementations.

2) The product forms are in a separate include file, and I've replaced commerce_product_* entity functions with generic entity ones (entity_load, entity_save, etc).
This is the first step towards having support for other entity types.
The module logic is generic enough that I don't see any reason why it shouldn't support line items as well out of the box.
Would need a new name then though, Commerce Inline Form? Something else?

The row listing still isn't abstracted out (sku and title are always added). I will post a proposed patch for that soon, unsure about the implementation (we need to make it alterable by the user, Ryan suggested having a custom entity view mode).
We'll need a way to alter the embedded forms, as well as a mechanism of declaring the callbacks for each entity type (a new hook is probably a better idea then further expanding hook_entity_info()).

I've removed the "Reference product" button. It has never been implemented, and I consider it unneeded.
The use case is pretty clear here: a widget to manage child entities from one place only (product display -> products, order -> line items).
For that the button is mostly visual noise, and our latest internal mockups did leave it out.

Comments

philipz’s picture

philipz commented

My first impression is: wow, This looks really great!

I'm using custom product type entity (machine name: towar) and I see some things are working out of the box and some are not.
Editing product opens the form with all the fields attached. After saving the product I get an error
Notice: Undefined index: widget in commerce_ipf_field_attach_submit() (line 394 of /sites/all/modules/commerce_inline_product_form/commerce_ipf.module).
and saving the form doesn't work either but this is probably simple to fix. Tommorow I'll look through the code to see what's happening inside.

My first module name suggestion was: Commerce Entity Widget.
But then I though that maybe this is even more generic widget type than just Commerce widget?
If so the name could be something like Entity Inline Form or Entity Inline Widget.

EDIT:
The first error was caused by my field_collection field attached to my product entity. After removing it the error was gone.

Next problem is that after saving inline form the Title gets overriden by SKU. Moreover there is no Title field on the inline form even though the field is attached.

Last thing is a PHP error on node form submit
PHP Fatal error: Call to undefined function entity_metadata_field_get_language() in /var/www/tacx-polska.tk/public_html/sites/all/modules/commerce_inline_product_form/commerce_ipf.module on line 397

bojanz’s picture

bojanz commented

Ah, we're progressing :)

Next problem is that after saving inline form the Title gets overriden by SKU. Moreover there is no Title field on the inline form even though the field is attached.

Yeah, this is like that intentionally. I'm going to change it to take the node title. A future plan is to append attributes (so "T-Shirt (Size: M, Color: Blue)")

PHP Fatal error:  Call to undefined function entity_metadata_field_get_language() in /var/www/tacx-polska.tk/public_html/sites/all/modules/commerce_inline_product_form/commerce_ipf.module on line 397

This means you don't have the latest Entity API dev.

philipz’s picture

philipz commented

Yeah, this is like that intentionally. I'm going to change it to take the node title. A future plan is to append attributes (so "T-Shirt (Size: M, Color: Blue)")

Appending attributes is great idea but I'm not sure why you want to override the title field. Right now it would be better to let the user input whatever is needed. The node title could be inserted automatically as product title on product creation inline form but product title should not be overwritten by SKU or node title after it has been created or edited by the user.

Of course updating the Entity API module helped solve the PHP error as you wrote.

Are you working on this module actively ? I want to help you but I don't know what should I start with. I would like to add the title field and allow to edit it as I wrote above. The next thing is custom columns displaying products referenced by node. What's your plan ? :)

bojanz’s picture

bojanz commented

Appending attributes is great idea but I'm not sure why you want to override the title field. Right now it would be better to let the user input whatever is needed. The node title could be inserted automatically as product title on product creation inline form but product title should not be overwritten by SKU or node title after it has been created or edited by the user.

It is a redundant field if you consider the fact that all product management goes through the widget.
So I'm adding a "T-Shirt" product display, and I want to add shirt products with specific colors and sizes.
Having the system set the title as "T-Shirt (Color: Blue, Size: M)" is enough for me to recognize it in a view or wherever else I'd look at individual products.
It's just a field of no clear use on a very busy screen.
That said, I'll modify the code so that the title is autogenerated only on "add". If the user edits the product manually from outside the widget, the title change won't be killed.

Are you working on this module actively ? I want to help you but I don't know what should I start with.

Yes, I'm working on it actively. Finished abstracting out products today (and adding support for multiple bundles / product types). My next task is making the product form prettier (separating attributes, etc).
If you want to help, you could do #1462136: Support line items (should be fairly straightforward, based on the current code), and / or debug that field_collection error you encountered. Not sure what you mean with "custom columns displaying products referenced by node".

philipz’s picture

philipz commented

The way I see the Title field is a "description" field and that's why I don't think it should be overwritten with SKU at any point.

Autogenerating based on attibutes is the way to go of course but if I didn't want it to be autogenerated while adding products to my product display I would want to add the Title (as description) to the each SKU manually. So I think:
1. Title field should be allowed to be edited manually (might be a setting on the widget settings page).
2. Autogeneration of Title field should be a setting on the widget field.
3. If Title was autogenerated there are two options now: first is allowing it to be tweaked by the user or the second one is it wouldn't appear in the inline form at all. This might also be a setting.

I see you're working hard on this so I'll start with debugging the field_collection error today and I'll get to #1462136: Support line items tomorrow if it won't be ready by then :)

By "custom columns displaying products referenced by node" I meant a way to select which fields of a product sould be visible as table columns on the widget (for example Price field). This is somehow related to #1459156: Abstract out the display of products isn't it?

lsolesen’s picture

lsolesen commented

I am getting a 500 when using the inline form when saving the product display (not when adding the products). However, no products are saved. I do not have the problem when just using an autocomplete field. See #1499382: 500 when hitting save for the product display.

I have tried to use the inline form with AUTOSKU, but that is not taken into consideration in the form.

bojanz’s picture

bojanz commented
Version: » 7.x-1.x-dev
Status: Active » Fixed

Nothing more to review here. We've had considerable progress since I first posted this issue, as well as internal reviews.

The title issue still stands, and is on my todo list.

Status: Fixed » Closed (fixed)

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

jddeli’s picture

jddeli commented
Priority: Normal » Critical

No its not fixed

jddeli’s picture

jddeli commented
Status: Closed (fixed) » Active

No its not fixed

bojanz’s picture

bojanz commented
Priority: Critical » Normal
Status: Active » Closed (fixed)

Please learn how to behave.

bojanz’s picture

bojanz commented
Issue summary: View changes

Substitute a word.