-
Notifications
You must be signed in to change notification settings - Fork 54
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
Fixes CustomerCenter state not refreshing when reopening #2202
base: main
Are you sure you want to change the base?
Fixes CustomerCenter state not refreshing when reopening #2202
Conversation
📸 Snapshot Test260 unchanged
🛸 Powered by Emerge Tools |
LaunchedEffect(Unit) { | ||
viewModel.loadCustomerCenter() |
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.
I see that loadCustomerCenter
does not cancel previous calls, or at least doesn't seem to.
Is it worth checking something like ... ?
override suspend fun loadCustomerCenter() {
if (_state.value is CustomerCenterState.Loading) {. <--- early exit if its already loading
return
}
if (_state.value !is CustomerCenterState.Loading) {
_state.value = CustomerCenterState.Loading
}
try {
val customerCenterConfigData = purchases.awaitCustomerCenterConfigData()
val purchaseInformation = loadPurchaseInformation(dateFormatter, locale)
_state.value = CustomerCenterState.Success(
customerCenterConfigData,
purchaseInformation,
supportedPathsForManagementScreen = customerCenterConfigData.getManagementScreen()?.let {
supportedPaths(purchaseInformation, it)
},
)
} catch (e: PurchasesException) {
_state.value = CustomerCenterState.Error(e.error)
}
}
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.
hmmm that's a good point. Actually we shouldn't be loading the initial state here. Let me rethink it
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.
What about a refreshIfNeeded
kind of thing?
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.
Just one question
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2202 +/- ##
=======================================
Coverage 80.54% 80.54%
=======================================
Files 277 277
Lines 9456 9456
Branches 1334 1334
=======================================
Hits 7616 7616
Misses 1280 1280
Partials 560 560 ☔ View full report in Codecov by Sentry. |
I found the root cause. It was a regression from #2171 |
I would love to add a test for this, but will do in a separate PR, it might be tricky |
Added a test |
We noticed that the customer center wasn't reloading when reopening it, only after switching to another tab in the Paywalls Tester.
I moved the
loadCustomerCenter
toLaunchedEffect
next to when we track the impression to fix it