-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add initial Brazil NFSe addon #402
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #402 +/- ##
==========================================
+ Coverage 78.88% 78.97% +0.09%
==========================================
Files 217 221 +4
Lines 10722 10773 +51
==========================================
+ Hits 8458 8508 +50
- Misses 1909 1910 +1
Partials 355 355 ☔ View full report in Codecov by Sentry. |
5659155
to
fefe7d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Some observations. Do the Charges and Discounts also need to be covered?
|
||
* https://www.planalto.gov.br/ccivil_03/leis/lcp/lcp116.htm | ||
`), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is is possible to add a Pattern
flag perhaps to validate the codes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose not to validate anything since every municipality can define their own list of codes without restrictions on the format. The list in LEI116 is commonly used, apparently, but there's no guarantee those will be the codes in the municipality.
} | ||
|
||
return validation.Validate(line, | ||
bill.RequireLineTaxCategory(br.TaxCategoryISS), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this also need to be repeated for Discount and Charge and lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I don't think it makes sense to support either Charges or Discounts: it doesn't seem possible to map them to an NFSe. So, I've added validations to ensure those aren't present in 177eb69
@samlown Thanks for your feedback! I think I addressed all your points and made an additional change (derived from your feedback as well) that let me get to 98% coverage. |
Describe your changes
Checklist before requesting a review
go generate .
to ensure that Schemas and Regime data are up to date.