feat: filter courses by user language by default

[navinkarkera]

Description

Adds a waffle switch to filter courses by selected site language by default

Useful information to include:

• Which edX user roles will this change impact? Learner

Private-ref: BB-8091

Testing instructions

• Setup master devstack and checkout this PR
• Add below snippet to lms/envs/private.py

FEATURES['ENABLE_COURSE_DISCOVERY'] = True

• Start lms and cms using make {lms,cms}-up

• Add courseware.discovery_default_language_filter waffle switch in django admin.

• Add a new course or update language of any existing course to some other language.

• Go to http://localhost:18000/courses

• If you have never set your language via account settings, you should see all the courses without any filters applied.

• Now change your language by going to Account page.
  

  • Only English option is available in the Language field.
  • You can add multiple languages by adding a new Darklang config in http://localhost:18000/admin/dark_lang/darklangconfig/

• Open http://localhost:18000/courses, a default language filter should be applied.

[openedx-webhooks]

Thanks for the pull request, @navinkarkera! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

• supporting documentation
• Open edX discussion forum threads
• timeline information ("this must be merged by XX date", and why that is)
• partner information ("this is a course on edx.org")
• any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

[open-craft-grove]

Sandbox deploy request received. Deployment will start soon.

[open-craft-grove]

Sandbox deployment started.

[open-craft-grove]

Sandbox update request received. Deployment will start soon.

[open-craft-grove]

Sandbox deployment successful.

Sandbox LMS is available at pr-33647-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33647-139931.staging.do.opencraft.hosting

[open-craft-grove]

Sandbox deployment started.

[open-craft-grove]

Sandbox deployment successful.

Sandbox LMS is available at pr-33647-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33647-139931.staging.do.opencraft.hosting

[tecoholic]

@navinkarkera 👍

• I tested this: Verified that the discovery page filters courses automatically based on the user's language.
• I read through the code
• I checked for accessibility issues
• Includes documentation

NOTE:

• I needed to run the make lms-static for the JS files to be regenerated.
• While feature flag contains the documentation about the flag, maybe there somewhere else this should be documented? eg., https://edx.readthedocs.io/projects/edx-installing-configuring-and-running/en/latest/feature_flags/feature_flag_index.html

[navinkarkera]

While feature flag contains the documentation about the flag, maybe there somewhere else this should be documented? eg., https://edx.readthedocs.io/projects/edx-installing-configuring-and-running/en/latest/feature_flags/feature_flag_index.html

openedx-unsupported/edx-documentation#2210

[open-craft-grove]

Sandbox update request received. Deployment will start soon.

[mphilbrick211]

Hi @navinkarkera! Is this ready for review?

[navinkarkera]

@mphilbrick211 Thanks for checking. Yes it is ready.

[open-craft-grove]

Sandbox deployment successful.

Sandbox LMS is available at pr-33647-139931.staging.do.opencraft.hosting
Sandbox Studio is available at studio.pr-33647-139931.staging.do.opencraft.hosting

Instance config : https://grove-stage-build-logs.nyc3.digitaloceanspaces.com/34602668-5639873435-config.yml
Instance requirements : https://grove-stage-build-logs.nyc3.digitaloceanspaces.com/34602668-5639873435-requirements.txt

[pomegranited]

Hi @mphilbrick211 @navinkarkera may I review/merge this as CC?

[navinkarkera]

@pomegranited That would be super helpful. Thanks!

[pomegranited]

Sure thing @navinkarkera , could you direct me to the ticket?

[navinkarkera]

@pomegranited BB-8091. Added the ticket info to description as well.

[pomegranited]

@navinkarkera This code is working exactly as described, thank you for such great testing instructions!

But I have some questions about the feature flag, and a request for a better test. Please let me know if you have any questions or concerns.

[pomegranited]

Since there's only ever one course in the returned list, this test doesn't actually verify that the filter was applied.
Could you add another course that uses a different language so we can see that this filter actually applies?

[pomegranited]

I noticed that when my preferred language doesn't match any of my course's languages that I see header text like this:

We couldn't find any results for ""

which I think students may find confusing, because the text is confusing, and because the student didn't actually perform an explicit search.

An easy way around this would be to avoid showing that header if there's no search text.

[navinkarkera]

Since there's only ever one course in the returned list, this test doesn't actually verify that the filter was applied.
Could you add another course that uses a different language so we can see that this filter actually applies?

@pomegranited Actually the test should not be about checking API response as it is mocked above by AjaxHelpers.respondWithJson(requests, JSON_RESPONSE);. So I modified to check the request body as it should include language filter automatically when default filter is enabled.

An easy way around this would be to avoid showing that header if there's no search text.

Makes sense. Done 👍

[pomegranited]

Is there an implementation reason why you chose to add a new settings.FEATURES flag instead of one of the toggles available from edx-toggles like SettingToggle or WaffleFlag?

It's my understanding from Feature Flags and Settings on edx-platform and OEP-17 that we should be using these edx-toggles now, and that FEATURES flags are deprecated.

[navinkarkera]

@pomegranited No specific reason for using feature flags, except for maybe the relationship with ENABLE_COURSE_DISCOVERY feature flag. After reading the the OEP, I think we can using WaffleSwitch here.

[pomegranited]

👍 Thank you for addressing my comments @navinkarkera :) I'll merge this tomorrow if there are no objections.

Could you update the PR description to reflect the use of a Waffle Switch instead of a feature flag to enable this feature? And will need an update to openedx-unsupported/edx-documentation#2210 too.

• I tested this on my devstack using the excellent testing instructions in the PR.
• I read through the code
• I checked for accessibility issues by using my keyboard to navigate the Courses list
• Includes documentation on the new switch
• User-facing strings are extracted for translation N/A

[navinkarkera]

@pomegranited We probably don't need openedx-unsupported/edx-documentation#2210 as it will be automatically documented here.

[pomegranited]

@navinkarkera Perfect, thank you for fixing that nit and updating your PR description.

We probably don't need openedx-unsupported/edx-documentation#2210 as it will be automatically documented here.

Sweet! Feel free to close that then.

Merging this now :)

[openedx-webhooks]

@navinkarkera 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.

[edx-pipeline-bot]

2U Release Notice: This PR has been deployed to the edX production environment.