refactor: filter out variant media from product media query

[willopez]

Impact: minor
Type: bugfix

Issue

The top level product media resolver was returning media from variants. This causes all media in child variants and options to be rendered on the parent top level product.

Solution

Filter out variant media from product media resolved.

Breaking changes

NONE

query product($productId: ID!, $shopId: ID!, $shouldIncludeVariantMedia: Boolean) {
  product(productId: $productId, shopId: $shopId, shouldIncludeVariantMedia: $shouldIncludeVariantMedia) {
    media(shouldIncludeVariantMedia: $shouldIncludeVariantMedia {
    _id
    URLs {
      small
    }
  }
  }
}

Variables:

{
  "productId": "myProductId",
  "shopId": "myShopId",
  "shouldIncludeVariantMedia": false
}

Testing

1. Execute product media query above and verify that only media for the top level product is returned.

[aldeed]

@willopez I think this was an intentional design decision, but I don't remember for sure. IDK which way people expect, but there might be people relying on this behavior so this could be considered a breaking change. @spencern or @zenweasel what do you think?

[mikemurray]

@willopez, @aldeed is correct. It "was" intentional, though not properly displayed in the Admin UI.

The media bubbles up to the top. So If I upload an image at the Option level, then the parent Variant inherits the option media, and the top-level Product then inherits all the Variant and Option media.

This is so you can organize the top-level product images if you want, and so you don't have to upload images to every single level. You could just upload only option media, and leave the other levels blank.

This made sense when the products were displayed on the storefront from the products collection. Now that we get the info from the Catalog, we can still have that functionality there, while doing something different with Products.

---

I can see @willopez intent here because...

When displaying the product images, the bubble-up effect is useful. When uploading images in the Admin UI, having child media in the uploader, especially at the product level, is overwhelming.

Perhaps adding a param the getProductMedia called like shouldIncludeVariantMedia could be an option to keep the old functionality while allowing the new.

Otherwise, we can always filter out the media in the UI and display a better user interface for managing direct media dependencies, while also allowing the organization of all product media at the top-level.

[willopez]

Adding a param shouldIncludeVariantMedia seems good to me.

[spencern]

Sounds good to me 👍

[brent-hoover]

@aldeed We don't currently rely on this behavior because we don't use Product Media at all but I think for building an admin screen it would be good to have the flexibility that the shouldIncludeVariantMedia parameter would provide

[aldeed]

@willopez shouldIncludeVariantMedia sounds good to me as long as it defaults to true when not set so that current behavior doesn't break. I think it should be an argument on the Product.media field rather than on the query, so that you can use it anywhere that a Product type is returned.

query product($productId: ID!, $shopId: ID!) {
  product(productId: $productId, shopId: $shopId) {
    media(shouldIncludeVariantMedia: false) {
      _id
      URLs {
        small
      }
    }
  }
}

[willopez]

@mikemurray @aldeed @spencern shouldIncludeVariantMedia argument added.

[aldeed]

A few comments

[mikemurray]

👍

The requested changes have been made