Closed rene-armida closed 9 years ago
create_order
can be called with a discount_percent
but the discount_percent
is not necessarily coming from a Coupon
. For example when we create a discount for 3-month, 6-month, or 12-month pre-paid (saas/views/billing.py: get_invoicable_options), it is a discount without a Coupon
.
The saas/humanize.py contains some templates strings we use for some description.
@smirolo I hadn't thought of discounts without coupons.
Would you still like me to move the code to saas/views/billing.py
like you suggested? I guess that would mean the caller would have to specify a description, and the view would build the description and pass it along?
yes. please do so. it is a good compromise until we find a better idea. thanks.
@smirolo the more I look at the create_order
function, the more I'm confused by it.
The goal appears to be to create a database record in response to an order that has already been processed. It's a static, and doesn't have much to do with the containing Subscription
class, so let's ignore that for now.
It supports convenience methods for figuring out what to store in the database - namely a description and amount for the transaction. The description can be auto-generated, if not already provided. The amount can be automatically calculated if a discount applies.
There's a subtle interaction between these features: if the description is supplied, then the discount percentage is ignored, and the amount cannot be automatically calculated.
Now we want to push the generation of description strings to the view layer, which makes MVC sense, but makes this optional convenience thing even more complicated: if descr
isn't provided, but discount_descr
is, then we build the string as requested. However, if descr
is provided, we ignore discount_descr
along with discount_percent
.
It seems to me like descr
should never be optional, and create_order
should accept one of two sets of arguments, depending on your design taste:
Coupon
instance to be stored in the database. The function does nothing convenient for you, and only creates and saves a Transaction
object.Coupon
instance; the function determines the discounted amount for you, and stores everything in a Transaction
object.In both of these cases, descr
is entirely the responsibility of the caller - in line with your suggestion for moving the string building code to saas/billing/views.py
. But that means any callers that currently rely on the auto-description-and-calculation feature will need to be rewritten.
create_order
creates an in-memory Transaction instance (it is eventually saved later in the database on execute_order
). The reason for that is that we generate 'options' (see comment in views/billing.py:CartBaseView and Pipeline Orders) a subscriber can pick up from (3-month, 6-month, etc. prepaid).
There is a call in create_order
to describe_buy_periods
(implemented in humanize.py) to build a description. This is important because we use the structure of the string itself later on to augment the description with HTML links in templates (templatetags/saas_tags.py:describe).
From your investigation, it seems a better idea would be to pass a Coupon
and/or discount_percent
to describe_buy_periods
through create_order
and create the description there.
What about we change the prototype create_order(discount_percent=
to create_order(discount=
and do some magic on the instance type (Coupon/Integer) inside describe_buy_periods
to generate a description?
We could augment the description with a link to the Coupon in as_html_description
if present...
I tried to follow the style of the existing arguments. Should address #21