Skip to content

feat: deduct vendor-country bank holidays from billable days#47

Closed
Sajjon wants to merge 1 commit into
mainfrom
feat/off-on-bank-holidays
Closed

feat: deduct vendor-country bank holidays from billable days#47
Sajjon wants to merge 1 commit into
mainfrom
feat/off-on-bank-holidays

Conversation

@Sajjon

@Sajjon Sajjon commented Jun 1, 2026

Copy link
Copy Markdown
Owner

Summary

Adds an opt-in off_on_bank_holidays flag to invoice ServiceFees. When enabled, public holidays in the vendor's country are deducted from billable working days for day- and **hour-**granularity rates (monthly/fortnightly fixed rates are unaffected — a fixed monthly fee doesn't shrink for a holiday).

Closes the feature request to be "off on bank holidays".

Feasibility outcome

  • API: Nager.Date — free, no key, no rate limit, 195 countries incl. 🇸🇪 SE / 🇺🇸 US / 🇬🇧 GB (verified against the live AvailableCountries endpoint). Sweden is fully covered by the global API; the Swedish-specific dryg.net is not needed.
  • Chosen over the offline py-holidays-rs crate (beta, 1★ — supply-chain risk).

Design (mirrors existing patterns)

  • Online fetch + RON disk-cache fallback, modeled 1:1 on the existing exchange-rate fetcher. Cache at ~/.klirr/cached_holidays.ron, keyed ISO country → year; repeat runs work offline.
  • Name → ISO mapping: the vendor's free-text country string is resolved via a curated alias table (Sweden/SverigeSE, England/UKGB, USAUS, ~45 countries).
  • Graceful degradation: an unmappable country or a failed lookup logs a warning and skips the deduction — it never blocks PDF generation.
  • Backward compatible: flag defaults to false and deserializes from legacy RON via #[serde(default)].

Changes by crate

  • foundation: CountryCode newtype + resolver; BankHolidays newtype; BankHolidaysFetcher behind a new bank-holidays feature; working_days_between/quantity_in_period now skip holiday dates (holidays on weekends are no-ops).
  • core-invoice: ServiceFees.off_on_bank_holidays; resolve_bank_holidays wired into prepare_invoice_input_data.
  • cli: yes/no prompt in the service-fees wizard.

Tests

  • Country resolver (Sweden/UK/US aliases, unknown → None).
  • Holiday fetch with mocked responses (Public-type filter, cache hit/miss, gibberish-cache reset) — no network in tests.
  • working_days_between deducting a known holiday; holiday-on-weekend no-op; hours = days×8 deduction.
  • ServiceFees legacy-RON deserialization (no field → false).
  • Graceful-degrade path (enabled flag + unmappable country → empty, no network).

All unit + doc tests pass; clippy clean for new code.

Note on CI/fixtures: the render-typst image-fixture tests are environment-sensitive (imagemagick) and fail identically on a clean main on the dev machine (same similarity value), so they're unrelated to this change — samples use the default flag=false and render identically.

🤖 Generated with Claude Code

Add an opt-in `off_on_bank_holidays` flag to invoice ServiceFees. When set,
public holidays in the vendor's country are deducted from billable working
days for day- and hour-granularity rates (monthly/fortnightly fixed rates are
unaffected).

Holidays are fetched from the free Nager.Date API and cached to disk
(~/.klirr/cached_holidays.ron), mirroring the existing exchange-rate
fetch+cache pattern, so repeat runs work offline. The vendor's free-text
country is mapped to an ISO 3166-1 alpha-2 code via a name->code table
(Sweden/Sverige->SE, England/UK->GB, USA->US, ...).

Resolution degrades gracefully: an unmappable country or a failed lookup logs
a warning and skips the deduction rather than failing PDF generation. The flag
defaults to false and deserializes from legacy RON (serde default), preserving
prior behavior.

- foundation: CountryCode + resolver, BankHolidays newtype, BankHolidaysFetcher
  (new `bank-holidays` feature), holiday-aware working-day counting.
- core-invoice: ServiceFees flag, resolve_bank_holidays wired into prepare.
- cli: yes/no prompt in the service-fees wizard.

Note: the render-typst fixture image tests are environment-sensitive
(imagemagick) and fail identically on a clean main on this machine; they are
unaffected by this change (samples use the default flag=false).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Sajjon Sajjon closed this Jun 1, 2026
@Sajjon Sajjon deleted the feat/off-on-bank-holidays branch June 1, 2026 10:41
@codecov

codecov Bot commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.11111% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.96%. Comparing base (b9c0ca6) to head (10bbe98).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/foundation/src/bank_holidays.rs 63.76% 25 Missing ⚠️
...re-invoice/src/logic/prepare_data/bank_holidays.rs 50.00% 7 Missing ⚠️
...-invoice/src/models/data/submodels/service_fees.rs 0.00% 1 Missing ⚠️
crates/foundation/src/models/country_code.rs 98.46% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #47      +/-   ##
==========================================
- Coverage   97.07%   95.96%   -1.11%     
==========================================
  Files         104      108       +4     
  Lines        2287     2452     +165     
==========================================
+ Hits         2220     2353     +133     
- Misses         67       99      +32     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant