Create simple_sitemap LinkGenerator plugin for Commerce products

Is this a plugin provided by that module? I don't see a LinkGenerator plugin in core?

I see. Feel free to write this patch yourself. It's not something the core dev team will have time to prioritize while we flesh out the core of Commerce 2.x itself.

Reclassifying as a feature request. Leaving this to the community, mglaman and I can't tackle this in april due to more pressing issues.

First of all thank you platinum1 for sponsoring the creation of the plugin. It now needs to be moved to commerce, I'm attaching the patch. It works well with simple_sitemap 8.x-2.x and has been already tested by a couple of people. Still I'm going to set the status to 'needs review' for good measure.

As the creator/maintainer of simple_sitemap I know you guys must have a full schedule, but it would be great, if you could push this in a timely manner.

Thanks for all your work which will undoubtedly make D8 a great commerce platform!

It's still a feature request, and not a task since this not a Drupal core feature, or required development to get 8.x-2.x out the door.

+/**
+ * @file
+ * Contains \Drupal\commerce_product\Plugin\simple_sitemap\LinkGenerator\CommerceProductType.
+ *
+ * Plugin for simple_sitemap link generation of commerce product entities.
+ */

This docblock should not exist.

+  /**
+   * {@inheritdoc}
+   */
+  public function getQuery($bundle) {
+    return $this->database->select('commerce_product_field_data', 'p')
+      ->fields('p', array('product_id', 'changed'))
+      ->condition('type', $bundle)
+      ->condition('status', 1);
+  }

Entity storage should never be accessed directly. It's not good that the parent module requires this.

This docblock should not exist.

Thanks mate, going to look into it.

Entity storage should never be accessed directly. It's not good that the parent module requires this.

This is debatable. A simple sql query is the quickest way of getting all entity ids and the approach is universal across all entities. I can see your point however in that field storage may change for non-core modules rendering plugins unusable.

Using an entityQuery() instead of the sql query would make the generation slower (as entities will have to be loaded for additional data), but I could live with this compromise. Would the following work for you?

  /**
   * {@inheritdoc}
   */
  public function getQuery($bundle) {
    return \Drupal::entityQuery('commerce_product')
      ->condition('status', 1)
      ->condition('type', $bundle);
  }

I think the proposed change looks good. Except you will want to use dependency injection to get he entity query service.

Using an entityQuery() instead of the sql query would make the generation slower (as entities will have to be loaded for additional data),

While true, it does also make this easier to implemented for entities since you only need to know the type. But any API adds abstraction and technically bloats performance, but we then have standardization.

This has now been resolved within the simple_sitemap module, as as of 8.x-2.5 it makes better use of the entity API to facilitate link generating functionality for contrib entities like commerce product. This means commerce does not need to implement a plugin to be supported by simple_sitemap anymore.

Thanks for your input guys.