fix: Cart item with additionalProperty #1325

tlgimenes · 2022-05-27T22:20:17Z

What's the purpose of this pull request?

When adding a product to cart with additionalProperties we were receiving the following error:

GraphQLError [Object]: Variable "$additionalProperty" got invalid value { propertyID: "4ba0a4d37b7f733da5ddb281ea50d883", name: "activeSubscriptions", value: "abonnements", valueReference: "SPECIFICATION" }; Field "propertyID" is not defined by type IStorePropertyValue.

This PR addresses this issue

Starters

vercel · 2022-05-27T22:20:20Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
faststore	✅ Ready (Inspect)	Visit Preview	May 30, 2022 at 2:12PM (UTC)

icazevedo

Doesn't this defeat the whole purpose of not having to serialize the attachment id on the frontend?

tlgimenes · 2022-05-30T12:30:29Z

Doesn't this defeat the whole purpose of not having to serialize the attachment id on the frontend?

Not really. We still don't need to serialize anything on the frontend.

icazevedo · 2022-05-30T12:41:10Z

Doesn't this defeat the whole purpose of not having to serialize the attachment id on the frontend?

Not really. We still don't need to serialize anything on the frontend.

It's not clear to me how it wouldn't. This is a required property on input, and we use it for the validateCart mutation. When people are sending attachments to the cart with the items, they don't have the id yet because they're creating this object on the frontend, and to have it, they must serialize it.

packages/api/src/platforms/vtex/utils/propertyValue.ts

codesandbox-ci · 2022-05-30T12:43:41Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 00fef03:

Sandbox	Source
Store UI Typescript	Configuration

tlgimenes · 2022-05-30T14:37:02Z

Doesn't this defeat the whole purpose of not having to serialize the attachment id on the frontend?

Not really. We still don't need to serialize anything on the frontend.

It's not clear to me how it wouldn't. This is a required property on input, and we use it for the validateCart mutation. When people are sending attachments to the cart with the items, they don't have the id yet because they're creating this object on the frontend, and to have it, they must serialize it.

Ok, I think I've precipitated myself on openning this PR that early, however I think it's working now.

We had two issues. The first one is that when fetching a product, additionalProperties was fetched alongside the propertyID. When adding this product to cart, this propertyID would be send to the backend, causing this error:

GraphQLError [Object]: Variable "$additionalProperty" got invalid value { propertyID: "4ba0a4d37b7f733da5ddb281ea50d883", name: "activeSubscriptions", value: "abonnements", valueReference: "SPECIFICATION" }; Field "propertyID" is not defined by type IStorePropertyValue.

To fix this issue, I added a non required field on IStorePropertyValue

The second issue was that adding a product to cart with additionalProperties would result in the item being removed from cart. This was because of the orderForm item lacking specifications and only having attachments. Adding a filter to getId functions seems to have fixed this issue.

A good case for this PR is this store , that has specifications set and after this PR, should have the bug fixed and the cart should work as intended. Also, I added links to this PR on our official starters

icazevedo

I understand the issues now. Thanks!

icazevedo · 2022-05-30T14:44:53Z

packages/api/src/platforms/vtex/resolvers/validateCart.ts


 const getId = (item: IStoreOffer) =>
  [
    item.itemOffered.sku,
    item.seller.identifier,
    item.price,
-    item.itemOffered.additionalProperty?.map(getPropertyId).join('-'),
+    item.itemOffered.additionalProperty
+      ?.filter(isAttachment)


Shouldn't this function be the same in the starters as well? How can we manage the fact that the starters can't know what attachments are?

On the starters side I think it's ok. The issue arises when mixing checkout API with @faststore/api. If you are using only one or the other I think you're safe

I see. So by that logic, we are able to insert VTEX concepts in the starters, as long as they follow @faststore/api schemas, right? That way we can guarantee they won't break if used with another platform (although it's not going to be a graceful experience).

Yeah, I mean, we do that with the channel. Channel is a concept of @faststore, but what goes inside that string differs from each platform.

tlgimenes requested a review from a team as a code owner May 27, 2022 22:20

vercel bot deployed to Preview May 27, 2022 22:25 View deployment

icazevedo reviewed May 28, 2022

View reviewed changes

tlgimenes force-pushed the fix/propertyid branch from 3a26303 to 900fba1 Compare May 30, 2022 12:33

icazevedo reviewed May 30, 2022

View reviewed changes

packages/api/src/platforms/vtex/utils/propertyValue.ts Outdated Show resolved Hide resolved

vercel bot deployed to Preview May 30, 2022 12:46 View deployment

id non required

cb2f0b1

tlgimenes force-pushed the fix/propertyid branch from 599d0bf to cb2f0b1 Compare May 30, 2022 13:14

vercel bot deployed to Preview May 30, 2022 13:20 View deployment

fix validateCart logic

00fef03

vercel bot deployed to Preview May 30, 2022 14:12 View deployment

tlgimenes changed the title ~~fix property id~~ fix: Cart item with additionalProperty May 30, 2022

tlgimenes mentioned this pull request May 30, 2022

fix: Cart item with additionalProperty vtex-sites/gatsby.store#74

Merged

icazevedo reviewed May 30, 2022

View reviewed changes

icazevedo approved these changes May 30, 2022

View reviewed changes

tlgimenes merged commit 8fa6aa1 into main May 30, 2022

tlgimenes deleted the fix/propertyid branch May 30, 2022 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Cart item with additionalProperty #1325

fix: Cart item with additionalProperty #1325

tlgimenes commented May 27, 2022 •

edited

Loading

vercel bot commented May 27, 2022 •

edited

Loading

icazevedo left a comment

tlgimenes commented May 30, 2022

icazevedo commented May 30, 2022

codesandbox-ci bot commented May 30, 2022 •

edited

Loading

tlgimenes commented May 30, 2022 •

edited

Loading

icazevedo left a comment

icazevedo May 30, 2022

tlgimenes May 30, 2022

icazevedo May 30, 2022

tlgimenes May 30, 2022 •

edited

Loading

icazevedo May 30, 2022

fix: Cart item with additionalProperty #1325

fix: Cart item with additionalProperty #1325

Conversation

tlgimenes commented May 27, 2022 • edited Loading

What's the purpose of this pull request?

Starters

vercel bot commented May 27, 2022 • edited Loading

icazevedo left a comment

Choose a reason for hiding this comment

tlgimenes commented May 30, 2022

icazevedo commented May 30, 2022

codesandbox-ci bot commented May 30, 2022 • edited Loading

tlgimenes commented May 30, 2022 • edited Loading

icazevedo left a comment

Choose a reason for hiding this comment

icazevedo May 30, 2022

Choose a reason for hiding this comment

tlgimenes May 30, 2022

Choose a reason for hiding this comment

icazevedo May 30, 2022

Choose a reason for hiding this comment

tlgimenes May 30, 2022 • edited Loading

Choose a reason for hiding this comment

icazevedo May 30, 2022

Choose a reason for hiding this comment

tlgimenes commented May 27, 2022 •

edited

Loading

vercel bot commented May 27, 2022 •

edited

Loading

codesandbox-ci bot commented May 30, 2022 •

edited

Loading

tlgimenes commented May 30, 2022 •

edited

Loading

tlgimenes May 30, 2022 •

edited

Loading