Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,12 @@ import androidx.work.PeriodicWorkRequestBuilder
import androidx.work.WorkInfo
import androidx.work.WorkManager
import com.woocommerce.android.ui.woopos.common.util.WooPosLogWrapper
import com.woocommerce.android.ui.woopos.util.datastore.WooPosPreferencesRepository
import dagger.hilt.android.qualifiers.ApplicationContext
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.launch
import java.util.Calendar
import java.util.concurrent.TimeUnit
import javax.inject.Inject
Expand All @@ -21,6 +26,7 @@ import javax.inject.Singleton
class WooPosLocalCatalogSyncScheduler @Inject constructor(
@ApplicationContext private val context: Context,
private val logger: WooPosLogWrapper,
private val preferencesRepository: WooPosPreferencesRepository,
) {

private companion object {
Expand All @@ -33,45 +39,51 @@ class WooPosLocalCatalogSyncScheduler @Inject constructor(
private val workManager by lazy { WorkManager.getInstance(context) }

fun schedulePeriodicFullCatalogSync() {
val syncWorkRequest = PeriodicWorkRequestBuilder<WooPosLocalCatalogSyncWorker>(
REFRESH_INTERVAL_HOURS,
TimeUnit.HOURS
)
.setInitialDelay(calculateDelayToNight(), TimeUnit.MILLISECONDS)
.setConstraints(getConstraints())
.setBackoffCriteria(
BackoffPolicy.EXPONENTIAL,
1,
TimeUnit.MINUTES
CoroutineScope(Dispatchers.IO).launch {
val constraints = getConstraintsBasedOnPreference()
val syncWorkRequest = PeriodicWorkRequestBuilder<WooPosLocalCatalogSyncWorker>(
REFRESH_INTERVAL_HOURS,
TimeUnit.HOURS
)
.setInitialDelay(calculateDelayToNight(), TimeUnit.MILLISECONDS)
.setConstraints(constraints)
.setBackoffCriteria(
BackoffPolicy.EXPONENTIAL,
1,
TimeUnit.MINUTES
)
.build()

workManager.enqueueUniquePeriodicWork(
WooPosLocalCatalogSyncWorker.WORK_NAME,
ExistingPeriodicWorkPolicy.REPLACE,
syncWorkRequest
)
.build()

workManager.enqueueUniquePeriodicWork(
WooPosLocalCatalogSyncWorker.WORK_NAME,
ExistingPeriodicWorkPolicy.KEEP,
syncWorkRequest
)

logger.d("POS local catalog full sync scheduled.")
logger.d("POS local catalog full sync scheduled.")
}
}

fun triggerManualFullCatalogSync() {
val oneTimeWorkRequest = OneTimeWorkRequestBuilder<WooPosLocalCatalogSyncWorker>()
.setConstraints(getConstraints())
.setBackoffCriteria(
BackoffPolicy.EXPONENTIAL,
1,
TimeUnit.MINUTES
CoroutineScope(Dispatchers.IO).launch {
val constraints = getConstraintsBasedOnPreference()
val oneTimeWorkRequest = OneTimeWorkRequestBuilder<WooPosLocalCatalogSyncWorker>()
.setConstraints(constraints)
.setBackoffCriteria(
BackoffPolicy.EXPONENTIAL,
1,
TimeUnit.MINUTES
)
.build()

workManager.enqueueUniqueWork(
ONE_TIME_WORK_NAME,
ExistingWorkPolicy.REPLACE,
oneTimeWorkRequest
)
.build()

workManager.enqueueUniqueWork(
ONE_TIME_WORK_NAME,
ExistingWorkPolicy.REPLACE,
oneTimeWorkRequest
)

logger.d("Manual POS local catalog sync triggered")
logger.d("Manual POS local catalog sync triggered")
}
}

fun isPeriodicWorkRunning(): Boolean {
Expand All @@ -86,9 +98,23 @@ class WooPosLocalCatalogSyncScheduler @Inject constructor(
return oneTimeWork.any { it.state == WorkInfo.State.RUNNING }
}

private fun getConstraints(): Constraints {
fun updateWorkConstraints() {
logger.d("Updating work constraints based on cellular preference change")
cancelPeriodicWork()
schedulePeriodicFullCatalogSync()
}

fun cancelPeriodicWork() {
workManager.cancelUniqueWork(WooPosLocalCatalogSyncWorker.WORK_NAME)
logger.d("Cancelled periodic work for constraint update")
}

private suspend fun getConstraintsBasedOnPreference(): Constraints {
val allowCellular = preferencesRepository.allowCellularDataUpdate.first()
val networkType = if (allowCellular) NetworkType.CONNECTED else NetworkType.UNMETERED

return Constraints.Builder()
.setRequiredNetworkType(NetworkType.CONNECTED)
.setRequiredNetworkType(networkType)
.setRequiresBatteryNotLow(true)
.build()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ import androidx.lifecycle.viewModelScope
import com.woocommerce.android.tools.SelectedSite
import com.woocommerce.android.ui.woopos.localcatalog.PosLocalCatalogSyncResult
import com.woocommerce.android.ui.woopos.localcatalog.WooPosLocalCatalogSyncRepository
import com.woocommerce.android.ui.woopos.localcatalog.WooPosLocalCatalogSyncScheduler
import com.woocommerce.android.ui.woopos.util.datastore.WooPosPreferencesRepository
import com.woocommerce.android.ui.woopos.util.datastore.WooPosSyncTimestampManager
import com.woocommerce.android.ui.woopos.util.format.WooPosDateFormatter
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.update
import kotlinx.coroutines.launch
import javax.inject.Inject
Expand All @@ -21,6 +24,8 @@ class WooPosSettingsLocalCatalogViewModel @Inject constructor(
private val localCatalogSyncRepository: WooPosLocalCatalogSyncRepository,
private val selectedSite: SelectedSite,
private val dateFormatter: WooPosDateFormatter,
private val preferencesRepository: WooPosPreferencesRepository,
private val syncScheduler: WooPosLocalCatalogSyncScheduler,
) : ViewModel() {
private val _state = MutableStateFlow(WooPosSettingsLocalCatalogState())
val state: StateFlow<WooPosSettingsLocalCatalogState> = _state.asStateFlow()
Expand All @@ -47,18 +52,25 @@ class WooPosSettingsLocalCatalogViewModel @Inject constructor(
lastFullUpdate = formattedTimestamp // TBD local catalog: Replace with full sync timestamp
)

val allowCellularDataUpdate = preferencesRepository.allowCellularDataUpdate.first()
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider making the cellular preference fully reactive by continuously listening to the repository instead of reading it once at startup. This way the UI state always stays in sync with the stored
preference, and the Switch can pass its new value directly rather than the ViewModel toggling the current state.

Something like that. Wdyt?

Refactor_cellular_data_update_handling_to_pass_boolean_value_and_improve_state_management.patch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion @kidinov! I'm wondering what is the benefit of observing, since the settings screen is the only place how this setting can be changed 🤔. The VM would be observing a repository that only the VM itself can change -> so the VM would update the setting and the VM would receive an update from the repo. I might be missing something, but I'm not sure what is the benefit.

Could you please elaborate on it a bit?

Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Unidirectional data flow: Data flows in one direction: UI → VM → Repo → VM → UI. This makes it easy to trace what's happening. No shortcuts where the VM updates itself directly.
  2. Less code: No need to flip !_state.value.allowCellularDataUpdate - the Switch already has the new value, just save it
  3. Can't get out of sync: Right now, if something goes wrong between updating the VM state and saving to repo, they could be different. With observing, that's impossible.
  4. Works if anything else changes it: currently there is assumption that it can be changed only from 1 place and in sync way, but if it changes asynchronously, it'll be out of sync with the view

Bottom line: We don't maintain two copies of the same data. The repo is the source of the truth

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 points, I've tested and applied the patch 🙇.

e54cf5c


_state.update {
it.copy(catalogStatus = catalogStatus)
it.copy(
catalogStatus = catalogStatus,
allowCellularDataUpdate = allowCellularDataUpdate
)
}
}
}

fun toggleCellularDataUpdate() {
viewModelScope.launch {
val newValue = !_state.value.allowCellularDataUpdate
_state.update {
it.copy(allowCellularDataUpdate = !it.allowCellularDataUpdate)
it.copy(allowCellularDataUpdate = newValue)
}
// TBD local catalog: Save preference to shared preferences or data store
preferencesRepository.setAllowCellularDataUpdate(newValue)
syncScheduler.updateWorkConstraints()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class WooPosPreferencesRepository @Inject constructor(
private val recentProductSearchesSiteSpecificKey = buildSiteSpecificKey(RECENT_PRODUCT_SEARCHES_KEY)
private val recentCouponSearchesSiteSpecificKey = buildSiteSpecificKey(RECENT_COUPON_SEARCHES_KEY)
private val wasOpenedOnceKey = booleanPreferencesKey(POS_WAS_OPENED_ONCE_KEY)
private val allowCellularDataUpdateKey = booleanPreferencesKey(ALLOW_FULL_SYNC_ON_CELLULAR_DATA_KEY)

val recentProductSearches: Flow<List<String>> = dataStore.data
.map { preferences ->
Expand All @@ -35,6 +36,11 @@ class WooPosPreferencesRepository @Inject constructor(
preferences[wasOpenedOnceKey] ?: false
}

val allowCellularDataUpdate: Flow<Boolean> = dataStore.data
.map { preferences ->
preferences[allowCellularDataUpdateKey] ?: false
}

suspend fun addRecentProductSearch(search: String) {
dataStore.edit { preferences ->
val currentSearches = preferences[recentProductSearchesSiteSpecificKey]?.let {
Expand Down Expand Up @@ -69,13 +75,20 @@ class WooPosPreferencesRepository @Inject constructor(
}
}

suspend fun setAllowCellularDataUpdate(allow: Boolean) {
dataStore.edit { preferences ->
preferences[allowCellularDataUpdateKey] = allow
}
}

private fun buildSiteSpecificKey(key: String): Preferences.Key<String> =
stringPreferencesKey("${selectedSite.getOrNull()?.siteId}-$key")

private companion object {
const val RECENT_PRODUCT_SEARCHES_KEY = "recent_product_searches_key"
const val RECENT_COUPON_SEARCHES_KEY = "recent_coupon_searches_key"
const val POS_WAS_OPENED_ONCE_KEY = "pos_was_opened_once_key"
const val ALLOW_FULL_SYNC_ON_CELLULAR_DATA_KEY = "allow_full_sync_on_cellular_data_key"

const val MAX_RECENT_SEARCHES_COUNT = 10
}
Expand Down
Loading