Skip to content

show courses and programs first in default sort #1104

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 18, 2024
Merged

show courses and programs first in default sort #1104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 92 additions & 5 deletions frontends/api/src/generated/v1/api.ts
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions learning_resources_search/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@

LEARN_SUGGEST_FIELDS = ["title.trigram", "description.trigram"]
COURSENUM_SORT_FIELD = "course.course_numbers.sort_coursenum"
DEFAULT_SORT = "-created_on"
DEFAULT_SORT = ["is_learning_material", "-created_on"]


def gen_content_file_id(content_file_id):
Expand Down Expand Up @@ -551,7 +551,7 @@ def construct_search(search_params):
sort = generate_sort_clause(search_params)
search = search.sort(sort)
elif not search_params.get("q"):
search = search.sort(DEFAULT_SORT)
search = search.sort(*DEFAULT_SORT)

if search_params.get("endpoint") == CONTENT_FILE_TYPE:
query_type_query = {"exists": {"field": "content_type"}}
Expand Down Expand Up @@ -598,7 +598,6 @@ def execute_learn_search(search_params):
"""

search = construct_search(search_params)

return search.execute().to_dict()


Expand Down
4 changes: 3 additions & 1 deletion learning_resources_search/api_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1428,6 +1428,7 @@ def test_execute_learn_search_for_learning_resource_query(opensearch):
"course.course_numbers.sort_coursenum",
"course.course_numbers.primary",
"resource_relations",
"is_learning_material",
]
},
}
Expand Down Expand Up @@ -1630,6 +1631,7 @@ def test_execute_learn_search_for_content_file_query(opensearch):
"course.course_numbers.sort_coursenum",
"course.course_numbers.primary",
"resource_relations",
"is_learning_material",
]
},
}
Expand Down Expand Up @@ -1760,7 +1762,7 @@ def test_document_percolation(opensearch, mocker):
[
("-views", None, [{"views": {"order": "desc"}}]),
("-views", "text", [{"views": {"order": "desc"}}]),
(None, None, [{"created_on": {"order": "desc"}}]),
(None, None, ["is_learning_material", {"created_on": {"order": "desc"}}]),
(None, "text", None),
],
)
Expand Down
2 changes: 2 additions & 0 deletions learning_resources_search/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class FilterConfig:
"platform": FilterConfig("platform.code"),
"offered_by": FilterConfig("offered_by.code"),
"learning_format": FilterConfig("learning_format.code"),
"is_learning_material": FilterConfig("is_learning_material"),
}

SEARCH_NESTED_FILTERS = {
Expand Down Expand Up @@ -368,4 +369,5 @@ class FilterConfig:
"course.course_numbers.sort_coursenum",
"course.course_numbers.primary",
"resource_relations",
"is_learning_material",
]
12 changes: 12 additions & 0 deletions learning_resources_search/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
from learning_resources_search.api import gen_content_file_id
from learning_resources_search.constants import (
CONTENT_FILE_TYPE,
COURSE_TYPE,
PROGRAM_TYPE,
)
from learning_resources_search.models import PercolateQuery
from learning_resources_search.utils import remove_child_queries
Expand Down Expand Up @@ -79,6 +81,8 @@ def serialize_learning_resource_for_update(
return {
"resource_relations": {"name": "resource"},
"created_on": learning_resource_obj.created_on,
"is_learning_material": learning_resource_obj.resource_type
not in [COURSE_TYPE, PROGRAM_TYPE],
**serialized_data,
}
Comment on lines 82 to 87
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki Jun 18, 2024

Choose a reason for hiding this comment

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

These three fields:

  1. resource_relations
  2. created_on
  3. is_learning_material

are "extra". None of them show up in http://localhost:8063/api/v1/learning_resources/.

(1) and (2) do not show up in http://localhost:8063/api/v1/learning_resources_search/, but (3) does. Do you know why?

Mostly asking out of curiosity. If removing (3) is easy, that might be worthwhile since then the response formats will match exactly, which has been the goal. That said, having (3) is not a problem—it's "extra" and it's not usable on the frontend since it doesn't show up in the openapi (good).

Copy link
Contributor Author

@abeglova abeglova Jun 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I hid the is_learning_material from the results. It's pretty redundant anyway. I guess it's a bit weird to filter by something that's not a field in the results but keeping the the output the same is good and cluttering the results of both the learning_resources api and learing_resources_search api with a field that's pretty specific to our ui seems unnecessary


Expand Down Expand Up @@ -175,6 +179,7 @@ def to_representation(self, obj):
"professional",
"free",
"learning_format",
"is_learning_material",
]

CONTENT_FILE_AGGREGATIONS = ["topic", "content_feature_type", "platform", "offered_by"]
Expand Down Expand Up @@ -261,6 +266,13 @@ class LearningResourcesSearchRequestSerializer(SearchRequestSerializer):
default=None,
help_text="True if the learning resource offers a certificate",
)
is_learning_material = ArrayWrappedBoolean(
required=False,
allow_null=True,
default=None,
help_text="True if the learning resource is a podcast, podcast episode, video, "
"video playlist, or learning path",
)
certification_choices = CertificationType.as_tuple()
certification_type = StringArrayField(
required=False,
Expand Down
8 changes: 8 additions & 0 deletions learning_resources_search/serializers_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@
"url": "http://xpro.mit.edu/courses/course-v1:xPRO+MCPO+R1/",
"resource_type": "course",
"platform": "globalalumni",
"is_learning_material": False,
},
}
],
Expand Down Expand Up @@ -278,6 +279,7 @@
"url": "http://xpro.mit.edu/courses/course-v1:xPRO+MCPO+R1/",
"resource_type": "course",
"platform": "globalalumni",
"is_learning_material": False,
}
],
"metadata": {
Expand Down Expand Up @@ -344,6 +346,7 @@
"last_modified": None,
"runs": [],
"course_feature": [],
"is_learning_material": True,
"user_list_parents": [],
},
}
Expand Down Expand Up @@ -507,6 +510,7 @@
"last_modified": None,
"runs": [],
"course_feature": [],
"is_learning_material": True,
"user_list_parents": [],
}
],
Expand Down Expand Up @@ -589,6 +593,7 @@ def test_serialize_learning_resource_for_bulk(resource_type, is_professional, no
"_id": resource.id,
"resource_relations": {"name": "resource"},
"created_on": resource.created_on,
"is_learning_material": resource.resource_type not in ["course", "program"],
**free_dict,
**LearningResourceSerializer(resource).data,
}
Expand Down Expand Up @@ -635,6 +640,7 @@ def test_serialize_course_numbers_for_bulk(
"resource_relations": {"name": "resource"},
"created_on": resource.created_on,
"free": False,
"is_learning_material": False,
**LearningResourceSerializer(resource).data,
}
expected_data["course"]["course_numbers"][0] = {
Expand Down Expand Up @@ -713,6 +719,7 @@ def test_learning_resources_search_request_serializer():
"certification": "false",
"certification_type": CertificationType.none.name,
"free": True,
"is_learning_material": True,
"offered_by": "xpro,ocw",
"platform": "xpro,edx,ocw",
"topic": "Math",
Expand All @@ -730,6 +737,7 @@ def test_learning_resources_search_request_serializer():
"sortby": "-start_date",
"professional": [True],
"certification": [False],
"is_learning_material": [True],
"certification_type": [CertificationType.none.name],
"free": [True],
"offered_by": ["xpro", "ocw"],
Expand Down
44 changes: 44 additions & 0 deletions openapi/specs/v1.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2093,6 +2093,7 @@ paths:
- professional
- free
- learning_format
- is_learning_material
type: string
description: |-
* `resource_type` - resource_type
Expand All @@ -2107,6 +2108,7 @@ paths:
* `professional` - professional
* `free` - free
* `learning_format` - learning_format
* `is_learning_material` - is_learning_material
description: Show resource counts by category
- in: query
name: certification
Expand Down Expand Up @@ -2254,6 +2256,13 @@ paths:
items:
type: integer
description: The id value for the learning resource
- in: query
name: is_learning_material
schema:
type: boolean
nullable: true
description: True if the learning resource is a podcast, podcast episode,
video, video playlist, or learning path
- in: query
name: learning_format
schema:
Expand Down Expand Up @@ -2494,6 +2503,7 @@ paths:
- professional
- free
- learning_format
- is_learning_material
type: string
description: |-
* `resource_type` - resource_type
Expand All @@ -2508,6 +2518,7 @@ paths:
* `professional` - professional
* `free` - free
* `learning_format` - learning_format
* `is_learning_material` - is_learning_material
description: Show resource counts by category
- in: query
name: certification
Expand Down Expand Up @@ -2655,6 +2666,13 @@ paths:
items:
type: integer
description: The id value for the learning resource
- in: query
name: is_learning_material
schema:
type: boolean
nullable: true
description: True if the learning resource is a podcast, podcast episode,
video, video playlist, or learning path
- in: query
name: learning_format
schema:
Expand Down Expand Up @@ -2920,6 +2938,7 @@ paths:
- professional
- free
- learning_format
- is_learning_material
type: string
description: |-
* `resource_type` - resource_type
Expand All @@ -2934,6 +2953,7 @@ paths:
* `professional` - professional
* `free` - free
* `learning_format` - learning_format
* `is_learning_material` - is_learning_material
description: Show resource counts by category
- in: query
name: certification
Expand Down Expand Up @@ -3081,6 +3101,13 @@ paths:
items:
type: integer
description: The id value for the learning resource
- in: query
name: is_learning_material
schema:
type: boolean
nullable: true
description: True if the learning resource is a podcast, podcast episode,
video, video playlist, or learning path
- in: query
name: learning_format
schema:
Expand Down Expand Up @@ -3337,6 +3364,7 @@ paths:
- professional
- free
- learning_format
- is_learning_material
type: string
description: |-
* `resource_type` - resource_type
Expand All @@ -3351,6 +3379,7 @@ paths:
* `professional` - professional
* `free` - free
* `learning_format` - learning_format
* `is_learning_material` - is_learning_material
description: Show resource counts by category
- in: query
name: certification
Expand Down Expand Up @@ -3498,6 +3527,13 @@ paths:
items:
type: integer
description: The id value for the learning resource
- in: query
name: is_learning_material
schema:
type: boolean
nullable: true
description: True if the learning resource is a podcast, podcast episode,
video, video playlist, or learning path
- in: query
name: learning_format
schema:
Expand Down Expand Up @@ -6873,6 +6909,7 @@ components:
- professional
- free
- learning_format
- is_learning_material
type: string
description: |-
* `resource_type` - resource_type
Expand All @@ -6887,6 +6924,7 @@ components:
* `professional` - professional
* `free` - free
* `learning_format` - learning_format
* `is_learning_material` - is_learning_material
x-enum-descriptions:
- resource_type
- certification
Expand All @@ -6900,6 +6938,7 @@ components:
- professional
- free
- learning_format
- is_learning_material
Article:
type: object
description: Serializer for LearningResourceInstructor model
Expand Down Expand Up @@ -9218,6 +9257,11 @@ components:
type: boolean
nullable: true
description: True if the learning resource offers a certificate
is_learning_material:
type: boolean
nullable: true
description: True if the learning resource is a podcast, podcast episode,
video, video playlist, or learning path
certification_type:
type: array
items:
Expand Down