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 order again button #20

Closed
wants to merge 2 commits into from
Closed

Conversation

enzomerca
Copy link

@enzomerca enzomerca commented Aug 30, 2023

What is the purpose of this pull request?

https://vtex-dev.atlassian.net/browse/B2BTEAM-1187

Due to a change on the OMS API used to retrieve order information (see: vtex-apps/b2b-organizations-graphql#124), now the clientProfileData.email field returns information a bit different. Before it would return something like [email protected] and now it returns something like: [email protected].

So, to decide if we should show the order again button or not, instead of using == to compare the email strings, we check if the order email starts with the same value as the current user email.

What problem is this solving?

Properly show the order again button after changing the OMS api used to retrieve order information (as the email field is returned a bit different now).

How should this be manually tested?

Install fixed versions of b2b-organizations-graphql and b2b-orders-history apps, create an order and use the admin UI to complete it. After that, do the following:

Access Orders (/orders-history) from My Account;
Click on Order Again;
A cart with the items from a previous order should be created (previously an error page would appear with the message "Sorry, an error occurred while processing your request.")

Screenshots or example usage

See description of: vtex-apps/b2b-organizations-graphql#124

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Requires change to documentation, which has been updated accordingly.

@vtex-io-ci-cd
Copy link

vtex-io-ci-cd bot commented Aug 30, 2023

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 😞

@vtex-io-docs-bot
Copy link

vtex-io-docs-bot bot commented Aug 30, 2023

Beep boop 🤖

I noticed you didn't make any changes at the docs/ folder

  • There's nothing new to document 🤔
  • I'll do it later 😞

In order to keep track, I'll create an issue if you decide now is not a good time

  • I just updated 🎉🎉

@enzomerca enzomerca requested a review from a team August 30, 2023 19:16
@enzomerca enzomerca marked this pull request as ready for review August 30, 2023 19:17
@@ -50,7 +49,7 @@ class OrderActions extends Component {
paymentData: { transactions },
} = order

const isOwner = email === this.state.loggedInEmail
const isOwner = email.startsWith(this.state.loggedInEmail)

Choose a reason for hiding this comment

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

How sure are we that this new email field will contain the original value as a prefix? It would be good to get a confirmation from the OMS team (if you haven't done it yet, of course). Maybe Mourão can help confirm that, if needed (I couldn't mention him here, he's probably no within this vtex-apps org yet).

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I was planning to reachout to them to understand the difference between the 2 endpoints. I will check this before merging this PR.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.14) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@enzomerca
Copy link
Author

dropped.

@enzomerca enzomerca closed this Dec 12, 2023
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