Skip to content

Conversation

@mbertrand
Copy link
Member

@mbertrand mbertrand commented Sep 16, 2024

What are the relevant tickets?

Part 2 of https://github.com/mitodl/hq/issues/5126, depends on mitodl/course-search-utils#123

Description (What does it do?)

  • Uses an updated version of course-search-utils (Add delivery as a query/facet option course-search-utils#123), which needs to be merged before this one. Once it is merged and released, then this PR will be updated to use the new release version. Currently it is pinned to the course-search-utils PR commit.
  • Removes the learning_format field from the LearningResource model
  • Renames learning_format to delivery in the Profile model
  • Removes "learning_format" from the ETL pipelines
  • Updates the frontend to use "delivery" instead of "learning_format" for filtering and facets

How can this be tested?

  • Run some searches, with and without using the "Format" facet, everything should work as expected.
  • Update your preferred format for your user profile, everything should work as expected.

Additional Context

@mbertrand mbertrand force-pushed the mb/delivery_frontend branch 4 times, most recently from 2e25701 to 3d8bcd4 Compare September 17, 2024 20:08
@mbertrand mbertrand added Needs Review An open Pull Request that is ready for review and removed Work in Progress labels Sep 17, 2024
@shanbady shanbady self-assigned this Sep 18, 2024
@shanbady shanbady self-requested a review September 18, 2024 13:51
Copy link
Contributor

@shanbady shanbady left a comment

Choose a reason for hiding this comment

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

Seems like this factory attribute still remains but otherwise looks good

@shanbady shanbady assigned mbertrand and unassigned shanbady Sep 18, 2024
@shanbady shanbady added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Sep 18, 2024
@mbertrand
Copy link
Member Author

@shanbady that "format" factory attribute is for LearningResourceOfferor.format which is not quite the same thing as LearningResource.learning_format. I made a bunch of changes based on feedback from Simone, because some more planning has to go into how the switchover to "delivery" will be presented on the front end:

  • Removed all "offline" values from courses via a migration and updated the OCW ETL pipeline to not add it unless OCW_OFFLINE_DELIVERY=True (default=False)
  • Reverted "Format"->"Delivery" frontend text changes
  • Updated package.json and yarn.lock to use the official new version of course-search-utils

@mbertrand mbertrand added Needs Review An open Pull Request that is ready for review and removed Waiting on author labels Sep 18, 2024
@shanbady shanbady added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Sep 18, 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.

lgtm

@mbertrand mbertrand merged commit 747ea76 into main Sep 19, 2024
@odlbot odlbot mentioned this pull request Sep 19, 2024
5 tasks
@mbertrand mbertrand deleted the mb/delivery_frontend branch October 23, 2024 12:51
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.

4 participants