CMM-1289: Add Subscribers tab cards for new stats screen#22634
CMM-1289: Add Subscribers tab cards for new stats screen#22634
Conversation
Introduces the shared infrastructure for the Subscribers tab including the wordpress-rs dependency update, data layer (3 new API endpoints), repository methods, card configuration with persistence, and the All-time Subscribers card showing current, 30-day, 60-day, and 90-day subscriber counts. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Adds an empty placeholder card for the Subscribers Graph that will be populated with chart data in a future iteration. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Displays a list of subscribers with their subscription dates. Includes a Show All CTA that opens a detail screen with the full subscriber list. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Adds the Emails card showing latest emails with opens and clicks counts, including a detail screen. Wires all 4 cards into the Subscribers tab with the tab ViewModel, content composable, add-card bottom sheet, and manifest registrations. Co-Authored-By: Claude Opus 4.6 <[email protected]>
adalpari
changed the title
Generated by 🚫 Danger |
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/base_manifest.txt 2026-03-05 15:05:59.513513060 +0000
+++ ./build/reports/diff_manifest/WordPress/wordpressVanillaRelease/head_manifest.txt 2026-03-05 15:06:08.103623463 +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.subscribers.subscriberslist.SubscribersListDetailActivity"
+ android:exported="false"
+ android:theme="@style/WordPress.NoActionBar" />
+ <activity
+ android:name="org.wordpress.android.ui.newstats.subscribers.emails.EmailsDetailActivity"
+ 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/25318/canvas?sid=019cbe83-e30f-49c1-9026-4dc204d013f6, click on the |
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/base_manifest.txt 2026-03-05 15:06:36.759565352 +0000
+++ ./build/reports/diff_manifest/WordPress/jetpackVanillaRelease/head_manifest.txt 2026-03-05 15:06:47.649638969 +0000
@@ -397,6 +397,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.subscribers.subscriberslist.SubscribersListDetailActivity"
+ android:exported="false"
+ android:theme="@style/WordPress.NoActionBar" />
+ <activity
+ android:name="org.wordpress.android.ui.newstats.subscribers.emails.EmailsDetailActivity"
+ 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/25318/canvas?sid=019cbe83-e310-4cdb-ba7a-1cffbe78c2b7, click on the |
Project dependencies changeslist! Upgraded Dependencies
rs.wordpress.api:android:1200-3a23a7389b3931e6241986a168b73a95d99151e0, (changed from trunk-a557060af03d5ed2eb41a21afd51f434fe864e20)
rs.wordpress.api:kotlin:1200-3a23a7389b3931e6241986a168b73a95d99151e0, (changed from trunk-a557060af03d5ed2eb41a21afd51f434fe864e20)tree +--- project :libs:fluxc
-| \--- rs.wordpress.api:android:trunk-a557060af03d5ed2eb41a21afd51f434fe864e20
-| +--- 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-a557060af03d5ed2eb41a21afd51f434fe864e20
-| | +--- 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:1200-3a23a7389b3931e6241986a168b73a95d99151e0
+| +--- 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:1200-3a23a7389b3931e6241986a168b73a95d99151e0
+| | +--- 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-a557060af03d5ed2eb41a21afd51f434fe864e20 (*)
+\--- rs.wordpress.api:android:1200-3a23a7389b3931e6241986a168b73a95d99151e0 (*) |
Adds 5 test files covering the new Subscribers tab functionality: - AllTimeSubscribersViewModelTest (12 tests) - SubscribersListViewModelTest (13 tests) - EmailsCardViewModelTest (13 tests) - StatsRepositorySubscribersTest (12 tests) - SubscribersCardsConfigurationRepositoryTest (14 tests) Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
|
|
|
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
- SubscribersTabViewModelTest (21 tests): config loading, card management (add/remove/move), flow observation, network status - SubscribersGraphViewModelTest (6 tests): placeholder state behavior Co-Authored-By: Claude Opus 4.6 <[email protected]>
…ats-subscribers-tab
- Redesign card with highlighted Current sub-card and 3 smaller sub-cards for 30/60/90 days ago - Fix subscribers API: use DAY unit instead of MONTH - Pass correct date parameter (today, -30d, -60d, -90d) for each call - Hide period selector when Subscribers tab is selected - Update tests for new date parameter Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Center-align opens/clicks columns in headers and rows - Add divider between column headers and items - Show "-" instead of "0" for zero opens/clicks values - Use lighter color for zero values to de-emphasize - Add proper spacing between title and numeric columns - Remove "Top N" truncation label from detail screen Co-Authored-By: Claude Opus 4.6 <[email protected]>
Replace the placeholder graph card with a Vico line chart showing subscriber counts over time. Add segmented button tabs for switching between Days (30d), Weeks (12w), Months (6m), and Years (3y) periods. Data is sorted chronologically (older to newer). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Add 10 new ViewModel tests covering tab switching params, chronological sorting, empty data, auth errors, and edge cases. Add 4 repository tests for fetchSubscribersGraph covering success, empty, error, and auth error. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Update wordpress-rs to b17d6e02dde5ea55773d527c1cb6ad2f889fc90e, handle nullable dateSubscribed, format subscriber dates as relative time (e.g. "30 days", "1 year, 45 days") instead of raw date strings. Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Extract BaseSubscribersCardViewModel to eliminate ViewModel duplication - Replace hardcoded error strings with string resources - Use Android plural resources for formatSubscriberDate localization - Fix refresh() to call statsRepository.init() before fetching - Use AtomicBoolean for thread-safe isLoading/isLoadedSuccessfully flags - Lazy load detail items (fetch 5 for card, 100 on demand) - Prevent cardsToLoad from re-triggering on every config change - Extract shared formatEmailStat to StatsFormatter utility - Add FormatSubscriberDateTest with 12 tests covering all branches - Update existing ViewModel tests for base class refactor Co-Authored-By: Claude Opus 4.6 <[email protected]>
Detail pages now fetch data via their own ViewModels with infinite scroll instead of receiving all items through Intent extras. Subscribers uses page-based pagination; Emails uses increasing quantity. Both use mutex- guarded loading with scroll-to-end detection. Co-Authored-By: Claude Opus 4.6 <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
...in/java/org/wordpress/android/ui/newstats/subscribers/subscriberslist/SubscribersListCard.kt
Fixed
Show fixed
Hide fixed
...g/wordpress/android/ui/newstats/subscribers/subscriberslist/SubscribersListDetailActivity.kt
Fixed
Show fixed
Hide fixed
- Bug #1: Add error state with retry to detail Activities (blank screen on error) - Bug #2: Cancel previous load job on tab switch to prevent race conditions - Bug #3: Reset isLoadedSuccessfully in refresh() to allow data reload - Bug #4: Add duplicate guard in addCard() to prevent duplicate cards - Bug #9: Use resource string instead of raw e.message for error display - Thread safety #7: Use AtomicBoolean for isInitialLoad in SubscribersTabViewModel - Performance #11: Pre-compute formatted dates in ViewModel, memoize in card composable - Update all tests to match new error handling and API signatures Co-Authored-By: Claude Opus 4.6 <[email protected]>
...in/java/org/wordpress/android/ui/newstats/subscribers/subscriberslist/SubscribersListCard.kt
Fixed
Show fixed
Hide fixed
...g/wordpress/android/ui/newstats/subscribers/subscriberslist/SubscribersListDetailActivity.kt
Fixed
Show fixed
Hide fixed
- Replace Resources with ContextProvider in SubscribersListDetailViewModel to avoid passing Activity resources into ViewModel (MVVM violation) - Fix refresh() race condition by canceling loadJob in BaseSubscribersCardViewModel - Add Mutex synchronization to SubscribersCardsConfigurationRepository - Fix isValidConfiguration no-op (filterIsInstance on typed list) - Expose PAGE_SIZE constants for test use instead of duplicating - Replace SimpleDateFormat with DateTimeFormatter in StatsDataSourceImpl - Log exceptions in fetchSubscribersGraph instead of swallowing silently - Use Period.between() for leap-year-accurate date formatting - Remove duplicate selectedTab from SubscribersGraphUiState.Loaded - Auto-load newly added cards in SubscribersTabContent - Add statsRepository.init() verification tests to all card ViewModels Co-Authored-By: Claude Opus 4.6 <[email protected]>
…suppressions Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Claude finished @adalpari's task in 3m 13s —— View job Code ReviewThe PR implements a well-structured Subscribers tab with 4 configurable cards, closely following the existing Traffic tab pattern. The architecture is clean, the test coverage is solid, and card management (add/remove/reorder/persist) is correctly implemented. Here are the issues I found, from most to least critical: 🔴 Critical Issues1. Race condition in private fun getOrCreateClient(): WpComApiClient {
val token = accessToken
check(token != null) { "DataSource not initialized" }
return wpComApiClient
?: wpComApiClientProvider.getWpComApiClient(token)
.also { wpComApiClient = it }
}The fields are @Synchronized
private fun getOrCreateClient(): WpComApiClient { ... }2. Security alert:
val resources = LocalContext.current.resources
val formattedDate = remember(item.subscribedSince) {
formatSubscriberDate(item.subscribedSince, resources)
}This is flagged by GitHub Advanced Security. 🟡 Medium Issues3. Missing unit tests for Danger bot flagged this explicitly. The base class contains significant shared logic — A concrete gap: there's no test verifying that calling 4. Emails pagination re-fetches all data on each "load more"
fun loadMore() {
currentQuantity += EMAILS_DETAIL_PAGE_SIZE
fetchEmails(currentQuantity, isInitial = false) // fetches ALL items from page 1
}Unlike 5.
All write operations ( 🟢 Observations / Minor6.
val resources = contextProvider.getContext().resourcesUsing 7.
val loadVisibleCards = {
visibleCards.dispatchToVisibleCards(...)
}This lambda captures 8. val currentSiteId = siteId // called inside flow collectThe SummaryThe implementation is solid and follows established patterns well. The three items that need attention before merging are: (1) the |
- Add @synchronized to StatsDataSourceImpl.getOrCreateClient() to prevent race condition during parallel API calls - Move date formatting from SubscribersListCard composable to SubscribersListViewModel to avoid stale resources capture - Make saveConfiguration private in ConfigurationRepository - Add comment documenting emails pagination API limitation - Add BaseSubscribersCardViewModelTest with 12 tests Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Rethrow CancellationException in all catch blocks to preserve structured concurrency (BaseSubscribersCardViewModel, detail VMs, StatsRepository) - Use safe cast in extractSubscriberCount to prevent runtime crash - Add try-catch to fetchSubscribersList and fetchEmailsSummary - Add mutex to getConfiguration() to fix TOCTOU race - Fix refresh() bypassing isLoading guard - Remove emails pagination (API limitation) and use static list - Fix premature loadMore trigger by initializing canLoadMore to false - Add !isLoading guard to shouldLoadMore derived state Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
@adalpari This is looking good and tested well. I did ask Claude to look for simplifications and I've attached its output. |
- Extract NoConnectionContent into shared composable used by both NewStatsActivity and SubscribersTabContent - Consolidate 4 card movement methods in SubscribersCardsConfigurationRepository via moveCard() helper - Consolidate 6 card action methods in SubscribersTabViewModel via cardAction() helper - Extract CardActions data class in SubscribersTabContent to reduce callback repetition Co-Authored-By: Claude Opus 4.6 <[email protected]>
nbradbury
left a comment
nbradbury
left a comment
There was a problem hiding this comment.
This looks good to me! Feel free to merge once you can switch to a trunk version of wp-rs 
…ats-subscribers-tab # Conflicts: # gradle/libs.versions.toml
- Update wordpress-rs library to commit 3a23a7389b3931e6241986a168b73a95d99151e0 - Adapt StatsEmailsSummarySortOrder -> WpApiParamOrder rename Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
4 participants
Description
Populates the Subscribers tab in the new stats screen with 4 configurable cards. Each card supports move, remove, and add operations with per-site persistence, matching the Traffic tab pattern. Uses 3 new wordpress-rs API endpoints (stats subscribers, subscribers by user type, stats emails summary).
Cards:
Testing instructions
Subscribers tab cards:
Card management:
Show All detail screens:
Error and empty states:
Screen_recording_20260303_124916.mp4