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: policy and token checkout integration #46

Merged

Conversation

Rudge
Copy link
Contributor

@Rudge Rudge commented Jan 30, 2024

What problem is this solving?

When the user uses the quote, there is an error and the price of the quote isn't set in the cart.

How to test it?

Use this quote https://revendapjba.ferimport.com.br/b2b-quotes/6ace0564-b175-11ee-8452-12c497aaea6f?workspace=usequote

Workspace

Screenshots or example usage:

image

image

Copy link

vtex-io-ci-cd bot commented Jan 30, 2024

Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖

Please select which version do you want to release:

  • Patch (backwards-compatible bug fixes)

  • Minor (backwards-compatible functionality)

  • Major (incompatible API changes)

And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.

  • No thanks, I would rather do it manually 😞

@Rudge Rudge requested a review from a team January 30, 2024 15:52
Copy link

github-actions bot commented Jan 30, 2024

Messages
📖 ❤️ Thanks!
📖

🎉 PR additions = 18, PR deletions = 10

Generated by 🚫 dangerJS against fd186a6

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@@ -346,7 +346,7 @@ export const Mutation = {
const {
clients: { masterdata, hub },
vtex,
vtex: { account, logger, storeUserAuthToken },
vtex: { account, logger, authToken },
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the difference between authToken and storeUserAuthToken? I'm afraid this change could break some use case that uses storeUserAuthToken

Copy link
Contributor Author

@Rudge Rudge Jan 31, 2024

Choose a reason for hiding this comment

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

The main differences are here. If we use the store token, we need to give this permission to all shoppers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright, I think I got it. We are using the app token to communicate with checkout (instead of the user token), which in this case seems to make sense (due to the permission issue you mentioned)

@@ -346,7 +346,7 @@ export const Mutation = {
const {
clients: { masterdata, hub },
vtex,
vtex: { account, logger, storeUserAuthToken },
vtex: { account, logger, authToken },
Copy link
Contributor

Choose a reason for hiding this comment

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

Alright, I think I got it. We are using the app token to communicate with checkout (instead of the user token), which in this case seems to make sense (due to the permission issue you mentioned)

@Rudge Rudge merged commit 0c39146 into master Jan 31, 2024
12 checks passed
@Rudge Rudge deleted the hotfix/B2BTEAM-1245-fix-policy-and-token-checkout-integration branch January 31, 2024 15:09
Copy link

vtex-io-ci-cd bot commented Jan 31, 2024

Your PR has been merged! App is being published. 🚀
Version 2.5.0 → 2.5.1

After the publishing process has been completed (check #vtex-io-releases) and doing A/B tests with the new version, you can deploy your release by running:

vtex deploy [email protected]

After that your app will be updated on all accounts.

For more information on the deployment process check the docs. 📖

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.

2 participants