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

Update asyncActions.js #3026

Closed
wants to merge 1 commit into from
Closed

Conversation

indranil-m
Copy link

The cart context should get the details if any error occurs

Description

TODO: Describe your changes in detail here.

Related Issue

Closes #ISSUE_NUMBER.

Acceptance

Verification Stakeholders

Specification

Verification Steps

  1. Go to the FOO page.
  2. Verify the BAR shows up.
  3. Make sure BAZ does a thing.

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.

The cart context should get the details if any error occurs
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Feb 26, 2021

Fails
🚫

node` failed.

🚫 A version label is required. A maintainer must add one.
🚫

Unit tests in the following files did not pass 😔. All tests must pass before this PR can be merged

  • packages/peregrine/lib/store/actions/cart/__tests__/asyncActions.spec.js
🚫

No linked issue found. Please link a relevant open issue by adding the text "closes #<issue_number>" or "closes JIRA-<issue_number>" in your PR.

🚫 Missing information in PR. Please fill out the "Verification Steps" section.
🚫 Missing information in PR. Please fill out the "Description" section.
🚫

The following file(s) were not formatted with prettier. Make sure to execute yarn run prettier locally prior to committing.

packages/peregrine/lib/store/actions/cart/asyncActions.js
Warnings
⚠️ Found the word "TODO" in the PR description. Just letting you know incase you forgot :)
Messages
📖 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.

Log

ERROR ON TASK: prettierCheck


ERROR ON TASK: unitTests


Error:  Danger had errors running. See message(s) above for more details.
danger-results://tmp/danger-results.json

If your PR is missing information, check against the original template here. At a minimum you must have the section headers from the template and provide some information in each section.

Generated by 🚫 dangerJS against 3c05d6e

@sirugh
Copy link
Contributor

sirugh commented Feb 26, 2021

Hello @indranil-m, would you please open an issue describing the problem and link it here as the PR template suggests? We will also need repro steps for the bug (what is wrong and how did you get it to occur) as well as steps we can use to check that this code fixes that problem.

@sirugh sirugh added the requires followup Some actions need to be taken after this code is merged. label Feb 26, 2021
@indranil-m
Copy link
Author

indranil-m commented Feb 26, 2021 via email

Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

Sorry, I don't understand the issue. We currently handle errors here and intentionally log and do not throw. I'd suggest opening an issue with a bug report so we can discuss what the problem is :)

@@ -347,10 +347,11 @@ export const getCartDetails = payload => {
try {
const { data } = await fetchCartDetails({
variables: { cartId },
fetchPolicy: 'network-only'
fetchPolicy: 'no-cache',
Copy link
Contributor

Choose a reason for hiding this comment

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

This was switched to network-only very recently to fix a bug: #2996
If you use no-cache then the mini cart will get out of date.

@sirugh
Copy link
Contributor

sirugh commented Mar 2, 2021

Closing this. In the future please use the BUG issue template to report any issues you might have found. This helps us assess and prioritize issues that arise. In this case, because there are no repro steps or an "expected vs actual", I can't verify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:peregrine requires followup Some actions need to be taken after this code is merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants