In #3000794: ProductVariation should implement EntityPublishedInterface (deprecate isActive / setActive) the isActive() and setActive() interface methods were deprecated in favour of isPublished() and setPublished(). At the same time, the relevant entity key mapping was changed from 'status' to 'published'.

Other contrib modules may need to support older Commerce versions for some time yet, so these should continue to use the deprecated methods.

But the isActive() and setActive() methods are now broken, and need to use the 'published' entity key to work correctly. Patch to come.

CommentFileSizeAuthor
#2 commerce-invalid-active-method-key-3021064-2.patch710 bytesJohn Pitcairn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

John Pitcairn created an issue. See original summary.

John Pitcairn’s picture

John Pitcairn CreditAttribution: John Pitcairn commented
Assigned: John Pitcairn » Unassigned
Priority: Minor » Major
Status: Active » Needs review
FileSize
commerce-invalid-active-method-key-3021064-2.patch710 bytes

And patch. I wasn't sure whether to change the key the methods use, or to restore the old 'status' key to the entity annotation and leave the methods as-is. I opted for the former.

  • bojanz committed bf24a29 on 8.x-2.x authored by John Pitcairn 
    Issue #3021064 by John Pitcairn: Deprecated active() methods are now...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Fixed

This is definitely the cleaner option.

Thank you John, appreciated.

agoradesign’s picture

agoradesign CreditAttribution: agoradesign commented
Status: Fixed » Needs work

While the getter now looks good, the setter looks very broken now, because you don't use the entity key, but try to set the property named 'published'. sorry for not providing a patch, but it should be something like that instead:

    $key = $this->getEntityType()->getKey('published');
    $this->set($key, (bool) $active);
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented

We need to bring back the removed test coverage for the two methods.

  • bojanz committed 9fc7354 on 8.x-2.x 
    Issue #3021064 by bojanz, agoradesign: Deprecated active() methods are...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs work » Fixed

Third try :)

Status: Fixed » Closed (fixed)

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