Skip to content

Fix v0/baskets/create_from_product so that baskets get discounts as we expect#3290

Merged
jkachel merged 7 commits into
mainfrom
jkachel/10103-consolidate-update-discount-validation
Feb 17, 2026
Merged

Fix v0/baskets/create_from_product so that baskets get discounts as we expect#3290
jkachel merged 7 commits into
mainfrom
jkachel/10103-consolidate-update-discount-validation

Conversation

@jkachel
Copy link
Copy Markdown
Contributor

@jkachel jkachel commented Feb 10, 2026

What are the relevant tickets?

Closes mitodl/hq#10103

Description (What does it do?)

The initial implementation/move of the v0/baskets/create_from_product API was pretty simple. When it created a basket, it was at least able to go try to fetch the applicable automatic discounts for the user and apply them to the basket. However, it did not do this correctly, and would attempt to apply all the auto discounts available to the basket. In Unified Ecommerce, this was fine; MITx Online only allows a single discount per basket, so if there were more than one, it would fail.

In working through this, I found too that it wasn't grabbing the financial assistance discounts and applying those, either. These get applied according to their own ruleset, but nevertheless count as "automatic" and should be applied to any normal basket. These would get applied at some point in the process - at worst, when the learner checks out - but they should be applied when the basket is created so the checkout page display has the correct pricing on it.

So, this PR does a handful of things:

  • Expands the get_auto_apply_discounts_for_basket internal API to also grab financial assistance discounts. (Previously, this didn't consider financial assistance discounts at all.) Additionally, this has been updated to respect the activation and expiration dates on the discount.
  • Updates the apply_discount_to_basket internal API to make sure the discount being applied is better (the logic needed some help).
  • Updates the _create_basket_from_product function so that it grabs existing applied discounts, applicable automatic discounts (including financial aid), and any discount provided to it, clears any discounts applied to the basket, and applies all of the collected discounts.
  • Adds more comprehensive tests for these APIs

How can this be tested?

Automated tests should pass.

To test the API itself, you will need a number of things set up:

  • A course run:
    • with a product
    • with financial assistance set up
  • A regular discount that the user can manually apply
  • A regular discount that is assigned to the user
  • A regular discount that is assigned to the course run product

The regular discounts should not have their Automatic flag set to True. Ideally, start testing with the discounts set to have no activation or expiration dates. All these discounts should ideally be for different amounts. For ease of testing, it's easier to set them all to fixed-price unlimited-use discounts.

The externally accessible part of this code is the /api/v0/baskets/create_from_product API. Test by both adding a product to the basket and adding a product and a discount to the basket.

The result of hitting the API should be a basket that has the specified product in it. The discount applied should be:

  • None if no discount is supplied, no discount is set to automatic, and no approved financial assistance exists
  • The best discount if any of these are true:
    • A discount is supplied
    • A discount is set to automatic, and is linked to no products or users
    • A discount is set to automatic, and is linked either to the product in the basket or the user
    • A financial assistance discount, if the user has an approved financial assistance request for the course associated with the product

So, for testing, hit the API after setting the user-attached discount to automatic, then turning it off and setting the product-attached one to automatic, etc. Finally, apply for financial assistance and approve your request to ensure that discount is applied as expected.

Additional Context

This is one of the APis that got moved over from UE - some of the things that MITx Online does UE didn't do by design, like financial assistance. So, this PR is mostly shifting the process that's in the legacy API for discounts into APIs and then calling them from the new v0 API.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 10, 2026

OpenAPI Changes

Show/hide ## Changes for v0.yaml:
## Changes for v0.yaml:


## Changes for v1.yaml:


## Changes for v2.yaml:


Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

@dsubak dsubak self-assigned this Feb 11, 2026
@jkachel jkachel force-pushed the jkachel/10103-consolidate-update-discount-validation branch from c89d0de to 23e2637 Compare February 11, 2026 19:21
existing_basket_discounts = [bd.redeemed_discount for bd in basket.discounts.all()]
discounts_to_apply = [
*existing_basket_discounts,
*list(get_auto_apply_discounts_for_basket(basket.id).all()),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: I think the all() is superfluous here since get_auto_apply_discounts_for_basket always kicks back a new, unevaluated queryset.

Copy link
Copy Markdown
Contributor

@dsubak dsubak left a comment

Choose a reason for hiding this comment

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

Does what it says on the testing plan. I validated this with James' help by going through the API directly.

It appears that there are some discrepancies between discount application logic here and in the learn cart, so for anyone else who plans on testing this, stick with hitting the API directly.

@jkachel jkachel force-pushed the jkachel/10103-consolidate-update-discount-validation branch from 2d80389 to 07086d7 Compare February 12, 2026 20:15
Comment thread ecommerce/api.py Outdated
Comment thread ecommerce/api.py Outdated
Comment thread ecommerce/api.py Outdated
@jkachel
Copy link
Copy Markdown
Contributor Author

jkachel commented Feb 13, 2026

@dsubak this should be ready for re-review now.

I went back and found what documentation is out there for discount assignments, and the ruleset that was here didn't match up with that (which is why the cart page kept moving things around on you). So, I changed the new code so that it matches up. I would like to be able to - in a future PR - strip out the existing discount application stuff out of the legacy views and have it use the new code too.

The workflow for discount application is:

  • Any automatic discounts are collected and applied - this includes the set of discounts assigned to the active user, financial assistance discounts, and discounts with automatic=True
  • Any existing discounts in the basket are applied
  • Supplied discount codes are applied

Codes are applied if:

  • They are valid - not expired, not inactive, applicable to the products in the basket if assigned to products
  • They are not financial assistance discounts (unless we're explicitly allowing those)
  • The discount offers a better price than any other discount that has been applied
  • Unless a user discount is applied, in which case the user discount supersedes all discounts but financial assistance discounts

In the case that there's a user discount and a financial assistance one, the financial assistance discount wins. This does mean that if the user discount is overridden, then it effectively doesn't count anymore and the user can supply a better discount by hand.

The discount product bit was also wrong, so that too was corrected - we don't apply discounts automatically based on products. We do gatekeep discounts, though, so we do check to make sure the discount's products are in the basket if it has any.

(This still doesn't touch B2B or verified program discounts because they do their own thing.)

FWIW, this is the info I found (in chronological order):
#708
https://github.com/mitodl/hq/discussions/145
https://github.com/mitodl/hq/discussions/344
https://github.com/mitodl/hq/discussions/358

It'll be a task to get some documentation written for this to consolidate it all into one place (including the special case stuff that we have for B2B and verified programs).

@jkachel jkachel requested a review from dsubak February 13, 2026 15:15
Comment thread ecommerce/api.py
- User discounts are automatically applied to the cart, regardless of the "automatic" flag
- The discount/product association is _not_ for automatically applying a discount - it's for validation on application

So now get_auto_apply_discounts_for_basket looks for discounts attached to the user, discounts that have the automatic flag set, or applicable financial assistance discounts.
User discounts apply whether or not there are other discounts other than financial aid.
@jkachel jkachel force-pushed the jkachel/10103-consolidate-update-discount-validation branch from 7e227de to bc23e5a Compare February 13, 2026 21:14
Copy link
Copy Markdown
Contributor

@dsubak dsubak left a comment

Choose a reason for hiding this comment

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

I think we're good! James and I worked through some hiccups where finaid wasn't being selected over user discounts in certain cases, but after his fixes it's behaving as expected.

@jkachel jkachel merged commit 15d0dc5 into main Feb 17, 2026
10 checks passed
@jkachel jkachel deleted the jkachel/10103-consolidate-update-discount-validation branch February 17, 2026 16:13
@odlbot odlbot mentioned this pull request Feb 19, 2026
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants