Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle type in CourseMode min_price #26508

Conversation

julianajlk
Copy link
Contributor

@julianajlk julianajlk commented Feb 12, 2021

REV-1632.

Background:
The min_price column in LMS CourseMode table takes integers, and should be updated to take decimals to match the price in ecommerce database, making the price across the learner pathway to purchase the same (49.99 vs. 49).

This ticket covers:
Before (2) is deployed in ecommerce, LMS must be able to handle receiving the price data as a non-integer. Originally had this check before .save() in CourseMode model, but per Emma's point, would be better to check this earlier - now in CourseRetrieveUpdateView (urls.py uses this view) for commerce API.

In order to decrease the risk of breaking anything with migrations, this change will be divided into 5 steps across edx-platform and ecommerce:

  1. Deploy LMS no.1 (this PR): make sure min_price can receive non-int.
  2. Deploy ecommerce no.1 (PR here): change LMSPublisher API to stop converting min_price to int.
  3. Deploy LMS no.2 (WIP PR here): add new column for price and make sure it's populated with the same min_price value.
  4. Deploy LMS no.3 : use new price column in Track Selection and Course Home pages.
  5. Deploy LMS no. 4 : remove original min_price column.

@julianajlk julianajlk force-pushed the julianajlk/REV-1632/price-discrepancy-coursemode-decimal branch 4 times, most recently from 712d068 to 38529d2 Compare February 16, 2021 18:33
@openedx openedx deleted a comment from edx-status-bot Feb 16, 2021
@julianajlk julianajlk force-pushed the julianajlk/REV-1632/price-discrepancy-coursemode-decimal branch from 38529d2 to c080e85 Compare February 17, 2021 16:21
@julianajlk julianajlk force-pushed the julianajlk/REV-1632/price-discrepancy-coursemode-decimal branch from c080e85 to 2e5fb79 Compare March 4, 2021 15:40
@openedx openedx deleted a comment from edx-status-bot Mar 4, 2021
@edx-status-bot
Copy link

Your PR has finished running tests. The following contexts failed:

  • jenkins/quality
  • jenkins/python

@arch-bom-gocd-alerts
Copy link

📣 💥 Heads-up: You must either rebase onto master or merge master into your branch to avoid breaking the build.

We recently removed diff-quality and introduced lint-amnesty. This means that the automated quality check that has run on your branch doesn't work the same way it will on master. If you have introduced any quality failures, they might pass on the PR but then break the build on master.

This branch has been detected to not have commit 2e33565 as an ancestor. Here's how to see for yourself:

git merge-base --is-ancestor 2e335653 julianajlk/REV-1632/price-discrepancy-coursemode-decimal && echo "You're all set" || echo "Please rebase onto master or merge master to your branch"

If you have any questions, please reach out to the Architecture team (either #edx-shared-architecture on Open edX Slack or #architecture on edX internal).

@julianajlk
Copy link
Contributor Author

Closing as this work was reprioritized.

@julianajlk julianajlk closed this Dec 7, 2023
@nedbat nedbat deleted the julianajlk/REV-1632/price-discrepancy-coursemode-decimal branch January 8, 2024 14:57
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.

3 participants