feat: Add discount payment types #1390

asadali145 · 2023-01-26T13:16:45Z

Pre-Flight checklist

What are the relevant tickets?

Closes #1074

What's this PR do?

Adds a new field payment_type in the discounts model.
Replaces all usages of for_flexible_pricing with payment_type.
Adds data migration to set the payment types for the existing discounts.
Updates the generate_discount_code management command to add a new argument.

How should this be manually tested?

There are a few things that we need to test.

Test that the old discounts are updated per the data migration logic /admin/ecommerce/discount/.
- If for_flexible_pricing=True then payment_type=financial-assistance
- if the discount code starts with CS- then payment_type=customer-support
- if the discount code starts with JPAL- then payment_type=staff
- if the discount code starts with MITALUM15-, 14750x-, 14740x-, 1473x-, 14310x-, JPAL102x- or CYB2022- then payment_type=marketing
- If discount code is CYBER2022 then payment_type=marketing
Test that the new field is added in the Django Admin list and detail view /admin/ecommerce/discount/.
Test that the new field is added in refine dashboard. The discounts filter works fine and the new field is also working fine in discounts create, view, and edit forms.
Test that the financial assistance request flow is fine. i.e Once a request is approved, the discount is applied automatically on the checkout page.
Test that the discounts work fine when applying manually on the checkout page.

pdpinch · 2023-01-26T13:52:16Z

ecommerce/migrations/0031_discount_payment_type.py

+        migrations.AddField(
+            model_name='discount',
+            name='payment_type',
+            field=models.CharField(choices=[('B2C', 'Marketing'), ('B2B', 'Sales'), ('FA', 'Financial Assistance'), ('CS', 'Customer Support')], max_length=3, null=True),


I need to confirm this list. Please don't merge until I've signed off.

@pdpinch Do you have an update on this?

Need to add one more: "staff"

pdpinch · 2023-01-26T13:52:33Z

How should we handle updating existing coupons?

asadali145 · 2023-01-27T14:10:34Z

How should we handle updating existing coupons?

I am not really sure about that. I have created a data migration for financial assistance discounts. I am not sure about all the other types of discounts. Maybe if have some kind of a pattern then we can create a data migration for other discounts as well.

For now, I have kept the new field nullable. So, It will be null for all the discounts apart from the ones that are related to financial assistance.

Do you have any information about any pattern that we used for the discounts that we can use and write a data migration?

pdpinch · 2023-02-01T14:41:38Z

I have created a data migration for financial assistance discounts.

great.

It will be null for all the discounts apart from the ones that are related to financial assistance.

Good idea. That's fine.

Do you have any information about any pattern that we used for the discounts that we can use and write a data migration?

@jkachel may have some suggestions for some of the coupons he created.

To be clear, it's not necessary that we backfill all of them. I just want to update the easy ones.

jkachel · 2023-02-01T15:10:13Z

We've only had to do a few runs of these so it shouldn't be too bad to do. The patterns I've been using are:

Customer Service - always starts with CS-
JPAL staff discounts - starts with JPAL-
MIT Alumni discounts - starts with MITALUM15-
There were also 100 codes each for $100 fixed price for the individual DEDP courses that start with an abbreviated version of the course number (i.e. 14750x-) - I can't find the ticket that we made these for though. (These are different than the JPAL discounts, which also have the course number in them.)

The create_discount_code management command also needs to be updated to support this. Didn't see that in the code but batches of codes (and thus codes that would generally need to be tagged in this manner) are generated using that command.

asadali145 · 2023-02-01T15:24:46Z

The create_discount_code management command also needs to be updated to support this.

Yeah, I will be updating a couple of commands, The work is in progress for that.

MIT Alumni discounts - starts with MITALUM15-

What should be the type for these discounts?

There were also 100 codes each

What should be the type for these course discounts?

codecov-commenter · 2023-02-02T09:47:48Z

Codecov Report

Merging #1390 (44126b5) into main (b1c97ee) will decrease coverage by 0.03%.
The diff coverage is 55.17%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #1390      +/-   ##
==========================================
- Coverage   83.22%   83.19%   -0.03%     
==========================================
  Files         309      309              
  Lines       13135    13148      +13     
  Branches      935      936       +1     
==========================================
+ Hits        10932    10939       +7     
- Misses       1936     1940       +4     
- Partials      267      269       +2

Impacted Files	Coverage Δ
...erce/management/commands/generate_discount_code.py	`0.00% <0.00%> (ø)`
...ement/commands/generate_legacy_enrollment_codes.py	`0.00% <0.00%> (ø)`
ecommerce/serializers.py	`68.38% <ø> (ø)`
...d/public/src/containers/pages/checkout/CartPage.js	`40.81% <0.00%> (ø)`
frontend/public/src/factories/course.js	`98.91% <ø> (ø)`
ecommerce/admin.py	`64.37% <100.00%> (ø)`
ecommerce/api.py	`78.06% <100.00%> (ø)`
ecommerce/constants.py	`100.00% <100.00%> (ø)`
ecommerce/factories.py	`100.00% <100.00%> (ø)`
ecommerce/models.py	`85.36% <100.00%> (ø)`
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

asadali145 · 2023-02-02T09:59:35Z

Hey @pdpinch & @jkachel,
I am doing the following mapping for the new payment_type field. Let me know if there is any issue with this.

Discounts with for_flexible_pricing=True will have payment_type=financial-assistance
Discounts with discount codes starting with CS- will have payment_type=customer-support
Discounts with discount codes starting with JPAL- will have payment_type=staff

Also, I am not sure about these 2 discounts. What should be the payment_type for these 2 types of discounts?

MIT Alumni discounts - starts with MITALUM15-

There were also 100 codes each for $100 fixed price for the individual DEDP courses that start with an abbreviated version of the course number (i.e. 14750x-) - I can't find the ticket that we made these for though. (These are different than the JPAL discounts, which also have the course number in them.)

jkachel · 2023-02-02T15:13:46Z

MIT Alumni discounts - starts with MITALUM15-

What should be the type for these discounts?

There were also 100 codes each

What should be the type for these course discounts?

After some thought, I think both of these are B2C.

I did find out the purpose for the last group of codes I mentioned. They're for the lottery - https://odl.zendesk.com/agent/tickets/145641 is the Zendesk ticket for that (which you'll probably not be able to get into, just including for completeness) and this is it in HQ: https://github.com/mitodl/hq/issues/558. The relevant info from the ticket is:

These coupons are used for a lottery that's run every term for DEDP. The specific products change each term because not every course is offered every term.

In addition, I also found a couple more groups that will need backfilling:

CYB2022- (batch) and CYBER2022 (singular) - we have a handful of these codes that were for this past Cyber Monday sale - those are definitely B2C. (There was an issue with the actual CYBER2022 singular code I'd set up for this so there are also a batch of codes to hand out to folks who had issues with the code.)
MM-prepaid- - there are a bunch of these from the MicroMasters migration. I'm not sure if these are B2C or CS, though, or if we should leave these as-is because this is a one-off special case.

asadali145 · 2023-02-03T07:11:17Z

@jkachel There are also some discount codes starting with JPAL102x-. Are they the same as JPAL-? Or are they more like lottery discounts? What should be the type of these? staff or marketing?

asadali145 · 2023-02-03T09:25:03Z

MM-prepaid- - there are a bunch of these from the MicroMasters migration. I'm not sure if these are B2C or CS, though, or if we should leave these as-is because this is a one-off special case.

@pdpinch What do you suggest about these?

jkachel · 2023-02-03T13:18:07Z

@jkachel There are also some discount codes starting with JPAL102x-. Are they the same as JPAL-? Or are they more like lottery discounts? What should be the type of these? staff or marketing?

JPAL102x- are lottery codes and JPAL- are for JPAL staff.

jkachel

Everything looks good - staff dashboard looked OK, management command is good, workflow is fine on the frontend. Approving conditionally because of two things:

waiting on resolution for MM-prepaid codes from @pdpinch (my thought is to leave them blank because we're never going to do anything like that again probably, or at least not without it being a whole thing that gets tracked like a project)
can you update the docs for generate_discount_code in docs/source/commands? It needs the new flag added.

Other than those two things, LGTM 👍

jkachel

Rescinding previous approval - did not notice but the financial assistance emails are broken. I can't link to a Sentry log for this because it's on my local Sentry instance but here's the relevant stack trace:

asadali145 · 2023-02-06T06:42:13Z

Rescinding previous approval - did not notice but the financial assistance emails are broken. I can't link to a Sentry log for this because it's on my local Sentry instance but here's the relevant stack trace:

@jkachel Could you please verify if this is a legit issue? It seems like a migrations issue to me. Not sure if you tried to change something there. Everything is working fine for me. I am getting the emails. I have also tried to reapply all the migrations as well.

jkachel · 2023-02-06T16:31:42Z

@jkachel Could you please verify if this is a legit issue? It seems like a migrations issue to me. Not sure if you tried to change something there. Everything is working fine for me. I am getting the emails. I have also tried to reapply all the migrations as well.

After some checking I have no idea. The local copy I use most of the time has issues (even with a fresh database and the migrations applied), but I tested it on a separate system and it works OK there. So, this does seem like a local-to-me issue or maybe at worst a macOS issue.

jkachel

LGTM 👍

pdpinch · 2023-02-08T15:27:33Z

waiting on resolution for MM-prepaid codes from @pdpinch (my thought is to leave them blank because we're never going to do anything like that again probably, or at least not without it being a whole thing that gets tracked like a project)

I think we should make a new category for these, "legacy"

…-support for legacy discount command

asadali145 · 2023-02-09T09:27:12Z

23f0b3d adds a new payment type legacy.
@jkachel Could you have one last look?

jkachel

LGTM 👍

asadali145 force-pushed the asad/1074-discount-payment-types branch from 7746aa9 to e76317c Compare January 26, 2023 13:19

pdpinch reviewed Jan 26, 2023

View reviewed changes

asadali145 added the Work in progress label Feb 2, 2023

asadali145 force-pushed the asad/1074-discount-payment-types branch from ffda830 to 0fd5e11 Compare February 3, 2023 13:46

asadali145 marked this pull request as ready for review February 3, 2023 13:49

asadali145 added Work in progress Needs Review and removed Work in progress labels Feb 3, 2023

asadali145 requested review from jkachel and pdpinch February 3, 2023 14:01

jkachel self-assigned this Feb 3, 2023

jkachel approved these changes Feb 3, 2023

View reviewed changes

jkachel requested changes Feb 3, 2023

View reviewed changes

jkachel added Waiting on Author and removed Needs Review labels Feb 3, 2023

asadali145 requested a review from jkachel February 6, 2023 08:23

asadali145 removed the Waiting on Author label Feb 6, 2023

asadali145 added the Needs Review label Feb 6, 2023

jkachel approved these changes Feb 6, 2023

View reviewed changes

asadali145 added 18 commits February 9, 2023 14:02

feat: Add payment_type field for discounts

8cf9055

backfill migration

c414194

replace for_flexible_pricing & update choices

311ee8d

fmt

7c54da2

lint & fmt

fa2f685

feat: Add staff as a payment type

3978202

feat(generate_discount_code): Add support for payment types

54e361c

fmt

7b0f5b0

isort

38db25d

remove usage of for_flexible_pricing from generate_discount codes

461f91f

feat(generate_legacy_enrollment_codes): sets payment_type to customer…

5f8be1c

…-support for legacy discount command

feat: update config_tiers command to set payment_type

7139ed3

isort & ran black

c4236ee

Updated data migration

11fa924

feat(backfill_payment_types: Add more backfill)

82e68bf

remove for_flexible_pricing field in favour of payment_type

8cf9468

update generate_discount_code docs

f528d19

add new payment_type legacy

23f0b3d

asadali145 force-pushed the asad/1074-discount-payment-types branch from 44126b5 to 23f0b3d Compare February 9, 2023 09:13

pdpinch mentioned this pull request Feb 10, 2023

tracking types of discounts #1074

Closed

2 tasks

jkachel approved these changes Feb 14, 2023

View reviewed changes

asadali145 merged commit 82d222e into main Feb 14, 2023

odlbot mentioned this pull request Feb 15, 2023

Release 0.61.0 #1430

Merged

2 tasks

rachellougee mentioned this pull request Feb 15, 2023

Update ecommerce_discount model for MITxOnline mitodl/ol-data-platform#566

Closed

1 task

asadali145 deleted the asad/1074-discount-payment-types branch March 17, 2023 12:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add discount payment types #1390

feat: Add discount payment types #1390

asadali145 commented Jan 26, 2023 •

edited

Loading

pdpinch Jan 26, 2023

asadali145 Feb 1, 2023

pdpinch Feb 1, 2023

pdpinch commented Jan 26, 2023

asadali145 commented Jan 27, 2023

pdpinch commented Feb 1, 2023

jkachel commented Feb 1, 2023

asadali145 commented Feb 1, 2023

codecov-commenter commented Feb 2, 2023 •

edited

Loading

asadali145 commented Feb 2, 2023

jkachel commented Feb 2, 2023

asadali145 commented Feb 3, 2023

asadali145 commented Feb 3, 2023

jkachel commented Feb 3, 2023

jkachel left a comment

jkachel left a comment

asadali145 commented Feb 6, 2023

jkachel commented Feb 6, 2023

jkachel left a comment

pdpinch commented Feb 8, 2023

asadali145 commented Feb 9, 2023

jkachel left a comment

feat: Add discount payment types #1390

feat: Add discount payment types #1390

Conversation

asadali145 commented Jan 26, 2023 • edited Loading

Pre-Flight checklist

What are the relevant tickets?

What's this PR do?

How should this be manually tested?

pdpinch Jan 26, 2023

Choose a reason for hiding this comment

asadali145 Feb 1, 2023

Choose a reason for hiding this comment

pdpinch Feb 1, 2023

Choose a reason for hiding this comment

pdpinch commented Jan 26, 2023

asadali145 commented Jan 27, 2023

pdpinch commented Feb 1, 2023

jkachel commented Feb 1, 2023

asadali145 commented Feb 1, 2023

codecov-commenter commented Feb 2, 2023 • edited Loading

Codecov Report

asadali145 commented Feb 2, 2023

jkachel commented Feb 2, 2023

asadali145 commented Feb 3, 2023

asadali145 commented Feb 3, 2023

jkachel commented Feb 3, 2023

jkachel left a comment

Choose a reason for hiding this comment

jkachel left a comment

Choose a reason for hiding this comment

asadali145 commented Feb 6, 2023

jkachel commented Feb 6, 2023

jkachel left a comment

Choose a reason for hiding this comment

pdpinch commented Feb 8, 2023

asadali145 commented Feb 9, 2023

jkachel left a comment

Choose a reason for hiding this comment

asadali145 commented Jan 26, 2023 •

edited

Loading

codecov-commenter commented Feb 2, 2023 •

edited

Loading