PWA-989: Real Data Wishlists and Wishlist Create #3041

eug123 · 2021-03-03T22:32:36Z

Description

Hook up the Wishlist page to pull actual real data from GraphQL (remove all the mock data)
Hook up the Create Wishlist button on the Favorites / Wishlist page so that it actually fires the createWishlist GQL mutation (and updates the page)

Related Issue

Closes PWA-989

Acceptance

Wishlist page should use real data from backend
Should be possible to create new wishlist, when new wishlist created - it appears on the page

Verification Stakeholders

@dpatil-magento

Specification

Verification Steps

Create a Private wish list and check the UI.
Create a Public wish list and check the UI.
Verify existing list are shown when user log-in.
Verify Private and Public lists with Max and Min characters and check the UI.
Verify functionality of Cancel Button
Verify wishlists are not accessible for guest users.
Verify Auth user can access wish list of their account only.
Verify UI on other browsers and devices (only if any UI changes)
Run MFTF

Screenshots / Screen Captures (if appropriate)

Checklist

I have added tests to cover my changes, if necessary.
I have added translations for new strings, if necessary.
I have updated the documentation accordingly, if necessary.

PWAStudioBot · 2021-03-03T22:43:45Z

	Messages
📖	Associated JIRA tickets: PWA-989.
📖	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).
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 77e12c2

tjwiebell

Some minor signature improvements to future proof, but overall looks great. I did run into an issue now that this is hooked up to live data.

If you create a brand new Customer, it appears the first wishlist is not automatically created via GraphQL, so the /wishlist page hangs. Likely an existing bug where some assumptions were made with mocked data that aren't correct. It does appear Luma will automatically create your first list for you, so we'll need to check with product if that's something Venia should do as well, or just render the page with no lists.

packages/peregrine/lib/talons/WishlistPage/createWishlist.gql.js

packages/peregrine/lib/talons/WishlistPage/useCreateWishlist.js

…o into PWA-989

tjwiebell

Clarified some points about refetch logic, this is real close. Ping me on Slack if you have any questions.

tjwiebell · 2021-03-12T17:43:38Z

packages/peregrine/lib/talons/WishlistPage/useCreateWishlist.js

+                    variables: {
+                        input: data
+                    },
+                    refetchQueries: [{ query: getCustomerWishlistsQuery }],


Right implementation, wrong query 😄 this ended up adding another network round trip. Here's a screenshot of network requests with that mutation, which additionally showcases why we should be giving operations unique names as well.

This will be a little more involved, but totally worth it.

Migrate wishlistPage.gql.js to Peregrine. Update useWishlistPage talon to use the defaultOperations merge pattern.

import { GET_CUSTOMER_WISHLIST } from './wishlistPage.gql'

Use that query here

Remove GET_CUSTOMER_WISHLISTS from createWishlist.gql.js

Additionally, you can scale down the response of CREATE_WISHLIST to just id, since we expect to refetch here anyway

Have not checked network(. Corrected it.

Thank you

tjwiebell · 2021-03-12T17:45:37Z

packages/venia-ui/lib/components/WishlistPage/wishlist.js

+    const visibilityLabel =
+        visibility === 'PUBLIC'
+            ? formatMessage({
+                  id: 'wishlist.publicText',
+                  defaultMessage: 'Public'
+              })
+            : formatMessage({
+                  id: 'wishlist.privateText',
+                  defaultMessage: 'Private'
+              });


Should we use the globals here instead?

Suggested change

const visibilityLabel =

visibility === 'PUBLIC'

? formatMessage({

id: 'wishlist.publicText',

defaultMessage: 'Public'

})

: formatMessage({

id: 'wishlist.privateText',

defaultMessage: 'Private'

});

const visibilityLabel =

visibility === 'PUBLIC'

? formatMessage({

id: 'global.public',

defaultMessage: 'Public'

})

: formatMessage({

id: 'global.private',

defaultMessage: 'Private'

});

Yes it makes sense for this phrase "global", and these global phrases already used in other wishlist component)
Fixed.
Thank you

sirugh · 2021-03-19T14:57:19Z

packages/peregrine/lib/talons/WishlistPage/__tests__/useCreateWishlist.spec.js


-        return root.findByType('i').props.talonProps;
-    };
+const createWishlistMock = {


How do you test the loading state of this mutation? I'm trying to do this in my PR but I'm having trouble...

Nvm, found it :D Just don't await the act'ed click.

dpatil-magento · 2021-03-21T04:04:36Z

Thanks @eug123 those 2 fixes looks good. Please check following -

On Sign-Out Apollo cache still has wishlist info saved. I think wish list data should be cleared on sign-out given its customer specific.

When Enable Multiple Wish Lists is not enabled then customer is not allowed to create one wishlist also.

The Enable Multiple Wish Lists can be store specific but currently once I create on enabled store the same lists can be viewed on non-enabled store view also.
CE default has wishlist and not Create List button.

Don't think we need to extend the scope of this PR, First issue is something I believe needs a fix but on other 3 observations lets Demo to @tkacheva and create a follow up story around admin configs if needed.

eug123 · 2021-03-22T21:20:50Z

@dpatil-magento,
Issues 1 and 2 are fixed, please check.

Thank you

tjwiebell

Let's simplify the new utility as much we can, but otherwise the rest looks good 👍 ping me if you have questions.

tjwiebell · 2021-03-22T21:49:15Z

packages/peregrine/lib/Apollo/clearWishlistDataFromCache.js

+ *
+ * @param {ApolloClient} client
+ */
+export const clearWishlistDataFromCache = async client => {


I'm not sure about another utility function here, since I would consider wishlist part of customer data. Fine to keep this, but would be better to have clearCustomerDataFromCache call it.

Alternatively, there is a data security pattern we might want to establish. The clearCustomerDataFromCache will clear entities that start with Customer and I think we've just gotten lucky so far that they followed this pattern. You could instead define keyFields for Wishlist and WishlistItem, and this extra utility function wouldn't be needed.

Wishlist: { keyFields: ({ id }) => `CustomerWishlist:${id}` }, WishlistItem: { keyFields: ({ id }) => `CustomerWishlistItem:${id}` },

Changed according your recommendations.
Thank you!

tjwiebell

Changes look perfect, nice catch covering all the WishlistItem implementations. It does look like Coveralls is dinging us a tiny bit for not testing the params of these new keyField functions, but not a blocker for approval.

dpatil-magento · 2021-03-23T16:59:30Z

QA Approved. Currently unable to check when multiple wishlist is disabled how the wishlist gonna look but will test it in pr 3048

evanc420 · 2021-03-24T17:13:39Z

packages/extensions/venia-sample-language-packs/i18n/fr_FR.json

@@ -350,8 +350,6 @@
    "validation.mustBeChecked": "Doit être vérifié.",
    "validation.validatePassword": "Un mot de passe doit contenir au moins 3 des éléments suivants: minuscules, majuscules, chiffres, caractères spéciaux.",
    "wishlist.emptyListText": "Il n'y a actuellement aucun élément dans cette liste",
-    "wishlist.privateText": "Privée",
-    "wishlist.publicText": "Publique",


packages/extensions/venia-sample-language-packs/i18n/fr_FR.json

PWA-989: Real Data Wishlists and Wishlist Create

56c450c

m2-community-project bot added the Progress: ready for review label Mar 3, 2021

PWAStudioBot added pkg:peregrine pkg:venia-ui labels Mar 3, 2021

eug123 added 3 commits March 5, 2021 10:03

PWA-989: Real Data Wishlists and Wishlist Create

7366994

PWA-989: Real Data Wishlists and Wishlist Create

fb11c50

Merge remote-tracking branch 'mainline/develop' into PWA-989

597f55d

eug123 added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Mar 9, 2021

Merge branch 'develop' into PWA-989

f0a90cd

eug123 marked this pull request as ready for review March 9, 2021 23:05

tjwiebell suggested changes Mar 11, 2021

View reviewed changes

m2-community-project bot assigned tjwiebell Mar 11, 2021

m2-community-project bot added Progress: changes requested and removed Progress: ready for review labels Mar 11, 2021

eug123 added 2 commits March 11, 2021 16:56

PWA-989: Real Data Wishlists and Wishlist Create

7908916

Merge branch 'PWA-989' of github.com:magento-obsessive-owls/pwa-studi…

586d663

…o into PWA-989

m2-community-project bot added Progress: review and removed Progress: changes requested labels Mar 11, 2021

Merge branch 'develop' into PWA-989

6bbfa3d

tjwiebell suggested changes Mar 12, 2021

View reviewed changes

m2-community-project bot added Progress: changes requested and removed Progress: review labels Mar 12, 2021

PWA-989: Real Data Wishlists and Wishlist Create

36712af

m2-community-project bot removed the Progress: changes requested label Mar 12, 2021

sirugh reviewed Mar 19, 2021

View reviewed changes

PWA-989: Real Data Wishlists and Wishlist Create

4b366a6

eug123 requested a review from tjwiebell March 22, 2021 21:23

tjwiebell suggested changes Mar 22, 2021

View reviewed changes

m2-community-project bot added Progress: changes requested and removed Progress: review labels Mar 22, 2021

PWA-989: Real Data Wishlists and Wishlist Create

8a907e3

m2-community-project bot added Progress: review and removed Progress: changes requested labels Mar 23, 2021

tjwiebell previously approved these changes Mar 23, 2021

View reviewed changes

m2-community-project bot added Progress: approved and removed Progress: review labels Mar 23, 2021

eug123 added 2 commits March 23, 2021 10:03

Merge branch 'develop' into PWA-989

52227e3

PWA-989: Real Data Wishlists and Wishlist Create

77e12c2

eug123 dismissed tjwiebell’s stale review via 77e12c2 March 23, 2021 15:52

tjwiebell approved these changes Mar 23, 2021

View reviewed changes

dpatil-magento merged commit c11ed3d into magento:develop Mar 23, 2021

dpatil-magento deleted the PWA-989 branch March 23, 2021 16:59

m2-community-project bot added Progress: done and removed Progress: approved labels Mar 23, 2021

evanc420 reviewed Mar 24, 2021

View reviewed changes

m2-community-project bot removed the Progress: done label Mar 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PWA-989: Real Data Wishlists and Wishlist Create #3041

PWA-989: Real Data Wishlists and Wishlist Create #3041

eug123 commented Mar 3, 2021 •

edited

Loading

PWAStudioBot commented Mar 3, 2021 •

edited

Loading

tjwiebell left a comment

tjwiebell left a comment

tjwiebell Mar 12, 2021

eug123 Mar 12, 2021

tjwiebell Mar 12, 2021

eug123 Mar 12, 2021

sirugh Mar 19, 2021

sirugh Mar 19, 2021

dpatil-magento commented Mar 21, 2021

eug123 commented Mar 22, 2021

tjwiebell left a comment

tjwiebell Mar 22, 2021

eug123 Mar 23, 2021

tjwiebell left a comment

dpatil-magento commented Mar 23, 2021

evanc420 Mar 24, 2021

PWA-989: Real Data Wishlists and Wishlist Create #3041

PWA-989: Real Data Wishlists and Wishlist Create #3041

Conversation

eug123 commented Mar 3, 2021 • edited Loading

Description

Related Issue

Acceptance

Verification Stakeholders

Specification

Verification Steps

Screenshots / Screen Captures (if appropriate)

Checklist

PWAStudioBot commented Mar 3, 2021 • edited Loading

tjwiebell left a comment

Choose a reason for hiding this comment

tjwiebell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpatil-magento commented Mar 21, 2021

eug123 commented Mar 22, 2021

tjwiebell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjwiebell left a comment

Choose a reason for hiding this comment

dpatil-magento commented Mar 23, 2021

Choose a reason for hiding this comment

eug123 commented Mar 3, 2021 •

edited

Loading

PWAStudioBot commented Mar 3, 2021 •

edited

Loading