Wishlists don't implement EntityOwnerInterface because orders don't.
However, orders don't because the customer isn't able to freely change their order once it has been placed.
There is no such restriction here, the customer can always modify their own wishlist.
So, let's implement EntityOwnerInterface and remove the customer methods.
We should also add uid and owner entity keys, like we did for products and variations in https://www.drupal.org/project/commerce/issues/3007275#comment-12822472

CommentFileSizeAuthor
#12 commerce_wishlist-3023998-entity_owner_interface-12.patch1.77 KBczigor
#7 commerce_wishlist-3023998-entity_owner_interface-7.patch9.76 KBczigor
#5 commerce_wishlist-3023998-entity_owner_interface-5.patch9.63 KBczigor
#3 commerce_wishlist-3023998-entity_owner_interface-3.patch7.57 KBczigor
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz created an issue. See original summary.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented

Note that this will also simplify permissions, since we'll no longer need to declare our "own" permission.

czigor’s picture

czigor CreditAttribution: czigor at Centarro commented
Status: Active » Needs review
FileSize
commerce_wishlist-3023998-entity_owner_interface-3.patch7.57 KB
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented

We can remove WishlistAccessControlHandler and WishlistPermissionProvider completely.

czigor’s picture

czigor CreditAttribution: czigor at Centarro commented
FileSize
commerce_wishlist-3023998-entity_owner_interface-5.patch9.63 KB

Fixed it.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Needs work 
- *     "access" = "Drupal\commerce_wishlist\WishlistAccessControlHandler",
- *     "permission_provider" = "Drupal\commerce_wishlist\WishlistPermissionProvider",

Not like that :) We still need to specify the previous parents.

czigor’s picture

czigor CreditAttribution: czigor at Centarro commented
Status: Needs work » Needs review
FileSize
commerce_wishlist-3023998-entity_owner_interface-7.patch9.76 KB

Ah ok, fixed it.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro for Ny Media AS commented
Status: Needs review » Reviewed & tested by the community

Thanks, looks good now.

  • agoradesign committed 3f0bad0 on 8.x-3.x authored by czigor 
    Issue #3023998 by czigor, bojanz: Wishlists should implement...
agoradesign’s picture

agoradesign CreditAttribution: agoradesign commented
Status: Reviewed & tested by the community » Fixed

keep pushin' on...

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro for Ny Media AS commented
Status: Fixed » Needs work

We forgot a bit:

We should also add uid and owner entity keys, like we did for products and variations in https://www.drupal.org/project/commerce/issues/3007275#comment-12822472

Query access will crash without those.

czigor’s picture

czigor CreditAttribution: czigor at Centarro for Ny Media AS commented
Status: Needs work » Needs review
FileSize
commerce_wishlist-3023998-entity_owner_interface-12.patch1.77 KB

Added the missing keys.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro for Ny Media AS commented
Status: Needs review » Reviewed & tested by the community

Looks good.

  • agoradesign committed 11a2ae6 on 8.x-3.x authored by czigor 
    Issue #3023998 by czigor, bojanz: Wishlists should implement...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro for Ny Media AS commented
Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

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

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented

Was there any particular reason for not using EntityOwnerTrait??

agoradesign’s picture

agoradesign CreditAttribution: agoradesign commented

hmm I guess that trait didn't exist at that time

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented

I created #3118707: Use the EntityOwnerTrait.