CMM-1973: Views card period selection and chart improvements

[adalpari]

Description

Adds chart improvements and drill-down interaction to the Views stats card:

• Persist chart type selection: Save the user's line/bar chart preference via SavedStateHandle (process death) and SharedPreferences (app restart)
• Single-period bar chart: Show only current period bars with wider bars (16dp) for better readability. Previous period is shown as a dashed grey line overlay
• Straight line connectors: Replace cubic curves with straight lines in both chart modes
• Drill-down on bar tap: Tapping a bar changes the period selector to the tapped period — monthly bars drill into the full month (by days), daily bars drill into that day (by hours). Hourly bars are ignored since there's no smaller granularity. Works for Last 6/12 Months and custom periods with monthly granularity
• Loading spinner: Show a centered spinner with dimmed content while drilling down, instead of replacing the entire card with a shimmer skeleton
• Tap guard: Ignore subsequent bar taps while a drill-down is already loading
• Unit tests: 10 new tests covering chart type persistence, drill-down edge cases (daily, monthly, hourly, custom monthly, invalid index), loading guard, and ChartType enum storage key conversion

Testing instructions

Persist chart type:

1. Open the new stats screen and navigate to the Views card
2. Switch the chart type from line to bar (or vice versa)
3. Leave the screen and return

• Verify the chart type selection is preserved

Drill down on bar tap:

1. Select "Last 7 Days" period and switch to bar chart
2. Tap on a specific day's bar

• Verify the period selector changes to a custom range for that day
• Verify the chart shows 24 hourly data points

3. Select "Last 12 Months" period
4. Tap on a specific month's bar

• Verify the period selector changes to the full month (first to last day)

• Verify the chart shows daily data points for that month

• Smoke tests bar taps

Screen_recording_20260324_140240.mp4

[dangermattic]

	2 Warnings
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️	PR is not assigned to a milestone.

Generated by 🚫 Danger

[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	pr22726-2c7d562
	Build Number	1488
	Application ID	org.wordpress.android.prealpha
	Commit	2c7d562
	Installation URL	0jgsvhj0ofjso

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

[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	pr22726-2c7d562
	Build Number	1488
	Application ID	com.jetpack.android.prealpha
	Commit	2c7d562
	Installation URL	0ljd55orgej30

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

[adalpari]

@claude

[claude]

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

---

Code Review: CMM-1973 Views Card Period Selection & Chart Improvements

• Gather context and understand the request
• Read changed files
• Analyze code quality, security, performance, and testing
• Post review feedback

---

Overall this is a solid PR with clean MVVM patterns, good test coverage, and well-structured UI state management. A few issues worth addressing before merging:

---

Bugs

1. Double-load race condition in onBarTapped

ViewsStatsViewModel.kt, lines 263–274

onBarTapped calls loadData() directly after onPeriodChanged(), but never sets loadingPeriod. When selectedPeriod changes, the composable will observe the change and call loadDataIfNeeded() — at that point loadingPeriod == null and loadedPeriod is the old period, so it fires a second concurrent network request.

fun onBarTapped(index: Int) {
    ...
    onPeriodChanged(newPeriod)
    loadData()  // ← sets no loadingPeriod guard
}

Fix: set loadingPeriod = newPeriod before calling loadData() inside onBarTapped, or call loadDataIfNeeded() from the composable instead and skip the direct loadData() call here (relying on the period change observation). Fix this →

---

2. drillDownPeriod parsing is fragile for monthly granularity

ViewsStatsViewModel.kt, lines 288–298

When isMonthlyGranularity is true, the code does rawPeriod.take(DATE_PREFIX_LENGTH) and parses it as ISO_LOCAL_DATE. This assumes monthly data always arrives in YYYY-MM-DD format. If the API ever returns YYYY-MM for monthly periods, take(10) yields "YYYY-MM" (7 chars) which ISO_LOCAL_DATE cannot parse, throwing a DateTimeParseException.

Consider either checking format before parsing, or handling the exception:

// Current code assumes YYYY-MM-DD — fragile if API changes
val date = LocalDate.parse(
    rawPeriod.take(DATE_PREFIX_LENGTH),  // could be 7 chars for "YYYY-MM"
    DateTimeFormatter.ISO_LOCAL_DATE
)

Fix this →

---

Policy Violations

3. Reflection used in test — violates CLAUDE.md guidelines

ViewsStatsViewModelTest.kt, lines 619–627

CLAUDE.md explicitly states: "No Reflection: Avoid using reflection in new code; prefer type-safe alternatives."

val field = ViewsStatsViewModel::class.java
    .getDeclaredField("_uiState")
field.isAccessible = true
@Suppress("UNCHECKED_CAST")
val uiState = field.get(viewModel) as kotlinx.coroutines.flow.MutableStateFlow<...>
uiState.value = ...

Consider exposing a @VisibleForTesting method on the ViewModel (e.g., fun simulateLoadingState()) or restructuring the test to trigger the loading state through the public API (e.g., using a CompletableDeferred in the repository mock). Fix this →

---

Performance

4. Long to Int overflow in chart data

ViewsStatsCard.kt, lines 512–513, 520, 524–528

series(chartData.currentPeriod.map { it.views.toInt() })

views is Long. For high-traffic sites with > ~2.1 billion views in a period, toInt() silently overflows. The Vico library accepts Number in some APIs — consider casting to Float instead, which has sufficient precision for chart rendering and covers the full view range.

---

Code Quality

5. StatItemCard selects icons via localized string comparison

ViewsStatsCard.kt, lines 719–725

val icon = when (stat.label) {
    stringResource(R.string.stats_views) -> Icons.Outlined.Visibility
    stringResource(R.string.stats_visitors) -> Icons.Default.PersonOutline
    ...
    else -> Icons.Outlined.Visibility  // silent fallback
}

Matching on translated display strings is fragile. If a string resource changes, the icon silently falls back to the default. Consider adding an icon type or enum to StatItem, set in buildBottomStats(). Fix this →

6. formatMonthlyLabel has no bounds check

ViewsStatsViewModel.kt, lines 519–522

private fun formatMonthlyLabel(period: String): String {
    val parts = period.split("-")
    val date = LocalDate.of(parts[0].toInt(), parts[1].toInt(), 1)

If period is malformed (e.g., missing -), this throws IndexOutOfBoundsException. A try-catch or getOrNull guard would make this resilient to unexpected API data.

---

Testing

7. Missing test: loadDataIfNeeded not double-called after bar tap

There is no test that verifies the double-load race condition (issue #1) is prevented — i.e., that when onBarTapped triggers loadData() internally, a subsequent loadDataIfNeeded() call from the composable does not trigger an additional network request. This would be a good test to add alongside the fix.

---

Minor Observations (no action required)

• The two-phase initialization in initializeWithPersistedPeriod (SavedStateHandle sync + SharedPreferences async) is well-structured. The isPeriodInitialized gate correctly prevents premature data loading.
• Parallel fetching of current/previous period in StatsRepository.fetchStatsForPeriod with coroutineScope + async is the right approach.
• The isLoadingNewPeriod + dimmed overlay + spinner UX pattern is clean and correctly guarded.
• ChartType.fromStorageKey returning null for unrecognised keys (forward compat) is a good defensive pattern.

[adalpari]

5. StatItemCard selects icons via localized string comparison

ViewsStatsCard.kt, lines 719–725

val icon = when (stat.label) {
    stringResource(R.string.stats_views) -> Icons.Outlined.Visibility
    stringResource(R.string.stats_visitors) -> Icons.Default.PersonOutline
    ...
    else -> Icons.Outlined.Visibility  // silent fallback
}

Matching on translated display strings is fragile. If a string resource changes, the icon silently falls back to the default. Consider adding an icon type or enum to StatItem, set in buildBottomStats()

This is expected because the label will always be localised.

[codecov]

Codecov Report

❌ Patch coverage is 41.75824% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.38%. Comparing base (d91e2f5) to head (2c7d562).
⚠️ Report is 3 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...s/android/ui/newstats/viewsstats/ViewsStatsCard.kt	0.00%	82 Missing ⚠️
...roid/ui/newstats/viewsstats/ViewsStatsViewModel.kt	84.41%	6 Missing and 6 partials ⚠️
.../android/ui/newstats/repository/StatsRepository.kt	8.33%	10 Missing and 1 partial ⚠️
.../wordpress/android/ui/newstats/NewStatsActivity.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #22726      +/-   ##
==========================================
+ Coverage   37.36%   37.38%   +0.02%     
==========================================
  Files        2316     2316              
  Lines      123376   123507     +131     
  Branches    16734    16758      +24     
==========================================
+ Hits        46096    46177      +81     
- Misses      73574    73623      +49     
- Partials     3706     3707       +1     

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nbradbury]

@adalpari This looks good, but one problem was flagged:

1. drillDownMonth() exception not caught (Medium)

File: ViewsStatsViewModel.kt:310-324

drillDownPeriod() catches DateTimeParseException, but drillDownMonth() can throw NumberFormatException from parts[0].toInt() / parts[1].toInt() if the string passes the regex but contains unexpected values. The catch block on line 300 won't handle it.

Also, when I first tapped a bar to drill down, it wasn't clear at first that the date range had changed. On the web the date is prominent:

But in the app the date is much less prominent:

This isn't something to address in this PR but I wanted to point it out.

[adalpari]

Also, when I first tapped a bar to drill down, it wasn't clear at first that the date range had changed. On the web the date is prominent:

Do you mean the selected period date? How would you suggest to make it more prominent?

On iOS, I just see the selector being changed as well 🤔

On the other hand, I want to improve the look of the bars. So maybe I can open another iteration PR

[adalpari]

@adalpari This looks good, but one problem was flagged:

1. drillDownMonth() exception not caught (Medium)

File: ViewsStatsViewModel.kt:310-324

drillDownPeriod() catches DateTimeParseException, but drillDownMonth() can throw NumberFormatException from parts[0].toInt() / parts[1].toInt() if the string passes the regex but contains unexpected values. The catch block on line 300 won't handle it.

Fixed!

[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

[nbradbury]

Do you mean the selected period date? How would you suggest to make it more prominent?

I do mean that, yes. I'm not sure how to make it more prominent. I'd suggest adding a large label at the top like our web app, but that would take up extra space where space is at a premium.

[nbradbury]

Looks good!