Skip to content

Enable remote search in Themes if custom themes are supported #24262

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@
"location" : "https://github.com/wordpress-mobile/WordPressKit-iOS",
"state" : {
"branch" : "wpios-edition",
"revision" : "554d3ef682af4cfd7888e33658789df48b67c408"
"revision" : "7dc4ed37223be387e95a5d68e23f1dc956db318b"
}
},
{
Expand Down
2 changes: 2 additions & 0 deletions WordPress/Classes/Services/ThemeService.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ typedef void(^ThemeServiceFailureBlock)(NSError *error);
*
* @param blogId The blog to get the themes for. Cannot be nil.
* @param page Results page to return.
* @param search Search string to filter themes.
* @param sync Whether to remove unsynced results.
* @param success The success handler. Can be nil.
* @param failure The failure handler. Can be nil.
Expand All @@ -56,6 +57,7 @@ typedef void(^ThemeServiceFailureBlock)(NSError *error);
*/
- (NSProgress *)getThemesForBlog:(Blog *)blog
page:(NSInteger)page
search:(NSString *)search
sync:(BOOL)sync
success:(ThemeServiceThemesRequestSuccessBlock)success
failure:(ThemeServiceFailureBlock)failure;
Expand Down
6 changes: 6 additions & 0 deletions WordPress/Classes/Services/ThemeService.m
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ - (NSProgress *)getActiveThemeForBlog:(Blog *)blog

- (NSProgress *)getThemesForBlog:(Blog *)blog
page:(NSInteger)page
search:(NSString *)search
sync:(BOOL)sync
success:(ThemeServiceThemesRequestSuccessBlock)success
failure:(ThemeServiceFailureBlock)failure
Expand All @@ -157,6 +158,10 @@ - (NSProgress *)getThemesForBlog:(Blog *)blog
NSAssert([self blogSupportsThemeServices:blog],
@"Do not call this method on unsupported blogs, check with blogSupportsThemeServices first.");

if (search.length == 0) {
search = nil;
}

if (blog.wordPressComRestApi == nil) {
return nil;
}
Expand All @@ -165,6 +170,7 @@ - (NSProgress *)getThemesForBlog:(Blog *)blog

if ([blog supports:BlogFeatureCustomThemes]) {
return [remote getWPThemesPage:page
search:search
freeOnly:![blog supports:BlogFeaturePremiumThemes]
success:^(NSArray<RemoteTheme *> *remoteThemes, BOOL hasMore, NSInteger totalThemeCount) {
NSArray * __block themeObjectIDs = nil;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -452,11 +452,12 @@ public protocol ThemePresenter: AnyObject {
}
}

fileprivate func syncThemePage(_ page: NSInteger, success: ((_ hasMore: Bool) -> Void)?, failure: ((_ error: NSError) -> Void)?) {
private func syncThemePage(_ page: NSInteger, search: String, success: ((_ hasMore: Bool) -> Void)?, failure: ((_ error: NSError) -> Void)?) {
assert(page > 0)
themesSyncingPage = page
_ = themeService.getThemesFor(blog,
page: themesSyncingPage,
search: search,
sync: page == 1,
success: {[weak self](themes: [Theme]?, hasMore: Bool, themeCount: NSInteger) in
if let success {
Expand Down Expand Up @@ -509,7 +510,7 @@ public protocol ThemePresenter: AnyObject {

func syncHelper(_ syncHelper: WPContentSyncHelper, syncContentWithUserInteraction userInteraction: Bool, success: ((_ hasMore: Bool) -> Void)?, failure: ((_ error: NSError) -> Void)?) {
if syncHelper == themesSyncHelper {
syncThemePage(1, success: success, failure: failure)
syncThemePage(1, search: searchName, success: success, failure: failure)
} else if syncHelper == customThemesSyncHelper {
syncCustomThemes(success: success, failure: failure)
}
Expand All @@ -518,7 +519,7 @@ public protocol ThemePresenter: AnyObject {
func syncHelper(_ syncHelper: WPContentSyncHelper, syncMoreWithSuccess success: ((_ hasMore: Bool) -> Void)?, failure: ((_ error: NSError) -> Void)?) {
if syncHelper == themesSyncHelper {
let nextPage = themesSyncingPage + 1
syncThemePage(nextPage, success: success, failure: failure)
syncThemePage(nextPage, search: searchName, success: success, failure: failure)
}
}

Expand Down Expand Up @@ -657,11 +658,65 @@ public protocol ThemePresenter: AnyObject {

// MARK: - Search support

private var searchDebounceTimer: Timer?
private let searchDebounceInterval: TimeInterval = 0.5

private func resetRemoteSearch() {
themesSyncingPage = 0

if blog.supports(BlogFeature.customThemes) {
themesSyncHelper.syncContent()
}
}

fileprivate func beginSearchFor(_ pattern: String) {
searchController.isActive = true
searchController.searchBar.text = pattern

searchName = pattern
updateSearchName(pattern)
}

private func updateSearchName(_ searchText: String) {
// Cancel any existing timer
searchDebounceTimer?.invalidate()

// If search text is empty, update immediately and reset remote search
if searchText.isEmpty {
self.searchName = searchText
self.fetchThemes()
self.resetRemoteSearch()
self.reloadThemes()
return
}

// Check if we have a previously longer search that is now under 3 characters
let previouslyHadRemoteSearch = self.searchName.count >= 3

// Create a new timer for debounce
searchDebounceTimer = Timer.scheduledTimer(withTimeInterval: searchDebounceInterval, repeats: false) { [weak self] _ in
guard let self else { return }
self.searchName = searchText

// Apply local search immediately
self.fetchThemes()

// Remote search only applies to WordPress.com themes and only if customThemes are supported.
// The remote endpoint support search just for 3+ characters
if self.blog.supports(BlogFeature.customThemes) {
if searchText.count >= 3 {
// Reset to first page when searching
self.themesSyncingPage = 0
self.themesSyncHelper.syncContent()
} else if previouslyHadRemoteSearch {
// If we previously had 3+ characters but now have less,
// we need to reset the remote search results
self.resetRemoteSearch()
}
}

// Always reload with local results
self.reloadThemes()
}
}

// MARK: - UISearchControllerDelegate
Expand All @@ -682,6 +737,7 @@ public protocol ThemePresenter: AnyObject {
hideSectionHeaders = false
searchName = ""
searchController.searchBar.text = ""
resetRemoteSearch()
}

open func didDismissSearchController(_ searchController: UISearchController) {
Expand Down Expand Up @@ -709,31 +765,44 @@ public protocol ThemePresenter: AnyObject {
// MARK: - UISearchResultsUpdating

open func updateSearchResults(for searchController: UISearchController) {
searchName = searchController.searchBar.text ?? ""
updateSearchName(searchController.searchBar.text ?? "")
}

// MARK: - NSFetchedResultsController helpers

fileprivate func searchNamePredicate() -> NSPredicate? {
guard !searchName.isEmpty else {
return nil
}

return NSPredicate(format: "name contains[c] %@", searchName)
}

fileprivate func browsePredicate() -> NSPredicate? {
return browsePredicateThemesWithCustomValue(false)
}

fileprivate func customThemesBrowsePredicate() -> NSPredicate? {
return browsePredicateThemesWithCustomValue(true)
let browsePredicate = browsePredicateThemesWithCustomValue(true)

// Search predicate for custom themes (local search only)
if !searchName.isEmpty {
let searchPredicate = NSPredicate(format: "name CONTAINS[cd] %@", searchName)
if let existingPredicate = browsePredicate {
return NSCompoundPredicate(andPredicateWithSubpredicates: [existingPredicate, searchPredicate])
} else {
return searchPredicate
}
}

return browsePredicate
}

fileprivate func browsePredicateThemesWithCustomValue(_ custom: Bool) -> NSPredicate? {
let blogPredicate = NSPredicate(format: "blog == %@ AND custom == %d", self.blog, custom ? 1 : 0)

let subpredicates = [blogPredicate, searchNamePredicate(), filterType.predicate].compactMap { $0 }
let subpredicates = [blogPredicate, filterType.predicate].compactMap { $0 }

// For regular themes, add local search predicate if:
// 1. Not using custom themes feature, or
// 2. Search term is less than 3 characters (we'll only search locally for short terms)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noting that it's an unfortunate limitation.

if !searchName.isEmpty && !custom && (!blog.supports(BlogFeature.customThemes) || searchName.count < 3) {
let searchPredicate = NSPredicate(format: "name CONTAINS[cd] %@", searchName)
return NSCompoundPredicate(andPredicateWithSubpredicates: subpredicates + [searchPredicate])
}

switch subpredicates.count {
case 1:
return subpredicates[0]
Expand Down
2 changes: 2 additions & 0 deletions WordPress/WordPressTest/ThemeServiceTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ - (void)testThatGetThemesForBlogWorks
XCTAssertNoThrow(service = [[ThemeService alloc] initWithCoreDataStack:self.manager]);
XCTAssertNoThrow([service getThemesForBlog:blog
page:1
search:nil
sync:NO
success:nil
failure:nil]);
Expand All @@ -124,6 +125,7 @@ - (void)testThatGetThemesForBlogThrowsExceptionWithoutBlog
XCTAssertNoThrow(service = [[ThemeService alloc] initWithCoreDataStack:self.manager]);
XCTAssertThrows([service getThemesForBlog:nil
page:1
search:nil
sync:NO
success:nil
failure:nil]);
Expand Down