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: display of bank invoice information #199

Closed
wants to merge 3 commits into from

Conversation

gaboulsvtex
Copy link

@gaboulsvtex gaboulsvtex commented Dec 27, 2024

What is the purpose of this pull request?

Fix a bug.

What problem is this solving?

In orders where the bank invoice payment method wasn't the first payment of the first transaction, its information wasn't displayed at the order placed page

How should this be manually tested?

There are 2 use cases for this change:

  • Orders with a single transaction with multiple payments, in which the one of them is a bank invoice and it isn't in the first position of the payments array;
  • Orders with multiple transactions in which one of them has a bank invoice payment, and this transaction isn't the first of the array.

Screenshots or example usage

Example of an order with bank invoice in the second payment of the array, where its information is not displayed:
https://gaboulstore.myvtex.com/checkout/orderPlaced/?og=1486610500025
image

Example of the same order, but in a workspace with this change implemented:
https://bankinvoice--gaboulstore.myvtex.com/checkout/orderPlaced/?og=1486610500025
image

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.

@gaboulsvtex gaboulsvtex requested review from a team as code owners December 27, 2024 20:32
@gaboulsvtex gaboulsvtex requested review from gabpaladino, vsseixaso and RodrigoTadeuF and removed request for a team December 27, 2024 20:32
Copy link
Contributor

vtex-io-ci-cd bot commented Dec 27, 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 😞

@vtex-io-docs-bot
Copy link

vtex-io-docs-bot bot commented Dec 27, 2024

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 🎉🎉

@iago1501 iago1501 added bug Something isn't working enhancement New feature or request labels Dec 27, 2024
@iago1501 iago1501 requested review from iago1501 and vmourac-vtex and removed request for a team and RodrigoTadeuF December 27, 2024 21:33
Copy link
Contributor

@iago1501 iago1501 left a comment

Choose a reason for hiding this comment

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

Algumas alterações rápidas de boas práticas/legibilidade, depois de verificado se ainda resolve o problema, vou validar de fato a usabilidade da alteração pra ver o impacto que ela vai ter

react/Notices.tsx Outdated Show resolved Hide resolved
react/Notices.tsx Outdated Show resolved Hide resolved
manifest.json Outdated Show resolved Hide resolved
Copy link
Author

@gaboulsvtex gaboulsvtex left a comment

Choose a reason for hiding this comment

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

Obrigado pelas sugestões.
Em meu novo commit eu fiz apenas alguns ajustes em cima da sua sugestão:

  • Removi os nested ternaries da linha 69;
  • Corrigi a função sortPayments(), que estava invertida e o TypeScript não aceitou a operação aritmética entre booleanos.

@gaboulsvtex gaboulsvtex requested a review from iago1501 January 7, 2025 14:25
react/Notices.tsx Outdated Show resolved Hide resolved
@iago1501 iago1501 mentioned this pull request Jan 9, 2025
4 tasks
@iago1501
Copy link
Contributor

iago1501 commented Jan 9, 2025

@gaboulsvtex , eu fiz uma outra proposta de implementação, dado que a ideia de reordenar só seria um workaround pro problema principal (retornar apenas um método de pagamento quando possui vários), se puder checar pra ver se resolve o seu problema, creio que resolvendo, a gente encerra esse e segue pelo outro: #200

@iago1501 iago1501 closed this Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants