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

fix: when there's no regions on the page #878

Merged
merged 2 commits into from
Feb 24, 2022

Conversation

jairo-bc
Copy link
Contributor

What?

Sometimes, when new theme is applied to the store, I don't receive regions object from the API.

Screenshots (if appropriate)

Screenshot 2022-02-18 at 17 57 12

cc @bigcommerce/storefront-team, @jkanive

@@ -31,6 +31,9 @@ async function getRenderedRegionsByPageType({ accessToken, storeUrl, pageType })
}),
});

if (!response.data.data) {
return { renderedRegions: []};
Copy link
Contributor

Choose a reason for hiding this comment

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

This would probably fix the problem from a stencil-cli perspective, but do we know if there a more root cause as to why this regions object isnt populating? Is it just not being passed in properly?

Copy link
Contributor

Choose a reason for hiding this comment

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

I had the same exact comment in the slack. I feel the issue is present somewhere else. getRenderedRegionByPageType has nothing to do with the theme. I have a feeling we are not passing down all the required params for the endpoint which could be the reason it's breaking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a follow up ticket to investigate this (posted over slack in threads). There is a 500 error from graphql.
Meanwhile, can we unblock with this temporary solution?
cc @jmwiese @jkanive @zvuki

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as we've captured the follow up work I think we can continue with this as a fix in the short term

@jairo-bc jairo-bc force-pushed the fix-issue-with-regions branch from f12499d to a94f205 Compare February 24, 2022 15:35
@jairo-bc jairo-bc requested a review from bc-jz February 24, 2022 16:02
@jairo-bc jairo-bc merged commit 1ad0520 into bigcommerce:master Feb 24, 2022
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.

3 participants