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

Issue/13213 disable notifications api request #13220

Merged
merged 10 commits into from
Jan 7, 2025
Merged

Issue/13213 disable notifications api request #13220

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
265a89b
Update notification settings using new API endpoint
JorgeMucientes Dec 29, 2024
4a45eed
Update fluxC changeset
JorgeMucientes Dec 30, 2024
9d20b87
Add loading indicator while site notifications are updated
JorgeMucientes Dec 30, 2024
afc3cc2
Add error handling
JorgeMucientes Dec 30, 2024
d4f33c4
Add unit tests for new logic
JorgeMucientes Dec 30, 2024
77ab4a8
Improve unit tests
JorgeMucientes Dec 30, 2024
6820df3
Fix detekt indentation issue
JorgeMucientes Dec 30, 2024
36c229b
Merge branch 'trunk' into issue/13213-disable-notifications-api-request
JorgeMucientes Dec 30, 2024
7b973cc
Fix wrong test name
JorgeMucientes Dec 31, 2024
0ae6499
Updates fluxc changeset
JorgeMucientes Jan 7, 2025
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
2 changes: 2 additions & 0 deletions ...kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/WooSitesVisibilityFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.woocommerce.android.ui.compose.composeView
import com.woocommerce.android.ui.main.AppBarStatus
import com.woocommerce.android.viewmodel.MultiLiveEvent
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ExitWithResult
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowDialog
import dagger.hilt.android.AndroidEntryPoint

@AndroidEntryPoint
Expand Down Expand Up @@ -40,6 +41,7 @@ class WooSitesVisibilityFragment : BaseFragment() {
when (event) {
is MultiLiveEvent.Event.Exit -> findNavController().navigateUp()
is ExitWithResult<*> -> navigateBackWithResult(WOO_SITES_VISIBILITY_UPDATED, event.data)
is ShowDialog -> event.showDialog()
}
}
}
Expand Down
23 changes: 17 additions & 6 deletions ...n/kotlin/com/woocommerce/android/ui/sitepicker/sitevisibility/WooSitesVisibilityScreen.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.rememberScrollState
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.foundation.verticalScroll
import androidx.compose.material.CircularProgressIndicator
import androidx.compose.material.Divider
import androidx.compose.material.Icon
import androidx.compose.material.IconButton
Expand Down Expand Up @@ -74,13 +76,21 @@ fun WooSitesVisibilityScreen(
},
backgroundColor = colorResource(id = R.color.color_toolbar),
actions = {
TextButton(
onClick = onSaveTapped,
enabled = state.isSaveButtonEnabled
) {
Text(
text = stringResource(id = R.string.save).uppercase()
if (state.isLoading) {
CircularProgressIndicator(
modifier = Modifier
.size(width = 56.dp, height = 16.dp)
.padding(horizontal = 16.dp)
)
} else {
TextButton(
onClick = onSaveTapped,
enabled = state.isSaveButtonEnabled
) {
Text(
text = stringResource(id = R.string.save).uppercase()
)
}
}
},
elevation = 0.dp
Expand Down Expand Up @@ -228,6 +238,7 @@ fun StoreVisibilityScreenPreview() {

),
isSaveButtonEnabled = true,
isLoading = false,
currentSite = WooStoreUi(
siteName = "Current Store",
siteUrl = "https://myselectedSite.com",
Expand Down
69 changes: 54 additions & 15 deletions ...otlin/com/woocommerce/android/ui/sitepicker/sitevisibility/WooSitesVisibilityViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package com.woocommerce.android.ui.sitepicker.sitevisibility

import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.asLiveData
import com.woocommerce.android.R
import com.woocommerce.android.tools.SelectedSite
import com.woocommerce.android.ui.sitepicker.SitePickerRepository
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.Exit
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ExitWithResult
import com.woocommerce.android.viewmodel.ScopedViewModel
Expand All @@ -12,33 +14,37 @@ import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.NotificationStore
import org.wordpress.android.fluxc.store.NotificationStore.SiteNotificationSetting
import javax.inject.Inject

@HiltViewModel
class WooSitesVisibilityViewModel @Inject constructor(
private val sitePickerRepository: SitePickerRepository,
private val selectedSite: SelectedSite,
private val visibleSitesDataStore: VisibleWooSitesDataStore,
private val notificationsStore: NotificationStore,
savedStateHandle: SavedStateHandle
) : ScopedViewModel(savedStateHandle) {
private var initiallySelectedSiteIds: List<Long> = emptyList()
private val _wooStores = MutableStateFlow(
private val _wooStoresState = MutableStateFlow(
WooStoresUiState(
wooStores = emptyList(),
currentSite = selectedSite.get().toWooStoreUi(isSiteVisible = false),
isSaveButtonEnabled = false
isSaveButtonEnabled = false,
isLoading = false
)
)
val viewState = _wooStores.asLiveData()
val viewState = _wooStoresState.asLiveData()

init {
launch {
_wooStores.value = _wooStores.value.copy(
_wooStoresState.value = _wooStoresState.value.copy(
wooStores = sitePickerRepository.getSites()
.filter { it.hasWooCommerce && it.siteId != selectedSite.get().siteId }
.map { it.toWooStoreUi(isSiteVisible(it.siteId)) }
)
initiallySelectedSiteIds = _wooStores.value.wooStores
initiallySelectedSiteIds = _wooStoresState.value.wooStores
.filter { it.isSelected }
.map { it.siteId }
}
Expand All @@ -49,26 +55,58 @@ class WooSitesVisibilityViewModel @Inject constructor(
}

fun onSaveTapped() {
_wooStoresState.value = _wooStoresState.value.copy(isLoading = true)
launch {
visibleSitesDataStore.updateSiteVisibilityStatus(
_wooStores.value.wooStores
.associate { it.siteId to it.isSelected }
)
triggerEvent(ExitWithResult(data = true))
notificationsStore.updateNotificationSettingsFor(
_wooStoresState.value.wooStores.map {
SiteNotificationSetting(
siteId = it.siteId,
newCommentEnabled = it.isSelected,
storeOrderEnabled = it.isSelected
)
}
).fold(
onSuccess = {
visibleSitesDataStore.updateSiteVisibilityStatus(
_wooStoresState.value.wooStores
.associate { it.siteId to it.isSelected }
)
triggerEvent(ExitWithResult(data = true))
},
onFailure = {
triggerEvent(
Event.ShowDialog(
titleId = R.string.site_picker_edit_store_list_error_title,
positiveButtonId = R.string.retry,
positiveBtnAction = { dialog, _ ->
dialog.dismiss()
onSaveTapped()
},
negativeButtonId = R.string.cancel,
negativeBtnAction = { dialog, _ ->
dialog.dismiss()
triggerEvent(Exit)
}
)
)
}
).also {
_wooStoresState.value = _wooStoresState.value.copy(isLoading = false)
}
}
}

fun onSiteTapped(wooStoreUi: WooStoreUi) {
_wooStores.value = _wooStores.value.copy(
wooStores = _wooStores.value.wooStores.map {
_wooStoresState.value = _wooStoresState.value.copy(
wooStores = _wooStoresState.value.wooStores.map {
when {
it.siteId == wooStoreUi.siteId -> it.copy(isSelected = !it.isSelected)
else -> it
}
}
)
_wooStores.value = _wooStores.value.copy(
isSaveButtonEnabled = _wooStores.value.wooStores
_wooStoresState.value = _wooStoresState.value.copy(
isSaveButtonEnabled = _wooStoresState.value.wooStores
.filter { it.isSelected }
.map { it.siteId } != initiallySelectedSiteIds
)
Expand All @@ -87,7 +125,8 @@ class WooSitesVisibilityViewModel @Inject constructor(
data class WooStoresUiState(
val wooStores: List<WooStoreUi>,
val currentSite: WooStoreUi,
val isSaveButtonEnabled: Boolean
val isSaveButtonEnabled: Boolean,
val isLoading: Boolean
)

data class WooStoreUi(
Expand Down
1 change: 1 addition & 0 deletions WooCommerce/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@
<string name="site_picker_edit_store_current_site_header">Current Store</string>
<string name="site_picker_edit_store_current_site_footer">Please switch to another store if you want to hide this one</string>
<string name="site_picker_edit_store_list_header">Other Stores</string>
<string name="site_picker_edit_store_list_error_title">There was an error when updating notification settings. Please try again</string>

<!--
My Store View
Expand Down
70 changes: 64 additions & 6 deletions ...n/com/woocommerce/android/ui/sitepicker/sitevisibility/WooSitesVisibilityViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import com.woocommerce.android.util.getOrAwaitValue
import com.woocommerce.android.util.runAndCaptureValues
import com.woocommerce.android.viewmodel.BaseUnitTest
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ExitWithResult
import com.woocommerce.android.viewmodel.MultiLiveEvent.Event.ShowDialog
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.flowOf
import org.assertj.core.api.Assertions.assertThat
Expand All @@ -17,6 +18,13 @@ import org.mockito.kotlin.any
import org.mockito.kotlin.doReturn
import org.mockito.kotlin.mock
import org.mockito.kotlin.verify
import org.mockito.kotlin.whenever
import org.wordpress.android.fluxc.store.NotificationStore
import org.wordpress.android.fluxc.store.NotificationStore.NotificationSettingErrorType
import org.wordpress.android.fluxc.store.NotificationStore.NotificationSettingsUpdateError
import org.wordpress.android.fluxc.store.NotificationStore.SiteNotificationSetting
import kotlin.test.assertFalse
import kotlin.test.assertTrue

@OptIn(ExperimentalCoroutinesApi::class)
class WooSitesVisibilityViewModelTest : BaseUnitTest() {
Expand Down Expand Up @@ -59,6 +67,8 @@ class WooSitesVisibilityViewModelTest : BaseUnitTest() {
onBlocking { isSiteVisible(any()) } doReturn flowOf(true)
}

private val notificationStore: NotificationStore = mock()

private lateinit var viewModel: WooSitesVisibilityViewModel

@Before
Expand All @@ -67,6 +77,7 @@ class WooSitesVisibilityViewModelTest : BaseUnitTest() {
sitePickerRepository = sitePickerRepository,
selectedSite = selectedSite,
visibleSitesDataStore = visibleWooSitesDataStore,
notificationsStore = notificationStore,
savedStateHandle = mock()
)
}
Expand All @@ -77,8 +88,8 @@ class WooSitesVisibilityViewModelTest : BaseUnitTest() {
viewModel.onSiteTapped(A_WOO_SITE_UI_MODEL)

val updatedState = viewModel.viewState.getOrAwaitValue()
assert(!updatedState.wooStores.last().isSelected)
assert(updatedState.isSaveButtonEnabled)
assertFalse(updatedState.wooStores.last().isSelected)
assertTrue(updatedState.isSaveButtonEnabled)
}

@Test
Expand All @@ -88,13 +99,15 @@ class WooSitesVisibilityViewModelTest : BaseUnitTest() {
viewModel.onSiteTapped(A_WOO_SITE_UI_MODEL)

val updatedState = viewModel.viewState.getOrAwaitValue()
assert(updatedState.wooStores.first().isSelected)
assert(!updatedState.isSaveButtonEnabled)
assertTrue(updatedState.wooStores.first().isSelected)
assertFalse(updatedState.isSaveButtonEnabled)
}

@Test
fun `given one site is unselected, when tapping save, then save site visibility status`() =
fun `given update notification settings succeeds, when tapping save, then save site's visibility locally`() =
testBlocking {
whenever(notificationStore.updateNotificationSettingsFor(any())).thenReturn(Result.success(Unit))

val hiddenSite = A_WOO_SITE_UI_MODEL
viewModel.onSiteTapped(hiddenSite)

Expand All @@ -106,12 +119,57 @@ class WooSitesVisibilityViewModelTest : BaseUnitTest() {
}

@Test
fun `when tapping save, then exit with result`() =
fun `given updating notification settings succeeds, when tapping save, then exit with result`() =
testBlocking {
val event = viewModel.event.runAndCaptureValues {
viewModel.onSaveTapped()
}.last()

assertThat(event).isEqualTo(ExitWithResult(data = true))
}

@Test
fun `given updating notification settings fails, when tapping save, then exit with result`() =
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, should be then show error dialog.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Fixed!

testBlocking {
whenever(notificationStore.updateNotificationSettingsFor(any()))
.thenReturn(
Result.failure(
NotificationSettingsUpdateError(
type = NotificationSettingErrorType.ApiError("Any error")
)
)
)

val event = viewModel.event.runAndCaptureValues {
viewModel.onSaveTapped()
}.last()

assertThat(event).isInstanceOf(ShowDialog::class.java)
}

@Test
fun `when tapping save, then site notification settings are updated on the backend`() =
testBlocking {
viewModel.onSaveTapped()

verify(notificationStore).updateNotificationSettingsFor(
AVAILABLE_WOO_SITES_TO_HIDE.map {
SiteNotificationSetting(
siteId = it.siteId,
newCommentEnabled = it.isSelected,
storeOrderEnabled = it.isSelected
)
}
)
}

@Test
fun `given save was tapped, when loading is done, then hide loading state`() =
testBlocking {
viewModel.onSaveTapped()

val updatedState = viewModel.viewState.getOrAwaitValue()

assertFalse(updatedState.isLoading)
}
}
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ stripe-terminal = '3.7.1'
tinder-statemachine = '0.2.0'
wiremock = '2.26.3'
wordpress-aztec = 'v2.1.4'
wordpress-fluxc = 'trunk-b329e031481c8653a57c0a41c4dedfc0ba6a510e'
wordpress-fluxc = '3122-c8c9f508066c0c8165420dee180b7d10cafbfbb6'
wordpress-login = '1.19.0'
wordpress-libaddressinput = '0.0.2'
wordpress-mediapicker = '0.3.1'
Expand Down
Loading