feat: adds sort widget to search manager and library component page [FC-0059] #1147

pomegranited · 2024-07-10T09:05:06Z

Description

Adds a widget to the library component search manager which sorts the search results based on the selected option.

Plus the Recently Modified components section from open-craft#52.

Supporting information

Part of #1038

Blocked by:

Private-ref: FAL-3758

Testing instructions

Run the latest master branch in Studio/CMS

Reindex studio to update the index settings: tutor dev run cms ./manage.py cms reindex_studio --experimental
Create a new library
Add components to the library.

From the shell, publish the library:

from openedx.core.djangoapps.content_libraries import api, models
library = models.ContentLibrary.objects.get(slug='<your library slug>')
api.publish_changes(library.library_key)

Add more components, but leave them unpublished.

To test this change:

Navigate to the new library from http://apps.local.edly.io:2001/course-authoring/libraries/
Check that the Recently Modified section shows your recently modified components in order of most recent.
Change the sort options and note changes in the Components list.
The Recently Modified section should not change when sort options change.
Click on the Components tab, and note sort option stays the same as previously selected.
Change sort options and note changes in the components list.
Select "Recently Published" -- should omit unpublished components from the list.

openedx-webhooks · 2024-07-10T09:05:11Z

Thanks for the pull request, @pomegranited!

What's next?

Please work through the following steps to get your changes 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.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/2u-tnl. Tag them in a comment and let them know that your changes are ready for review.

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:

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.

codecov · 2024-07-10T09:14:53Z

Codecov Report

Attention: Patch coverage is 99.09910% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.88%. Comparing base (649863d) to head (2d1b213).
Report is 2 commits behind head on master.

Files	Patch %	Lines
src/search-manager/data/apiHooks.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1147      +/-   ##
==========================================
+ Coverage   92.82%   92.88%   +0.05%     
==========================================
  Files         747      750       +3     
  Lines       13372    13468      +96     
  Branches     2856     2880      +24     
==========================================
+ Hits        12413    12510      +97     
+ Misses        924      923       -1     
  Partials       35       35

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

open-craft-grove · 2024-07-10T09:52:20Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-10T15:50:51Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-11T15:35:24Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-12T09:14:22Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-12T14:15:42Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-12T14:35:47Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-12T15:24:02Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-12T15:29:14Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-19T23:38:54Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-19T23:39:06Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-20T07:28:22Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-20T07:38:54Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-24T00:26:22Z

Sandbox deployment failed 💥
Please check the settings and requirements.
Retry deployment by pushing a new commit or updating the requirements/settings in the pull request's description.
📜 Failure Logs
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-24T00:38:23Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

* feat: Add Recently Modified library section * feat: Add "View All" to library sections The "View All" action appears on sections that pass in a view all action and contain content that exceeds the defined preview limit, which defaults to 4. * feat: Use intl library section titles * test: Update tests

pomegranited · 2024-07-25T22:42:08Z

@bradenmacdonald @rpenido CC @yusuf-musleh

Select "Recently Published" -- should omit unpublished components from the list.

I'm having trouble filtering out "unpublished" components from the search results? I think there's two problems, but I can't work out how to fix them. Can you help?

When "Recently Published" is selected sort option, I add an extraFilter with last_published IS NOT EMPTY, which shows up fine in the meilisearch POST request:

But it fails to filter out components with last_published: null from the response :(

If we can't work out how to make this happen with the filter query, I could add another boolean field for not_published, but it seems like we should be able to do this with just last_published?
2. The last_published date for the XBlock doesn't always seem to be updated when we publish the library?
My tests should have covered this, but when I manually publish the library (using the PR test instructions), it doesn't update all of the library components last_published values.
I'm testing using blocks imported to my library from the Demo Course, and some of these have their last_published set, but some don't, and I don't understand why..

Thanks in advance for any advice you have!

open-craft-grove · 2024-07-25T23:09:25Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-25T23:09:40Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

rpenido · 2024-07-26T02:53:40Z

But it fails to filter out components with last_published: null from the response :(

I think meilisearch treats null and empty differently

Meilisearch does not treat null values as empty. To match null fields, use the IS NULL operator.

I'll check tomorrow and update here!

rpenido · 2024-07-26T12:31:27Z

src/search-manager/SearchManager.ts

+  const sort: SearchSortOption[] = (searchSortOrderToUse === defaultSortOption ? [] : [searchSortOrderToUse]);
+  // Selecting SearchSortOption.RECENTLY_PUBLISHED also excludes unpublished components.
+  if (searchSortOrderToUse === SearchSortOption.RECENTLY_PUBLISHED) {
+    extraFilter.push('last_published IS NOT EMPTY');


Can you try this @pomegranited?

Suggested change

extraFilter.push('last_published IS NOT EMPTY');

extraFilter.push('last_published IS NOT NULL');

🤦‍♀️ That was totally it.. Thank you @rpenido !

And re the last_published date stored in the index, I think I've found the problem, but am not sure how to address it, so have asked Dave.

rpenido · 2024-07-26T12:35:59Z

src/library-authoring/LibraryHome.tsx

-  }
+  const renderEmptyState = () => {
+    if (componentCount === 0) {
+      return searchKeywords === '' ? <NoComponents /> : <NoSearchResults />;


I found a small bug here.

If we use the Recently Published sort and don't have any published components we have componentCount === 0 with searchKeywords === '', rendering the wrong state.

Maybe we should add an isFiltered flag inside our search context to check this condition and use it here.

Good catch! Maybe we can adjust the message.. have asked Product.

Addressed with 94ac0a1.

… components

open-craft-grove · 2024-07-26T16:23:34Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-26T16:24:12Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

This handles the case where "Recently Published" sort is selected, which filters out published components.

open-craft-grove · 2024-07-27T20:36:26Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-27T20:39:52Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-27T23:09:00Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-27T23:10:30Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

rpenido

LGTM 👍
Thank you for your work, @pomegranited!
I left a comment about the defaultProps (but it is not blocking, in my opinion).

I tested this using the instructions from the PR instructions
I read through the code
I checked for accessibility issues
Includes documentation

src/search-manager/ClearFiltersButton.tsx

bradenmacdonald

Looks great! I can merge tomorrow if you address the minor comments from me and @rpenido

bradenmacdonald · 2024-07-29T17:15:14Z

src/library-authoring/LibraryAuthoringPage.test.tsx

@@ -50,6 +50,21 @@ const returnEmptyResult = (_url, req) => {
  return mockEmptyResult;
 };

+const returnLowNumberResults = (_url, req) => {


Please add a comment explaining what this function is doing; it's not obvious to me from the name.

src/search-manager/SearchManager.ts

* use only a single state value (searchParams) * wrap returnSetter in useCallback to ensure the setter remains constant * don't modify searchParams directly -- copy and use setter only.

open-craft-grove · 2024-07-29T19:23:19Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-29T19:23:22Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-29T19:56:30Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-07-29T19:58:42Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

openedx-webhooks · 2024-07-30T16:41:13Z

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

pomegranited requested a review from a team as a code owner July 10, 2024 09:05

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 10, 2024

pomegranited requested a review from yusuf-musleh July 10, 2024 09:14

This was referenced Jul 10, 2024

feat: adds sort widget to search manager and library component page [FC-0059] open-craft/frontend-app-authoring#51

Closed

feat: adds sortable fields to studio content search index [FC-0059] openedx/edx-platform#35103

Merged

pomegranited added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Jul 10, 2024

pomegranited mentioned this pull request Jul 20, 2024

feat: Add "Recently Modified" library section [FC-0059] #1151

Closed

pomegranited and others added 2 commits July 26, 2024 07:26

feat: sort library components

6924591

pomegranited force-pushed the jill/FAL-3758-sort-components branch from 19c869a to f1389e3 Compare July 25, 2024 22:22

rpenido reviewed Jul 26, 2024

View reviewed changes

fix: filter out last_published = NULL when sorting Recently Published…

ca80bb5

… components

pomegranited mentioned this pull request Jul 27, 2024

fix: content search last_published date [FC-0059] openedx/edx-platform#35195

Merged

fix: show "No matching components found" if filtering

94ac0a1

This handles the case where "Recently Published" sort is selected, which filters out published components.

fix: rearrange failing test

3fe6f86

pomegranited requested a review from rpenido July 28, 2024 19:07

rpenido approved these changes Jul 29, 2024

View reviewed changes

src/search-manager/ClearFiltersButton.tsx Outdated Show resolved Hide resolved

src/search-manager/ClearFiltersButton.tsx Outdated Show resolved Hide resolved

src/search-manager/ClearFiltersButton.tsx Outdated Show resolved Hide resolved

bradenmacdonald reviewed Jul 29, 2024

View reviewed changes

pomegranited added 3 commits July 30, 2024 04:07

fix: don't use defaultProps

74c2d42

docs: adds comments for test functions

cc2dd74

fix: useStateWithUrlSearchParam to use best practices

2d1b213

* use only a single state value (searchParams) * wrap returnSetter in useCallback to ensure the setter remains constant * don't modify searchParams directly -- copy and use setter only.

pomegranited requested a review from bradenmacdonald July 29, 2024 19:26

bradenmacdonald approved these changes Jul 30, 2024

View reviewed changes

bradenmacdonald merged commit 4f88948 into openedx:master Jul 30, 2024
7 checks passed

bradenmacdonald deleted the jill/FAL-3758-sort-components branch July 30, 2024 16:41

mphilbrick211 added the FC Relates to an Axim Funded Contribution project label Jul 30, 2024

pomegranited mentioned this pull request Jul 30, 2024

test: fix flaky library-authoring test [FC-0059] #1193

Merged

pomegranited mentioned this pull request Aug 21, 2024

UI fixes for Sort Library Component [FC-0059] #1222

Merged

	extraFilter.push('last_published IS NOT EMPTY');
	extraFilter.push('last_published IS NOT NULL');

feat: adds sort widget to search manager and library component page [FC-0059] #1147

feat: adds sort widget to search manager and library component page [FC-0059] #1147

Conversation

pomegranited commented Jul 10, 2024 • edited Loading

Description

Supporting information

Testing instructions

openedx-webhooks commented Jul 10, 2024 • edited Loading

What's next?

🔘 Get product approval

🔘 Provide context

🔘 Get a green build

🔘 Let us know that your PR is ready for review:

Who will review my changes?

Where can I find more information?

When can I expect my changes to be merged?

codecov bot commented Jul 10, 2024 • edited Loading

Codecov Report

open-craft-grove commented Jul 10, 2024

open-craft-grove commented Jul 10, 2024

open-craft-grove commented Jul 11, 2024

open-craft-grove commented Jul 12, 2024

open-craft-grove commented Jul 12, 2024

open-craft-grove commented Jul 12, 2024

open-craft-grove commented Jul 12, 2024

open-craft-grove commented Jul 12, 2024

open-craft-grove commented Jul 19, 2024

open-craft-grove commented Jul 19, 2024

open-craft-grove commented Jul 20, 2024

open-craft-grove commented Jul 20, 2024

open-craft-grove commented Jul 24, 2024

open-craft-grove commented Jul 24, 2024

pomegranited commented Jul 25, 2024 • edited Loading

open-craft-grove commented Jul 25, 2024

open-craft-grove commented Jul 25, 2024

rpenido commented Jul 26, 2024

rpenido Jul 26, 2024

Choose a reason for hiding this comment

pomegranited Jul 26, 2024

Choose a reason for hiding this comment

pomegranited Jul 26, 2024

Choose a reason for hiding this comment

rpenido Jul 26, 2024

Choose a reason for hiding this comment

pomegranited Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

pomegranited Jul 27, 2024

Choose a reason for hiding this comment

open-craft-grove commented Jul 26, 2024

open-craft-grove commented Jul 26, 2024

open-craft-grove commented Jul 27, 2024

open-craft-grove commented Jul 27, 2024

open-craft-grove commented Jul 27, 2024

open-craft-grove commented Jul 27, 2024

rpenido left a comment

Choose a reason for hiding this comment

bradenmacdonald left a comment

Choose a reason for hiding this comment

bradenmacdonald Jul 29, 2024

Choose a reason for hiding this comment

open-craft-grove commented Jul 29, 2024

open-craft-grove commented Jul 29, 2024

open-craft-grove commented Jul 29, 2024

open-craft-grove commented Jul 29, 2024

openedx-webhooks commented Jul 30, 2024

pomegranited commented Jul 10, 2024 •

edited

Loading

openedx-webhooks commented Jul 10, 2024 •

edited

Loading

codecov bot commented Jul 10, 2024 •

edited

Loading

pomegranited commented Jul 25, 2024 •

edited

Loading

pomegranited Jul 26, 2024 •

edited

Loading