Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PDPs show intended Product only #2697

Merged
merged 2 commits into from
Sep 11, 2020
Merged

Conversation

supernova-at
Copy link
Contributor

@supernova-at supernova-at commented Sep 11, 2020

Description

Venia does not support Grouped, Bundled, or Virtual products yet, but the Venia sample data does include them.
This PR adds a bit of filtering logic into the useProduct root component talon to only return the product we requested (by url_key).

Related Issue

Closes PWA-895.

Acceptance

Verification Stakeholders

Specification

Verification Steps

  1. Set your MAGENTO_BACKEND_URL to a 2.4.1+ backend (this PR deployment may be that already)
  2. Go to the /gold-veritas-cuff-set.html page
  3. See that you are presented with the simple product, not the bundled product set

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have updated the documentation accordingly, if necessary.

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Sep 11, 2020

Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Associated JIRA tickets: PWA-895.

Generated by 🚫 dangerJS against 7c2ec53

@supernova-at supernova-at added the version: Patch This changeset includes backwards compatible bug fixes. label Sep 11, 2020
Copy link
Contributor

@tjwiebell tjwiebell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion to future proof this fix a little more. Test coverage to awesome 🙌 thanks for adding that.

return null;
}

const supportedProducts = data.products.items.filter(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To future proof this fix (in a distant future where we support grouped products), we can instead just search for the url_key we intended to query for. const product = data.product.items.find(item => item.url_key === urlKey)

// The product isn't in the cache and we don't have a response from GraphQL yet.
return null;
// Pick the first supported product.
return mapProduct(supportedProducts[0]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't believe mapProduct is still around, it was a compatibility fix for 2.3.1 which is long out of support. I'll create a tech debt task to remove it, which should clean this talon up a little bit.

@supernova-at supernova-at changed the title Venia to only show supported Products PDPs show intended Product only Sep 11, 2020
@dpatil-magento dpatil-magento merged commit 6da17c1 into develop Sep 11, 2020
@dpatil-magento dpatil-magento deleted the supernova/895_products branch September 11, 2020 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:peregrine Progress: done version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants