Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix buyerIp warning #836

Merged
merged 5 commits into from
May 10, 2023
Merged

Fix buyerIp warning #836

merged 5 commits into from
May 10, 2023

Conversation

frandiox
Copy link
Contributor

@frandiox frandiox commented May 1, 2023

The issue mentioned in this discussion #814 has two parts:

  1. We were adding buyerIp directly to the SFAPI request headers (so it worked) but we were not doing it via hydrogen-react, which was showing a warning. -- 🟩 Fixed in this PR
  2. MiniOxygen is not providing a buyerIP header for Hydrogen to forward to the SFAPI. This will be fixed in the MiniOxygen package. -- 🟩 Fixed in MiniOxygen

@frandiox frandiox requested a review from a team May 1, 2023 07:15
Comment on lines 165 to 166
defaultHeaders[STOREFRONT_REQUEST_GROUP_ID_HEADER] =
storefrontHeaders?.requestGroupId || requestGroupId || generateUUID();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@frehner Not related to this PR but should we move STOREFRONT_REQUEST_GROUP_ID_HEADER to hydrogen-react?

Copy link
Contributor

Choose a reason for hiding this comment

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

Everything that is common to using the SFAPI (and not specific to Hydrogen) should be in hydrogen-react, yeah.

Copy link
Contributor

@frehner frehner left a comment

Choose a reason for hiding this comment

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

Thanks!

@frandiox
Copy link
Contributor Author

frandiox commented May 2, 2023

Going to wait until a new version of MiniOxygen is released with 2 fixed, since that's also required to hide the warning.

@frandiox frandiox merged commit 9c2e67c into 2023-04 May 10, 2023
@frandiox frandiox deleted the fd-fix-buyerip branch May 10, 2023 02:34
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants