Quick question - we're looking at upgrading our Wishlist Rest API module to 1.6.0 from 1.4.3 and we'

U017Y69D9U4

Quick question - we're looking at upgrading our Wishlist Rest API module to 1.6.0 from 1.4.3 and we've noticed that in the src/Spryker/Zed/WishlistsRestApi/Business/WishlistItem/WishlistItemDeleter.php on line 53 we're requiring the UUID of a Wishlist item - but wishlist items don't seem to have a UUID.

I believe previously this was checking whether a SKU had been provided which seems correct. Just wondering if this is a bug (I don't see any table schema changes related to this in this diff - https://github.com/spryker/wishlists-rest-api/compare/1.4.3...1.6.0 that would add a UUID column to the spy_wishlist_item table)?

U01A5ARAXP0

It seems the Uuid it was already in that earlier version. There is a console command (WishlistsUuidWriterConsole.php) which will generate and add uuid where missing

U017Y69D9U4

That command is deprecated in favour of using UuidGeneratorConsole which when run against that table returns an error to say there is no field in the table for UUID? 🤔

U017Y69D9U4

(exact message - Table spy_wishlist_item does not contain uuid field )

U02V3L4CKD4

Perhaps there’s a missing schema change in the Wishlist module (for context i’m working with @U017Y69D9U4 😅)

U01A5ARAXP0

the uuid behavior is added to the spy_wishlist table, not the spy_wishlist_item table

U017Y69D9U4

Yes that's correct - but in WishlistItemDeleter in the latest version Spryker seems to be checking for a UUID on the wishlist item

U02V3L4CKD4

src/Spryker/Zed/WishlistsRestApi/Business/WishlistItem/WishlistItemDeleter.php:53

U017Y69D9U4

$wishlistItemRequestTransfer->requireIdCustomer()
            ->requireUuidWishlist()
            ->requireUuid();

U017Y69D9U4

Whereas in the previous version of this module, it was:

$wishlistItemRequestTransfer->requireIdCustomer()
            ->requireUuidWishlist()
            ->requireSku();

U017Y69D9U4

Perhaps this was added erroneously if it's not meant to be on the wishlist item 🙂 :

https://github.com/spryker/wishlists-rest-api/blob/master/src/Spryker/Shared/WishlistsRestApi/Transfer/wishlists_rest_api.transfer.xml#L48

U01A5ARAXP0

Yes, that seems a bug then

U017Y69D9U4

Cool, I'll submit a bug report 👍

U017Y69D9U4

Which might lead me to the next question - this seems to have changed since I last raised a bug report - where do I now need to go to raise a bug ticket? 😄

Valerii Trots

@UQKSAARKN can you confirm please if this is really an issue?

U017Y69D9U4

It looks like it was this commit which introduced it (at least on the transfer object):

https://github.com/spryker/wishlists-rest-api/commit/08743e00c118aba94334a05b4d678f55da385f69

And instead of using a UUID, we're just setting a SKU string into it

Valerii Trots

Yep, wanted to ask @U0140NZMDTN as well but he looks like offline.

Dmitriy Aseev

Hey, Guys!
It's not real UUID, it is identifier (for wishlist item = SKU)
Check GLUE layer: \Spryker\Glue\WishlistsRestApi\Processor\WishlistItems\WishlistItemDeleter::createWishlistItemRequest

$wishlistUuid = $resource->getId();

return (new WishlistItemRequestTransfer())
     ->setUuid($idResource)
     ->setUuidWishlist($wishlistUuid)
     ->setIdCustomer($surrogateIdentifier);

U017Y69D9U4

Isn't that a bit misleading? Wouldn't it read better as SKU which is what is expected, rather than a UUID?

Dmitriy Aseev

Yes, I totally agreed, but we have to support some related features (and SKU can't be unique reference):
• In Product Offer feature we concatenate SKU+offer-reference and set it to UUID
• In Configurable product feature we concatenate SKU+configurable_product_hash and set it to UUID
But for other cases (concrete products) - we set SKU to UUID

U017Y69D9U4

Seems then that we are therefore mis-using the term UUID elsewhere - perhaps a field called uniqueIdentifier would be better than UUID ? 🙂

Dmitriy Aseev

Yes, you are right 👍
But for now, we couldn't rename it in order to keep BC