Invoice cadence and granularity #21
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
==========================================
- Coverage 96.90% 95.74% -1.17%
==========================================
Files 76 92 +16
Lines 1650 2113 +463
==========================================
+ Hits 1599 2023 +424
- Misses 51 90 +39 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
* WIP: cadence and rate logic * sample_other * Implement proper elapsed_periods_since for YearMonthAndFortnight * more tests * more tests * set MSRV and README badges * make it possible to pass time off in hours. * Refactor TUI, removed superflous generics
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces invoice cadence and granularity features, allowing bi-weekly invoicing and flexible rate structures (hourly, daily, per fortnight, or monthly). It also replaces the TypedBuilder crate with Bon across the codebase and adds comprehensive testing infrastructure.
Key changes include:
- Added support for bi-weekly invoice cadence alongside existing monthly cadence
- Introduced granular rate structures (hourly, daily, fortnight, monthly) instead of forcing daily rates
- Replaced TypedBuilder with Bon builder pattern throughout the codebase
- Added extensive unit testing with
sample_other()implementations for better test coverage
Reviewed Changes
Copilot reviewed 152 out of 155 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/render/src/typst_context/content.rs | Updated builder derive from TypedBuilder to Builder |
| crates/render/src/render_test_helpers.rs | Added generic period type support and mocked exchange rate fetcher |
| crates/render/src/render.rs | Updated test cases to use new period types and rate structures |
| crates/render/src/lib.rs | Replaced TypedBuilder import with Bon Builder |
| crates/render/Cargo.toml | Updated dependencies: removed typed-builder, added bon and derive_more |
| crates/core/src/models/*.rs | Comprehensive updates replacing TypedBuilder with Builder throughout model definitions |
Comments suppressed due to low confidence (3)
crates/render/src/render_test_helpers.rs:50
- [nitpick] The generic parameter name 'Period' is ambiguous. Consider using a more descriptive name like 'TPeriod' or 'InvoicePeriod' to clearly indicate this is a type parameter for period types.
pub fn compare_image_against_expected<Period: IsPeriod>(
crates/render/src/render_test_helpers.rs:136
- [nitpick] The generic parameter name 'Period' is ambiguous. Consider using a more descriptive name like 'TPeriod' or 'InvoicePeriod' to match the naming convention used in the public function above.
fn generate_pdf_into_png_image<Period: IsPeriod>(
crates/core/src/models/data/submodels/year_month_and_fortnight.rs:742
- The test validates parsing but doesn't verify that the parsed values are correctly assigned to the struct fields. Consider adding assertions to check that sut.year, sut.month, and sut.half contain the expected values after parsing.
fn from_str_valid() {
Important
Breaking API change, you MUST re-run
klirr data initand input your info again. But before you do you can runklirr data dumpto print it out in the terminal for easier copy paste.New Features
Cadence::BiWeekly.klirr data dumpto see your data - even if it fails validation, useful for re-initInfra
TypedBuildercrate withBonsince TypedBuilder is not so actively maintained and the author recommendedBon- Bon has the advantage that custom validation (buildreturningResult<T>is very easily added). However, Bon has the disadvantage of worseIntoimpl, resulting in me having to sprinkleinto()all over the place.sample_otherfunction toHasSample,HasSample