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

Feat/add attachments to cartitem #53

Merged
merged 6 commits into from
May 25, 2022

Conversation

icazevedo
Copy link
Contributor

@icazevedo icazevedo commented May 19, 2022

What's the purpose of this pull request?

Updates the starter to take attachments into account when calculating the item ID: vtex/faststore#1304

How does it work?

The implementation uses the same logic as FastStore API's getId function

How to test it?

Add items to the cart and check the browser storage (indexed db). It should work just like before, but with id separator as :: instead of the previous : (this change was introduced to match the logic inside FastStore API)

Checklist

You may erase this after checking them all ;)

  • Added an entry in the CHANGELOG.md at the beginning of its due section. The latest version should comes first.

  • Added the PR number with the PR link at the entry in the CHANGELOG.md. E.g., New items in the pull_request_template.md (#4)

  • PR description

  • Updated the Storybook - if applicable.

  • Added a label according to the PR goal - Breaking change, Enhancement, Bug or Chore.

  • Added the component, hook, or pathname in-between backticks (``) - If applicable. E.g., ComponentName component.

  • Identified the function or parameter in the PR - If applicable. E.g., useWindowDimensions hook.

  • For documentation changes, ping @carolinamenezes or @Mariana-Caetano to review and update the changes.

@icazevedo icazevedo self-assigned this May 19, 2022
@vercel
Copy link

vercel bot commented May 19, 2022

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

Name Status Preview Updated
gatsby-store-storybook ✅ Ready (Inspect) Visit Preview May 25, 2022 at 2:53PM (UTC)

@vtex-sites
Copy link

vtex-sites bot commented May 19, 2022

Lighthouse Reports

Here are the Lighthouse reports of this Pull Request

📝 Based on commit d1da4ea

Lighthouse Report by page
📎   /
📎   /apple-magic-mouse-99988212/p
📎   /office

@vtex-sites
Copy link

vtex-sites bot commented May 19, 2022

Preview is ready

This pull request generated a Preview

👀   Preview: https://sfj-d1da4ea--gatsby.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit d1da4ea

@vercel
Copy link

vercel bot commented May 25, 2022

Deployment failed with the following error:

Resource is limited - try again in 15 minutes (more than 100, code: "api-deployments-free-per-day").

@vercel
Copy link

vercel bot commented May 25, 2022

Deployment failed with the following error:

Resource is limited - try again in 10 minutes (more than 100, code: "api-deployments-free-per-day").

item.price,
serializeAdditionalProperty(item.itemOffered),
]
.filter(Boolean)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to filter? If addicionalProperty is null, than the final key will be ...::null which doesn't impact the final key. What do you think?

src/sdk/cart/validate.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@tlgimenes tlgimenes left a comment

Choose a reason for hiding this comment

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

only small nits, but overall, LGTM

Copy link
Contributor

@tlgimenes tlgimenes left a comment

Choose a reason for hiding this comment

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

only small nits, but overall, LGTM

@vercel
Copy link

vercel bot commented May 25, 2022

Deployment failed with the following error:

Resource is limited - try again in 13 seconds (more than 100, code: "api-deployments-free-per-day").

@tlgimenes tlgimenes merged commit d1da4ea into main May 25, 2022
@tlgimenes tlgimenes deleted the feat/add-attachments-to-cartitem-id branch May 25, 2022 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants