Skip to content

CMM-1936: Add Insights tab to new stats screen#22711

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

CMM-1936: Add Insights tab to new stats screen#22711
adalpari wants to merge 20 commits intotrunkfrom
feat/CMM-1936-create-insights-tab-complete

Conversation

@adalpari
Copy link
Contributor

@adalpari adalpari commented Mar 19, 2026

Description

Note: This is the result of multiple individual PRs merged into this Feature branch.

Adds the Insights tab to the new stats screen with the following cards:

  • Year in Review — 2x2 grid showing posts, words, likes, and comments per year, with a detail screen for all years
  • All-Time Stats — total posts, views, visitors, and best views day
  • Most Popular Day — day of the week with highest average views
  • Most Popular Time — hour of the day with highest average views (bar chart)
  • Tags & Categories — top tags/categories by views, with expandable groups and a detail screen

Also includes:

  • Centralized data fetching via InsightsViewModel coordinating StatsSummaryUseCase, StatsInsightsUseCase, and StatsTagsUseCase
  • Card configuration (add/remove/reorder) via InsightsCardsConfigurationRepository persisted in AppPrefs
  • Pull-to-refresh, no-connection handling, and add-card bottom sheet for the Insights tab
  • Generic AddCardBottomSheet<T> extracted to deduplicate three identical bottom sheet implementations
  • Error handling fix in StatsTagsUseCase for in-flight request coalescing
  • Comprehensive unit tests for all new ViewModels, use cases, and repositories

Testing instructions

Smoke tests the whole new stats Insights tab.

Screenshot_20260319-164022

adalpari and others added 16 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>
…, 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>
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>
)

* 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>
)

* 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>
* 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>
# Conflicts:
#	WordPress/src/main/AndroidManifest.xml
#	WordPress/src/main/java/org/wordpress/android/ui/newstats/NewStatsActivity.kt
#	WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSource.kt
#	WordPress/src/main/java/org/wordpress/android/ui/newstats/datasource/StatsDataSourceImpl.kt
#	WordPress/src/main/java/org/wordpress/android/ui/newstats/repository/StatsRepository.kt
#	WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefs.java
#	WordPress/src/main/java/org/wordpress/android/ui/prefs/AppPrefsWrapper.kt
#	gradle/libs.versions.toml
* 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>

* Fetch tags data independently in detail screen instead of using static field

The detail screen now has its own ViewModel that fetches up to 100
items directly from the API, while the card continues to fetch 10.
This removes the static data holder pattern that was prone to data
loss on process death.

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

* Extract shared mapper, add detail VM tests, guard double calls, show all 10 card items

- Extract TagsAndCategoriesMapper to deduplicate TagGroupData to
  TagGroupUiItem mapping between card and detail ViewModels
- Add unit tests for TagsAndCategoriesDetailViewModel
- Add isLoaded/isLoading guards to detail VM to prevent double fetches
- Remove CARD_MAX_ITEMS limit so card displays all 10 fetched items
- Remove unused import in DetailActivity

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

* Skip data fetching for hidden Insights cards

Only call summary/insights endpoints when visible cards need them,
avoiding unnecessary network calls for hidden cards. Track which
endpoint groups have been fetched so re-adding a hidden card
triggers a fetch for its missing data.

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

* Early return in fetchData when no endpoints are needed

Prevents setting isDataLoaded=true when no cards require
fetching, which would block future fetches when cards are
re-added to the visible list.

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

* Address code review findings: reduce duplication and improve tests

- Replace duplicate shimmer animation in YearInReviewCard with
  shared rememberShimmerBrush() utility
- Extract StatsTagsUseCase to centralize token validation and
  repository init, removing duplication between Tags ViewModels
- Add card reordering tests for middle elements in
  InsightsCardsConfigurationRepositoryTest
- Fix locale-dependent assertions in MostPopularDayViewModelTest

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

* Suppress LargeClass detekt warning on InsightsViewModelTest

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

* Fix process-death restore and add caching to StatsTagsUseCase

- Call loadData() unconditionally in TagsAndCategoriesDetailActivity
  onCreate to handle process-death restore (loadData guard prevents
  double fetch on rotation)
- Add Mutex-protected in-memory cache to StatsTagsUseCase, consistent
  with StatsSummaryUseCase and StatsInsightsUseCase
- Pass forceRefresh=true on pull-to-refresh in TagsAndCategoriesViewModel

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

* Extract BaseTagsAndCategoriesViewModel and use localized error messages

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

* Fix thread safety and error handling in ViewModels

Use AtomicBoolean with compareAndSet in InsightsViewModel to prevent
race conditions, rethrow CancellationException in base tags ViewModel,
and only mark endpoints as fetched on success results.

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

* Extract isCacheHit method to fix detekt ComplexCondition

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

* Cancel in-flight fetch job before refreshing in InsightsViewModel

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

* Fix compilation errors after trunk merge

Update wordpress-rs to version with stats tags types and remove
duplicate NoConnectionContent composable and redundant else branches
that caused -Werror failures.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Address code review findings and add base ViewModel tests

- Move network call outside mutex in StatsTagsUseCase to
  avoid blocking concurrent callers during slow requests
- Add main-thread-confinement comments for fetchJob fields
- Document why TAGS_AND_CATEGORIES is absent from
  needsSummary/needsInsights (uses its own fetch path)
- Restore card max items to 7 (was unintentionally changed
  to 10 during refactoring)
- Add tests for BaseTagsAndCategoriesViewModel

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix detekt findings: suppress ReturnCount, remove unused composable

- Suppress ReturnCount on StatsTagsUseCase.invoke (3 returns are
  clearer than restructuring with nested conditions)
- Remove unused PlaceholderTabContent composable

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Fix TOCTOU race, config fetch trigger, and detail empty state

- StatsTagsUseCase: use in-flight Deferred to coalesce concurrent
  requests with the same params, eliminating the TOCTOU race where
  two callers could both miss cache and fire duplicate requests
- BaseTagsAndCategoriesViewModel: make isLoaded private since no
  subclass accesses it directly
- InsightsViewModel: trigger loadDataIfNeeded() from
  updateFromConfiguration when new endpoints are required, instead
  of relying on the UI to call it
- TagsAndCategoriesDetailActivity: add empty-state message when
  the loaded items list is empty

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Deduplicate detail VM tests, fix Mockito import, add cancellation test

- Remove duplicate tests from TagsAndCategoriesDetailViewModelTest
  that are already covered by BaseTagsAndCategoriesViewModelTest;
  keep only initial-state and maxItems-specific tests
- Replace fully-qualified org.mockito.Mockito.times() with the
  imported mockito-kotlin times() throughout InsightsViewModelTest
- Add test verifying that rapid double-refresh cancels the first
  job so only one forceRefresh call completes

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Clear stats caches on screen open to prevent stale data

Add clearCache() to StatsInsightsUseCase, StatsSummaryUseCase, and
StatsTagsUseCase. Call all three from InsightsViewModel init so that
reopening the insights screen always fetches fresh data while still
sharing results between cards within a single session.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Update wordpress-rs to trunk-262a778ead5f163f3450d62adfac21fb32048714

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
- Fix critical bug in StatsTagsUseCase where an exception from fetchTags()
  would leave the CompletableDeferred incomplete, hanging all awaiters
- Extract generic AddCardBottomSheet<T> to replace three duplicate
  implementations (Stats, Insights, Subscribers)
- Merge duplicate LaunchedEffect(cardsToLoad) blocks in InsightsTabContent

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dangermattic
Copy link
Collaborator

dangermattic commented Mar 19, 2026

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 closed this Mar 19, 2026
@adalpari adalpari deleted the feat/CMM-1936-create-insights-tab-complete branch March 19, 2026 13:56
@adalpari adalpari restored the feat/CMM-1936-create-insights-tab-complete branch March 19, 2026 13:56
@adalpari adalpari reopened this Mar 19, 2026
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 19, 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-19 15:50:43.007665376 +0000
+++ ./build/reports/diff_manifest/WordPress/jetpackRelease/head_manifest.txt	2026-03-19 15:50:50.807648389 +0000
@@ -397,6 +397,14 @@
             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" />
+        <activity
             android:name="org.wordpress.android.ui.newstats.subscribers.subscriberslist.SubscribersListDetailActivity"
             android:exported="false"
             android:theme="@style/WordPress.NoActionBar" />

Go to https://buildkite.com/automattic/wordpress-android/builds/25570/canvas?sid=019d06c6-2658-4133-8a2d-29971c6d7ebe, click on the Artifacts tab and audit the files.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 19, 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-19 15:51:24.200876723 +0000
+++ ./build/reports/diff_manifest/WordPress/wordpressRelease/head_manifest.txt	2026-03-19 15:51:33.210936020 +0000
@@ -205,6 +205,14 @@
             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" />
+        <activity
             android:name="org.wordpress.android.ui.newstats.subscribers.subscriberslist.SubscribersListDetailActivity"
             android:exported="false"
             android:theme="@style/WordPress.NoActionBar" />

Go to https://buildkite.com/automattic/wordpress-android/builds/25570/canvas?sid=019d06c6-2657-4f0d-bcb9-a189e186ad69, click on the Artifacts tab and audit the files.

@wpmobilebot
Copy link
Contributor

Project dependencies changes

list
! Upgraded Dependencies
rs.wordpress.api:android:trunk-262a778ead5f163f3450d62adfac21fb32048714, (changed from trunk-502e9561f2a68294f0065867bab9214cc9a6b78c)
rs.wordpress.api:kotlin:trunk-262a778ead5f163f3450d62adfac21fb32048714, (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:trunk-262a778ead5f163f3450d62adfac21fb32048714
+|         +--- 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-262a778ead5f163f3450d62adfac21fb32048714
+|         |    +--- 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:trunk-262a778ead5f163f3450d62adfac21fb32048714 (*)

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 19, 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
Versionpr22711-e22b613
Build Number1487
Application IDcom.jetpack.android.prealpha
Commite22b613
Installation URL6b19m5ku41smg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Mar 19, 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
Versionpr22711-e22b613
Build Number1487
Application IDorg.wordpress.android.prealpha
Commite22b613
Installation URL5gpjs4ek3elhg
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@codecov
Copy link

codecov bot commented Mar 19, 2026

Codecov Report

❌ Patch coverage is 28.48189% with 2054 lines in your changes missing coverage. Please review.
✅ Project coverage is 37.34%. Comparing base (2bd4f59) to head (e22b613).

Files with missing lines Patch % Lines
.../wordpress/android/ui/newstats/NewStatsActivity.kt 0.00% 268 Missing ⚠️
...d/ui/newstats/mostpopularday/MostPopularDayCard.kt 0.00% 265 Missing ⚠️
...ewstats/tagsandcategories/TagsAndCategoriesCard.kt 0.00% 253 Missing ⚠️
...droid/ui/newstats/yearinreview/YearInReviewCard.kt 0.00% 243 Missing ⚠️
...droid/ui/newstats/alltimestats/AllTimeStatsCard.kt 0.00% 208 Missing ⚠️
...ui/newstats/mostpopulartime/MostPopularTimeCard.kt 0.00% 189 Missing ⚠️
...gsandcategories/TagsAndCategoriesDetailActivity.kt 0.00% 166 Missing ⚠️
...s/tagsandcategories/TagsAndCategoriesComponents.kt 0.00% 126 Missing ⚠️
...ewstats/yearinreview/YearInReviewDetailActivity.kt 0.00% 123 Missing ⚠️
...roid/ui/newstats/datasource/StatsDataSourceImpl.kt 0.00% 103 Missing ⚠️
... and 17 more
Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #22711      +/-   ##
==========================================
- Coverage   37.52%   37.34%   -0.19%     
==========================================
  Files        2286     2314      +28     
  Lines      120570   123367    +2797     
  Branches    16514    16693     +179     
==========================================
+ Hits        45244    46066     +822     
- Misses      71651    73600    +1949     
- Partials     3675     3701      +26     

☔ 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.

- Fix race condition in InsightsViewModel.refreshData() by nulling
  fetchJob before cancel and guarding finally block for current job
- Fix StatsTagsUseCase cancellation propagation: complete deferred
  with error instead of completeExceptionally for CancellationException
- Remove redundant hiddenCards constructor param, make it a computed
  property; use PersistedConfig for backward-compatible JSON migration
- Key expandedGroups on items in TagsAndCategoriesCard and detail
  activity so expansion state resets on data refresh
- Use StatsCardContainer in AllTimeStatsCard and MostPopularDayCard
  instead of duplicating border/clip/background pattern
- Replace Year.now() (API 26+) with Calendar in YearInReviewViewModel
- Move @Suppress off inline catch to function level in StatsTagsUseCase
- Fix fully qualified references in repository test (import never, Gson)

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

@claude

@claude
Copy link
Contributor

claude bot commented Mar 19, 2026

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


Code Review: CMM-1936 — Insights Tab

  • Read and analyze key source files
  • Review architecture and data flow
  • Check error handling and edge cases
  • Review testing coverage and quality
  • Check for security and performance concerns
  • Post comprehensive review feedback

This is a well-structured PR. The architecture follows established patterns in the codebase and the test coverage for the new ViewModels and use cases is solid. Below are the issues I found, ordered by severity.


Bugs

1. StatsTagsUseCaseCancellationException resolves deferred with an error instead of cancelling it

StatsTagsUseCase.kt:84-90

} catch (e: CancellationException) {
    mutex.withLock { inFlight = null }
    deferred.complete(TagsResult.Error("Request cancelled"))  // ← problem
    throw e
}

Any concurrent caller waiting on deferred.await() when the owner's coroutine is cancelled will receive TagsResult.Error("Request cancelled") rather than having their own coroutine cancelled. This means the detail screen and the card screen (which use different ViewModels with their own scopes) could show an error state after the user triggers a pull-to-refresh that cancels an in-flight request. The non-cancellation path on line 94 correctly uses completeExceptionally(e) — the cancellation path should use deferred.cancel(e) or deferred.completeExceptionally(e) (and not re-throw, since await() will then rethrow for waiters).

Fix this →


2. InsightsViewModel.updateFromConfiguration — newly-added card may never load

InsightsViewModel.kt:263-279

private fun updateFromConfiguration(config: InsightsCardsConfiguration) {
    ...
    val needsNewFetch = (cards.needsSummary() && !summaryFetched.get()) || ...
    _cardsToLoad.value = config.visibleCards
    if (needsNewFetch) {
        isDataLoaded.set(false)
        loadDataIfNeeded()
    }
}

If a user adds a card while a fetch is already in progress (isDataLoading = true), loadDataIfNeeded() returns immediately because compareAndSet(false, true) fails. The fetch that's already running uses the old _cardsToLoad.value. After the fetch completes the finally block sets isDataLoading.set(false) and isDataLoaded.set(true) — but the newly required data was never fetched. The card will stay in its Loading state until the user manually pulls to refresh.


3. MostPopularTimeViewModel|| in NoData guard hides partial data

MostPopularTimeViewModel.kt:60-63

if (data.highestDayPercent == 0.0 || data.highestHourPercent == 0.0) {
    return MostPopularTimeCardUiState.NoData
}

Using || means the card returns NoData when either value is zero, even if the other contains valid data. A new site where the highest hour has 0% traffic would show NoData for the whole card. && would be the appropriate guard if the intent is "no useful data in either field".


Performance

4. StatsSummaryUseCase / StatsInsightsUseCase — mutex held for entire network call duration

StatsSummaryUseCase.kt:30-45, StatsInsightsUseCase.kt:30-45

Both use cases hold the mutex across the full statsRepository.fetch*() call:

return mutex.withLock {
    ...
    val result = statsRepository.fetchStatsSummary(siteId) // network call inside lock
    ...
}

This serialises all callers for the entire round-trip (vs StatsTagsUseCase which correctly releases the lock before the network call). While not incorrect for correctness, it means the coroutineScope { launch { fetchSummary(...) }; launch { fetchInsights(...) } } parallel execution in InsightsViewModel provides no benefit — both fetches block each other on their respective mutex.


Code Quality

5. YearInReviewViewModelCalendar.getInstance() inconsistency

YearInReviewViewModel.kt:77

val currentYear = Calendar.getInstance().get(Calendar.YEAR).toString()

All other date/time handling in this PR uses java.time. This should use java.time.Year.now().value.toString() for consistency and to remove the Calendar import.

Fix this →


6. BaseTagsAndCategoriesViewModel — empty success result renders an empty card

BaseTagsAndCategoriesViewModel.kt:109-118

When the API returns a successful empty list, the state transitions to Loaded(items = emptyList(), maxViewsForBar = 1L). This will render a card with no rows, giving users no feedback. Adding a NoData check — consistent with how MostPopularDayViewModel handles a blank viewsBestDay — would be more user-friendly:

if (items.isEmpty()) return TagsAndCategoriesCardUiState.NoData

7. StatsDataSourceImpl.init() — non-atomic check-then-set on @Volatile fields

StatsDataSourceImpl.kt:78-83

override fun init(accessToken: String) {
    if (this.accessToken != accessToken) {
        this.accessToken = accessToken
        wpComApiClient = null
    }
}

The read-check-write sequence on accessToken/wpComApiClient is not atomic. Two concurrent init() calls with the same token could both pass the if check, and one could null out a client that was just created. Since init() is called from the use cases before each fetch, this is a real concurrent path. @Synchronized or a synchronized(this) block should be used here (the existing getOrCreateClient() already uses @Synchronized).


8. NewStatsActivity — period selector shown on the Insights tab

NewStatsActivity.kt:176-225

if (currentTab != StatsTab.SUBSCRIBERS) { /* show period menu */ }

The period selector is shown when on the Insights tab, but none of the Insights cards (Year in Review, All-Time Stats, Most Popular Day/Time, Tags & Categories) are time-range filtered. This may mislead users into thinking changing the period affects Insights data.


9. AddCardItem — missing accessibility content description

AddStatsCardBottomSheet.kt:122-147

The AddCardItem Row is clickable but has no contentDescription. Screen readers will announce the Text content but not communicate that the element is a button or what action it performs. A semantics { contentDescription = "Add $label" } modifier on the Row would help.


Minor Observations

  • InsightsCardsConfigurationRepository.isValidConfiguration (line 288–293): The @Suppress("USELESS_CAST") and the cast to List<Any?> for a null check is a fragile pattern. The comment at line 313-317 indicates this is for JSON migration (the EnumWithFallbackValueTypeAdapterFactory may deserialise unknown enum values as null). This is fine, but worth a brief comment explaining why null can appear in a typed list.

  • MostPopularDayViewModel.parseBestDay (line 82–108): Silently falls back to raw bestDay string on parse failure. Consider logging the parse error at DEBUG level so unusual date formats from the API don't go unnoticed.

  • TagsAndCategoriesDetailViewModel has no refresh() method (unlike TagsAndCategoriesViewModel which has one). The detail screen presumably can't be refreshed, which may be intentional — but worth confirming.


| Branch

adalpari and others added 3 commits March 19, 2026 16:34
- Fix StatsTagsUseCase cancellation propagation (deferred.cancel instead of error)
- Fix InsightsViewModel card-add race by cancelling in-flight fetch on config change
- Release mutex during network calls in StatsSummaryUseCase and StatsInsightsUseCase
- Replace Calendar with java.time.Year in YearInReviewViewModel
- Add NoData state for empty tags response instead of empty Loaded card
- Show period selector only on Traffic tab, not Insights
- Add accessibility onClickLabel to AddCardItem bottom sheet

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The mutex refactor introduced a third return path (guard clause
pattern) which is idiomatic and readable.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud
Copy link

@adalpari adalpari marked this pull request as ready for review March 19, 2026 16:14
@adalpari adalpari requested a review from a team as a code owner March 19, 2026 16:14
@adalpari adalpari requested review from nbradbury and removed request for a team March 19, 2026 16:14
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.

3 participants