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

Search all posts and group results into sections #10837

Conversation

@chipsnyder
Copy link
Contributor

chipsnyder commented Nov 22, 2019

Fixes #9183

Description

This is to handle an enhancement to the current search in the Posts section of the App. Today the app only searches the published posts, where the goal will be to add the other types of posts to the results and mirror the look and feel of the Pages section of the app.

To do this in PostListItemDataSource the app will fetch the next list of data from GET /sites/$site/posts/ (sorted by date) and them group the posts into the sections. It then inserts the headers (if the section has results) and returns back a list with the grouped elements.

Notes:

  • In my testing GET /sites/$site/posts/ was throwing a random error. Removing "meta" from the fields seems to resolve this in my testing. This seems to be an API issue as I'm seeing it with the existing requests as well. We'll be letting the API team know so we can raise an issue.
  • The approach taken here follows the pattern done by WooCommerce but does have the potential for a user to scroll past a section, trigger another page of data, and get updates to a section they passed. Narrowing the results by adding the the search query more will limit this issue.

To test:

Searching Prompt
Given the user is on the Blog Posts Search screen
Then they should see "Search posts" instead of "Search published posts"

Searching with results
Given the user is on the Blog Posts Search screen
When they perform a search with results in multiple statuses
Then they should see the results separated into each status group sorted by date

Searching with results and scroll
Given the user is on the Blog Posts Search screen
When they perform a search with results in multiple statuses
And they trigger a new paged to be fetched
Then they should see the sections update with the new pages added.

Searching with no results (Unchanged behavior)
Given the user is on the Blog Posts Search screen
When they perform a search with no results
Then they should see a prompt "No posts matching your search"

PR submission checklist:

  • I have considered adding unit tests where possible.

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

chipsnyder added 15 commits Nov 20, 2019
- Modified PostListType to have the SEARCH type inclide all post statuses

- Modified PostListViewModel to to limit the number of posts per fetch because I was getting frequent 500 errors from the API when requesting 60 items per request
When searching after a page is fetched from the API the results are now grouped into sections.

Also reverted the change to ask for smaller page sizes as it appears that including "meta" in the "fields" parameter is the item that periodically causes a 500 from the server.
Moved Published to come before drafts to mirror the ordering of the tabs in the post list.
Reverted the other english translations. These will be translated automatically later.
…t is already available

postListType which is already provided gives the needed information on if this is a search or not
@peril-wordpress-mobile

This comment has been minimized.

Copy link

peril-wordpress-mobile bot commented Nov 22, 2019

Warnings
⚠️ PR is missing at least one label.
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@chipsnyder

This comment has been minimized.

Copy link
Contributor Author

chipsnyder commented Nov 22, 2019

Closing PR to resolve missing data

@chipsnyder chipsnyder closed this Nov 22, 2019
@chipsnyder chipsnyder reopened this Nov 22, 2019
@chipsnyder

This comment has been minimized.

Copy link
Contributor Author

chipsnyder commented Nov 22, 2019

@planarvoid Here's the PR opened to the main Repo

@chipsnyder chipsnyder mentioned this pull request Nov 25, 2019
2 of 2 tasks complete
Copy link
Contributor

planarvoid left a comment

Looks really good 👍 Thanks for the changes and for the new updated PR

@planarvoid planarvoid merged commit 722dcd2 into wordpress-mobile:develop Nov 25, 2019
4 checks passed
4 checks passed
Peril Found some issues. Don't worry, everything is fixable.
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: strings-check Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.