Skip to content
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.

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

drawer topic carousels should not be filtered on just courses #1984

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gumaerc
Copy link
Contributor

@gumaerc gumaerc commented Jan 23, 2025

What are the relevant tickets?

https://github.com/mitodl/hq/issues/6565

Description (What does it do?)

When adding the topic carousels to the new learning resource drawer, the TopicCarouselConfig class was reused from the Dashboard. This config was set up by default to filter on only courses. The carousels in the drawer are not supposed to be this way. By default the TopicCarouselConfig does not filter on courses now, and that is only set in the Dashboard.

How can this be tested?

  • Spin up this branch of mit-learn
  • Ensure you have a variety of resources of different types populated in your database
  • Visit the search page at http://localhost:8062/search/
  • Click on some search results and verify that the topic carousels pull in more than just courses
  • You should test several resources as some topics may show all courses just by chance

@gumaerc gumaerc added the Needs Review An open Pull Request that is ready for review label Jan 23, 2025
) => ResourceCarouselProps["config"]

const TopicCarouselConfig: TopicCarouselConfigProps = (
topic: string | undefined,
resourceType?: LearningResourcesSearchRetrieveResourceTypeEnum[] | undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

resourceTypes ? or maybe filteredTypes ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review An open Pull Request that is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants