Skip to content

Conversation

@mbertrand
Copy link
Member

@mbertrand mbertrand commented Sep 3, 2024

What are the relevant tickets?

Partially addresses https://github.com/mitodl/hq/issues/5126

Description (What does it do?)

  • Adds a delivery field to LearningResource with a default value the same as for learning_format
  • Modifies the OCW and Sloan ETL pipelines to populate the values correctly. The default value is fine for all other sources.
  • Updates the search index to include the delivery field.

How can this be tested?

  • Ingest these OCW courses if you haven't done so already - you'll need to assign OCW_ env settings to the same as RC.
./manage.py backpopulate_ocw_data --course-name 16-810-engineering-design-and-rapid-prototyping-january-iap-2007 --skip-contentfiles
./manage.py backpopulate_ocw_data --course-name res-7-006-7-03x-genetics --skip-contentfiles

)

  • Run docker compose up, the new migration should complete.
  • Check that the 16.810 course has a delivery value of ["online", "offline"], while the 7.006 course has a value of just ["online"]
  • You can run the mgmt commands again, though this will temporarily make 7.006 include "offline" too until Include hide_download in data.json ocw-hugo-themes#1445 is merged and a mass publish is run.

Additional Context

The learning_format field is not removed in this PR because doing so would cause complications with several related repos (course-search-utils, open-api-clients). After both of these repos are updated to include delivery, then learning_format can be removed, and the front end can be updated to use delivery instead.

@mbertrand mbertrand force-pushed the mb/learning_format2delivery branch 2 times, most recently from d37b00e to e9522d0 Compare September 4, 2024 19:53
@mbertrand mbertrand changed the title Rename learning_format to delivery Add LearningResource.delivery field (eventually to replace learning_format) Sep 4, 2024
@mbertrand mbertrand force-pushed the mb/learning_format2delivery branch 2 times, most recently from 9ae4181 to 3d4e0a0 Compare September 5, 2024 15:45
@mbertrand mbertrand added Needs Review An open Pull Request that is ready for review and removed Work in Progress labels Sep 5, 2024
@abeglova abeglova self-assigned this Sep 6, 2024
Copy link
Contributor

@abeglova abeglova left a comment

Choose a reason for hiding this comment

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

Looks like you started to add delivery to the search api but did not finish. Other than that, works great

"professional",
"free",
"learning_format",
"delivery",
Copy link
Contributor

Choose a reason for hiding this comment

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

Search aggregated by delivery does not work RN you need to add "delivery": FilterConfig("delivery.code"), to SEARCH_FILTERS in learning_resource_search/constants.py for delivery to be a valid aggregation option.

Additionally you should add delivery to LearningResourcesSearchRequestSerializer

@abeglova abeglova added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Sep 6, 2024
@mbertrand mbertrand force-pushed the mb/learning_format2delivery branch 2 times, most recently from e69ec4f to de1fa19 Compare September 6, 2024 20:01
@mbertrand mbertrand force-pushed the mb/learning_format2delivery branch from de1fa19 to 2ee56b5 Compare September 9, 2024 13:10
@mbertrand mbertrand merged commit a2ddda6 into main Sep 9, 2024
@odlbot odlbot mentioned this pull request Sep 12, 2024
17 tasks
@rhysyngsun rhysyngsun deleted the mb/learning_format2delivery branch February 7, 2025 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants