Skip to content

Conversation

@irfano
Copy link
Contributor

@irfano irfano commented Dec 23, 2025

Closes WOOMOB-1843

Description

This PR resolves a long-standing NullPointerException crash in DashboardDataStore and DashboardRepository occurring during app initialization or site switching.

DashboardDataStore and DashboardRepository were attempting to access selectedSite.siteComponent using the non-null assertion operator (!!) during their initialization.

If the application started in a state where no site was strictly selected (e.g., fresh install, after logout, or race conditions during startup. I'm not sure because I couldn't reproduce it), siteComponent would be null, causing an immediate crash.

Instead of assuming a site exists at initialization time, I refactored both classes to be Reactive:

Also I updated SelectedSite to support this reactive approach.

Test Steps

Since I can't reproduce the crash, it's not possible to test this PR on running app. But I wrote unit tests to reproduce crash cases.

  • DashboardDataStoreTest: If you check out this commit and run the test, you can reproduce a NullPointerException crash at selectedSite.siteComponent!!, line. Then check out the latest commit and confirm the crash was fixed.
  • Add DashboardRepositoryTest for initialization without crash: If you check out this commit and run the test, you can reproduce a NullPointerException crash at selectedSite.siteComponent!!, line. Then checkout the latest commit and confirm the crash was fixed.

Images/gif

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@irfano irfano added this to the 23.9 milestone Dec 23, 2025
@irfano irfano added type: crash The worst kind of bug. feature: dashboard Related to home screen project labels Dec 23, 2025
Comment on lines +90 to -101
// Create a new site component tied to the lifecycle of the selected site
siteComponent = siteComponentProvider.get()
.setSite(siteModel)
.setCoroutineScope(createSiteCoroutineScope())
.build()

wasReset = false
state.value = siteModel
PreferenceUtils.setInt(getPreferences(), SELECTED_SITE_LOCAL_ID, siteModel.id)

// Notify listeners
getEventBus().post(SelectedSiteChangedEvent(siteModel))

// Create a new site component tied to the lifecycle of the selected site
siteComponent = siteComponentProvider.get()
.setSite(get())
.setCoroutineScope(createSiteCoroutineScope())
.build()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this change because state.value = siteModel triggers observers, and it was causing them to receive the old siteComponent. Now, I update siteComponent before updating the state so observers get the correct value.

val widgets: Flow<List<DashboardWidgetDataModel>> = dataStore.data
.catch { exception ->
val widgets: Flow<List<DashboardWidgetDataModel>> = dataStoreFlow.flatMapLatest { dataStore ->
val flow = dataStore?.data ?: flow { throw IOException("Site component is null") }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now we pass null site component case to the existing catch block.

@wpmobilebot
Copy link
Collaborator

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App NameWooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commiteef1113
Direct Downloadwoocommerce-wear-prototype-build-pr15138-eef1113.apk

@wpmobilebot
Copy link
Collaborator

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commiteef1113
Direct Downloadwoocommerce-prototype-build-pr15138-eef1113.apk

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 54.54545% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.67%. Comparing base (4cfbab7) to head (eef1113).
⚠️ Report is 2 commits behind head on trunk.

Files with missing lines Patch % Lines
...ce/android/ui/dashboard/data/DashboardDataStore.kt 33.33% 8 Missing and 2 partials ⚠️
...tlin/com/woocommerce/android/tools/SelectedSite.kt 0.00% 4 Missing ⚠️
...e/android/ui/dashboard/data/DashboardRepository.kt 92.85% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #15138      +/-   ##
============================================
+ Coverage     38.64%   38.67%   +0.03%     
- Complexity    10456    10466      +10     
============================================
  Files          2181     2181              
  Lines        123905   123892      -13     
  Branches      17100    17102       +2     
============================================
+ Hits          47877    47914      +37     
+ Misses        71205    71149      -56     
- Partials       4823     4829       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: dashboard Related to home screen project type: crash The worst kind of bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants