Skip to content

feat: add bieannial and triannial billing frequency#8

Open
HenningWendtland wants to merge 4 commits into
version-16from
extend-billing-frequency
Open

feat: add bieannial and triannial billing frequency#8
HenningWendtland wants to merge 4 commits into
version-16from
extend-billing-frequency

Conversation

@HenningWendtland

@HenningWendtland HenningWendtland commented Jun 15, 2026

Copy link
Copy Markdown
Member

Addition of biennial and triennial billing (every 2 and 3 years).
This was easily possible for the period_type == "start date" but not for "calendar months" type, so just implemented for the first type.
To improve the UI the correct types are displayed dynamically

Reference: AIT-46

@HenningWendtland HenningWendtland changed the title feat: ad bieannial and triannial billing frequency feat: add bieannial and triannial billing frequency Jun 15, 2026
@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The core period-computation logic is correct, well-tested, and backed by unit tests that cover both period types and edge cases around block boundaries.

The frequency arithmetic and billing-block alignment are mathematically sound, and the new test cases verify the boundary behaviour. The two noted concerns — a stale translation key and the first-block skip for mid-year CalendarMonths subscriptions — are non-blocking: one is a localisation quality issue and the other mirrors pre-existing behaviour for Yearly subscriptions.

Translation files (de.po / main.pot) should be regenerated after aligning the error message string in simple_subscription.py.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[get_from_and_to_date] --> B{period_type?}
    B -->|StartDate| C{billing_time?}
    B -->|CalendarMonths| D{billing_time?}
    C -->|AtBeginning| E[get_date_period\neval_date]
    C -->|AfterEnd| F[get_date_period\ncurrent_start - 1 day]
    D -->|AtBeginning| G[get_calendar_period\neval_date]
    D -->|AfterEnd| H[get_calendar_period\ncurrent_start - 1 day]
    G --> I{frequency in\nMULTI_YEAR?}
    H --> I
    I -->|No| J[INVOICE_MONTH_MAP\nlookup → from_date]
    I -->|Yes| K[block_start_year =\nstart_date.year +\nfloor div aligned span]
    K --> L[from_date = Jan 1\nof block_start_year]
    J --> M[to_date = from_date\n+ frequency months - 1 day]
    L --> M
    E --> N[months = frequency.value\nrelativedelta arithmetic]
    F --> N
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[get_from_and_to_date] --> B{period_type?}
    B -->|StartDate| C{billing_time?}
    B -->|CalendarMonths| D{billing_time?}
    C -->|AtBeginning| E[get_date_period\neval_date]
    C -->|AfterEnd| F[get_date_period\ncurrent_start - 1 day]
    D -->|AtBeginning| G[get_calendar_period\neval_date]
    D -->|AfterEnd| H[get_calendar_period\ncurrent_start - 1 day]
    G --> I{frequency in\nMULTI_YEAR?}
    H --> I
    I -->|No| J[INVOICE_MONTH_MAP\nlookup → from_date]
    I -->|Yes| K[block_start_year =\nstart_date.year +\nfloor div aligned span]
    K --> L[from_date = Jan 1\nof block_start_year]
    J --> M[to_date = from_date\n+ frequency months - 1 day]
    L --> M
    E --> N[months = frequency.value\nrelativedelta arithmetic]
    F --> N
Loading

Reviews (3): Last reviewed commit: "feat: implement biennial and trieannial ..." | Re-trigger Greptile

@HenningWendtland

Copy link
Copy Markdown
Member Author

@greptileai

@HenningWendtland

Copy link
Copy Markdown
Member Author

@greptileai

@barredterra barredterra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One last nitpick

Comment on lines +215 to +227
def get_calendar_period(
eval_date: date, frequency: Frequency, start_date: date | None = None
) -> tuple[date, date]:
"""Return the first and last day of the calendar period containing `eval_date`."""
if frequency in MULTI_YEAR_FREQUENCIES and not start_date:
frappe.throw(
_("Start Date is required for frequency {0}.").format(
_(frequency.name, context="Frequency of Subscription")
)
)

if frequency in MULTI_YEAR_FREQUENCIES:
years_span = frequency.value // 12

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
def get_calendar_period(
eval_date: date, frequency: Frequency, start_date: date | None = None
) -> tuple[date, date]:
"""Return the first and last day of the calendar period containing `eval_date`."""
if frequency in MULTI_YEAR_FREQUENCIES and not start_date:
frappe.throw(
_("Start Date is required for frequency {0}.").format(
_(frequency.name, context="Frequency of Subscription")
)
)
if frequency in MULTI_YEAR_FREQUENCIES:
years_span = frequency.value // 12
def get_calendar_period(
eval_date: date, frequency: Frequency, start_year: int | None = None
) -> tuple[date, date]:
"""Return the first and last day of the calendar period containing `eval_date`."""
if frequency in MULTI_YEAR_FREQUENCIES:
if not start_year:
raise ValueError("start_date is required for multi-year frequencies")
years_span = frequency.value // 12
  • This method only needs the start year, not the entire start date (less for the reader to think about)
  • Calendar utils were meant to be independent of frappe, so they should raise regular python exceptions instead of frappe.throw(). A framework-caller can then catch these and frappe.throw(). This keeps the separation between pure date math and framework clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants