As part of #working-groups:build-test-release testing effort, we have found unexpected behaviour, or lets say something you should be able to do but you can’t (As a course author/staff)
Case in summary
course author can’t hide a course in /courses page when discovery service is enabled[1]
When course discovery service in enabled, courses that are disbaled in <lms.domain>/courses page will be retrieved from search discovery service. However course discovery service doesn’t in the current implmentation know or get “catalog_visibility” settings, the advance setting of a course.
So in case a course author/staff wants to hide a partucuilar course, while discovery search is enabled, they can’t as how they used to with catalog.
The proposed solution:
The solution which I am proposing is that edx-search service would by default exclude courses with a filed value key of “catalog_visibility” exists and its value is “none” or “about”. If the “catalog_visibility” field doesn’t exist then it will be ignored meaing it will be visible.
As this will lead to breaking changes, I am posting this here to ask people about their opinion, do you see this change rational?
Alternatives solutions
I can’t think of other alternative unless it would envolve adding a new setting value, e.g. discovery_visbility which would lead in my opinion making the platform more complex to manage, at that this point the course author/staff needs to know weather they are using discovery service or not, and accordingly use the approiate settings…etc.
Hey @ghassan, I did some searching and found some information about how this field is handled in Discovery. I am completely unfamiliar with edx-search and how it is connected to LMS, CMS and Discovery, so I do not know whether or not this information will be helpful to you.
Anyway, I found the CourseRun.hidden field in Discovery:
This field seems to control whether the course should be returned from the Discovery APIs, and it’s apparently controlled by the Studio catalog_visibility setting.
So, it sounds like that if one invokes Disovery’s refresh_course_metadata command, the hidden field in Discovery will be updated from edx-platform’s catalog_visibility setting. Again, not sure how this relates to edx-search.
So in lms homepage it was hidden beacuse it used course_discovery, however for /courses, the result came straight form edx-search, where lms just proxy request to edx-search.
So a good question to ask here (which I don’t know the answer of) is why /courses got the result from edx-search instead of course_discovery is there a clear reason for that?, are those two services completely equivalent? Answer to those question might shed some light to a possible alterinative… i.e. can we replace the edx-search usage with course_discovery?
Why doesn’t edx-search require the Discovery service?
Because the Discovery Service isn’t enabled by default, but the /courses page needs to work out-of-the-box on a new instance. So, edx-search can only depend on LMS/Studio by default.
Why doesn’t edx-search use the Discovery service when it IS enabled?
It would make sense if edx-search could be configured to use the Discovery service when the service is enabled. I believe the only reason this isn’t the case is because of history.
The /courses page was written in ~2013, and edx-search was created in ~2015. The Discovery service didn’t come around until ~2016 (I’ve realized that the “course_discovery” references in edx-search aren’t a reference to Discovery service; it’s just the general idea of discovering courses. The service didn’t exist yet).
By now, edx.org hasn’t used the LMS /courses page at all for a long time: instead, they use a closed-sourced marketing site, which uses the Discovery service as a backend. So, edX/2U doesn’t think about the edx-search library very much. The /courses page in particular is entirely in the hands of the community.
Can we replace the edx-search usage with course_discovery ?
Unless we require all Open edX instances to run Discovery, I don’t think we’d want to replace the usage. But providing the Discovery service as an alternative backend sounds like a great idea. I don’t think edX/2U would be interested in the change, but as long as it didn’t introduce any regressions, I can’t imagine they’d oppose the change either.
Thank you for the detail explanation, it almost more sesne now.
The only thing that doesn’t make much sense is that, for the /courses will get the result from edx-search if the discovery service is enabeld.
Hence:
So the lms will use edx-search for /courses if discovery service is enabled. And that exact part of code what made mix them togather (edx-search and discovery) do you have any context on that?
On the other hand, while I was going over testing for my changes, I find that edx-search has a setting for overriding filter class, and the LMS does override it and it use this filter instead edx-platform/lms_filter_generator.py at a27247d14d2b77234fd004988f96258e29d0d000 · openedx/edx-platform · GitHub . And filter shuold skip course visibility to none if SEARCH_SKIP_SHOW_IN_CATALOG_FILTERING = False. I am not 100% sure about that, since I just found about it and didn’t test it yet.
Based on the docs for FEATURES['ENABLE_COURSE_DISCOVERY'], I think that the toggle refers the old idea of “course discovery” (that is, just finding new courses) as opposed to the newer meaning (the Discovery service):
So, enabling that toggle isn’t telling LMS to use the Discovery service; it’s telling the LMS to have a course search page as the LMS landing page.
If I would get one certain thing this this thread, is that the project needs serious review of naming, but that story of another day.
Here is my conculstion to recap:
Open edX has a featuer called Course Discovery ( repo is called edx-search) and it has a service Course Discovery (its repo is called course-discovery).
Though both of them have their dedicated repo, but both are being used/referred to in edx-platform, and sometimes refered with identical term: which is exactly course_discovery.
And I think in the begging of solving this weired behaviour of courses being visible, I did mix them up, (as I beleive folks already do).
Now can you confirm this, @kmccormick
Both of them have their settings/configuration. And should not be dependent on each other, meaning you you can use discovery service (course-discovery), without enabled discovery featuer (edx-search) which is responsible for /courses only
Yes, I believe this is nearly correct, with two notes:
The edx-search repo contains code for both course search (the old meaning of “course discovery”) as well as courseware search (searching through content in courses).
Very few people call course search “course discovery” any more, it seems like it’s just those code references you unluckily came upon. I would just call this “course search”.
Yes, I think it’s important for software projects to continuously review and update naming as features are added, changed, and removed. I think Open edX has struggled with this in the past, and while I think it’s starting to get better (for example, the XModule->XBlock cleanup is doing a lot of good renaming work) we still have a long way to go.
For this particular issue, I think doing a backwards-compatible rename of FEATURES['ENABLE_COURSE_DISCOVERY'] to FEATURES['ENABLE_COURSE_SEARCH'] would be a big improvement.