Minor refactoring to improve clarity

MAILANDR-410
This commit is contained in:
Johannes Prueller
2023-05-08 18:08:46 +02:00
committed by Marino Meneghel
parent 36f6fcca4e
commit 0ec1cbc73e
7 changed files with 57 additions and 51 deletions
+6 -1
View File
@@ -7,6 +7,11 @@
<option value="$PROJECT_DIR$/config/detekt/config.yml" />
</set>
</option>
<option name="configurationFiles">
<list>
<option value="$PROJECT_DIR$/config/detekt/config.yml" />
</list>
</option>
<option name="enableFormatting" value="true" />
<option name="pluginJarPaths">
<set>
@@ -21,4 +26,4 @@
<configPaths>$PROJECT_DIR$/config/detekt/config.yml</configPaths>
<pluginPaths>$PROJECT_DIR$/detekt-rules/build/libs/detekt-rules.jar</pluginPaths>
</component>
</project>
</project>
@@ -103,7 +103,7 @@ class ConversationRepositoryImpl @Inject constructor(
) ?: return emptyList<ConversationWithContext>().right()
return conversationRemoteDataSource.getConversations(userId = userId, pageKey = adaptedPageKey)
.onRight { conversations -> insertConversations(userId, adaptedPageKey, conversations) }
.onRight { conversations -> upsertConversations(userId, adaptedPageKey, conversations) }
}
override suspend fun markAsStale(userId: UserId, labelId: LabelId) =
@@ -308,7 +308,7 @@ class ConversationRepositoryImpl @Inject constructor(
return addLabel(userId, conversationId, labelId)
}
private suspend fun insertConversations(
private suspend fun upsertConversations(
userId: UserId,
pageKey: PageKey,
conversations: List<ConversationWithContext>
@@ -68,7 +68,6 @@ import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue
@SuppressWarnings("LargeClass")
class ConversationRepositoryImplTest {
private val userId = UserIdSample.Primary
@@ -701,7 +700,7 @@ class ConversationRepositoryImplTest {
}
@Test
fun `move to trash removes all labels from messages, except AllMail, AllDraft, AllSent`() = runTest {
fun `move to trash removes all labels from messages, except AllMail, AllDraft and AllSent`() = runTest {
// given
val conversationId = ConversationIdSample.WeatherForecast
val conversation = ConversationSample.WeatherForecast
@@ -24,24 +24,24 @@ import androidx.paging.compose.LazyPagingItems
import ch.protonmail.android.mailmailbox.presentation.mailbox.MailboxScreenState
import ch.protonmail.android.mailmailbox.presentation.mailbox.model.MailboxItemUiModel
@Suppress("ReturnCount")
fun LazyPagingItems<MailboxItemUiModel>.mapToUiStates(): MailboxScreenState {
if (this.loadState.isLoading()) {
return if (this.itemCount == 0) {
MailboxScreenState.Loading
} else {
MailboxScreenState.LoadingWithData(this)
return when {
this.loadState.isLoading() -> {
if (this.itemCount == 0) {
MailboxScreenState.Loading
} else {
MailboxScreenState.LoadingWithData(this)
}
}
} else if (this.loadState.refresh is LoadState.Error) {
return if (this.itemCount == 0) {
MailboxScreenState.Error
} else {
MailboxScreenState.ErrorWithData(this)
this.loadState.refresh is LoadState.Error -> {
if (this.itemCount == 0) {
MailboxScreenState.Error
} else {
MailboxScreenState.ErrorWithData(this)
}
}
} else if (this.itemCount == 0) {
return MailboxScreenState.Empty
} else {
return MailboxScreenState.Data(this)
this.itemCount == 0 -> MailboxScreenState.Empty
else -> MailboxScreenState.Data(this)
}
}
@@ -51,9 +51,9 @@ import ch.protonmail.android.mailmailbox.presentation.mailbox.model.MailboxViewA
import ch.protonmail.android.mailmailbox.presentation.mailbox.model.UnreadFilterState
import ch.protonmail.android.mailmailbox.presentation.mailbox.previewdata.MailboxStateSampleData
import ch.protonmail.android.mailmailbox.presentation.mailbox.reducer.MailboxReducer
import ch.protonmail.android.mailmailbox.presentation.paging.MailboxPagerFactory
import ch.protonmail.android.mailsettings.domain.model.FolderColorSettings
import ch.protonmail.android.mailsettings.domain.usecase.ObserveFolderColorSettings
import ch.protonmail.android.mailmailbox.presentation.paging.MailboxPagerFactory
import ch.protonmail.android.testdata.contact.ContactTestData
import ch.protonmail.android.testdata.label.LabelTestData
import ch.protonmail.android.testdata.mailbox.MailboxItemUiModelTestData.buildMailboxUiModelItem
@@ -395,32 +395,34 @@ class MailboxViewModelTest {
fun `mailbox items for the current location are requested when location changes`() = runTest {
// Given
val currentLocationFlow = MutableStateFlow<MailLabelId>(initialLocationMailLabelId)
val initialMailboxState = createMailboxDataState()
val userIds = listOf(userId)
every { selectedMailLabelId.flow } returns currentLocationFlow
every { pagerFactory.create(any(), any(), any(), any()) } returns mockk mockPager@{
every { pagerFactory.create(userIds, any(), false, Message) } returns mockk mockPager@{
every { this@mockPager.flow } returns flowOf(PagingData.from(listOf(unreadMailboxItem)))
}
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxState()
every { mailboxReducer.newStateFrom(any(), any()) } returns initialMailboxState
every {
mailboxReducer.newStateFrom(
any(),
initialMailboxState,
MailboxEvent.NewLabelSelected(
MailLabelId.System.Spam.toMailLabel(),
UnreadCountersTestData.labelToCounterMap[MailLabelId.System.Spam.labelId]!!
)
)
} returns createMailboxState(selectedMailLabelId = MailLabelId.System.Spam)
} returns createMailboxDataState(selectedMailLabelId = MailLabelId.System.Spam)
mailboxViewModel.items.test {
// Then
awaitItem()
verify { pagerFactory.create(listOf(userId), MailLabelId.System.Archive, false, Message) }
verify { pagerFactory.create(userIds, initialLocationMailLabelId, false, Message) }
// When
currentLocationFlow.emit(MailLabelId.System.Spam)
// Then
awaitItem()
verify { pagerFactory.create(listOf(userId), MailLabelId.System.Spam, false, Message) }
verify { pagerFactory.create(userIds, MailLabelId.System.Spam, false, Message) }
cancelAndIgnoreRemainingEvents()
}
}
@@ -438,7 +440,7 @@ class MailboxViewModelTest {
val pagingData = PagingData.from(listOf(unreadMailboxItem, readMailboxItem))
every { this@mockk.flow } returns flowOf(pagingData)
}
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxState()
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxDataState()
val differ = MailboxAsyncPagingDataDiffer.differ
// When
@@ -473,7 +475,7 @@ class MailboxViewModelTest {
val pagingData = PagingData.from(listOf(unreadMailboxItemWithLabel))
every { this@mockk.flow } returns flowOf(pagingData)
}
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxState()
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxDataState()
val differ = MailboxAsyncPagingDataDiffer.differ
// When
@@ -510,7 +512,7 @@ class MailboxViewModelTest {
val pagingData = PagingData.from(listOf(unreadMailboxItem, readMailboxItem))
every { this@mockk.flow } returns flowOf(pagingData)
}
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxState()
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxDataState()
val differ = MailboxAsyncPagingDataDiffer.differ
// When
mailboxViewModel.items.test {
@@ -664,13 +666,13 @@ class MailboxViewModelTest {
@Test
fun `mailbox pager is recreated when unread filter state changes`() = runTest {
// Given
val expectedMailBoxState = createMailboxState(unreadFilterState = false)
val expectedMailBoxState = createMailboxDataState(unreadFilterState = false)
every { mailboxReducer.newStateFrom(any(), any()) } returns expectedMailBoxState
every { pagerFactory.create(any(), any(), any(), any()) } returns mockk mockPager@{
every { pagerFactory.create(listOf(userId), Archive, any(), Message) } returns mockk mockPager@{
every { this@mockPager.flow } returns flowOf(PagingData.from(listOf(unreadMailboxItem)))
}
every { mailboxReducer.newStateFrom(expectedMailBoxState, MailboxViewAction.EnableUnreadFilter) } returns
createMailboxState(unreadFilterState = true)
createMailboxDataState(unreadFilterState = true)
mailboxViewModel.items.test {
// When
@@ -722,7 +724,7 @@ class MailboxViewModelTest {
@Test
fun `mailbox pager is recreated when selected mail label state changes`() = runTest {
// Given
val expectedMailBoxState = createMailboxState(selectedMailLabelId = initialLocationMailLabelId)
val expectedMailBoxState = createMailboxDataState(selectedMailLabelId = initialLocationMailLabelId)
val inboxLabel = MailLabelId.System.Inbox
val currentLocationFlow = MutableStateFlow<MailLabelId>(initialLocationMailLabelId)
every { selectedMailLabelId.flow } returns currentLocationFlow
@@ -738,7 +740,7 @@ class MailboxViewModelTest {
UnreadCountersTestData.labelToCounterMap[inboxLabel.labelId]!!
)
)
} returns createMailboxState(selectedMailLabelId = inboxLabel)
} returns createMailboxDataState(selectedMailLabelId = inboxLabel)
mailboxViewModel.items.test {
// When
@@ -762,7 +764,7 @@ class MailboxViewModelTest {
fun `pager is not recreated when any state beside selectedLabel, unreadFilter, viewMode or primaryUser changes`() =
runTest {
// Given
val expectedMailBoxState = createMailboxState(Effect.empty())
val expectedMailBoxState = createMailboxDataState(Effect.empty())
every { mailboxReducer.newStateFrom(any(), any()) } returns expectedMailBoxState
every { pagerFactory.create(any(), any(), any(), any()) } returns mockk mockPager@{
every { this@mockPager.flow } returns flowOf(PagingData.from(listOf(unreadMailboxItem)))
@@ -772,7 +774,7 @@ class MailboxViewModelTest {
expectedMailBoxState,
MailboxEvent.ItemDetailsOpenedInViewMode(unreadMailboxItemUiModel, NoConversationGrouping)
)
} returns createMailboxState(
} returns createMailboxDataState(
Effect.of(OpenMailboxItemRequest(MailboxItemId(unreadMailboxItem.id), unreadMailboxItem.type))
)
@@ -792,7 +794,7 @@ class MailboxViewModelTest {
@Test
fun `verify mapped paging data is cached`() = runTest {
// Given
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxState()
every { mailboxReducer.newStateFrom(any(), any()) } returns createMailboxDataState()
every { pagerFactory.create(any(), any(), any(), any()) } returns mockk mockPager@{
every { this@mockPager.flow } returns flowOf(PagingData.from(listOf(unreadMailboxItem)))
}
@@ -812,7 +814,7 @@ class MailboxViewModelTest {
}
}
private fun createMailboxState(
private fun createMailboxDataState(
openEffect: Effect<OpenMailboxItemRequest> = Effect.empty(),
unreadFilterState: Boolean = false,
selectedMailLabelId: MailLabelId.System = initialLocationMailLabelId
@@ -27,10 +27,10 @@ import io.mockk.mockk
import org.junit.Assert.assertEquals
import org.junit.Test
class PagingLoadingStateMapperTest {
class PagingLoadingStateMapperKtTest {
@Test
fun `mapToUiStates() should return Loading when source is loading and itemCount is 0`() {
fun `when source is loading and itemCount is 0 then Loading is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { itemCount } returns 0
every { loadState.source.refresh } returns LoadState.Loading
@@ -39,7 +39,7 @@ class PagingLoadingStateMapperTest {
}
@Test
fun `mapToUiStates() should return Loading when mediator is loading and itemCount is 0`() {
fun `when mediator is loading and itemCount is 0 then Loading is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { loadState.mediator } returns mockk(relaxed = true)
every { itemCount } returns 0
@@ -50,7 +50,7 @@ class PagingLoadingStateMapperTest {
}
@Test
fun `mapToUiStates() should return Loading when append is loading and itemCount is 0`() {
fun `when append is loading and itemCount is 0 then Loading is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { loadState.mediator } returns mockk(relaxed = true)
every { itemCount } returns 0
@@ -62,7 +62,7 @@ class PagingLoadingStateMapperTest {
}
@Test
fun `mapToUiStates() should return LoadingWithData when source is loading and itemCount is larger than 0`() {
fun `when source is loading and itemCount is larger than 0 then LoadingWithData is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { itemCount } returns 10
every { loadState.source.refresh } returns LoadState.Loading
@@ -71,7 +71,7 @@ class PagingLoadingStateMapperTest {
}
@Test
fun `mapToUiStates() should return LoadingWithData when mediator is loading and item count is larger than 0`() {
fun `when mediator is loading and item count is larger than 0 then LoadingWithData is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { itemCount } returns 10
every { loadState.source.refresh } returns LoadState.NotLoading(false)
@@ -82,7 +82,7 @@ class PagingLoadingStateMapperTest {
}
@Test
fun `mapToUiStates() should return Error when loadState is error and item count is 0`() {
fun `when loadState is error and item count is 0 then Error is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { itemCount } returns 0
every { loadState.refresh } returns LoadState.Error(Exception())
@@ -94,7 +94,7 @@ class PagingLoadingStateMapperTest {
}
@Test
fun `mapToUiStates() should return ErrorWithData when loadState is error and item count larger than 0`() {
fun `when loadState is error and item count larger than 0 then ErrorWithData is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { itemCount } returns 10
every { loadState.refresh } returns LoadState.Error(Exception())
@@ -106,7 +106,7 @@ class PagingLoadingStateMapperTest {
}
@Test
fun `mapToUiStates() should return Empty when itemCount is 0`() {
fun `when itemCount is 0 then Empty is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { itemCount } returns 0
every { loadState.source } returns mockk(relaxed = true)
@@ -119,7 +119,7 @@ class PagingLoadingStateMapperTest {
}
@Test
fun `mapToUiStates() should return Data when itemCount is larger than 0`() {
fun `when itemCount is larger than 0 then Data is returned`() {
val items = mockk<LazyPagingItems<MailboxItemUiModel>> {
every { itemCount } returns 10
every { loadState.source } returns mockk(relaxed = true)
@@ -93,7 +93,7 @@ class MessageRepositoryImpl @Inject constructor(
return remoteDataSource.getMessages(
userId = userId,
pageKey = adaptedPageKey
).onRight { messages -> insertMessages(userId, adaptedPageKey, messages) }
).onRight { messages -> upsertMessages(userId, adaptedPageKey, messages) }
}
override suspend fun markAsStale(userId: UserId, labelId: LabelId) = localDataSource.markAsStale(userId, labelId)
@@ -221,7 +221,7 @@ class MessageRepositoryImpl @Inject constructor(
return addLabel(userId = userId, messageId = messageId, labelId = labelId)
}
private suspend fun insertMessages(
private suspend fun upsertMessages(
userId: UserId,
pageKey: PageKey,
messages: List<Message>