Skip to content

CMM-1936: New stats add Insights tab#22666

Open
adalpari wants to merge 13 commits intotrunkfrom
feat/CMM-1936-create-insights-tab
Open

CMM-1936: New stats add Insights tab#22666
adalpari wants to merge 13 commits intotrunkfrom
feat/CMM-1936-create-insights-tab

Conversation

@adalpari
Copy link
Contributor

@adalpari adalpari commented Mar 9, 2026

Description

Populates the Insights tab in the new stats screen with its first card: Year in Review.

This follows the same architecture as the Traffic tab: card management (add/remove/move), data layer, ViewModel pattern, and comprehensive tests.

What's included:

  • Card type system: InsightsCardType enum, InsightsCardsConfiguration, and InsightsCardsConfigurationRepository for card management with persistence via AppPrefs
  • Data layer: StatsDataSource.fetchStatsInsights() and StatsRepository.fetchInsights() for fetching yearly insights data
  • Year in Review card: Compose UI showing the current year's stats (posts, words, likes, comments) in a 2×2 mini-card grid with shimmer loading and error states
  • Detail screen: YearInReviewDetailActivity showing all years with full stats (totals + averages) in a table layout, accessible via "View all" link
  • Current year guarantee: The current year is always shown in the card, even if not returned by the API (displayed with zero values)
  • Card management: InsightsViewModel handles add/remove/move card operations with the same pattern as NewStatsViewModel

Testing instructions

NOTE: You will see the period selector in the Insights screen, but it's useless. I've not hidden it because I already hid it in this PR (waiting for the RS layer to be approved) and I don't want to create any conflict.

Year in Review card:

  1. Navigate to Stats → Insights tab
  2. Verify the Year in Review card loads with the current year's stats
  • Card shows current year title (e.g., "2026 in review")
  • Stats display in a 2×2 grid (Posts, Words, Likes, Comments)
  • "View all" link is visible

View all detail screen:

  1. Tap "View all" on the Year in Review card
  • Detail screen opens with all years listed newest-first
  • Each year shows 7 stats: Posts, Total comments, Avg comments/post, Total likes, Avg likes/post, Total words, Avg words/post
  • Back button returns to Insights tab
Screen_recording_20260309_170610.mp4

adalpari and others added 7 commits March 9, 2026 11:13
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Populate the Insights tab with card management (add, remove, move)
mirroring the Traffic tab architecture. Add Year in Review as the
first Insights card, fetching yearly summaries (posts, words, likes,
comments) via the wordpress-rs statsInsights endpoint.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update the card title to show "YEAR in review" instead of a generic
title. Replace the table layout with a 2x2 grid of mini cards, each
displaying an icon, label, and formatted value for Posts, Words,
Likes, and Comments, matching the web design.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add a detail screen showing all years with full stats (posts, comments,
avg comments/post, likes, avg likes/post, words, avg words/post). The
card shows the most recent year and a "View all" link when multiple
years are available. Years are sorted newest first.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ow All

Ensure the current year is always present in the data, adding it with
zero values if not returned by the API. The Show All footer is now
always visible regardless of the number of years available.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use resource string for exception errors instead of raw e.message
- Add duplicate guard in addCard()
- Change Error from data class to class to fix lambda equality
- Use Year.now() per-call instead of cached static val
- Fix isValidConfiguration to check for null entries
- Remove 0dp Spacer from detail screen
- Add StatsFormatterTest with 12 tests
- Add repository moveCard and addCard duplicate tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dangermattic
Copy link
Collaborator

dangermattic commented Mar 9, 2026

1 Error
🚫 This PR includes a PR version of wordpress-rs (1230-d4c9317af369c7231b414998986fd2e773eebf7d). Please merge the corresponding wordpress-rs PR and update to a trunk version before merging.
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

@adalpari adalpari changed the base branch from trunk to feat/CMM-1936-new-stats-create-insights-tab-feature-branch March 9, 2026 13:01
@adalpari
Copy link
Contributor Author

adalpari commented Mar 9, 2026

@claude

@claude
Copy link
Contributor

claude bot commented Mar 9, 2026

Claude finished @adalpari's task in 3m 28s —— View job


Code Review: CMM-1936 – Insights Tab with Year in Review Card

  • Gather context and understand the PR
  • Review data layer (StatsDataSource, StatsRepository)
  • Review card configuration and management
  • Review ViewModels
  • Review UI components (YearInReviewCard, YearInReviewDetailActivity)
  • Review tests
  • Review AppPrefs changes
  • Compile and post final review

Overall the PR is well-structured and follows the established Traffic-tab patterns closely. The code is clean, the test coverage is solid, and the architecture is consistent. Below are the issues worth addressing before merging.


Bugs / Correctness

1. isLoadedSuccessfully stays true after a failed refresh — error state persists on re-entry

YearInReviewViewModel.kt:41,106

When refresh() succeeds once, isLoadedSuccessfully = true. If a subsequent refresh() throws or returns an error, the state becomes Error, but isLoadedSuccessfully is never reset to false. When the user navigates away and back, loadDataIfNeeded() sees isLoadedSuccessfully = true and does nothing — the card stays stuck in the error state with no automatic recovery path.

// loadDataInternal – sets true on success but never resets on failure
is InsightsResult.Success -> {
    isLoadedSuccessfully = true   // ← never reset on subsequent failure
    ...
}

Fix: reset isLoadedSuccessfully = false when the error state is set inside loadDataInternal, or clear the flag at the start of refresh(). Fix this →


2. Race condition in InsightsCardsConfigurationRepository mutation operations

InsightsCardsConfigurationRepository.kt:53-128

Every mutation method (removeCard, addCard, moveCardUp, …) follows a read-modify-write pattern:

suspend fun removeCard(siteId: Long, cardType: InsightsCardType) =
    withContext(ioDispatcher) {
        val current = getConfiguration(siteId)   // read
        ...
        saveConfiguration(siteId, ...)            // write
    }

withContext is not a mutex — if two coroutines call mutations concurrently (e.g. the user rapidly taps remove and the OS also triggers a config reload), the second read can return stale data and overwrite the first write. The equivalent StatsCardsConfigurationRepository for the Traffic tab likely has the same pattern.

Recommended fix: use a Mutex to serialize mutations. Fix this →


3. siteId silently falls back to 0L — operations proceed with an invalid site ID

InsightsViewModel.kt:43-45

private val siteId: Long
    get() = selectedSiteRepository.getSelectedSite()?.siteId ?: 0L

If getSelectedSite() returns null (e.g. during logout), all card mutation calls (removeCard, addCard, move operations) will execute with siteId = 0L, silently writing to the wrong preferences key. Consider guarding early and returning/logging an error instead of using 0. Fix this →


Design / API Inconsistency

4. fetchStatsInsights takes siteId: String while all other interface methods take siteId: Long

StatsDataSource.kt:217-219

// All other methods:
suspend fun fetchStatsVisits(siteId: Long, ...): ...

// Insights:
suspend fun fetchStatsInsights(siteId: String): StatsInsightsDataResult

The conversion to String happens in StatsRepository.fetchInsights (siteId.toString()). The inconsistency leaks through the interface and can confuse future implementors. Consider aligning the interface to Long and letting StatsDataSourceImpl handle the toULong() conversion internally (as it does for all other methods). Fix this →


UX Issues

5. Period selector (date/calendar toolbar button) is visible on the Insights tab

NewStatsActivity.kt:138-208

The TopAppBar period selector is always rendered regardless of the active tab. Insights data is year-based and not period-dependent, so showing a period picker on the Insights tab is misleading. The action should be hidden or disabled when the Insights (or Subscribers) tab is active.


Minor Issues

6. resetToDefault does not emit to configurationFlow

InsightsCardsConfigurationRepository.kt:188-196

When an invalid config is found and reset, _configurationFlow is not updated, so any active observers won't be notified of the reset. Since resetToDefault is only called from loadConfiguration (which is invoked from getConfiguration), and getConfiguration returns the result directly to the caller, this is not a practical problem today — but it makes the flow semantics inconsistent. Consider emitting there as well.

7. YearInReviewCardUiState.Error is a plain class, not data class

YearInReviewCardUiState.kt:13-16

Loading and Loaded use data object/data class, but Error is a plain class because it holds a lambda. This is correct (lambdas can't be compared by value), but it means Error has no toString(), no structural equality, and no copy(). This asymmetry is worth documenting with a comment explaining why it can't be a data class.

8. Loading shimmer has no accessibility semantics

YearInReviewCard.kt:112-181

The shimmer LoadingContent composable has no contentDescription or semantic annotation. Screen readers will announce nothing while the card loads. Consider adding a semantics { contentDescription = stringResource(R.string.loading) } to the outer container.


Test Coverage

The test suite is comprehensive and covers the key scenarios well (success, error, retry, refresh, loadDataIfNeeded deduplication, current-year guarantee). The main gap is:

  • No test for the "error after successful load → navigate away → return" scenario described in bug Menu Drawer Item refactor #1 above.
  • No test for concurrent mutations in InsightsCardsConfigurationRepositoryTest (the existing mutation tests are sequential).

Summary

Severity Count
Bug 2 (#1 stale error state, #2 race condition)
Correctness concern 1 (#3 siteId fallback)
Design inconsistency 1 (#4 String vs Long siteId)
UX 1 (#5 period selector on Insights tab)
Minor/polish 3 (#6–8)

@adalpari adalpari changed the base branch from feat/CMM-1936-new-stats-create-insights-tab-feature-branch to trunk March 9, 2026 13:03
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 9, 2026

Project manifest changes for WordPress

The following changes in the WordPress's merged AndroidManifest.xml file were detected (build variant: wordpressRelease):

--- ./build/reports/diff_manifest/WordPress/wordpressRelease/base_manifest.txt	2026-03-13 15:23:07.333694357 +0000
+++ ./build/reports/diff_manifest/WordPress/wordpressRelease/head_manifest.txt	2026-03-13 15:23:14.503705447 +0000
@@ -203,6 +203,14 @@
         <activity
             android:name="org.wordpress.android.ui.newstats.authors.AuthorsDetailActivity"
             android:exported="false"
+            android:theme="@style/WordPress.NoActionBar" />
+        <activity
+            android:name="org.wordpress.android.ui.newstats.yearinreview.YearInReviewDetailActivity"
+            android:exported="false"
+            android:theme="@style/WordPress.NoActionBar" />
+        <activity
+            android:name="org.wordpress.android.ui.newstats.tagsandcategories.TagsAndCategoriesDetailActivity"
+            android:exported="false"
             android:theme="@style/WordPress.NoActionBar" /> <!-- Account activities -->
         <activity
             android:name="org.wordpress.android.ui.main.MeActivity"

Go to https://buildkite.com/automattic/wordpress-android/builds/25463/canvas?sid=019ce7c3-3258-45b7-b784-ced02fa25ced, click on the Artifacts tab and audit the files.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 9, 2026

Project manifest changes for WordPress

The following changes in the WordPress's merged AndroidManifest.xml file were detected (build variant: jetpackRelease):

--- ./build/reports/diff_manifest/WordPress/jetpackRelease/base_manifest.txt	2026-03-13 15:22:50.072242607 +0000
+++ ./build/reports/diff_manifest/WordPress/jetpackRelease/head_manifest.txt	2026-03-13 15:22:56.662259042 +0000
@@ -395,6 +395,14 @@
         <activity
             android:name="org.wordpress.android.ui.newstats.authors.AuthorsDetailActivity"
             android:exported="false"
+            android:theme="@style/WordPress.NoActionBar" />
+        <activity
+            android:name="org.wordpress.android.ui.newstats.yearinreview.YearInReviewDetailActivity"
+            android:exported="false"
+            android:theme="@style/WordPress.NoActionBar" />
+        <activity
+            android:name="org.wordpress.android.ui.newstats.tagsandcategories.TagsAndCategoriesDetailActivity"
+            android:exported="false"
             android:theme="@style/WordPress.NoActionBar" /> <!-- Account activities -->
         <activity
             android:name="org.wordpress.android.ui.main.MeActivity"

Go to https://buildkite.com/automattic/wordpress-android/builds/25463/canvas?sid=019ce7c3-3259-4974-af06-55c2dfd95700, click on the Artifacts tab and audit the files.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 9, 2026

Project dependencies changes

list
! Upgraded Dependencies
rs.wordpress.api:android:1230-d4c9317af369c7231b414998986fd2e773eebf7d, (changed from trunk-4fac42b1a262e93adcbc0cd0353aafa0369671d2)
rs.wordpress.api:kotlin:1230-d4c9317af369c7231b414998986fd2e773eebf7d, (changed from trunk-4fac42b1a262e93adcbc0cd0353aafa0369671d2)
tree
 +--- project :libs:fluxc
-|    \--- rs.wordpress.api:android:trunk-4fac42b1a262e93adcbc0cd0353aafa0369671d2
-|         +--- 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-4fac42b1a262e93adcbc0cd0353aafa0369671d2
-|         |    +--- 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-d4c9317af369c7231b414998986fd2e773eebf7d
+|         +--- 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-d4c9317af369c7231b414998986fd2e773eebf7d
+|         |    +--- 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-4fac42b1a262e93adcbc0cd0353aafa0369671d2 (*)
+\--- rs.wordpress.api:android:1230-d4c9317af369c7231b414998986fd2e773eebf7d (*)

…, siteId fallback, API type inconsistency

- Reset isLoadedSuccessfully on error/exception so loadDataIfNeeded recovers after failed refresh
- Add Mutex to InsightsCardsConfigurationRepository to prevent concurrent mutation races
- Guard siteId in InsightsViewModel mutations to avoid operating with invalid 0L
- Align fetchStatsInsights parameter from String to Long for interface consistency

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@adalpari
Copy link
Contributor Author

adalpari commented Mar 9, 2026

Claude sugestions achieved here

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 9, 2026

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

App NameJetpack Android
Build TypeDebug
Versionpr22666-a2d0dbf
Build Number1487
Application IDcom.jetpack.android.prealpha
Commita2d0dbf
Installation URL2kfoj6312j3m8
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 9, 2026

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

App NameWordPress Android
Build TypeDebug
Versionpr22666-a2d0dbf
Build Number1487
Application IDorg.wordpress.android.prealpha
Commita2d0dbf
Installation URL037md267cm9p0
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 9, 2026

🤖 Build Failure Analysis

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

@adalpari adalpari marked this pull request as ready for review March 9, 2026 16:09
@adalpari adalpari requested a review from a team as a code owner March 9, 2026 16:09
@adalpari adalpari requested review from nbradbury and removed request for a team March 9, 2026 16:09
@nbradbury
Copy link
Contributor

I found it odd that the four labels are all totals for the year, but only the Words and Likes labels include the word "Total." The web app doesn't say "Total" anywhere and I think we can do the same.

total

Card uses short labels (Words, Likes) without "Total" prefix.
Detail screen uses exact web labels: Total posts, Total comments,
Avg comments per post, Total likes, Avg likes per post, Total words,
Avg words per post.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@adalpari
Copy link
Contributor Author

adalpari commented Mar 9, 2026

I found it odd that the four labels are all totals for the year, but only the Words and Likes labels include the word "Total." The web app doesn't say "Total" anywhere and I think we can do the same.

Good call! This was because of reusing words. Changed!
share_908030740176707496

Copy link
Contributor

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

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

Looks good! I'll approve this and it can be merged when Buildkite is feeling better :shipit:

)

* Update wordpress-rs library hash

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add stats summary data layer with shared caching

Add fetchStatsSummary endpoint and StatsSummaryData model to
StatsDataSource. Implement Mutex-based caching in StatsRepository
so All-time Stats and Most Popular Day cards share a single API call.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add All-time Stats Insights card

New card showing Views, Visitors, Posts, and Comments from the
statsSummary endpoint. Uses rememberShimmerBrush for loading state.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add Most Popular Day Insights card

New card showing the best day for views with date, view count, and
percentage. Shares the statsSummary API call with All-time Stats via
repository caching.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Wire All-time Stats and Most Popular Day into Insights tab

Add card types to InsightsCardType enum and default card list.
Wire ViewModels and cards into InsightsTabContent. Add config
migration to automatically show new card types for existing users.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix review issues: locale, empty best day, config migration, tests

- Create DateTimeFormatter at format time instead of caching in
  companion object to respect locale changes
- Handle empty viewsBestDay by returning loaded state with empty
  values instead of throwing
- Fix config migration by persisting hiddenCards field so new card
  types can be distinguished from user-removed cards
- Add missing tests: empty viewsBestDay, zero views percentage,
  exception path, dayAndMonth assertion, addNewCardTypes migration,
  full config no-migration

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Update RS library and fix insights locale, detekt, and date casing

- Update wordpress-rs to 1de57afce924622700bcc8d3a1f3ce893d8dad5b
- Add locale parameter to StatsInsightsParams matching other endpoints
- Fix detekt MagicNumber and TooGenericExceptionCaught violations
- Capitalize first letter of formatted date in Most Popular Day card

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Move stats summary cache from StatsRepository to InsightsViewModel

StatsRepository was not @singleton, so the shared cache for statsSummary
data was broken — each ViewModel got its own instance. Move caching to
InsightsViewModel which is activity-scoped and shared. Child ViewModels
now receive a summary provider function wired from the UI layer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix double-write and concurrency in InsightsCardsConfigurationRepository

addNewCardTypes wrote to prefs directly, then the caller wrote again via
saveConfiguration. Also getConfiguration was not mutex-protected. Make
addNewCardTypes pure, extract loadAndMigrate for atomic read-migrate-save
within the mutex, and make getConfiguration go through the mutex.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Rename hiddenCards() to computeHiddenCards() to avoid naming conflict

InsightsCardsConfiguration had both a hiddenCards property (persisted
list) and a hiddenCards() method (computed from visibleCards). Rename
the method to make the distinction clear.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add NoData state for MostPopularDay and fix percentage precision

When a site has no best day data, show a dedicated NoData state with a
"No data yet" message instead of a blank Loaded card. Also reduce
percentage format from 3 to 1 decimal place for consistency with
typical stats UIs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Suppress TooGenericExceptionThrown detekt warning in test files

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix thread-safety, DI design, and Error state in Insights cards

- Add @volatile to isLoading/isLoadedSuccessfully flags in ViewModels
- Extract StatsSummaryUseCase singleton to replace summaryProvider var,
  removing temporal coupling and null-check error paths
- Change Error from class to data class, move onRetry out of state and
  into composable parameters to avoid unnecessary recompositions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
adalpari and others added 2 commits March 13, 2026 11:32
)

* Add Most Popular Time Insights card

Introduce a new "Most popular time" card in the stats insights tab
that shows the best day of week and best hour with their view
percentages. The card reuses the insights API endpoint via a new
shared StatsInsightsUseCase (following the StatsSummaryUseCase
caching pattern), which also refactors YearInReviewViewModel to
use the same use case.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Clean up MostPopularTimeViewModel: locale-aware hour formatting and remove unnecessary @volatile

Use DateTimeFormatter.ofLocalizedTime instead of hardcoded AM/PM to
respect device locale settings. Remove unnecessary @volatile annotations
since all access is on the main thread via viewModelScope.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add tests for MostPopularTimeViewModel and StatsInsightsUseCase

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix day-of-week mapping, NoData condition, and add bounds check

- Fix day mapping: WordPress API uses 0=Monday (not Sunday)
- Show NoData when either day or hour percent is zero (not both)
- Add bounds check for invalid day values (returns empty string)
- Update and add tests for new behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix detekt: suppress LongMethod and remove unused import

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Centralize Insights data fetching in InsightsViewModel

Move data fetching from individual card ViewModels to InsightsViewModel
as coordinator, ensuring each API endpoint (stats summary and insights)
is called only once per load. Card ViewModels now receive results via
SharedFlow instead of fetching independently, reducing duplicate
network calls from 4 to 2.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix race condition, consistent onRetry pattern, and remove unused siteId

- Set isDataLoading in refreshData() to prevent duplicate fetches
- Move onRetry from YearInReviewCardUiState.Error to composable param
- Remove unused siteId property, use resolvedSiteId() directly

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add formatHour bounds check, remove duplicate string, clean up import

- Guard formatHour against invalid hour values (crash prevention)
- Remove duplicate stats_insights_percent_of_views string resource
- Use import for kotlin.math.round instead of fully qualified call

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix detekt LongMethod: extract fetchSummary and fetchInsights

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Reduce duplication in MostPopularTimeCard using shared components

Replace manual card container, header, error content, and shimmer
boxes with StatsCardContainer, StatsCardHeader, StatsCardErrorContent,
and ShimmerBox. Extract repeated day/hour section into StatSection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Rename views percent string resource and add NoData preview

- Rename stats_insights_most_popular_day_percent to
  stats_insights_views_percent for neutral naming
- Add missing NoData preview to MostPopularTimeCard

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Trigger PR checks

* Use device 24h/12h setting for hour formatting

Use android.text.format.DateFormat.is24HourFormat() to respect
the device time format preference instead of relying on locale.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix thread safety, CancellationException handling, and lambda allocation

- Add @volatile to isDataLoaded/isDataLoading flags
- Rethrow CancellationException to preserve structured concurrency
- Wrap onRetryData lambda with remember to avoid recomposition allocations

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@adalpari adalpari changed the title CMM-1936: Add Insights tab with Year in Review card CMM-1936: New stats add Insights tab Mar 13, 2026
* Add Most Popular Time Insights card

Introduce a new "Most popular time" card in the stats insights tab
that shows the best day of week and best hour with their view
percentages. The card reuses the insights API endpoint via a new
shared StatsInsightsUseCase (following the StatsSummaryUseCase
caching pattern), which also refactors YearInReviewViewModel to
use the same use case.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Clean up MostPopularTimeViewModel: locale-aware hour formatting and remove unnecessary @volatile

Use DateTimeFormatter.ofLocalizedTime instead of hardcoded AM/PM to
respect device locale settings. Remove unnecessary @volatile annotations
since all access is on the main thread via viewModelScope.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add tests for MostPopularTimeViewModel and StatsInsightsUseCase

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix day-of-week mapping, NoData condition, and add bounds check

- Fix day mapping: WordPress API uses 0=Monday (not Sunday)
- Show NoData when either day or hour percent is zero (not both)
- Add bounds check for invalid day values (returns empty string)
- Update and add tests for new behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix detekt: suppress LongMethod and remove unused import

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Centralize Insights data fetching in InsightsViewModel

Move data fetching from individual card ViewModels to InsightsViewModel
as coordinator, ensuring each API endpoint (stats summary and insights)
is called only once per load. Card ViewModels now receive results via
SharedFlow instead of fetching independently, reducing duplicate
network calls from 4 to 2.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix race condition, consistent onRetry pattern, and remove unused siteId

- Set isDataLoading in refreshData() to prevent duplicate fetches
- Move onRetry from YearInReviewCardUiState.Error to composable param
- Remove unused siteId property, use resolvedSiteId() directly

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add formatHour bounds check, remove duplicate string, clean up import

- Guard formatHour against invalid hour values (crash prevention)
- Remove duplicate stats_insights_percent_of_views string resource
- Use import for kotlin.math.round instead of fully qualified call

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix detekt LongMethod: extract fetchSummary and fetchInsights

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Reduce duplication in MostPopularTimeCard using shared components

Replace manual card container, header, error content, and shimmer
boxes with StatsCardContainer, StatsCardHeader, StatsCardErrorContent,
and ShimmerBox. Extract repeated day/hour section into StatSection.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Rename views percent string resource and add NoData preview

- Rename stats_insights_most_popular_day_percent to
  stats_insights_views_percent for neutral naming
- Add missing NoData preview to MostPopularTimeCard

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Trigger PR checks

* Use device 24h/12h setting for hour formatting

Use android.text.format.DateFormat.is24HourFormat() to respect
the device time format preference instead of relying on locale.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix thread safety, CancellationException handling, and lambda allocation

- Add @volatile to isDataLoaded/isDataLoading flags
- Rethrow CancellationException to preserve structured concurrency
- Wrap onRetryData lambda with remember to avoid recomposition allocations

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add Tags & Categories insights card

Add a new top-list card to the Insights tab showing tags and categories
with view counts. Includes expandable multi-tag groups, percentage bars,
folder/tag icons, and a detail screen via Show All. Updates wordpress-rs
to 1230 for the statsTags endpoint.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add expand/collapse for multi-tag groups in detail screen

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add tests for Tags & Categories feature

ViewModel, repository, and display type unit tests covering
success/error states, data mapping, refresh, and edge cases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Simplify Tags & Categories by reusing shared components

- Use StatsCardContainer, StatsCardHeader, StatsListHeader,
  StatsCardEmptyContent, StatsListRowContainer from StatsCardCommon
- Extract TagTypeIcon and ExpandedTagsSection into shared
  TagsAndCategoriesComponents to eliminate duplication between
  Card and DetailActivity
- Add fromTagType() to TagGroupDisplayType to avoid list allocation
  per tag in ExpandedTagsSection
- Add modifier parameter to StatsListRowContainer for clickable rows
- Remove duplicated constants (CardCornerRadius, BAR_BACKGROUND_ALPHA,
  VERTICAL_LINE_ALPHA)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Remove unused stubUnknownError from ViewModel test

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix review issues: error recovery, conditional refresh, loading state

- Only set isLoaded on success so loadData() retries after errors
- Guard pull-to-refresh to skip tags refresh when card is hidden
- Move loading state into refresh() so callers don't need showLoading()
- Remove showLoading() public method
- Add test for loadData() retry after error

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Address review feedback: deduplicate row composable, remove unused link field, fix thread safety and Intent size

- Extract shared TagGroupRow composable into TagsAndCategoriesComponents.kt with optional position parameter, removing duplicate from Card and DetailActivity
- Remove unused TagData.link field from data source, impl, and all tests
- Replace Intent extras with in-memory static holder in DetailActivity to avoid TransactionTooLargeException risk
- Remove unnecessary Parcelable from UI models
- Use AtomicBoolean for isLoaded/isLoading flags in ViewModel for thread safety

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix concurrent refresh, process death, isExpandable duplication, and accessibility

- Cancel in-flight fetch job on refresh() to prevent stale overwrites
- Finish detail activity on process death instead of showing blank screen
- Extract TagGroupUiItem.isExpandable computed property to deduplicate logic
- Add content descriptions for TagTypeIcon and expand/collapse chevron icons

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Update configuration tests to include TAGS_AND_CATEGORIES card type

Fixes CI test failures caused by the new TAGS_AND_CATEGORIES card
not being reflected in test fixtures and assertions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@sonarqubecloud
Copy link

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants