Properly placed pages tests #8303
Conversation
| set(value) { | ||
| _pageMap = value | ||
| _pages.postValue(pageMap.values.toList()) | ||
| field = value |
| @@ -194,16 +190,16 @@ class PagesViewModel | |||
| _isNewPageButtonVisible.postOnUi(isNotEmpty && currentPageType != TRASHED && _isSearchExpanded.value != true) | |||
| } | |||
|
|
|||
| fun onSearch(searchQuery: String) { | |||
| fun onSearch(searchQuery: String, delay: Long = 200) { | |||
There was a problem hiding this comment.
We could extract the number literal into a constant.
| @@ -226,7 +222,7 @@ class PagesViewModel | |||
| _isSearchExpanded.value = false | |||
| clearSearch() | |||
|
|
|||
| launch { | |||
| launch(uiContext) { | |||
| delay(500) | |||
There was a problem hiding this comment.
Same here, this can be a constant (I know it's my code 🙃).
|
|
||
| pageStore.uploadPageToServer(changed) | ||
| } | ||
| } | ||
| } | ||
| action.onSuccess = { | ||
| launch(CommonPool) { | ||
| launch(commonPoolContext) { | ||
| reloadPages() | ||
|
|
||
| delay(100) |
|
|
||
| checkIfNewPageButtonShouldBeVisible() | ||
|
|
||
| pageStore.deletePageFromServer(page) | ||
| } | ||
| } | ||
| action.onSuccess = { | ||
| launch(CommonPool) { | ||
| launch(commonPoolContext) { | ||
| delay(100) |
| pageStore.updatePageInDb(page) | ||
| refreshPages() | ||
|
|
||
| pageStore.uploadPageToServer(page) | ||
| } | ||
| } | ||
| action.onSuccess = { | ||
| launch(CommonPool) { | ||
| launch(commonPoolContext) { | ||
| delay(100) |
| if (page.parent?.remoteId != parentId) { | ||
| page.parent = _pageMap[parentId] | ||
| pageMap = _pageMap |
There was a problem hiding this comment.
By removing this line, the logic in the setter is not called and we lost the instant updates. We could just extract that code and call it from here explicitly.
There was a problem hiding this comment.
you're right! I missed that. I'll call the setter explicitly 👍
edit: Actually I noticed that the PageModel object is being modified directly and I don't think that's a good practice so I changed it to use the copy editing instead of setters and I'm doing the same for the pageMap
| pageMap[page.remoteId]?.let { changed -> | ||
| changed.parent = _pageMap[oldParent] | ||
| pageMap = _pageMap |
There was a problem hiding this comment.
Same here, lost instant updates.
| } | ||
|
|
||
| @Test | ||
| fun onPageEditFinishedReloadSite() = runBlocking<Unit> { |
There was a problem hiding this comment.
Are you planning to add the test() function as you did in FluxC?
|
@0nko Everything should be fixed now 👍 |
|
Looking great! |
I'm using injected
uiContextandcommonPoolContextin view models inSearchListViewModelandPagesViewModel. I've separated the tests for these two to become truly unit tests. I've removed the_pageMapwhich was unnecessary because you can keep the value inpageMap. I've made it immutable because immutable collections are much easier to maintain and much less error-prone. I've added some test coverage but I feel like there are too many changes in this PR already so I'll add more in a separate PR.I'm removing
LiveDataUtilsbecause it's not necessary any more since we're using synchronousliveData.setValueinstead of asyncliveData.postValue.To test: Go through the new Pages activity and check that everything still works as expected.