Product implements it now - via EntityPublishedTrait (with method setPublished() ) - see #2870672: Product should implement EntityPublishedInterface.
Why Variation uses own method setActive instead of this standardized way?
For me Published and Active means same thing.

I need to publish/unpublish Products and Variations based on date (coming from Migration) via Scheduled Executable. I had a plan with common Action plugin accepting both entity types via ContentEntityInterface. But i have to create separate plugin(or logic) for the both entity types.

So: is there some reason why not to implement EntityPublishedInterface (and probably mark setActive() method as deprecated) or should i create patch?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

karlos007 created an issue. See original summary.

bojanz’s picture

Back when variations were implemented, there was no such thing as EntityPublishedInterface. We converted products, which were already node-like, but didn't find any benefit to doing so (deprecating old methods, introducing new ones) for variations.

For now your best bet is to have two actions. I am willing to consider converting variations, but it's not something we'd land in the next month, due to other work in progress.

kmajzlik’s picture

So there is no reason to decline this and if i work on patch it can be accepted when ok?

bojanz’s picture

I am hoping so. There might be impact that I haven't taken into account, but so far it sounds like a good idea.

bojanz’s picture

Title: Should Variation implement EntityPublishedInterface » ProductVariation should implement EntityPublishedInterface (deprecate isActive / setActive)

Giving this issue a definitive title, since we agreed to do this.

czigor’s picture

Status: Active » Needs review
FileSize
6.03 KB
czigor’s picture

I wonder if we need an update hook to clear cache because of the annotation change.

czigor’s picture

Added an update hook to add the new entity key.

bojanz’s picture

Status: Needs review » Needs work

Looks good!

+    $row['status'] = $entity->isPublished() ? $this->t('Active') : $this->t('Inactive');

We need to update all UI strings as well (Active/Inactive -> Published/Unpublished).

bojanz’s picture

Assigned: Unassigned » bojanz

I'll wrap this up.

bojanz’s picture

+  $definition_update_manager->updateEntityType('commerce_product_variation');

This method doesn't allow a string to be passed. The whole update hook needs to be replaced, we used one for products already, we can just copy it.

Also, the variation field label is still "Active". We should be able to use the published trait's definition as a base.

  • bojanz committed 7977ad4 on 8.x-2.x authored by czigor
    Issue #3000794 by czigor, bojanz: ProductVariation should implement...
bojanz’s picture

Status: Needs work » Fixed

Done.

Status: Fixed » Closed (fixed)

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