-
Notifications
You must be signed in to change notification settings - Fork 119
Bookings: Update service/event filter to search by name only #16306
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
Conversation
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had two small comments, but it works as described 👍
| case .all: | ||
| case .all, .name: | ||
| let searchFields: [ProductSearchField] = { | ||
| if filter == .name { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This extra layer of if makes this a bit hard to read. I'd rather extract try await remote.searchProducts(for: siteID, .... to a helper functuion and do something like
switch filter {
case .all:
products = try await searchByKeyword(searchFields: [])
case .name:
products = try await searchByKeyword(searchFields: [.name])
case .sku:
products = try await remote.searchProductsBySKU(
for: siteID,
keyword: keyword,
pageNumber: pageNumber,
pageSize: pageSize
)
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 26476a9.
|
|
||
| // Takes precedence over `search_name_or_sku` from WC 10.1+ and is combined with `search` value | ||
| parameters.updateValue([SearchField.name, SearchField.sku, SearchField.globalUniqueID], forKey: ParameterKey.searchFields) | ||
| parameters.updateValue([ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you've only updated the mapping here, but the documentation for this function seems to lag, as it still says it searches in "name, description, and short_description". Could you please update it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in 1414442.

Part of WOOMOB-1240
Description
This PR improves the search experience on Bookings' Service/Event filter.
Previously in #16294 I got a feedback regarding the search behavior of Service/Event:
The reason for this is that currently we're searching products without defining which fields to search. By default, this would mean matching keywords with product names, descriptions, short descriptions, SKUs, and global unique identifiers. On the Service/Event filter view, the only visible detail is product name, so returning results matching other fields might make it look like the search results are incorrect.
This PR improves the experience by asking the remote to search only by product names when filtering booking service/events. This comes with updates to the Networking and Yosemite layers to include the
search_fieldsin the network requests.Test Steps
Regression test: Since I updated the search request for POS in
ProductsRemote, we need to ensure that I didn't cause any regression.Screenshots
Simulator.Screen.Recording.-.iPad.mini.A17.Pro.-.2025-11-03.at.16.18.31.mov
RELEASE-NOTES.txtif necessary.