Skip to content

Complete PT regime and SAF-T addon #453

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

Merged
merged 10 commits into from
Feb 4, 2025
Merged

Complete PT regime and SAF-T addon #453

merged 10 commits into from
Feb 4, 2025

Conversation

cavalle
Copy link
Contributor

@cavalle cavalle commented Jan 29, 2025

This PR completes the PT regime and the SAF-T by adding:

  • Product type extension
  • European Article Number (EAN) identity
  • Series and code format validation
  • Validations for all required extensions
  • Normalizations to set default codes for several extensions
  • Support for foreign tax rates (i.e. customer-rates)
  • A new stamp to store the full signature hash

Part of APP-101

Pre-Review Checklist

  • I have performed a self-review of my code.
  • I have added thorough tests with at least 90% code coverage.
  • I've modified or created example GOBL documents to show my changes in use, if appropriate.
  • When adding or modifying a tax regime or addon, I've added links to the source of the changes, either structured or in the comments.
  • I've run go generate . to ensure that the Schemas and Regime data are up to date.
  • All linter warnings have been reviewed and fixed.
  • I've been obsessive with pointer nil checks to avoid panics.
  • The CHANGELOG.md has been updated with an overview of my changes.
  • Requested a review from @samlown.

Copy link

codecov bot commented Jan 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.08%. Comparing base (00a47a1) to head (6c87de8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #453      +/-   ##
==========================================
+ Coverage   84.57%   85.08%   +0.51%     
==========================================
  Files         250      254       +4     
  Lines       12348    12518     +170     
==========================================
+ Hits        10443    10651     +208     
+ Misses       1553     1518      -35     
+ Partials      352      349       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cavalle cavalle marked this pull request as ready for review January 29, 2025 09:39
@cavalle cavalle requested a review from samlown January 29, 2025 09:39
Copy link
Collaborator

@samlown samlown left a comment

Choose a reason for hiding this comment

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

This is looking really nice! I've added a few suggestions that are worth discussing before merge.

@samlown
Copy link
Collaborator

samlown commented Jan 30, 2025

BTW, I'd also aim for 100% test coverage if possible. I've been forcing myself recently, and feeling happier about it :-)

@cavalle cavalle requested a review from samlown January 30, 2025 08:58
@cavalle
Copy link
Contributor Author

cavalle commented Jan 30, 2025

BTW, I'd also aim for 100% test coverage if possible. I've been forcing myself recently, and feeling happier about it :-)

I also like large coverage as it often reveals issues you didn't anticipate. However, taken to the extreme, it forces to test certain logic in private functions that is there only as mere safeguards (cases that must not happen). I'm not always sure the effort of covering those cases is worth it. But I'll see if I can get to 100% here.

@cavalle
Copy link
Contributor Author

cavalle commented Jan 30, 2025

100% coverage achieved!

Copy link
Collaborator

@samlown samlown left a comment

Choose a reason for hiding this comment

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

🔝 🎩

@samlown samlown merged commit 6c7a5ff into main Feb 4, 2025
2 of 4 checks passed
@samlown samlown deleted the saft branch February 4, 2025 11:53
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