Changeset View
Standalone View
looper/templatetags/looper.py
| from decimal import Decimal | |||||
| from typing import Mapping, Any | |||||
| from django import template | from django import template | ||||
| from looper.models import PlanVariation | |||||
| import looper.money | |||||
| import looper.taxes | |||||
| register = template.Library() | register = template.Library() | ||||
| @register.filter | @register.filter | ||||
| def pricing(pricable) -> str: | def pricing(pricable) -> str: | ||||
| if pricable is None: | if pricable is None: | ||||
| return '-' | return '-' | ||||
| price: str = pricable.price.with_currency_symbol_nonocents() | price: str = pricable.price.with_currency_symbol_nonocents() | ||||
| if pricable.interval_length == 1: | if pricable.interval_length == 1: | ||||
| return f'{price} / {pricable.interval_unit}' | return f'{price} / {pricable.interval_unit}' | ||||
| return f'{price} / {pricable.interval_length}\u00A0{pricable.interval_unit}s' | return f'{price} / {pricable.interval_length}\u00A0{pricable.interval_unit}s' | ||||
| @register.filter | |||||
| def get_plan_variation(plan_variation_id: int) -> PlanVariation: | |||||
| """Return a PlanVariation with a matching ID, if it exists.""" | |||||
| return PlanVariation.objects.get(pk=plan_variation_id) | |||||
sybren: This conflicts with the declared return type. | |||||
| @register.simple_tag(takes_context=True) | |||||
| def get_taxable( | |||||
| context: Mapping[Any, Any], price: looper.money.Money, product_type: str | |||||
| ) -> looper.taxes.Taxable: | |||||
| user = context.get('user') | |||||
| tax_type = looper.taxes.TaxType.NO_CHARGE | |||||
| tax_rate = Decimal(0) | |||||
| if user and user.is_authenticated: | |||||
| customer = user.customer | |||||
| # TODO(anna): fallback to geo country when there's no Customer | |||||
| tax_type, tax_rate = customer.get_tax(product_type) | |||||
Done Inline ActionsI'm not a fan of "set these variables, and then immediately set them to something else" code. To me this is clearer: if not user or user.is_anonymous:
return looper.taxes.Taxable(price, tax_type=None, tax_rate=None)
tax_type, tax_rate = user.customer.get_tax(product_type)
return looper.taxes.Taxable(price, tax_type=tax_type, tax_rate=tax_rate)But I can also see the advantage of having one looper.taxes.Taxable() call. Maybe it shows that "get the tax type/rate for the current user" should be a different function. That function can then handle the differentiation between anonymous and authenticated users. If nothing else, this exercise shows that the call to looper.taxes.Taxable(..., None, None) is invalid, because neither Taxable.tax_type nor Taxable.tax_rate are allowed to be None. sybren: I'm not a fan of "set these variables, and then immediately set them to something else" code. | |||||
Done Inline ActionsThis code has been updated: at least there's no invalid call with Nones. railla: This code has been updated: at least there's no invalid call with `None`s.
I'd rather not… | |||||
Done Inline Actions👍 sybren: :+1: | |||||
| return looper.taxes.Taxable(price, tax_type=tax_type, tax_rate=tax_rate) | |||||
This conflicts with the declared return type.