Skip to content

Conversation

@mbertrand
Copy link
Member

@mbertrand mbertrand commented Nov 7, 2023

What are the relevant tickets?

Closes #195

Description (What does it do?)

  • Allows search results to be sorted by primary course number
  • If there is a departments filter applied, sort by course numbers for that department
  • Modifies LEARNING_RESOURCE_SORTBY_OPTIONS to be a dict that is used to provide more user-friendly sort options to the user. For example, the user can sort by mitcoursenumber and that will actually sort by course.course_numbers.sort_coursenum
  • Adjusts the department filter so that it works by department id instead of name (ex: "CMS-W" instead of "Comparative Media Studies/Writing")

How can this be tested?

  • Temporarily modify learning_resources_search.constants.SOURCE_EXCLUDED_FIELDS to be an empty list so you will be able to see the course.course_numbers.sort_coursenum values.

  • Import a bunch of OCW courses, among them 21H.104J+fall_2010 and a few other courses with the department_id "11" (Urban Studies and Planning), plus some other departments.

    ./manage.py backpopulate_ocw_data --course-num 21h-104j-riots-strikes-and-conspiracies-in-american-history-fall-2010
    
  • Go to this url to sort by course number, filtered by department:
    http://localhost:8063/api/v1/learning_resources_search/?department=11&sortby=mitcoursenumber

  • Verify the results are in ascending order by course.course_numbers.sort_coursenum for the respective department ("11"). The course mentioned above, 21H.104J+fall_2010, has a cross-listed id within that department of "11.015J" so it should appear in the correct position relative to the other courses imported from that department.

  • Go to this url to sort by course number, unfiltered by department:
    http://localhost:8063/api/v1/learning_resources_search/?sortby=mitcoursenumber

  • Verify the results are sorted by course.course_numbers.sort_coursenum where listing_type=primary, and not by any cross-listed numbers.

  • Check that the schema documentation for this endpoint shows some user-friendly sort options:

Screenshot 2023-11-08 at 1 29 24 PM

I did not try to make the departments parameter schema documentation more user-friendly in this PR, leaving that for a future one because the use of a custom field class (StringArrayField) for that parameter makes it seem at first glance that doing so might be somewhat involved.

@codecov
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9b919af) 76.79% compared to head (bc71b04) 76.81%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #197      +/-   ##
==========================================
+ Coverage   76.79%   76.81%   +0.02%     
==========================================
  Files         313      313              
  Lines       13645    13653       +8     
  Branches     2239     2243       +4     
==========================================
+ Hits        10478    10487       +9     
  Misses       2894     2894              
+ Partials      273      272       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mbertrand mbertrand added Needs Review An open Pull Request that is ready for review and removed Work in Progress labels Nov 9, 2023
@ChristopherChudzicki ChristopherChudzicki self-assigned this Nov 13, 2023
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki left a comment

Choose a reason for hiding this comment

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

This is working as expected and looks good 👍

I’m also noticing a few possible ETL issues that seem unrelated to this work…

@ChristopherChudzicki ChristopherChudzicki added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Nov 13, 2023
@mbertrand mbertrand merged commit e1df2a0 into main Nov 14, 2023
@mbertrand mbertrand deleted the mb/sort_coursenum branch January 16, 2024 13:14
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.

Allow search API results to be sorted by department course numbers

3 participants