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.

Sign up for GitHub

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

Jump to bottom

Search backend #668

Merged
merged 17 commits into from
Apr 20, 2023
Merged

Search backend #668

merged 17 commits into from
Apr 20, 2023

Conversation

thagikura
Copy link
Contributor

@thagikura thagikura commented Apr 17, 2023 •
edited
Loading

Adds search UI (same change as #656)

  • Empty search screen if the search result is empty
  • Topics contents that reuses the TopicsTabContent for InterestsScreen
  • Updates contents that reuses the newsFeed for ForYouScreen
    TODO: Needs to add RecentSearch

Adds search backend that wires the backend data with the search UI

  • Add Fts related tables.
  • Add SearchContentsRepository that takes care of populating the Fts tables and search the contents from the Fts tables.
  • Wire the SearchContentsRepository with the SearchViewModel

Screenshot:

@thagikura
Add UI skeleton for the search feature
86fe3b2 
This CL adds an UI skeleton of the search feature including:
- The search navigation icon in the NiaTopAppBar
- The text field to enter the search query
- Almost empty ViewModel

Bug: 216019540
Change-Id: Ic2355a7634e5c5c60f72276741175d9e637db5eb
@thagikura
Fix copyright header
5e28769 
Change-Id: I70a0732f67617a96b6dd20eccbc6c02a9dafdabd
@thagikura
Fix formatting issues
2eddf0b 
Change-Id: Id9efb259bd8ac6a83af19fb370869aa19f78831d
@thagikura
Implement SearchScreen
13e6cc4 
- Empty search screen if the search result is empty
- Topics contents that reuses the TopicsTabContent for InterestsScreen
- Updates contents that reuses the newsFeed for ForYouScreen

TODO: Needs to add RecentSearch
Change-Id: I19179336a60b5165e8b59508fb03dd8f55a16f96
@thagikura
Fix format issues
83b640a 
Change-Id: I5b87f89d824961cb9aec039281e784d6dec0716b
@thagikura
Wire the SearchScreen and the backend of search related data
6f9d2fe 
Change-Id: I103c903c936137c41d7d11be0731438d9ee557d9
@thagikura
Fix format issues
477838f
@thagikura thagikura mentioned this pull request Apr 17, 2023
Closed
@thagikura
Address review feedback
37d4b65 
Change-Id: If336af8fa5c58d6401dfd7a561a1a340ff388175
@thagikura
Fix format issues
55483e8 
Change-Id: I94aefab87ff9e05819800c516a0f2979f5f89d76
@thagikura thagikura mentioned this pull request Apr 18, 2023
Closed
@thagikura
Copy link
Contributor Author

Note that #670 that implements recent searches is opened.

To make the review easier, it may be better to merge this PR (#668) first then review #670 afterwards.

@mmoczkowski mmoczkowski self-requested a review April 19, 2023 08:34
mmoczkowski
mmoczkowski approved these changes Apr 19, 2023
View reviewed changes
Copy link
Contributor

@mmoczkowski mmoczkowski left a comment

Choose a reason for hiding this comment

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

Great feature, neatly implemented. Just a couple of nits and questions.

...com/google/samples/apps/nowinandroid/core/data/repository/DefaultSearchContentsRepository.kt Show resolved Hide resolved
...m/google/samples/apps/nowinandroid/core/data/repository/fake/FakeSearchContentsRepository.kt Outdated Show resolved Hide resolved
.../main/java/com/google/samples/apps/nowinandroid/core/database/model/NewsResourceFtsEntity.kt Show resolved Hide resolved
...n/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetSearchContentsUseCase.kt Outdated Show resolved Hide resolved
...n/src/main/java/com/google/samples/apps/nowinandroid/core/domain/GetSearchContentsUseCase.kt Outdated Show resolved Hide resolved
...com/google/samples/apps/nowinandroid/core/testing/repository/TestSearchContentsRepository.kt Outdated Show resolved Hide resolved
.../search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchViewModel.kt Outdated Show resolved Hide resolved
.../search/src/main/java/com/google/samples/apps/nowinandroid/feature/search/SearchViewModel.kt Outdated Show resolved Hide resolved
...main/java/com/google/samples/apps/nowinandroid/feature/search/navigation/SearchNavigation.kt Outdated Show resolved Hide resolved
GallFox99
GallFox99 reviewed Apr 20, 2023
View reviewed changes
...com/google/samples/apps/nowinandroid/core/data/repository/DefaultSearchContentsRepository.kt

val newsResourcesFlow = newsResourceIds
.mapLatest { it.toSet() }
.distinctUntilChanged()
Copy link

Choose a reason for hiding this comment

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

flatMapLatest {
newsResourceDao.getNewsResources(filterNewsIds = it)
}
val topicsFlow = topicIds
.mapLatest { it.toSet() }

         .distinctUntilChanged()
         .flatMapLatest(topicDao::getTopicEntities)
     return combine(newsResourcesFlow, topicsFlow) { newsResources, topics ->
         SearchResult(
             topics = topics.map { it.asExternalModel() },
             newsResources = newsResources.map { it.asExternalModel() },

GallFox99
GallFox99 reviewed Apr 20, 2023
View reviewed changes
Copy link

@GallFox99 GallFox99 left a comment

Choose a reason for hiding this comment

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

     .distinctUntilChanged()
     .flatMapLatest(topicDao::getTopicEntities)
 return combine(newsResourcesFlow, topicsFlow) { newsResources, topics ->
     SearchResult(
         topics = topics.map { it.asExternalModel() },
         newsResources = newsResources.map { it.asExternalModel() },

@thagikura thagikura merged commit 37b0917 into thagikura/search Apr 20, 2023
thagikura added a commit that referenced this pull request Apr 25, 2023
@thagikura
Search backend (#668)
9c0d6fc 
Adds search UI (same change as #656)

- Empty search screen if the search result is empty
- Topics contents that reuses the TopicsTabContent for InterestsScreen
- Updates contents that reuses the newsFeed for ForYouScreen
TODO: Needs to add RecentSearch

Adds search backend that wires the backend data with the search UI

- Add Fts related tables.
- Add SearchContentsRepository that takes care of populating the Fts tables and search the contents from the Fts tables.
- Wire the SearchContentsRepository with the SearchViewModel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoc081098 hoc081098 hoc081098 left review comments

@GallFox99 GallFox99 GallFox99 left review comments

@mmoczkowski mmoczkowski mmoczkowski approved these changes

@dturner dturner Awaiting requested review from dturner

@neelanshsahai neelanshsahai Awaiting requested review from neelanshsahai

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@thagikura @mmoczkowski @hoc081098 @GallFox99