On Conditional Offers

[Paul Stevens]

Hi all,

As some of you may be aware of, I've been working on improving the offer module for some time now. I cleaned up the proxy resolution, allowed multiple benefits per line item, and have now pending the definition of allowed combinations of offers. I noticed someone else already backported our code to define excluded categories on ranges. Thanks Diederik. Excellent!

Specifically I'm now looking into https://github.com/django-oscar/django-oscar/issues/2788 where repeated consumption of basketlines are marked as problematic.

Working on this issue the state of the offer implementation coming into focus more and more, which I'd like to share with you for further feedback and discussion.

Currently I'm operating on following hypothesis:

• the default offer code is rich in domain-knowledge, but quite immature as to implementation. There are some very good ideas there, but the code is in need of structural improvements.

To wit,

• The code should be rich in mechanisms, and very poor in policy assumptions. It should be all about mechanisms, and any policy assumption should be strictly shunned. This is currently not the case, so changing this will cause breakage where users depend on implied policy assumptions.
  
• Ideally I'd like the offer code to allow following logic: when a basket satisfies a (set of) conditions, this will lead to the application of a (set of) benefits.

Which means,

• the proxy setup must go. It is too restrictive, doesn't allow sharing of (sets of) conditions, or (sets of) benefits between distinct conditional offers. I'd like a conditional offer to depend on one or *more* conditions, and to lead to one or *more* benefits to be applied. This is a long-term goal.
  
• On the short term: application of an offer on a basket must be idem-potent; repeated application must not change the state of the benefits if the state of the basket hasn't changed between offer applications.
• This requires that satisfaction of a condition is independent of an offer's application. Only the state of the basket matters.
• It also requires that application of a benefit is directly implied and enforced by an offer's application.
  

Of course, being about mechanisms means that it must accommodate rich policies.

What I'm currently struggling with is two fields that touch on policy: offer.max_applications and benefit.max_affected_items

It would seem to me that both try to implement the same limitation: the number of line items that may be affected by an benefit. It seems to me that max_applications is an incomplete attempt at imposing restrictions of applying benefits that rely on too many side-effects, and breaking the idem-potency rule I'd like to enforce. The benefit.max_affected_items does look like a promising and useful feature to limit benefit applications, and I'd like to make that one the core variable to enforce such limits.

I would appreciate if you could share with me your usage of max_applications and max_affected_items to I can validate (or discard) my assumptions.

have a nice weekend!

-- 
Lab Digital
Paul J Stevens | Systems Engineering
M: +31 (0)62 460 6876
T: +31 (0)85 060 3980
Kanaalweg 14-G, 3526 KL Utrecht

[Samir Shah]

Hi Paul,

> As some of you may be aware of, I've been working on improving the offer module for some time now. I cleaned up the proxy resolution, allowed multiple benefits per line item.

Thanks a lot of your efforts on this!

> What I'm currently struggling with is two fields that touch on policy: offer.max_applications and benefit.max_affected_items

I can't see any max_applications field/property on the Offer model - are you referring to max_basket_applications? max_basket_applications does indeed seem to be an attempt at achieving similar objectives to Benefit.max_affected_items . My best guess is that this was done to allow some degree of control without having to write a custom benefit class - which to be honest doesn't feel like very good justification, and you can easily end up with counter-intuitive results depending on how the benefit(s) used by a particular offer apply themselves.

The Offer model also has max_global_applications, max_user_applications whose uses cases are fairly common for vouchers and/or limited-availability offers. I don't think these are contentious.

> The proxy setup must go.

Agree entirely on this.

Samir

[Paul Stevens]

On 25-03-19 06:42, Samir Shah wrote:
>
> > What I'm currently struggling with is two fields that touch on
> policy: offer.max_applications and benefit.max_affected_items
>
> I can't see any max_applications field/property on the Offer model -
> are you referring to max_basket_applications? max_basket_applications
> does indeed seem to be an attempt at achieving similar objectives to
> Benefit.max_affected_items . My best guess is that this was done to
> allow some degree of control without having to write a custom benefit
> class - which to be honest doesn't feel like very good justification,
> and you can easily end up with counter-intuitive results depending on
> how the benefit(s) used by a particular offer apply themselves.
>

Yes, I meant max_basket_applications. But I'll just leave it alone - for
now.

And what's up with codecov/changes failures? Overall coverage improved,
but that single check still complains. How useful is that, or not?

[Samir Shah]

And what's up with codecov/changes failures? Overall coverage improved, but that single check still complains. How useful is that, or not?

I don't think it is anything to worry about - see https://docs.codecov.io/docs/unexpected-coverage-changes#section-reasons-for-unexpected-changes . We do also have a handful of time-sensitive tests (separate issue) which always pass, but follow inconsistent code paths that also result in "unexpected coverage changes" between builds.

Samir