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?
Comment | File | Size | Author |
---|---|---|---|
#8 | interdiff-6-8.txt | 937 bytes | czigor |
#8 | commerce-3000794-published_variations-8.patch | 7.04 KB | czigor |
|
Comments
Comment #2
bojanz CreditAttribution: bojanz at Centarro commentedBack 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.
Comment #3
kmajzlik CreditAttribution: kmajzlik at Ciklum Western Europe commentedSo there is no reason to decline this and if i work on patch it can be accepted when ok?
Comment #4
bojanz CreditAttribution: bojanz at Centarro commentedI am hoping so. There might be impact that I haven't taken into account, but so far it sounds like a good idea.
Comment #5
bojanz CreditAttribution: bojanz at Centarro commentedGiving this issue a definitive title, since we agreed to do this.
Comment #6
czigor CreditAttribution: czigor at Centarro commentedComment #7
czigor CreditAttribution: czigor at Centarro commentedI wonder if we need an update hook to clear cache because of the annotation change.
Comment #8
czigor CreditAttribution: czigor at Centarro commentedAdded an update hook to add the new entity key.
Comment #9
bojanz CreditAttribution: bojanz at Centarro commentedLooks good!
We need to update all UI strings as well (Active/Inactive -> Published/Unpublished).
Comment #10
bojanz CreditAttribution: bojanz at Centarro commentedI'll wrap this up.
Comment #11
bojanz CreditAttribution: bojanz at Centarro commentedThis 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.
Comment #13
bojanz CreditAttribution: bojanz at Centarro commentedDone.