feat: home studio search filters

[johnvente]

Description

This PR adds a search input and two dropdown menus to filter courses in studio home:

• first dropdown menu: Allows us to filter by "All courses," "Active," or "Archived" courses.
• second dropdown menus: Allow for ascending/descending course names and the newest/oldest courses.

NOTE: this applies only for "courses" tab

This Uses ENABLE_HOME_PAGE_COURSE_API_V2 in the environment

DEMO

studio-home-courses-filter-demo.mp4

Supporting information

To review a little bit more about this implementation, please check this:
https://openedx.atlassian.net/wiki/spaces/OEPM/pages/4032856101/Studio+Home+incremental+upgrades+-+product+approach

How to test it

1. Set up your edx-platform

    cd path-to-your-edx-platform
    git fetch origin pull/34175/head:MJG/course-home-api-v2-pagination
    git checkout MJG/course-home-api-v2-pagination
   

2. Check it out this branch inside frontend-app-course-authoring

    cd path-to-frontend-app-course-authoring
    git fetch origin pull/846/head:jv/feat-home-studio-filters
    git checkout jv/feat-home-studio-filters
    nvm use && npm install
    npm start
   

[openedx-webhooks]

Thanks for the pull request, @johnvente! 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.

[codecov]

Codecov Report

Attention: Patch coverage is 92.30769% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 92.01%. Comparing base (99a144a) to head (a13068d).
Report is 5 commits behind head on master.

❗ Current head a13068d differs from pull request most recent head ef793fe. Consider uploading reports for the commit ef793fe to get more accurate results

Files	Patch %	Lines
src/studio-home/tabs-section/courses-tab/index.jsx	66.66%	7 Missing ⚠️
...tabs-section/courses-tab/courses-filters/index.jsx	97.22%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #846      +/-   ##
==========================================
+ Coverage   92.00%   92.01%   +0.01%     
==========================================
  Files         612      618       +6     
  Lines       10746    10835      +89     
  Branches     2305     2320      +15     
==========================================
+ Hits         9887     9970      +83     
- Misses        830      836       +6     
  Partials       29       29              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[felipemontoya]

I tested this PR when reviewing openedx/edx-platform#34173. It worked great, but I have a minor wording suggestion:

When the sum of filter+search returns an empty list, the message looks like an error happened.

I would only suggest that we make it less error like and just say that "There are no courses with the current filters. --Clear filters"

That would be wording + removing the alert icon.

[johnvente]

I tested this PR when reviewing openedx/edx-platform#34173. It worked great, but I have a minor wording suggestion:

When the sum of filter+search returns an empty list, the message looks like an error happened. I would only suggest that we make it less error like and just say that "There are no courses with the current filters. --Clear filters"

That would be wording + removing the alert icon.

Hi @felipemontoya I changed the message for something like this it doesn't look like an error now

Could you check again please?

[johnvente]

Note for reviewer: this PR will use ENABLE_HOME_PAGE_COURSE_API_V2 feature flag when this one is merged

[mariajgrimaldi]

I'm guessing all the changes made in #825 so I'll re-review once that's done. However, I still tested the filters locally, and they worked as described in the cover letter. Thank you.

[johnvente]

I won't be able to review this in a week, so I will check for any changes on April 1st

[jmakowski1123]

This was one of the first projects we put through both product review and design first, before implementing, so this already had product review and is good to go! Original spec here.

This looks great, very exciting to see in action!

One very nit-picky thing, @johnvente . When a user types in the search field, do they have to hit enter in order to display the results, or do the results display in real time as they are typing? I'm asking because the in-course search is being implemented now too, and I believe the behavior there is that the results being to display immediately as the user types. It'd be nice to have consistent behavior across both search experiences. Pinging @bradenmacdonald here for more input on the in-course side.

[jesperhodge]

@johnvente my thoughts on this.

• the x bug needs to be fixed
• for the shifting layout, could we leave a spinner in the search box (left of the X for example) to show the user that some action is underway, but only replace the list after the results come back?

@felipemontoya I love that idea.

[johnvente]

• I found a bug where if you type in a search query and then not click x, but remove it with backspace again, often it does not go back to the default and you have no way of clearing the query other than refreshing the page. See my screen recording.

Hi @jesperhodge I added the fix for this bug, and changed the debounced flow for your recommendation here

• for the shifting layout, could we leave a spinner in the search box (left of the X for example) to show the user that some action is underway, but only replace the list after the results come back?
  @felipemontoya I added a spinner of the left of the X and I replace the list when it has finished to bring the data

Sorry for the delay,

after today I won't be able to check this pr but please let know to @felipemontoya @mariajgrimaldi any
any additional change

[mariajgrimaldi]

I'm still facing this issue:

I found a bug where if you type in a search query and then not click x, but remove it with backspace again, often it does not go back to the default and you have no way of clearing the query other than refreshing the page. See my screen recording.

I'm working on fixing it.

[mariajgrimaldi]

The coverage upload is failing even after a few attempts. @jesperhodge: could you help us run it manually? Thanks!

[mariajgrimaldi]

I'm still facing this issue:

I found a bug where if you type in a search query and then not click x, but remove it with backspace again, often it does not go back to the default and you have no way of clearing the query other than refreshing the page. See my screen recording.

I'm working on fixing it.

I believe I solved this. Now, the coverage upload is still failing.

[mariajgrimaldi]

@jesperhodge: thank you for running the coverage upload manually, but I think it's also happening in other PRs:
https://github.com/openedx/frontend-app-course-authoring/actions/runs/8604487493/job/23601521237#step:6:38
https://github.com/openedx/frontend-app-course-authoring/actions/runs/8617944055/job/23619770035#step:6:38
I'm not sure if it's a 3rd party failure or related to the changes themselves.

[jesperhodge]

Thanks, @mariajgrimaldi . Yeah usually the coverage upload works after retrying, but it seems broken. From my side, I reviewed and everything looks and works great. I'm waiting for a confirmation from @cablaa77 for the UX but I don't expect any objections at all, then I'll approve.
I think after that I'll just go ahead and merge despite the failing upload, assuming I have the permissions for that. I'll also open a github request with axim to have a look at the coverage problem if nobody has done that yet.
Fantastic work on this. I really like the look and feel of it.

[jesperhodge]

I'm not sure @cablaa77 is available today, so I'm approving this. I don't expect any pushback on the UX and since this is behind a feature flag it's fine to merge.

[jesperhodge]

@mariajgrimaldi sorry this is all a lot of back and forth and taking a while. You can see I opened a request with axim to look into the CodeCov situation. I plan to just wait for them to get back to me, so if there's any need to merge this sooner than that please just notify me and I can take care of it.

[mariajgrimaldi]

Thank you for all the help and the review, @jesperhodge! Due to time constraints, we'd like to merge this PR soon. If the coverage check is a blocking issue, could we check the report another way?

Feel free to let me know if you folks need anything else from us. Thanks again!

[jesperhodge]

Hi @mariajgrimaldi , if you rebase or merge master into your branch, it should fix the pipeline.

[mariajgrimaldi]

@jesperhodge: done! Thank you so much for all the help :)

[openedx-webhooks]

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