Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FC-0059] feat: Add filters/sorting for lib v2 tab #1117

Conversation

yusuf-musleh
Copy link
Contributor

@yusuf-musleh yusuf-musleh commented Jun 20, 2024

Description

This PR adds searching and sorting functionality in the V2 Libraries tab.

Screenshot 2024-06-20 at 6 19 03 PM

Supporting information

Related Tickets:

Testing instructions

  1. Run this branch in your local env
  2. Make sure you have [FC-0059] feat: Add order query param to lib v2 API edx-platform#35005 running locally as well
  3. Make sure you have a few v2 libraries created
  4. Navigate to the Libraries (v2) tab (or http://apps.local.edly.io:2001/course-authoring/libraries)
  5. Confirm that the search bar and the sorting dropdown appear
  6. Perform a serach and confirm that the correct results show up
  7. Confirm that changing sorting changes the order of the results
  8. Confirm that clearing the search, lists all libraries
  9. Confirm that searching for a non-existant library shows "No results found"
  10. Confirm that clicking on the Clear filters, clears both the search and resets the sort/order to default

Private-ref: FAL-3752

@openedx-webhooks
Copy link

openedx-webhooks commented Jun 20, 2024

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

Copy link

codecov bot commented Jun 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.68%. Comparing base (292663a) to head (e5ce2ae).
Report is 680 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1117      +/-   ##
==========================================
+ Coverage   92.67%   92.68%   +0.01%     
==========================================
  Files         696      694       -2     
  Lines       12392    12359      -33     
  Branches     2714     2663      -51     
==========================================
- Hits        11484    11455      -29     
+ Misses        876      873       -3     
+ Partials       32       31       -1     

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

@yusuf-musleh yusuf-musleh force-pushed the yusuf-musleh/sort-filter-v2-libraries branch 2 times, most recently from 345b5fa to 4f005bd Compare June 20, 2024 15:30
@yusuf-musleh yusuf-musleh marked this pull request as ready for review June 20, 2024 15:40
@yusuf-musleh yusuf-musleh requested a review from a team as a code owner June 20, 2024 15:40
Copy link
Contributor

@pomegranited pomegranited left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This looks and works great, thank you @yusuf-musleh !

  • I tested this on my tutor dev stack using the PR test instructions.
  • I read through the code
  • I checked for accessibility issues by using the keyboard only to navigate the search/filter form.
  • Includes documentation N/A
  • User-facing strings are extracted for translation

@yusuf-musleh yusuf-musleh force-pushed the yusuf-musleh/sort-filter-v2-libraries branch 2 times, most recently from 0c2b719 to 49b7142 Compare June 23, 2024 15:16
@mphilbrick211
Copy link

@openedx/2u-tnl this is ready for review! Thanks!

@bradenmacdonald
Copy link
Contributor

@mphilbrick211 This is part of FC-0059, sorry we didn't have the FC number in the PR title. TNL is welcome to review but we can find a reviewer from one of our core contributors too :)

@mphilbrick211
Copy link

@mphilbrick211 This is part of FC-0059, sorry we didn't have the FC number in the PR title. TNL is welcome to review but we can find a reviewer from one of our core contributors too :)

Thanks, @bradenmacdonald! OK to have a CC review.

@yusuf-musleh yusuf-musleh changed the title feat: Add filters/sorting for lib v2 tab [FC-0059] feat: Add filters/sorting for lib v2 tab Jun 26, 2024
Copy link
Contributor

@rpenido rpenido left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @yusuf-musleh!
LGTM 👍 Great work!

I added a comment about a default value for the order, but feel free to push back.

  • I tested this on my tutor dev stack using the testing instructions
    • Note: The order was incorrect when I loaded the page, but this is a frontend issue (not related to this PR). The frontend is not passing the order field to the API until we use the dropdown.
      image
      image
  • I read through the code
  • I checked for accessibility issues N/A
  • Includes documentation

@yusuf-musleh yusuf-musleh force-pushed the yusuf-musleh/sort-filter-v2-libraries branch from 49b7142 to 6888565 Compare June 29, 2024 13:31
@yusuf-musleh
Copy link
Contributor Author

@rpenido Thanks for the review! And great catch on the default order issue, I've fixed it here 6888565

@yusuf-musleh yusuf-musleh force-pushed the yusuf-musleh/sort-filter-v2-libraries branch from 6888565 to 2469f04 Compare July 5, 2024 14:32
@yusuf-musleh yusuf-musleh force-pushed the yusuf-musleh/sort-filter-v2-libraries branch from fd13676 to 5d7591a Compare July 7, 2024 10:51
@yusuf-musleh yusuf-musleh force-pushed the yusuf-musleh/sort-filter-v2-libraries branch from 34d8ed6 to 07542c0 Compare July 8, 2024 13:34
@bradenmacdonald bradenmacdonald enabled auto-merge (squash) July 8, 2024 14:29
@bradenmacdonald bradenmacdonald merged commit 83489b0 into openedx:master Jul 8, 2024
4 checks passed
@openedx-webhooks
Copy link

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

@bradenmacdonald bradenmacdonald deleted the yusuf-musleh/sort-filter-v2-libraries branch July 8, 2024 14:41
@mphilbrick211 mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FC Relates to an Axim Funded Contribution project open-source-contribution PR author is not from Axim or 2U
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Sort & Filter on Libraries Tab of Studio Home
6 participants