feat: [FC-86] implement multivalue faceted search using Meilisearch and Elasticsearch

[Serj-N]

Summary

Course discovery needs to support filtering courses by multiple options within a single facet category.
The purpose of this PR is to add logic to both Meilisearch and Elasticsearch engines to support this kind of multivalue filtering.

Current Behavior

Using the "language" facet as an example.
After selecting "English" in the Language facet category, the language section in the filters area only displays the selected value - "English".

New Behavior

After selecting "English" in the Language facet category, the language section in the filters area displays all language values ("English", "French", "Ukrainian", etc.) as long as they are available considering the rest of the currently selected filters (organization, mode).

Note on the Tests

The PR started as only Meilisearch-specific, but this led to an issue with running the tests. The api layer abstracts away the selection of the search engine, which means the new logic automatically affects the Elasticsearch-specific parts and some tests start to fail. Consequently, it was decided to make this PR cover both Meilisearch and Elasticsearch.

[openedx-webhooks]

Thanks for the pull request, @Serj-N!

This repository is currently maintained by @Ali-Salman29.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[e0d]

@Serj-N
I've added you to the openedx-triage team as you are covered under an entity CLA. This will mean that tests will run automatically when you submit a pull request in the future. Please look for an invitation in your email and accept it.

[Serj-N]

@e0d Hello and thank you! Invitation accepted

[e0d]

@Serj-N It looks like there are some failed checks, please have a look.

[Serj-N]

@Serj-N It looks like there are some failed checks, please have a look.

@e0d Hi! Just wanted to say that all the tests pass now, and the PR is ready for review. Only the code coverage is still failing which, as far as I know, is a common thing and may be ignored - at least at this point, while the review is in progress.

[sarina]

@ormsbee any chance you can provide a review here, or suggest an appropriate reviewer?

[sarina]

@Serj-N you will need to address the codecov/patch check. The project check is sometimes wonky but the patch check is usually accurate, and is saying you've only tested 51% of lines changed. Can you add more test coverage?

[Serj-N]

@Serj-N you will need to address the codecov/patch check. The project check is sometimes wonky but the patch check is usually accurate, and is saying you've only tested 51% of lines changed. Can you add more test coverage?

@sarina Thanks, got it! The coverage is fixed now. Previously, it was saying 51% because I purposefully excluded one test module from running, as it contained integration tests that depend on the code from another PR. But it was also covering the existing code, so codecove didn't like it. I realized that I can simply include those integration tests in that other PR, while this PR can run all the tests as before without excluding anything.
Bottom line, I think we can proceed with the review now.

[Serj-N]

Most of the concerns I have here stem from trying to work with this messy codebase.

Agree, edx-search would definitely benefit from a major overhaul. But for this PR, I guess we'll have to deal with whatever we got now.
Either way, I would like to take the time to thank you for a thorough review!

[sarina]

Hey @bradenmacdonald - if you have questions holistically about the "why" we're doing this work in edx-search, I'm available for questions. We're trying to get this PR and the follow-on (draft) in as soon as we can, so we can get a working MFE replacement for Course Catalog/About pages into Ulmo. I definitely understand it's not ideal to make more changes within edx-search but I think refactoring/replacement will need to wait for a follow-up piece of work.

Feel free to reach out on the PR or in Slack with any questions!

[sarina]

@Serj-N I noticed you re-requested a review from Braden, but it would be best to explicitly leave a comment that this is ready for review again.

[Serj-N]

@bradenmacdonald I implemented and/or commented on the requested chages, and the PR is ready for another test and, hopefully, merge.
Above, Sarina provided a link to the related draft PR, and I'll leave it here, too, for your reference:
raccoongang#13

[bradenmacdonald]

As for the manual testing with Elasticsearch, it does indeed require some additional steps:

Thanks for the instructions. I didn't really want to enable the discovery plugin just to test Elasticsearch, so I create a new plugin that does everything necessary to test Elasticsearch and makes it really easy, based on your instructions: https://github.com/open-craft/tutor-contrib-elasticsearch-legacy

[bradenmacdonald]

👍 @Serj-N nice work on this PR. And thanks for addressing all my feedback.

I wish we could have put your talents to work on a better abstraction API for search instead of modifying this one, but alas the timing didn't work out that way.

• I tested this: both the built-in discovery view ( http://local.openedx.io:8000/courses ) and the Learning MFE courseware search tool, both with is_multivalue=False and with is_multivalue=True (the latter is nicer even if the current MFE doesn't allow multiple selections; at least it now shows the options you didn't select), both using Meilisearch and Elasticsearch.
• I read through the code
• I checked for accessibility issues: n/a, no UI
• Includes documentation: docstrings etc.

Someone else will have to actually merge this for you, as I'm not a Core Contributor on this repo.

[ormsbee]

@bradenmacdonald: We should probably fix that, given that you know more about this repo than most, and certainly more than me. But I'll do the merge.