fix: fetch proper courses when keyword searching with applied filters #1497
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @bra-i-am! What's next?Please work through the following steps to get your changes ready for engineering review: 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo 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:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Let us know that your PR is ready for review:Who will review my changes?This repository is currently maintained by 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:
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:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1497 +/- ##
=======================================
Coverage 92.93% 92.93%
=======================================
Files 1073 1073
Lines 21091 21091
Branches 4559 4545 -14
=======================================
Hits 19601 19601
Misses 1418 1418
Partials 72 72 ☔ View full report in Codecov by Sentry. |
|
@arbrandes @bradenmacdonald: could you give us a hand reviewing this PR? It fixes #1477. Thank you! |
| @@ -95,7 +95,7 @@ const CoursesFilters = ({ | |||
|
|
|||
| const handleSearchCoursesDebounced = useCallback( | |||
| debounce((value) => handleSearchCourses(value), 400), | |||
| [], | |||
| [activeOnly, archivedOnly], | |||
There was a problem hiding this comment.
Just one question: should order not be included as well ?
There was a problem hiding this comment.
Also, going back to this fix that addressed this issue in particular about the sorting default, after searching even if sorting is set to "newest" or "oldest" the results are returned sorted only by display name. Could that be fixed by adding order here?
There was a problem hiding this comment.
@bradenmacdonald @mariajgrimaldi: thanks for your feedback. I already updated the branch
please let me know if anything else
When active/archived filters were on or there was selected any order filter, the search skipped these values and it was just returned the courses list without the respective filters. Additionally, when a search keyword was applied and a filter was selected, the keyword stayed stuck and the search list returned were not the appropriate
bra-i-am
force-pushed
the
bc/fix-filtered-search
branch
from
d5692e5 to
d60f019
Compare
bra-i-am
changed the title
|
FYI @farhaanbukhsh Here's also the backport for you to review: #1496 |
There was a problem hiding this comment.
👍
- ✅ I tested this on Tutor with Sumac and master
- ✅ I read through the code
- ❌ I checked for accessibility issues
- ❌ Includes documentation
- ❌ I made sure any change in configuration variables is reflected in the corresponding client's
configuration-securerepository.
|
@farhaanbukhsh: can you help us merge this PR? Thank you! |
|
@mariajgrimaldi I can but I don't want to bypass the CC for the repo :) and don't worry we can backport this to sumac.1 as well. |
|
@farhaanbukhsh: I understand. Thank you! @bradenmacdonald: Could you give us a hand with a re-review? Thank you! |
When active/archived filters were on or there was selected any order filter, the search skipped these values and it was just returned the courses list without the respective filters. Additionally, when a search keyword was applied and a filter was selected, the keyword stayed stuck and the search list returned were not the appropriate
Description
This PR makes the Studio Home search bar work as expected. When active/archived filters were on or there was selected any order filter, the search skipped these values and just returned the courses list without the respective filters. Additionally, when a search keyword was applied and a filter was selected, the keyword stayed stuck and the search list returned was not the appropriate
useCallbackdependencies so if any of this filters changes, they are applied properly and retrieved the proper dataSupporting information
GitHub Issue: #1477
Testing instructions
Make sure you count on at least one course and do the following:
frontend-app-authoringMFE to test itArchivedin the dropdown beside the search barActiveand try to look for an archived courseScreenshots
Archive/active filter is taken into account
The sorting filter is applied despite changing the other filters
The search stayed stuck due to a specific keyword was preserved when a filter was applied: