CMM-2302: Retire the shadowed Jetpack feature-removal phases - #23215
Draft
nbradbury wants to merge 16 commits into
Draft
CMM-2302: Retire the shadowed Jetpack feature-removal phases#23215nbradbury wants to merge 16 commits into
nbradbury wants to merge 16 commits into
Conversation
… and Three The WordPress app fetches these flags from https://public-api.wordpress.com/wpcom/v2/mobile/feature-flags. In production jp_removal_self_hosted is true, and getCurrentPhase() checks it first, so the resolved phase is always PhaseSelfHostedUsers. Phases One, Two and Three are shadowed and can never be reached, even though their flags are also true. Removed: - PhaseOne/PhaseTwo/PhaseThree and their configs, BuildConfig fields and remote fields (jp_deadline, phase_two_blog_post) - The per-feature overlay path: JetpackFeatureOverlayContentBuilder.build(), JetpackFeatureOverlayContentBuilderParams, and the overlay frequency throttling (JetpackOverlayConnectedFeature, JetpackFeatureRemovalOverlayPhase) - The showJetpackOverlay plumbing in the Reader, Notifications, Stats and Stats-detail view models and fragments - The phase-three deadline countdown branding and JetpackBrandingUiState - The My Site Jetpack badge, which only ever showed in phases One to Three - shouldShowDashboard() and shouldShowQuickStart(), which had no callers - 21 orphaned string keys across 31 locales Collapsed shouldShowJetpackBrandingForPhaseOne/ForPhaseTwo into a single shouldShowJetpackBanner(), and getBrandingTextForScreen(screen) into getBrandingText(), since every branch resolved to the same string. Incidentally fixes two latent bugs by deleting the code they lived in: the swapped NOTIFICATIONS/READER SharedPrefs keys in JetpackFeatureOverlayShownTracker and the `else -> TODO()` in JetpackFeatureOverlayContentBuilder. No behaviour change: every removed branch was unreachable under PhaseSelfHostedUsers.
…phase The static-posters phase replaced Reader, Notifications and Stats with poster screens on the way to full removal. jp_removal_static_posters is true in production, but jp_removal_self_hosted is checked first in getCurrentPhase(), so PhaseStaticPosters is shadowed and shouldShowStaticPage() is always false. Removed: - PhaseStaticPosters, its config, BuildConfig field and manifest entry - ui/main/jetpack/staticposter/ and JetpackStaticPosterActivity - shouldShowStaticPage() and its 16 call sites, including the bottom navigation's poster/real fragment swapping in WPMainNavigationView - ActivityLauncher.showJetpackStaticPoster(), SiteNavigationAction.ShowJetpackRemovalStaticPostersView, DeepLinkNavigator.NavigateAction.OpenJetpackStaticPosterView and JetpackPoweredScreen.WithStaticPoster - The debug fragment-preview mechanism, whose only registered entry was the static poster: PreviewFragmentActivity, the PREVIEWS registry, UiItem.preview and the preview icon in DebugSettingsItemViewHolder - FeatureCollectionPhase.Final and two else branches that -Werror flagged as redundant once the remaining whens became exhaustive - 8 orphaned string keys across 29 locales Also removes two string keys missed by the previous commit, where a shell append concatenated them into a single malformed entry. No behaviour change: PhaseStaticPosters was already unreachable.
…hemes shouldShowJetpackBanner() required shouldShowJetpackBranding(), which requires *not* being in a removal phase, AND shouldShowBranding(), which requires being in PhaseFour. Those cannot both hold, so the banner never showed on the five screens that used it. This predates the branch: on trunk the same screens used shouldShowJetpackBrandingForPhaseOne/ForPhaseTwo, and both underlying methods returned false for every phase that satisfied shouldShowJetpackBranding(). Note the banner is currently hidden on every other screen too, since production resolves to PhaseSelfHostedUsers and shouldShowJetpackBranding() therefore returns false. jetpack_banner.xml and its remaining call sites are left in place because they are still reachable if the removal phase is ever turned off. Removed: - JetpackBrandingUtils.shouldShowJetpackBanner() and JetpackFeatureRemovalBrandingUtil.shouldShowBranding() - The dead banner block and layout include on ScanActivity, ScanHistoryFragment, PeopleListFragment, ThemeBrowserActivity, and the equivalent badge block on PersonDetailFragment - ThemeBrowserActivity's two private banner animation helpers and the injected JetpackBrandingUtils/UiHelpers fields left unused on those screens - ScrollableViewInitializedListener from ScanActivity and ThemeBrowserActivity, where the banner was the only implementation and both were left empty; the dispatching fragments already check the type before calling - The unused JetpackPoweredScreen.WithStaticText constants HOME, PEOPLE, PERSON
Collaborator
Generated by 🚫 Danger |
Contributor
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/jetpackRelease/base_manifest.txt 2026-08-14 14:37:12.543518612 +0000
+++ ./build/reports/diff_manifest/WordPress/jetpackRelease/head_manifest.txt 2026-08-14 14:37:16.233938749 +0000
@@ -1339,11 +1339,6 @@
android:label="@string/blaze_campaigns_page_title"
android:theme="@style/WordPress.NoActionBar" />
<activity
- android:name="org.wordpress.android.ui.jetpackoverlay.JetpackStaticPosterActivity"
- android:exported="false"
- android:label="@string/stats"
- android:theme="@style/WordPress.NoActionBar" />
- <activity
android:name="org.wordpress.android.ui.mysite.personalization.PersonalizationActivity"
android:exported="false"
android:label="@string/stats"Go to https://buildkite.com/automattic/wordpress-android/builds/28122/canvas?sid=01a000b2-5128-4374-8dfa-81e9e9a814c1, click on the |
Contributor
Project manifest changes for WordPressThe following changes in the --- ./build/reports/diff_manifest/WordPress/wordpressRelease/base_manifest.txt 2026-08-14 14:37:14.981007767 +0000
+++ ./build/reports/diff_manifest/WordPress/wordpressRelease/head_manifest.txt 2026-08-14 14:37:18.680224551 +0000
@@ -1314,11 +1314,6 @@
android:label="@string/blaze_campaigns_page_title"
android:theme="@style/WordPress.NoActionBar" />
<activity
- android:name="org.wordpress.android.ui.jetpackoverlay.JetpackStaticPosterActivity"
- android:exported="false"
- android:label="@string/stats"
- android:theme="@style/WordPress.NoActionBar" />
- <activity
android:name="org.wordpress.android.ui.mysite.personalization.PersonalizationActivity"
android:exported="false"
android:label="@string/stats"Go to https://buildkite.com/automattic/wordpress-android/builds/28122/canvas?sid=01a000b2-5128-4fda-a0c3-2bbe919a61ba, click on the |
Contributor
|
|
Contributor
|
|
Contributor
🤖 Build Failure AnalysisThis build has failures. Claude has analyzed them - check the build annotations for details. |
- Add JetpackFeatureRemovalOverlayUtilTest covering the surviving phase-four
feature-collection overlay logic: the -1 show-once sentinel, never-shown,
frequency elapsed / not elapsed, and the missing-timestamp path. The previous
commits deleted the tests for that class because every case targeted the
feature-specific overlay, leaving the frequency rules uncovered.
- Delete AnalyticsTracker.Stat.JETPACK_REMOVE_FEATURE_OVERLAY_LINK_TAPPED, whose
only caller was removed, and its stale mapping in AnalyticsTrackerNosaraTest.
It duplicated the event name of ..._LEARN_MORE_TAPPED, which remains in use.
- Unwrap a leftover no-op `run { }` in ReaderPostPagerActivity.handleDeepLinking.
- Collapse five double blank lines left behind where properties were removed.
- Restore the short-name phase imports in getCurrentPhase() instead of fully
qualifying them, which had pushed one line to the 120-character limit.
Cleanup debris left by the earlier removals. None of it could surface in a compile: unused imports and unused constructor properties are warnings to kotlinc and javac, not errors. - Delete 22 unused imports across 15 files. - Drop JetpackFeatureRemovalPhaseHelper from the constructors of TodaysStatsViewModelSlice, ListItemActionHandler and StatsLinkHandler, which no longer use it, along with the matching mocks and constructor arguments in their tests. - Drop the isFirstStart parameter from StatsDetailFragment. It was only read by the overlay observer that was removed, so it is now unused through setupObservers, initializeViewModels and the call site. - Remove three blank lines before closing braces. - Delete res/drawable/ic_external_v2.xml, whose only reference was the deleted static poster screen. detekt, checkstyle and lintWordPressRelease all pass. The one remaining lint warning (InsecurePermissionProtectionLevel in src/wordpress/AndroidManifest.xml) predates this branch and does not fail the build.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #23215 +/- ##
==========================================
- Coverage 37.93% 37.86% -0.07%
==========================================
Files 2347 2335 -12
Lines 127837 127056 -781
Branches 17781 17588 -193
==========================================
- Hits 48495 48110 -385
+ Misses 75374 75014 -360
+ Partials 3968 3932 -36 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The WordPress app does not ship Reader, Stats or Notifications; the Jetpack app does. That was decided at runtime from the jp_removal_* remote flags. This makes it a build-flavor check instead: shouldRemoveJetpackFeatures() is now simply !isJetpackApp. Unlike the preceding commits, this is a deliberate behaviour change. Why: - The flag was a server-side constant. The WP.com /mobile/feature-flags endpoint returns jp_removal_self_hosted=true for every build number and both app identifiers, with no rollout or cohort logic. - The in-app default was "not removed", so anything that emptied the flag store un-removed the features. Signing out calls AppInitializer's removeWpComUserRelatedData -> appConfig.clear(), which wipes the flags in memory and on disk; until the next successful fetch the WordPress app showed the full bottom nav with working Reader and Notifications. A failed fetch left it that way indefinitely. A fresh install has the same problem. There is now no flag to wipe. Removed: - JetpackFeatureRemovalPhase and the PhaseFour/NewUsers/SelfHostedUsers configs, their BuildConfig fields, PhaseFourBlogPostLinkConfig and PhaseFourOverlayFrequencyConfig - Per-phase branching in JetpackFeatureOverlayContentBuilder, JetpackFeatureCardHelper and JetpackFeatureRemovalOverlayUtil, including the phase-four overlay frequency rules; the collection overlay is now shown once - The switch-to-Jetpack menu card, gated on PhaseFour only and therefore never shown in production: view holder, view model slice, layout, menu, MySiteCardAndItem.Card.JetpackSwitchMenu and its preferences - jp_all_features_* Lottie files and 6 orphaned string keys across 30 locales Kept deliberately: - The literal "self_hosted", now JETPACK_REMOVAL_TRACKING_NAME. Three SharedPrefs keys embed the phase name, so dropping it would re-show the feature card to users who dismissed it and re-show the overlay to users who saw it. It also pins the Tracks `phase` property so events stay comparable. - The "switch to Jetpack" overlay and the My Site feature card, which are the live migration prompts. Trade-off: rolling the removal back now needs an app release rather than a flag flip. Tests rewritten around the flavor invariant. detekt, checkstyle and lintWordPressRelease all pass.
Follow-up cleanup on code this branch already touched. No behaviour change; the
overlay renders identically, verified by an A/B build of the same screen.
- Remove the "Learn more at jetpack.com" link from the switch-to-Jetpack overlay.
Its URL came from phase_four_blog_post, supplied only in PhaseFour, and its
visibility flag was false for the phase that actually shipped, so it has never
been reachable. This takes out the MaterialButton from both overlay layouts,
the migrationInfoText/migrationInfoUrl plumbing in JetpackFeatureOverlayContent
and JetpackFeatureOverlayComponentVisibility, the fragment's click listener and
OpenMigrationInfoLink handler, the view model method, the tracking call and the
orphaned JETPACK_REMOVE_FEATURE_OVERLAY_LEARN_MORE_TAPPED stat.
- Fold JetpackFeatureRemovalBrandingUtil into JetpackBrandingUtils. It was down
to one delegating method and one returning a constant, with a single consumer.
- Collapse JetpackFeatureOverlayContentBuilder's build -> getState -> getContent
indirection, which only existed to organise the per-phase branches.
- Flatten `sealed class DeepLinkPhase { class All }` to a single class.
- Make the overlay-shown preference key private; it had no external callers once
the phase-suffix builder was gone.
barrier2 in both overlay layouts now references only new_users_content_view_parent.
Both referenced views are GONE at runtime and that one already resolved lower, so
positions are unchanged: migration_helper_text, primary_button and secondary_button
report identical bounds before and after.
The new-users content block is deliberately left in place. It is unreachable for
the same reason, but it is designed, laid out for both orientations and
translated, so it is worth keeping if the migration prompt is ever revisited.
detekt, checkstyle and lintWordPressRelease all pass.
- Add JetpackFeatureOverlayShownTrackerTest. JETPACK_REMOVAL_TRACKING_NAME is embedded in the stored preference keys, so changing it would silently re-show the switch-to-Jetpack overlay and the feature card to every user who has already dismissed them. It reads like a vestigial phase name and nothing guarded it; the test pins both the constant and the key the tracker writes. - Remove the DeletablePrefKey constants SWITCH_TO_JETPACK_MENU_CARD_SHOWN_TIMESTAMP and SHOULD_HIDE_SWITCH_TO_JETPACK_MENU_CARD, orphaned when the switch-to-Jetpack menu card was deleted. Their only remaining reference was their own declaration. AppPrefs.reset() iterates this enum on logout, so an existing install keeps one stale timestamp entry that will no longer be cleaned; that is preferable to keeping dead constants alive purely for cleanup. - Delete wp_jetpack_feature_removal_overlay_learn_more_migration_text across all 29 locales. Its last usage went with the "Learn more at jetpack.com" link and it was the only orphan left behind by that change. detekt, checkstyle and lintWordPressRelease all pass.
…e-dead-jp-removal-phase
…e-dead-jp-removal-phase
…e-dead-jp-removal-phase
CMM-2301 (#23213) documented the Me row's gate in terms of JetpackFeatureRemovalPhaseHelper.getCurrentPhase, which this branch had already deleted. The KDoc link no longer resolved and the reasoning described a method that no longer exists. The conclusion still holds, just for a different reason: shouldRemoveJetpackFeatures() is now !isJetpackApp, so it is false in the Jetpack app and the item never builds there. Reworded to say that. Found by grepping comments for every symbol this branch deletes; this was the only stale reference. Neither detekt nor checkstyle validates KDoc links, so nothing else would have caught it.
Both are naming cleanups the earlier commits left behind. No behaviour change. - Rename JetpackFeatureRemovalPhaseHelper to JetpackFeatureRemovalHelper, and the injected properties to match. The class has no phases left: getCurrentPhase and JetpackFeatureRemovalPhase are gone and six of its seven methods are one-line build-flavor checks. The name outliving the concept already produced one stale KDoc, fixed in the previous commit. - Replace the single-constant enum JetpackFeatureRemovalSiteCreationPhase with JETPACK_DEEPLINK_TRACKING_NAME, alongside the existing JETPACK_REMOVAL_TRACKING_NAME. The Tracks `phase` property was being sourced two different ways for the same field. getDeepLinkPhase() becomes getDeepLinkTrackingName() and still returns null in the Jetpack app, so deep-link events keep emitting phase="two" in the WordPress app and phase=null in Jetpack exactly as before. The deep-link overlay is gated on whether the Jetpack app package is installed rather than on build flavor, so the null branch is reachable and was preserved deliberately. Import order is untouched: checkstyle's ImportOrder sets ordered="false", so alphabetical placement is not enforced and reordering 22 files would have been churn. detekt, checkstyle and the affected unit tests all pass.
…e-dead-jp-removal-phase
The comment claimed the value differs from JETPACK_REMOVAL_TRACKING_NAME "because deep links were rolled out on their own schedule". That reason was invented; nothing supports it. What "one" and "two" actually were, per getDeepLinkPhase() on trunk, is a coarser two-bucket view of the same rollout: "one" while the Jetpack-powered features were still present (PhaseOne/Two/Three/StaticPosters) and "two" once they were removed (PhaseFour/NewUsers/SelfHostedUsers). Production sits in the second bucket, hence "two". The value and the behaviour were already correct; only the explanation was wrong. The now-deleted enum was called JetpackFeatureRemovalSiteCreationPhase despite having nothing to do with site creation, which likely encouraged the wrong reading in the first place.
…e-dead-jp-removal-phase
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes CMM-2302.
The
JetpackFeatureRemovalPhasesystem was built in 2022–2023 to stage the removal of Reader, Stats and Notifications from the WordPress app across seven phases. The last change to the logic was August 2023.Those removal phases have been completed for quite some time, leaving us with a lot of dead code. This PR removes it.
Rollback note
The use of the
jp_removal_self_hostedflag was also removed here. This was the remote flag that controlled whether the WordPress app contains Jetpack features, so this change means we can no longer remotely control whether Jetpack features can appear in the WordPress app.My assumption is this should be fine - this flag has disabled Jetpack features for three years, and I can't imagine we'd want to roll that back.
Testing instructions