Allow modules to drupal_alter results of commerce_license_get_order_licenses

I'm wondering about this bit:

   if (!in_array($line_item->type, commerce_license_line_item_types())) {
+    // Allow other modules to create licenses for different product types.
+    drupal_alter('commerce_license_line_item_presave', $line_item);
     return;

Since that block returns (since the line item type is not licensable) should the drupal_alter() come before the $line_item->type check? That's also basically what's happening in the activate licenses block.

Otherwise it looks good, very simple change and something we need to get Commerce Product bundle working with Commerce License. Thanks!

This works great, but I think we should actually move the alter() hook down so it's within the function commerce_license_get_order_licenses(). This way any other modules that need to call this function to retrieve an array of licenses can do so there, rather than in the child functions, e.g. commerce_license_activate_order_licenses().

This is much better.

@bojanz et al please consider committing this. For custom product structures an alter hook for licenses gives us much more flexibility.

Tagging for sprint. This feature will allow me to continue work on Commerce Product Bundle, which we need.

Re-rolled based on @bojanz feedback from IRC regarding the documentation.

Fixing title to match the proposed solution

Typo is fixed.

So this ultimately an approach that will not work. We shouldn't be tackling commerce_order_get_order_licenses at all as it isn't the root of the problem. The action all happens in commerce_license_commerce_line_item_presave, which can be examined here: http://cgit.drupalcode.org/commerce_license/tree/commerce_license.module...

Conceptually, this function does the following things:

1. Determines if a line item might need to have a license attached to it. (If not, exit.)
2. Determines if the line item already HAS that license attached to it.
3. a. If yes, make sure that license's key metadata matches what the line item expects.
3. b. If not, fix that metadata and save it.
4. a. If not (to #2), create the license that the line item expects and save it.
4. b. Attach the newly-created license to the line item as a reference.

Let's think of #1 through #4 as a conceptual interface -- this is the logic that defines whether a license is created. Here's how the current implementation works:

1. Is the line item's type in the array of types defind in commerce_license_line_item_types()?
2. Is there a license on the $line_item->commerce_license reference field?
3. a. Does the product ID on the license match the commerce_produce reference on the line item?
3. b. Change the product ID to match the product reference.
4. a. The license is created with a product ID, UID, and license type defined on the line item product.
4. b. The license is added to the single-value $line_item->commerce_license reference field.

The problem is that this implementation needs to be substantially generalized, with hooks, to allow other modules to modify it. Pieces like the product_id, single-value reference field from line item to license, and the 1-1 mapping of line item to license would all need to be given alter hooks in some way. It would end up looking something like this:

1. Figure out whether a line item is licensable. (Do other modules need to alter this result?)
2. Figure out which licenses a line item demands. Allow other modules (commerce_product_bundle) to alter this result.
3. Match those desired licenses to the ones already on the line item. (Do we need a matcher function? Can other modules alter the matching process?)
4. If any of the licenses match, check if they need to be updated. (Can modules alter which properties trigger an update? Right now only product_id does.)
5. If any licenses are missing, create them. (Modules provided the created values in #2.)
6. Save the recomputed array of licenses onto the $line_item->commerce_license field.

Now we need to go find all the places where the ->commerce_license field is regarded as single value and change it to planning for multiple values. The commerce_order_get_order_licenses function is one example of this, but there are others around that need to be fixed up.

Once all that's done, THEN we need to think about the assumptions that CLB makes around recurring billing and the product_id. Ugh.

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.