CMM-1952: stats insights: tags card and some fixes

[adalpari]

Description

Adds the data layer and ViewModel improvements for the new stats cards detail screens, building on the Insights tab feature.

Fix: Detail screen crash on process death

Previously, TagsAndCategoriesDetailActivity received its data via a static field set by the card ViewModel before navigation. If Android killed the process and later restored the activity, the static field would be null, causing the screen to immediately finish() — the user would see it flash and close. The fix replaces this with a dedicated TagsAndCategoriesDetailViewModel that fetches its own data independently from StatsTagsUseCase (with caching, so it doesn't re-hit the network). The detail screen is now fully self-contained and survives process death.

Other changes

• Shared base ViewModel: Extracted BaseTagsAndCategoriesViewModel to eliminate duplication between the card and detail ViewModels, with thread-safe atomic loading guards and fetch-job cancellation.
• StatsTagsUseCase: New singleton use case with mutex-based caching for tags data fetching.
• TagsAndCategoriesMapper: Extracted shared mapping logic from the ViewModel into a reusable mapper.
• InsightsViewModel improvements: Replaced @Volatile booleans with AtomicBoolean for proper thread safety, cancels in-flight jobs on refresh, and added conditional data fetching — the ViewModel now checks which endpoints are actually needed by the currently visible cards (needsSummary() / needsInsights()) and skips API calls for hidden cards entirely. When the card configuration changes (e.g., a user adds a card back), it only triggers a fetch if the newly visible cards require data that hasn't already been fetched.
• Compilation fixes: Updated wordpress-rs to a version including stats tags types, removed duplicate NoConnectionContent composable, and removed redundant else branches causing -Werror failures.
• Tests: Added tests for InsightsViewModel, TagsAndCategoriesDetailViewModel, InsightsCardsConfigurationRepository, and updated existing TagsAndCategoriesViewModelTest.

Testing instructions

Tags & Categories card:

1. Open the app and navigate to new Stats → Insights tab
2. Ensure the Tags & Categories card is visible (add it from the "Add card" bottom sheet if needed)

• Verify the card loads and displays tag groups with view counts

Tags & Categories detail screen:

1. Tap "Show all" on the Tags & Categories card

• Verify the detail screen opens and loads data
• Verify multi-tag groups can be expanded/collapsed

2. Kill the app from recents, then restore it

No duplicate API calls for hidden cards:

1. Inspect network calls
2. Hide all cards from the Insights tab
3. Pull to refresh or go back

• Verify no API calls are made when no cards need data

4. Re-add a single card (e.g., "All Time Stats"), pull to refresh

• Verify only the required endpoint (summary) is fetched, not both summary and insights

Screen_recording_20260317_181655.mp4

[dangermattic]

	1 Error
🚫	This PR includes a PR version of wordpress-rs (1230-38a8a892ab0f19cc8290d563505c6f84da11acf5). Please merge the corresponding wordpress-rs PR and update to a trunk version before merging.

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

	1 Message
📖	This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

[wpmobilebot]

Project dependencies changes

list

! Upgraded Dependencies
rs.wordpress.api:android:1230-38a8a892ab0f19cc8290d563505c6f84da11acf5, (changed from trunk-502e9561f2a68294f0065867bab9214cc9a6b78c)
rs.wordpress.api:kotlin:1230-38a8a892ab0f19cc8290d563505c6f84da11acf5, (changed from trunk-502e9561f2a68294f0065867bab9214cc9a6b78c)

tree

 +--- project :libs:fluxc
-|    \--- rs.wordpress.api:android:trunk-502e9561f2a68294f0065867bab9214cc9a6b78c
-|         +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
-|         +--- com.squareup.okhttp3:okhttp-tls:5.3.2
-|         |    +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
-|         |    +--- com.squareup.okio:okio:3.16.4 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.2.21 -> 2.3.10 (*)
-|         +--- net.java.dev.jna:jna:5.18.1
-|         +--- rs.wordpress.api:kotlin:trunk-502e9561f2a68294f0065867bab9214cc9a6b78c
-|         |    +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
-|         |    +--- com.squareup.okhttp3:okhttp-tls:5.3.2 (*)
-|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
-|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.10 (*)
-|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.10 (*)
+|    \--- rs.wordpress.api:android:1230-38a8a892ab0f19cc8290d563505c6f84da11acf5
+|         +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
+|         +--- com.squareup.okhttp3:okhttp-tls:5.3.2
+|         |    +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
+|         |    +--- com.squareup.okio:okio:3.16.4 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.2.21 -> 2.3.10 (*)
+|         +--- net.java.dev.jna:jna:5.18.1
+|         +--- rs.wordpress.api:kotlin:1230-38a8a892ab0f19cc8290d563505c6f84da11acf5
+|         |    +--- com.squareup.okhttp3:okhttp:5.3.2 (*)
+|         |    +--- com.squareup.okhttp3:okhttp-tls:5.3.2 (*)
+|         |    +--- org.jetbrains.kotlinx:kotlinx-coroutines-core:1.10.2 (*)
+|         |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.10 (*)
+|         \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.21 -> 2.3.10 (*)
-\--- rs.wordpress.api:android:trunk-502e9561f2a68294f0065867bab9214cc9a6b78c (*)
+\--- rs.wordpress.api:android:1230-38a8a892ab0f19cc8290d563505c6f84da11acf5 (*)

[wpmobilebot]

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

	App Name	Jetpack Android
	Build Type	Debug
	Version	pr22701-95cdee9
	Build Number	1487
	Application ID	com.jetpack.android.prealpha
	Commit	95cdee9
	Installation URL	3f9j488vj8pu8

Note: Google Login is not supported on these builds.

[wpmobilebot]

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

	App Name	WordPress Android
	Build Type	Debug
	Version	pr22701-95cdee9
	Build Number	1487
	Application ID	org.wordpress.android.prealpha
	Commit	95cdee9
	Installation URL	7iq28323nodv8

Note: Google Login is not supported on these builds.

[adalpari]

@claude

[claude]

Claude finished @adalpari's task in 2m 55s —— View job

---

Code Review

• Gather context and read changed files
• Review core logic (BaseTagsAndCategoriesViewModel, StatsTagsUseCase, mapper)
• Review InsightsViewModel changes
• Review TagsAndCategoriesDetailActivity
• Review tests
• Post final review

---

Overall this is well-structured work. The process-death fix is solid, the base-class extraction is clean, and the conditional-fetch optimization is genuinely useful. A few issues worth discussing below.

---

Bugs / Correctness

1. StatsTagsUseCase — TOCTOU race on cache

StatsTagsUseCase.kt:34-48

// reads cache under lock…
val cached = mutex.withLock { cachedTags }
if (!forceRefresh && isCacheHit(cached, siteId, max)) {
    return TagsResult.Success(cached!!.third)
}
// …then fetches outside the lock
val result = statsRepository.fetchTags(siteId = siteId, max = max)
mutex.withLock {
    if (result is TagsResult.Success) {
        cachedTags = Triple(siteId, max, result.data)
    }
}

The comment explains this avoids holding the mutex during network I/O, which is correct. However there is a classic TOCTOU problem: two concurrent callers with the same (siteId, max) will both see a cache miss, both fire a network request, and both write their result. The second write silently overwrites the first. Worse, if callers have different max values, the first to write wins — the caller that asked for max=100 could end up caching a result for max=10 that a concurrent card-ViewModel wrote.

In practice this is unlikely to cause visible bugs (both requests succeed, the last writer wins), but a proper solution would be to reuse an in-flight Deferred (or hold the mutex during the fetch for truly singleton semantics). At minimum a comment acknowledging the trade-off would help future readers. Fix this →

---

2. BaseTagsAndCategoriesViewModel — isLoaded is protected but unused in subclasses

BaseTagsAndCategoriesViewModel.kt:33

protected val isLoaded = AtomicBoolean(false)

No current subclass reads or writes isLoaded directly. Exposing internal state to subclasses is a code smell that invites misuse — a subclass could set isLoaded=true without a successful fetch, silently suppressing future retries. Consider making this private. If a future subclass genuinely needs it, the field can be re-exposed at that time with a clear contract. Fix this →

---

3. InsightsViewModel — updateFromConfiguration does not trigger a fetch

InsightsViewModel.kt:250-265

private fun updateFromConfiguration(config: InsightsCardsConfiguration) {
    …
    if (needsNewFetch) {
        isDataLoaded.set(false)  // marks as needing data
    }
    _cardsToLoad.value = config.visibleCards
}

When a card that requires a new endpoint is added (e.g. user adds "Year in Review"), isDataLoaded is set to false, but no fetch is actually triggered — the next call to loadDataIfNeeded() from the UI will do it. This depends on the UI observing cardsToLoad and calling loadDataIfNeeded() in response. Is that guaranteed? If the UI only calls loadDataIfNeeded() on first composition (not on config changes), the new card will sit in the loading state indefinitely. Worth verifying the UI wiring, or triggering the fetch directly from updateFromConfiguration. Fix this →

---

4. TagsAndCategoriesDetailActivity — no empty-state UI

TagsAndCategoriesDetailActivity.kt:177-253

DetailLoadedContent renders an empty LazyColumn with only a header if items is empty. A user with no tags would see a blank screen with no explanation. Consider adding a simple empty-state message inside the LazyColumn. Fix this →

---

Design Concerns

5. TagsAndCategoriesDetailViewModel has no refresh mechanism

The detail screen exposes no refresh() function — the error-state retry button calls viewModel.loadData(), which re-uses the cached result on the second attempt if the first error was a network error but the use-case had actually cached a stale success. More importantly there's no pull-to-refresh. If the user navigates back, refreshes the card, and opens the detail again, they will still see cached (potentially stale) data in the detail screen since forceRefresh=false always. This may be acceptable for now, but worth flagging as a follow-up.

6. Cache lifetime in StatsTagsUseCase is unbounded

The singleton cache stores one (siteId, max, data) triple for the application lifetime. The cache key includes both siteId and max, so the card (max=7) and the detail screen (max=100) will always miss each other's cache. This is correct, but means the first time the detail screen opens it always makes a network request even if the card just fetched data seconds ago. This is a known trade-off mentioned in the PR description.

---

Tests

7. Significant duplication between TagsAndCategoriesDetailViewModelTest and BaseTagsAndCategoriesViewModelTest

Most tests in TagsAndCategoriesDetailViewModelTest are verbatim duplicates of tests in BaseTagsAndCategoriesViewModelTest (no-site error, fetch error, exception, success, empty result, double-loadData guard, retry-on-error). The concrete DetailViewModel only overrides maxItems=100, so the base test already covers all these paths. Consider keeping only the maxItems-specific test in TagsAndCategoriesDetailViewModelTest and removing the duplicates.

8. Inconsistent use of org.mockito.Mockito.times() vs. times() import

InsightsViewModelTest.kt:493-496, 749-751, 803-813

verify(statsSummaryUseCase, org.mockito.Mockito.times(1)).invoke(any(), any())

The file already imports mockito.kotlin.times (line 16 would normally be the import block). These calls should use the imported times(1) rather than the fully-qualified org.mockito.Mockito.times(1). Fix this →

9. Missing test: job cancellation on refresh

There is no test verifying that refreshData() actually cancels an in-flight fetchJob. The test when refresh called, then all visible endpoints are re-fetched verifies the final outcome but not the cancellation of the previous job. A test that starts a slow/infinite fetch, calls refreshData(), and verifies only the refreshed results arrive would give stronger confidence.

---

Minor

10. Magic number DETAIL_EXPANDED_START_PADDING = 52

TagsAndCategoriesDetailActivity.kt:47

52 dp is used as start padding for expanded tag items but its meaning is not documented. A brief comment (e.g., "aligns with tag-type icon + gap") would help reviewers understand the intent.

---

Blocking items: None — the wordpress-rs PR dependency (flagged by Danger) is the only thing blocking merge.

[wpmobilebot]

🤖 Build Failure Analysis

This build has failures. Claude has analyzed them - check the build annotations for details.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud