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

Change frontend to use resource_prices field instead of prices field #1737

Open
wants to merge 4 commits into
base: mb/price_model_01
Choose a base branch
from

Conversation

mbertrand
Copy link
Member

@mbertrand mbertrand commented Oct 24, 2024

What are the relevant tickets?

PR 2 of 5 to address https://github.com/mitodl/hq/issues/5134 and https://github.com/mitodl/hq/issues/4685

Dependent on PR 1 here: #1736

Description (What does it do?)

  • Updates the front end to use the resource_prices field and display the correct currency symbol.
  • Adds a new JS package currency-symbol-map to display the correct symbol for the currency.

Screenshots (if appropriate):

If price is in Euros (EUR):
Screenshot 2024-10-23 at 1 05 50 PM

If price is in Swiss francs (CHF, no symbol):

Screenshot 2024-10-23 at 1 03 54 PM

In dollars (USD):
Screenshot 2024-10-23 at 1 07 46 PM

How can this be tested?

  • If you don't already have some courses with prices, run these commands on the main branch:
    ./manage.py backpopulate_mitxonline_data
    ./manage.py backpopulate_xpro_data
    
  • Log in as an admin user to avoid caching and go to http://open.odl.local:8062/search?platform=mitxonline&platform=xpro, you will see a mix of courses that should all either be paid with certificate (xpro) or free with a paid certificate option (mitxonline). If you click on a course card you will see the same price in the drawer.
  • Switch to this branch, run docker compose build web celery (new python package) and restart containers. Make sure the new migrations complete successfully.
  • Go to http://open.odl.local:8062/search?platform=mitxonline&platform=xpro again. This time, you will see "Paid" instead of an actual price on the course cards because the index has not been updated. If you click on one you should see the actual price in the drawer, because that is coming straight from the database.
  • Run ./manage.py update_index --courses --programs, then wait a minute or 2.
  • Go to http://open.odl.local:8062/search?platform=mitxonline&platform=xpro again. Now you should see the actual prices on the course cards.
  • Run ./manage.py recreate_index --all, it should complete without errors.
  • You can test the currency by running this code:
    from learning_resources.models import LearningResource
    from learning_resources.utils import resource_upserted_actions
    resource = LearningResource.objects.get(title="Applying Machine Learning to Engineering and Science")
    resource.resource_prices.all().update(currency="EUR")
    resource_upserted_actions(resource, False)
  • Now wait a minute then go to http://open.odl.local:8062/search?q=%22Applying%20Machine%20Learning%20to%20Engineering%20and%20Science%22. The price should show the euro symbol €1559 instead of $1559 on the course card and resource drawer.
  • Check and uncheck the "Free" checkbox on the search page. The results should change as expected.
  • Running the following pipelines should still complete successfully (you'll need the appropriate env values from RC for SEE_* and EDX_API_*):
    ./manage.py backpopulate_sloan_data
    ./manage.py backpopulate_mit_edx_data [--api_course_file=edx.json --api_program_datafile=edx_programs.json]
    
  • Use the django debug toolbar to measure the # and speed of SQL queries for the URL http://api.open.odl.local:8063/api/v1/learning_resources/?platform=mitxonline. It should not be significantly slower than the same url on the main branch, but there will be 2 additional queries.
Screenshot 2024-10-23 at 1 59 26 PM

vs

Screenshot 2024-10-23 at 2 01 03 PM
  • Finally if you want to revert your price data before you go back to the main branch, run this and the prices field should be restored with values:
    ./manage.py migrate learning_resources 0070_learningresource_location
    

Additional Context

  • There will be 3 subsequent PR's before these https://github.com/mitodl/hq/issues/5134 is closed:
    • 3: Modify the LearningResource.prices and LearningResourceRun.prices fields to be the same as resource_prices, and update schema. (Note: this will cause index updates to fail until a reindex completes)
    • 4: Change frontend to use prices field again
    • 5: Remove resource_prices from models and schemas.

@mbertrand mbertrand changed the base branch from main to mb/price_model_01 October 24, 2024 15:51
@mbertrand mbertrand added the Needs Review An open Pull Request that is ready for review label Oct 24, 2024
@mbertrand mbertrand force-pushed the mb/price_model_01 branch 2 times, most recently from 7065765 to 94b6e25 Compare October 25, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review An open Pull Request that is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant