-
Notifications
You must be signed in to change notification settings - Fork 528
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
Migrate getTopicList() to DataProvider #2253
Conversation
TopicListController.getTopicList() was just a LiveData before, and this disallows us from leveraging other DataProviders tools. Plus, the general pattern now is for all asynchronous data in controllers to be given as DataProviders.
@jcqli PTAL. |
@BenHenning ok I think I'm understanding this now, thanks! FYI the robolectric tests are failing on cases that I think are related to this change |
This file comes from #1904. We need it to simplify Bazel build commands locally (bazelrc lets us automatically add arguments to certain commands like build to make it easier to type the commands in CLI locally). This file will also be used by Android Studio when using the Android Studio-with-Bazel plugin.
These new syncs are needed since we're no longer using an immediate LiveData for the topic list--background threads need to be sync'd against the UI.
Thanks @jcqli! The tests should be fixed now. PTAL! |
I think the two failures so far are flakes. Will re-run when app module tests finish. |
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 looks good and all makes sense to me!
Thanks @jcqli! |
…#2246) * Use BindableAdapter for displaying Welcome layout, still updating PromotedStoryList layout * Refactor PromotedStoryListViewModel to update using BindableAdapter * Migrate TopicListViewHolder logic * Bind TopicSummary padding and HomeItemList * Compute margins and bind to layout in topic summary view * Add databinding for topic summary margins * Pushing latest changes to compare * Return int to layout margins * Clean up commented code * Use app-defined margin layouts for data binding * Add binding to HomeViewModel * Remove use of ViewModelProvider in HomeFragmentPresenter * Bind PromotedStoryList layout values * Add data binding for sotry & layout of PromotedStory card * Add BindingAdapter for layout height * Create custom recycler view for PromotedStoryList * Remove unused imports & TopicListAdapter * Construct adapter in custom view * Debugging layout data binding * Add PromotedStoryCard to custom promoted story list view * Remove unused imports * Set topic summary position to start at 1 * Set adapter in initializer * Refactor to process TopicList later * Revert "Refactor to process TopicList later" This reverts commit fd9e418. * Process welcome in home view model * Move processing logic for all view models to home * Add model for home fragment data to combine data providers * Apply patch for combining home view model data and adjust topic list margins * Add orientation to tablet layouts * Use promoted story list for itemCount * Bind list of ChapterSummaryViewModel instead of ChapterSummary to fix lesson crash * Fix linter errors & remove PromotedStoryListAdapter * Add viewmodels to bazel build * Fix more linter errors * Fix more linter errors * Revert "Add viewmodels to bazel build" This reverts commit f090330. * Apply test changes from #2253 * Address review comments * Fix linter errors * Fix linter errors * Add view models to bazel build file * Refactor all topics list to return empty list if no topics * Remove duplicated HomeViewModel in bazel build * Fix linter error & filepath for AllTopicsViewModel in bazel build * Remove test helper for home fragment * Add completing all topics to story progress test helper * Add tests for no promtoed stories on home fragment and all topics completed * Address review comments * Set View All button visiblity in view model * Fix linter issues * Add comment to explain test topic exploration progress in test helper * Address review comments * Set routing / listeners in home activity * More review comments * Address review comments * Fix linter issues * Fix linter errors * Fix test errors (linter & bazel) * Fix bazel errors and add tests for no user progress case * Remove double declaration of TopicSummaryViewModel * Move PromotedStoryViewModel to be declared with resources * Remove curly brace on padding dimensions in promotedstorylistview * Remove Int type from padding value * Address review comments * Add BindingAdapter for padding end * Remove unused import * Fix comments * Use int in BindingAdapter * Rename BindingAdapter? * Refactor BindingAdapter naming * Add view to bazel build * Add SnapHelper to build * Create HomeFragmentView to handle layout * Handle layout in presenter again * Fix linter errors * Add recyclerview to snap helper in build * Add databidning dependency * WOrk on bazel errors * WOrk on bazel errors * WOrk on bazel errors * Enable data-binding in build? * Remove enable data binding for snap helper * Add custom package? * Use shim for bindings in PromotedStoryListView * Address review comments * Attach to window and use view component * Use application injection in PromotedStoryListView, add bounds check for getSpanSize * Fix linter errors * Use fragment injection in custom recycler view, override equals in view models * Shorten error message * Shorten error message * Add hashcode to view models and address review comments * Start tests for view model equals * Add view model tests * Use test activity to get fragment * Create intent in test activity * Add test for WelcomeViewModel * Clarify test names and start PromotedStoryListViewModel * Add tests for equals definition and hashcode of WelcomeViewModel and PromotedStoryViewModel * Add tests for PromotedStoryListViewModel * Add tests for TopicSummaryViewModel * Shorten comment * Address review comments * Separate WelcomeViewModel into own test * Separate PromotedStoryViewModel test * Separate PromotedStoryListViewModel tests * Separate TopicSummaryViewModel tests * Remove combined ViewModel tests * Fix lint errors * Create fragment in view model test * Remove fragment transaction from test activity * Fix linter errors * Use helpers and improve readability of promoted story list view model test * Improve readability for WelcomeViewModel tests * Simplilfy promtoed story view model and welcome view model tests * Simplify topic summary and promtoed story view models * Clarify test names * Move test helpers beneath test cases * Address review comments * Address comments for HomeActivityTest * Import ActivityScenario.launch for view model tests * Use named parameters in StoryProgressTestHelperTest * Clarify new user used for no story progress in ProfileTestHelpr * Order view model alphabetically * Add readability fixes from review * Fix comments * Add dummy data in promoted story list view to appropriately initiate adapter creation * Use BindingAdapter on the data list of promtoed stories * Remove static binding adapter for data-binding in promoted story list view * Address review comments * Remove extra space in layout * Improve test readability * Check column count before testing span in HomeActivityTest * Add test for promoted story list view bug * Rename tag for clarity * Comments for test * Address review comments * Address review comments * Fix linter errors * Fix linter errors * Add test to check correct max number of promoted stories shown * Delete home view model test * Fix import * Address review comments * Change function name * Add null check for promoted story list data * Fix typo
* Revert "Add viewmodels to bazel build" This reverts commit f090330. * fixed recommended list * Apply test changes from #2253 * Address review comments * Fix linter errors * Fix linter errors * fixed index error * added timestamp for recently played * Add view models to bazel build file * Refactor all topics list to return empty list if no topics * Remove duplicated HomeViewModel in bazel build * Fix linter error & filepath for AllTopicsViewModel in bazel build * removed walkthrough field * writting testcases * fixing testcase * Seperated UI part from this PR * Removed UI implementation from this PR * Update TopicListControllerTest.kt * Remove test helper for home fragment * Add completing all topics to story progress test helper * Add tests for no promtoed stories on home fragment and all topics completed * Address review comments * Set View All button visiblity in view model * Fix linter issues * Add comment to explain test topic exploration progress in test helper * renamed functions * updated testcases * Update topic.proto * updated proto. * Address review comments * Set routing / listeners in home activity * More review comments * Address review comments * Fix linter issues * Fix linter errors * Fix test errors (linter & bazel) * Update TopicListController.kt * Fix bazel errors and add tests for no user progress case * Remove double declaration of TopicSummaryViewModel * Move PromotedStoryViewModel to be declared with resources * Remove curly brace on padding dimensions in promotedstorylistview * Remove Int type from padding value * updated nit changes. * fixed nit changes * Update TopicListController.kt * Delete gradle.xml * Update strings.xml * Update TopicListControllerTest.kt * Update TopicListControllerTest.kt * Address review comments * Add BindingAdapter for padding end * Remove unused import * Fix comments * Use int in BindingAdapter * Rename BindingAdapter? * Refactor BindingAdapter naming * Add view to bazel build * Add SnapHelper to build * Create HomeFragmentView to handle layout * Handle layout in presenter again * Fix linter errors * Add recyclerview to snap helper in build * Add databidning dependency * WOrk on bazel errors * WOrk on bazel errors * WOrk on bazel errors * Enable data-binding in build? * Remove enable data binding for snap helper * Add custom package? * Use shim for bindings in PromotedStoryListView * Address review comments * Attach to window and use view component * Use application injection in PromotedStoryListView, add bounds check for getSpanSize * Fix linter errors * updated implementaion with stories for you * updated code. * Update TopicListControllerTest.kt * Update topic.proto * fixed nit changes and lint error * Update topic.proto * Delete coming_soon_topics.json * Update TopicListControllerTest.kt * Use fragment injection in custom recycler view, override equals in view models * Shorten error message * Shorten error message * fixed nit changes * Add hashcode to view models and address review comments * Start tests for view model equals * Add view model tests * Use test activity to get fragment * Create intent in test activity * Add test for WelcomeViewModel * Clarify test names and start PromotedStoryListViewModel * Update test_exp_id_4.json * Add tests for equals definition and hashcode of WelcomeViewModel and PromotedStoryViewModel * Add tests for PromotedStoryListViewModel * Update TopicListController.kt * Add tests for TopicSummaryViewModel * Shorten comment * Address review comments * Separate WelcomeViewModel into own test * Separate PromotedStoryViewModel test * Separate PromotedStoryListViewModel tests * Separate TopicSummaryViewModel tests * Remove combined ViewModel tests * Fix lint errors * Create fragment in view model test * Remove fragment transaction from test activity * Fix linter errors * Use helpers and improve readability of promoted story list view model test * Improve readability for WelcomeViewModel tests * Simplilfy promtoed story view model and welcome view model tests * Simplify topic summary and promtoed story view models * Clarify test names * Update TopicController.kt * Fixed nit. * fixed issues * Update TopicListController.kt * Update model/src/main/proto/topic.proto Co-authored-by: Ben Henning <[email protected]> * Removed ongoingstory list. * Move test helpers beneath test cases * Address review comments * Optimized code. * Update StoryProgressController.kt * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Ben Henning <[email protected]> * fixing nit changes * Update HomeViewModel.kt * Address comments for HomeActivityTest * Import ActivityScenario.launch for view model tests * Use named parameters in StoryProgressTestHelperTest * Clarify new user used for no story progress in ProfileTestHelpr * Order view model alphabetically * Add readability fixes from review * Fix comments * Add dummy data in promoted story list view to appropriately initiate adapter creation * Merge commit 'refs/pull/2246/head' of https://github.com/oppia/oppia-android into refactor-topic-list-adapter-to-use-bindable-adapter-veena * Use BindingAdapter on the data list of promtoed stories * Remove static binding adapter for data-binding in promoted story list view * Address review comments * updated * Remove extra space in layout * updated topic ontroller * Update promoted_story_list.xml * fixed implementation and updated testcases * updated * fixed lint errors * Update StoryProgressTestHelperTest.kt * Update StoryProgressTestHelperTest.kt * fixed lint * Update topic.proto * Update TopicListController.kt * Fixed issues * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Ben Henning <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> * fixed nit changes * Update TopicListController.kt * Update TopicListController.kt * Update TopicListControllerTest.kt * Update TopicListControllerTest.kt * updated implementation * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Ben Henning <[email protected]> * updated storyProgressController * updated changes * Update topic.proto * Updated changes * Update domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt Co-authored-by: Ben Henning <[email protected]> * implemented changes * updated list ids * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Farees Hussain <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Farees Hussain <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Farees Hussain <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Farees Hussain <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Farees Hussain <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/StoryProgressTestHelper.kt Co-authored-by: Farees Hussain <[email protected]> * updated * added helper tests * Update StoryProgressTestHelperTest.kt * Update TopicController.kt * Updated Suggestion logic * Update TopicListController.kt * rename testcase * Updated implementation changes and testcases as per discussion. * updated with develop code to pass ci tests. * fixed changes * Update TopicListControllerTest.kt * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> * Update domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt Co-authored-by: Ben Henning <[email protected]> * fixed testcases and implementation * Update TopicListController.kt * fixing changes * Update TopicListControllerTest.kt * Update TopicListControllerTest.kt * Fixed suggested changes * Update StoryProgressTestHelper.kt * Update StoryProgressTestHelperTest.kt * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> * Update TopicListControllerTest.kt * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> * Fixed itestcases * Update TopicListControllerTest.kt * Update domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt Co-authored-by: Ben Henning <[email protected]> * Update domain/src/test/java/org/oppia/android/domain/topic/TopicListControllerTest.kt Co-authored-by: Ben Henning <[email protected]> * Update domain/src/main/java/org/oppia/android/domain/topic/TopicListController.kt Co-authored-by: Ben Henning <[email protected]> Co-authored-by: jacqueli <[email protected]> Co-authored-by: Ben Henning <[email protected]> Co-authored-by: Farees Hussain <[email protected]>
TopicListController.getTopicList()
was just aLiveData
before which prevents us from leveraging otherDataProviders
tools. This PR migrates it to aDataProvider
to follow the general pattern to always provide data from controllers inDataProvider
s and to let us interop better with otherDataProvider
s.