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

Boleto Validator - Support "convenio" and return which partial is wrong #55

Open
renatoagds opened this issue Oct 25, 2019 · 10 comments
Open
Labels
help wanted Extra attention is needed

Comments

@renatoagds
Copy link

renatoagds commented Oct 25, 2019

This issue propose 2 features:

  1. Add support for "convenio" in "boleto" validator, since it related formatter (format-boleto) supports "convenio"

  2. Returns which partial is wrong in is-valid-boleto. To avoid break the current contract, we could just add a second parameter, that will be a flag to turn on this feature.


I could work on both, just checking if are valid features

@renatoagds
Copy link
Author

cc/ @hyanmandian

@hyanmandian
Copy link
Member

Hello @renatoagds, we are working in a fork project to refactoring the entire project and we plan to create 2 separated functions for that: format-bank-slip and format-covenant (I really don't know if these are the best names for that functions). If you need to format both cases, you can just compose these functions (or maybe we can create another function for that).

About is-valid-boleto, I think that we can fix this issue together with the refactoring.

Today I will create a branch with this fork and you could work with us :D

Thanks for the ideas and sorry for the delay.

@hyanmandian
Copy link
Member

Hey @renatoagds can you check next branch and confirm if we fixed your issue?

@renatoagds
Copy link
Author

@hyanmandian for sure! will test today and let you know.

@renatoagds
Copy link
Author

renatoagds commented Jan 14, 2020

@hyanmandian Confirming that's not the expected.

  1. The next branch doesn't include support for convenio in format and isValid (this is actually a regression, since format in master supports convenio).

  2. The current isValid for boleto doesn't return which block is wrong, it only returns false/true.

Codesandbox that I used: https://codesandbox.io/s/brazilian-utils-rc-4qrj4

Can I help with the requested features in next?

@hyanmandian
Copy link
Member

Yes, I can help! What you think to add a second param to "config" the expected return? Something like:

isValidBoleto("1541541587487") // false
isValidBoleto("1541541587487", { block: true }) // [false, false, false, false, false, false]

I think that is better because we don't break our current API and it's work for all packages we have.

@hyanmandian
Copy link
Member

About convenio I think we need to create another package to do that and another package to format and validate boleto and convenio together, something like isValidBoletoCovenant or isValidBoletoConvenio and don't know a good name for convenio :(

@renatoagds
Copy link
Author

renatoagds commented Jan 14, 2020

What you think to add a second param to "config" the expected return?

That's exactly what I was thinking about. We could call this new param options maybe.

About convenio I think we need to create another package to do that

Why do you think this is necessary? That's some case that wouldn't use convenio? I think we could split the internal logic between convenio and conventional boleto, but in the final package, it should be exposed just one function, that detects if starts with 8 and switch between handlers.

@hyanmandian
Copy link
Member

I think is better to have 3 packages: boleto, convenio and boletoOrConvenio (we need to think in better names, ok? HahahHAh). In that way, we cover all use cases, what you think?

@renatoagds
Copy link
Author

In that way, we cover all use cases, what you think?

Sounds good! 🚀

@hyanmandian hyanmandian added the help wanted Extra attention is needed label Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants