Skip to content

Commit b75a654

Browse files
Sajjonclaude
andauthored
feat: off-on-bank-holidays setting with per-invoice override (#48)
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). For hourly rates each weekday holiday removes 8h. 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 whole year is fetched and cached at once; only holidays falling on a weekday within the billed period are deducted. The vendor's free-text country is resolved to an ISO 3166-1 alpha-2 code via a generated table covering all 122 countries the Nager API serves, with aliases (England/UK->GB, USA/America->US, Sverige->SE) and accent-free spellings (Turkey, Aland Islands). 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. Add a per-invoice `--worked-holidays` (`-w`) override that treats holidays in the target period as worked (zero deduction), overriding the persisted setting. It is checked before any country lookup or network request. Also fix net payment terms beyond 31 days: NetDays reused the calendar `Day` type (validated 1..=31), so "Net 35"/"Net 60"/"Net 90" failed to parse. Add a dedicated `DueDays` newtype (1..=365) used by NetDays, the DueInDays trait, and Date::advance_days. Crates: - foundation: CountryCode + 122-country resolver, BankHolidays, BankHolidaysFetcher (new `bank-holidays` feature), holiday-aware working-day counting, DueDays. - core-invoice: ServiceFees flag, resolve_bank_holidays, ValidInput.worked_holidays, NetDays/PaymentTerms on DueDays. - cli: off-on-bank-holidays prompt in the wizard; --worked-holidays flag. Tests cover the resolver (all 122 codes, aliases, accents), the fetcher (mocked, cache hit/miss, no network), holiday deduction in the calendar, the CLI flag, the override short-circuit, and Net-term parsing beyond 31 days. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ffabf03 commit b75a654

27 files changed

Lines changed: 1768 additions & 75 deletions

.tarpaulin.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
[all]
22
exclude-files = [
33
"crates/foundation/src/exchange_rates.rs",
4+
"crates/foundation/src/bank_holidays.rs",
5+
"crates/core-invoice/src/logic/prepare_data/bank_holidays.rs",
46
"crates/render-typst/src/compare_images.rs",
57
"crates/render-typst/src/render.rs",
68
"crates/cli/src/init_logging.rs",

_typos.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
extend-exclude = [
33
"crates/core-invoice/src/models/l10n/swedish.rs",
44
"crates/core-invoice/src/models/l10n/snapshots/klirr_core_invoice__models__l10n__localization__tests__l10n_swedish.snap",
5+
# Flat table of ISO country codes and names (e.g. "ba"->BA, "fo"->FO,
6+
# "viet nam") which the spell checker flags as typos.
7+
"crates/foundation/src/models/country_code.rs",
58
]
69

710
[default.extend-words]

crates/cli/src/input/get_input/get_input.rs

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,21 @@ pub struct InvoiceInput {
7979
#[arg(long, short = 'e')]
8080
#[builder(default = false)]
8181
email: bool,
82+
83+
/// Treat bank holidays in the target period as worked, deducting none of
84+
/// them from billable days. Overrides the vendor's `off_on_bank_holidays`
85+
/// setting for this invoice only; has no effect if that setting is off.
86+
#[arg(long, short = 'w')]
87+
#[builder(default = false)]
88+
#[getset(get = "pub")]
89+
worked_holidays: bool,
90+
91+
/// Re-fetch bank holidays from the API instead of using the cached copy,
92+
/// then update the cache. Use this if a country's published holidays change.
93+
#[arg(long, short = 'r')]
94+
#[builder(default = false)]
95+
#[getset(get = "pub")]
96+
refresh_holidays: bool,
8297
}
8398

8499
impl InvoiceInput {
@@ -128,6 +143,8 @@ impl InvoiceInput {
128143
.layout(*self.layout())
129144
.items(items)
130145
.language(*self.language())
146+
.worked_holidays(self.worked_holidays)
147+
.refresh_holidays(self.refresh_holidays)
131148
.maybe_maybe_output_path(self.out)
132149
.maybe_email(email_config)
133150
.build();
@@ -276,6 +293,36 @@ mod tests {
276293
let input = CliArgs::parse_from([BINARY_NAME, "invoice"]);
277294
assert_eq!(input.command.unwrap_invoice().out, None);
278295
}
296+
297+
#[test]
298+
fn test_input_parsing_worked_holidays_flag() {
299+
let input = CliArgs::parse_from([BINARY_NAME, "invoice", "--worked-holidays"]);
300+
assert!(input.command.unwrap_invoice().worked_holidays);
301+
}
302+
303+
#[test]
304+
fn test_input_parsing_worked_holidays_short_flag() {
305+
let input = CliArgs::parse_from([BINARY_NAME, "invoice", "-w"]);
306+
assert!(input.command.unwrap_invoice().worked_holidays);
307+
}
308+
309+
#[test]
310+
fn test_input_parsing_worked_holidays_default() {
311+
let input = CliArgs::parse_from([BINARY_NAME, "invoice"]);
312+
assert!(!input.command.unwrap_invoice().worked_holidays);
313+
}
314+
315+
#[test]
316+
fn test_input_parsing_refresh_holidays_flag() {
317+
let input = CliArgs::parse_from([BINARY_NAME, "invoice", "--refresh-holidays"]);
318+
assert!(input.command.unwrap_invoice().refresh_holidays);
319+
}
320+
321+
#[test]
322+
fn test_input_parsing_refresh_holidays_default() {
323+
let input = CliArgs::parse_from([BINARY_NAME, "invoice"]);
324+
assert!(!input.command.unwrap_invoice().refresh_holidays);
325+
}
279326
}
280327

281328
mod tests_parsed_input {
@@ -322,6 +369,27 @@ mod tests {
322369
);
323370
}
324371

372+
#[test]
373+
fn test_input_parsing_worked_holidays_threads_to_valid_input() {
374+
let input = InvoiceInput::builder().worked_holidays(true).build();
375+
let input = input.parsed(Cadence::Monthly).unwrap();
376+
assert!(*input.worked_holidays());
377+
}
378+
379+
#[test]
380+
fn test_input_parsing_worked_holidays_defaults_to_false() {
381+
let input = InvoiceInput::builder().build();
382+
let input = input.parsed(Cadence::Monthly).unwrap();
383+
assert!(!*input.worked_holidays());
384+
}
385+
386+
#[test]
387+
fn test_input_parsing_refresh_holidays_threads_to_valid_input() {
388+
let input = InvoiceInput::builder().refresh_holidays(true).build();
389+
let input = input.parsed(Cadence::Monthly).unwrap();
390+
assert!(*input.refresh_holidays());
391+
}
392+
325393
#[test]
326394
#[should_panic]
327395
fn test_input_parsing_out_at_root_crashes() {

crates/cli/src/input/tui/helpers/build_service_fees.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use inquire::{CustomType, Text, error::InquireResult};
1+
use inquire::{Confirm, CustomType, Text, error::InquireResult};
22

33
use crate::{
44
Cadence, Granularity, InvoiceDataFromTuiError, Rate, Result, ServiceFees, UnitPrice,
@@ -33,10 +33,20 @@ pub fn build_service_fees(default: &ServiceFees) -> Result<ServiceFees> {
3333

3434
let rate = Rate::from((unit_price, granularity));
3535

36+
let off_on_bank_holidays = Confirm::new("Off on bank holidays?")
37+
.with_help_message(
38+
"If yes, public holidays in the vendor's country are deducted from billable \
39+
working days (day/hour rates only), looked up online and cached. Don't also \
40+
log a holiday as time off, or it is deducted twice.",
41+
)
42+
.with_default(*default.off_on_bank_holidays())
43+
.prompt()?;
44+
3645
Ok(ServiceFees::builder()
3746
.name(name)
3847
.cadence(cadence)
3948
.rate(rate)
49+
.off_on_bank_holidays(off_on_bank_holidays)
4050
.build()
4151
.unwrap())
4252
}

crates/core-invoice/Cargo.toml

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ documentation = "https://docs.rs/klirr-core-invoice"
99
repository = "https://github.com/Sajjon/klirr"
1010

1111
[dependencies]
12-
klirr-foundation = { workspace = true, features = ["crypto", "exchange-rates"] }
12+
klirr-foundation = { workspace = true, features = [
13+
"crypto",
14+
"exchange-rates",
15+
"bank-holidays",
16+
] }
1317

1418
bon.workspace = true
1519
chrono.workspace = true

crates/core-invoice/src/logic/calendar_logic.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
2-
Cadence, Date, Error, Granularity, InvoiceNumber, Quantity, RecordOfPeriodsOff, RelativeTime,
3-
Result, TimestampedInvoiceNumber,
2+
BankHolidays, Cadence, Date, Error, Granularity, InvoiceNumber, Quantity, RecordOfPeriodsOff,
3+
RelativeTime, Result, TimestampedInvoiceNumber,
44
};
55
use klirr_foundation::{
66
CalendarError, calculate_period_number, normalize_period_end_date_for_cadence as normalize,
@@ -145,6 +145,7 @@ pub fn calculate_invoice_number(
145145
/// Granularity::Day,
146146
/// Cadence::Monthly,
147147
/// &RecordOfPeriodsOff::default(),
148+
/// &BankHolidays::default(),
148149
/// )
149150
/// .unwrap();
150151
///
@@ -155,9 +156,16 @@ pub fn quantity_in_period(
155156
granularity: Granularity,
156157
cadence: Cadence,
157158
record_of_periods_off: &RecordOfPeriodsOff,
159+
bank_holidays: &BankHolidays,
158160
) -> Result<Quantity> {
159-
quantity_in_period_inner(target_date, granularity, cadence, record_of_periods_off)
160-
.map_err(map_calendar_error)
161+
quantity_in_period_inner(
162+
target_date,
163+
granularity,
164+
cadence,
165+
record_of_periods_off,
166+
bank_holidays,
167+
)
168+
.map_err(map_calendar_error)
161169
}
162170

163171
#[cfg(test)]
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
use crate::{BankHolidays, CountryCode, Data, Date, Granularity};
2+
use log::{debug, warn};
3+
4+
/// The disk-cached bank-holiday fetcher, re-exported from the foundation crate.
5+
pub type BankHolidaysFetcher<T = ()> = klirr_foundation::BankHolidaysFetcher<T>;
6+
7+
/// Resolves the public holidays to deduct from billable days for an invoice,
8+
/// degrading gracefully so holiday lookup never blocks PDF generation.
9+
///
10+
/// Returns an empty set (no deduction) when:
11+
/// - `worked_holidays` is set (the per-invoice `--worked-holidays` override),
12+
/// - the rate granularity is `Month`/`Fortnight` (holidays only affect day- and
13+
/// hour-billed invoices),
14+
/// - the vendor has not opted into `off_on_bank_holidays`,
15+
/// - the vendor's free-text country cannot be mapped to an ISO country code, or
16+
/// - the holiday API request fails (and nothing is cached).
17+
///
18+
/// Otherwise it returns the (possibly cached) holidays for the vendor's country
19+
/// in the invoice period's year.
20+
///
21+
/// The override and granularity checks come first, so an invoice for which
22+
/// holidays are irrelevant never triggers a country lookup or network request.
23+
///
24+
/// When `refresh` is `true`, a cache hit is ignored and holidays are re-fetched
25+
/// from the API (the `--refresh-holidays` flag), picking up any corrections.
26+
pub fn resolve_bank_holidays(
27+
data: &Data,
28+
target_period_end_date: &Date,
29+
worked_holidays: bool,
30+
refresh: bool,
31+
) -> BankHolidays {
32+
if worked_holidays {
33+
debug!("--worked-holidays set; deducting no bank holidays for this invoice.");
34+
return BankHolidays::default();
35+
}
36+
37+
let granularity = data.service_fees().rate().granularity();
38+
if !matches!(granularity, Granularity::Day | Granularity::Hour) {
39+
debug!(
40+
"Rate granularity {granularity:?} is unaffected by bank holidays; \
41+
skipping holiday resolution."
42+
);
43+
return BankHolidays::default();
44+
}
45+
46+
if !data.service_fees().off_on_bank_holidays() {
47+
return BankHolidays::default();
48+
}
49+
50+
let country_name = data.vendor().postal_address().country();
51+
let Some(country_code) = CountryCode::from_country_name(country_name) else {
52+
warn!(
53+
"off_on_bank_holidays is enabled but vendor country '{country_name}' could not be \
54+
resolved to an ISO country code; skipping bank-holiday deduction."
55+
);
56+
return BankHolidays::default();
57+
};
58+
59+
let year = *target_period_end_date.year();
60+
debug!("Resolving bank holidays for {country_code} {year} (refresh: {refresh}).");
61+
match BankHolidaysFetcher::default().holidays_for(&country_code, year, refresh) {
62+
Ok(holidays) => holidays,
63+
Err(error) => {
64+
warn!(
65+
"Failed to fetch bank holidays for {country_code} {year}: {error}. \
66+
Skipping bank-holiday deduction."
67+
);
68+
BankHolidays::default()
69+
}
70+
}
71+
}
72+
73+
#[cfg(test)]
74+
mod tests {
75+
use super::*;
76+
use crate::{HasSample, ServiceFees};
77+
use test_log::test;
78+
79+
/// Builds invoice data with a daily rate, the given `off_on_bank_holidays`
80+
/// setting, and vendor country, reusing the sample vendor for everything else.
81+
fn data_with(off_on_bank_holidays: bool, country: &str) -> Data {
82+
data_with_rate(
83+
off_on_bank_holidays,
84+
country,
85+
crate::Rate::daily(rust_decimal::dec!(100.0)),
86+
)
87+
}
88+
89+
/// As [`data_with`], but with an explicit rate so granularity can be varied.
90+
fn data_with_rate(off_on_bank_holidays: bool, country: &str, rate: crate::Rate) -> Data {
91+
let service_fees = ServiceFees::builder()
92+
.name("Consulting".to_string())
93+
.rate(rate)
94+
.cadence(crate::Cadence::Monthly)
95+
.off_on_bank_holidays(off_on_bank_holidays)
96+
.build()
97+
.unwrap();
98+
99+
let vendor = crate::CompanyInformation::sample_vendor();
100+
let address = vendor
101+
.postal_address()
102+
.clone()
103+
.with_country(country.to_string());
104+
let vendor = vendor.with_postal_address(address);
105+
106+
Data::builder()
107+
.information(crate::ProtoInvoiceInfo::sample())
108+
.vendor(vendor)
109+
.client(crate::CompanyInformation::sample_client())
110+
.payment_info(crate::PaymentInformation::sample())
111+
.service_fees(service_fees)
112+
.expensed_periods(crate::ExpensedPeriods::sample())
113+
.build()
114+
}
115+
116+
#[test]
117+
fn disabled_flag_returns_empty() {
118+
// Sample data has off_on_bank_holidays = false.
119+
let data = Data::sample();
120+
let period_end = Date::sample();
121+
assert!(resolve_bank_holidays(&data, &period_end, false, false).is_empty());
122+
}
123+
124+
#[test]
125+
fn enabled_with_unresolved_country_returns_empty_without_network() {
126+
// A vendor whose country cannot be mapped must degrade to empty without
127+
// attempting (or depending on) any network call.
128+
let data = data_with(true, "Atlantis");
129+
assert!(resolve_bank_holidays(&data, &Date::sample(), false, false).is_empty());
130+
}
131+
132+
#[test]
133+
fn worked_holidays_override_wins_even_when_enabled_without_network() {
134+
// off_on_bank_holidays is on AND the country (Sweden) is resolvable, so
135+
// without the override this would hit the network. The --worked-holidays
136+
// override must short-circuit to empty before any country lookup/fetch.
137+
let data = data_with(true, "Sweden");
138+
assert!(resolve_bank_holidays(&data, &Date::sample(), true, false).is_empty());
139+
}
140+
141+
#[test]
142+
fn refresh_alone_degrades_gracefully_for_unresolved_country() {
143+
// refresh=true still degrades to empty (no panic / no dependency) when
144+
// the country can't be resolved.
145+
let data = data_with(true, "Atlantis");
146+
assert!(resolve_bank_holidays(&data, &Date::sample(), false, true).is_empty());
147+
}
148+
149+
#[test]
150+
fn monthly_granularity_skips_resolution_without_network() {
151+
// A fixed monthly rate is unaffected by holidays, so even with the
152+
// setting on and a resolvable country, no country lookup/fetch happens.
153+
let data = data_with_rate(
154+
true,
155+
"Sweden",
156+
crate::Rate::monthly(rust_decimal::dec!(50000.0)),
157+
);
158+
assert!(resolve_bank_holidays(&data, &Date::sample(), false, false).is_empty());
159+
}
160+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
mod bank_holidays;
12
mod exchange_rates;
23
#[allow(clippy::module_inception)]
34
mod prepare_input_data;
45

6+
pub use bank_holidays::*;
57
pub use exchange_rates::*;
68
pub use prepare_input_data::*;

crates/core-invoice/src/logic/prepare_data/prepare_input_data.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
22
Currency, Data, ExchangeRates, ExchangeRatesMap, Item, LineItemsPricedInSourceCurrency,
3-
PreparedData, Result, ValidInput,
3+
PreparedData, Result, ValidInput, normalize_period_end_date_for_cadence, resolve_bank_holidays,
44
};
55
use log::debug;
66
use log::info;
@@ -32,7 +32,15 @@ pub fn prepare_invoice_input_data(
3232
fetcher: impl FetchExchangeRates,
3333
) -> Result<PreparedData> {
3434
info!("Preparing invoice input data for PDF generation...");
35-
let partial = data.to_partial(input)?;
35+
let cadence = *data.service_fees().cadence();
36+
let target_period_end_date = normalize_period_end_date_for_cadence(*input.date(), cadence)?;
37+
let bank_holidays = resolve_bank_holidays(
38+
&data,
39+
&target_period_end_date,
40+
*input.worked_holidays(),
41+
*input.refresh_holidays(),
42+
);
43+
let partial = data.to_partial(input, &bank_holidays)?;
3644
let currency = *partial.payment_info().currency();
3745
let exchange_rates = fetcher.fetch_for_line_items(currency, partial.line_items())?;
3846
let data_typst_compat = partial.to_typst(exchange_rates)?;

0 commit comments

Comments
 (0)