Adding course topics #2210
Adding course topics #2210
Conversation
annagav
force-pushed
the
ag/add_topics
branch
2 times, most recently
from
67bc5aa
to
7720306
Compare
annagav
force-pushed
the
ag/add_topics
branch
2 times, most recently
from
f4aa3e8
to
a2b0ea8
Compare
courses/models.py
Outdated
|
|
||
| def parent_topics_with_annotated_course_counts(self): | ||
| """ | ||
| Returns parent course topics with annotated course counts including the child topic course counts as well. |
There was a problem hiding this comment.
It would be good to specify that the course counts are based on filtering defined in get_catalog_course_filter.
courses/utils.py
Outdated
| @@ -166,3 +166,27 @@ def get_unenrollable_courses(queryset): | |||
| .filter(courseruns__id__in=courseruns_qs.values_list("id", flat=True)) | |||
| .distinct() | |||
| ) | |||
|
|
|||
|
|
|||
| def get_catalog_course_filter(relative_filter=""): | |||
There was a problem hiding this comment.
Do the filters in this method align with the filters on our catalog page?
There was a problem hiding this comment.
I am not sure. Maybe the start date is irrelevant, since we show archived courses. What do you think?
There was a problem hiding this comment.
So it looks like this is the place where courses get filtered for the catalog class CourseFilterSet.
The main criteria is_live and is_enrollable.
There was a problem hiding this comment.
Do you know what defines "is_enrollable" in this case?
There was a problem hiding this comment.
It might be better to try and reuse whatever filtering already exists. It's not realistic to assume that a developer will update this method when updating the filtering logic within CourseFilterSet.
There was a problem hiding this comment.
So I looked into a bit, and In order to reuse the the existing filtering I would need to refactor a bunch of existing code, and might introduce a bug. I think it would be better to do that refactor in a separate PR, and test it thoroughly.
|
Can we pre-populate the topics with the ones from this spreadsheet: https://docs.google.com/spreadsheets/d/1Euu4ezPKeYkiTThvmxB5vFONFNG4fOULNW42F6qLuFI/edit?pli=1#gid=431773359 |
Should the first row be included? |
|
Yeah, it's a hierarchy. The first row are the top-level topics. The other ones are children. |
annagav
force-pushed
the
ag/add_topics
branch
2 times, most recently
from
9432946
to
f502aa8
Compare
|
@collinpreston make sure reverse all three migrations before checking out the changes: |
annagav
force-pushed
the
ag/add_topics
branch
3 times, most recently
from
fa21f4e
to
9e1483d
Compare
for more information, see https://pre-commit.ci
Following the example in xpro, no programs don't have topics. |
What are the relevant tickets?
Fix https://github.com/mitodl/hq/issues/4110
Description (What does it do?)
Adding topics in MITx Online.
The ability to create delete topics through the cms, and add them to courses.
Populates topics with existing department names. I don't think it is necessary, but this is how it was done in mitxpro. And I think it is nice to have some topics to begin with.
Screenshots (if appropriate):
How can this be tested?
Run migrations
Run the app, go to the cms and make sure you can see Course Topics.
Click on it and try to add or delete a course topic.
Go to Course page add topic to a course page.
Also view
http://mitxonline.odl.local:8013/api/v2/parent_course_topics/