Repository navigation
Replace the Jetpack features removal remote flags with a fixed rule - #26280
Merged
Merged
Conversation
Collaborator
Generated by 🚫 Danger |
crazytonyli
approved these changes
Oct 29, 2026
crazytonyli
self-requested a review
October 29, 2026 20:43
Contributor
|
@jkmassel There are conflicts in this PR. |
Formatting only, so the follow-up change has a readable diff.
The server returns the same values for these eight flags to every app version, so the app now hard-codes the phase they produce: - WordPress app with a WordPress.com account: static screens - WordPress app without one: simplified UI - Any other app: normal Site creation follows the same pattern, and the duplicate-notifications check is always on, matching its server value and app default.
The phase no longer falls back to normal when no flag has been fetched, so tests that never set up the root view would see whichever phase their account state implies. Pin the test host to the normal UI type, and restore the value RootViewCoordinatorTests changes.
jkmassel
force-pushed
the
jkmassel/remove-jetpack-removal-flags
branch
from
October 29, 2026 20:55
1bd97ca to
9dd247c
Compare
jkmassel
enabled stacked auto-merge
October 29, 2026 21:27
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
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.
Replaces the eight Jetpack features removal remote flags with a fixed rule that gives the result the server already sends. Users see no change, apart from the timing fix described under Behaviour change.
Why
The server sends the same values to every app and version we checked (
org.wordpressandcom.automattic.jetpack, marketing versions 25.0, 26.5, 27.3 and 27.5, checked 2026-10-28):jp_removal_one,jp_removal_two,jp_removal_three,jp_removal_static_posterstruejp_removal_four,jp_removal_new_usersfalsejp_removal_self_hostedtrueanonymous,falsewith a WordPress.com tokenprevent_duplicate_notifs_remote_fieldtrueThe app only uses
jp_removal_self_hostedwhen there is no WordPress.com account, and without an account it fetches the flags anonymously. So it effectively readstrue.Changes
JetpackFeaturesRemovalCoordinator.generalPhase(): WordPress app with a WordPress.com account →.staticScreens; WordPress app without one →.selfHosted; any other app →.normal. Same result as the old flag chain with the values above.siteCreationPhase(): WordPress app →.twoif the user has a site, otherwise.normal. Other apps →.normal.JetpackNotificationMigrationService: the duplicate-notifications check is always on. The flag's server value and its app default were alreadytrue.RemoteFeatureFlag: the eight cases are gone. The server keys stay, because older app versions still read them.RootViewCoordinator,JetpackBrandingTextProviderandJetpackBrandingMenuCardPresenterno longer take aRemoteFeatureFlagStore. The last two take aphaseProviderclosure instead, so their existing per-phase tests still run until Remove the Jetpack features removal phases no one can reach #26281 deletes those phases.JetpackFeaturesRemovalCoordinatorTestsis rewritten in Swift Testing. Both phase functions take the account and site checks as@autoclosuredefaults, so the suite runs without Core Data or other shared state and is safe to run in parallel.TestingAppDelegatesetscurrentAppUIType = .normalat launch. On trunk, an unfetched flag meant the normal phase, so tests that checkjetpackFeaturesEnabled()saw every feature on. With the fixed rule a test process with no WordPress.com account would be.selfHosted;DomainCreditEligibilityTestsfailed without this.RootViewCoordinatorTestsnow restores the value it changes.The phases no one can reach (
.oneto.four,.newUsers) are left in place to keep this diff reviewable. #26281 deletes them.The first commit only runs
swift-formaton the touched files. Review the second commit for the actual change.Rebased onto the 27.4 Reader work
The 27.4 merge-back (#26297) added "Reader in WordPress app" (
readerInWordPressApp), which builds on the same phase logic. The rebase keeps all of it. The fixed rule simply feeds into it:RootViewCoordinator.appUIType(for:isReaderInWordPressAppEnabled:)maps.staticScreensto.staticScreensWithReaderwhen the flag is on. The account-change observer and reload tracking are unchanged.testReloadUIWhenSwitchingFromStaticScreensWithReadersigns out with a never-fetched flag store, which trunk resolves to.normal. With the fixed rule it is.simplified. This is case 2 under Behaviour change, which on trunk can now also happen through the account-change observer, not only on relaunch.RootViewCoordinatorTestsdrop their flag-store helpers. The normal-UI tab bar test now uses the Jetpack app, since the WordPress app can't reach the normal UI.staticScreensWithReaderstate tests are kept, in a small XCTest class next to the Swift Testing suite, because they set a shared static.Behaviour change
On trunk, the self-hosted phase depends on a cached flag whose app default is
false. The UI type is chosen when the app launches, so a user with no WordPress.com account gets the normal UI (Reader, Notifications and Me tabs) until the flag is cached and the app is relaunched:false, so trunk keeps the normal UI until a later anonymous fetch and relaunch. This one comes from reading the code; it was not reproduced on a simulator.With the fixed rule these users get the simplified UI immediately.
The trunk screenshot uses the normal UI's tab bar. This PR's screenshot is the simplified site menu, with no tab bar.
Test plan
WordPressUnitTestsplan on an iPhone 17 Pro (iOS 27.0) simulator — 4,199 passed, 0 failed, 5 skippedWordPressUnitTestsplan on an iPhone 18 Pro (iOS 27.0) simulator — 4,354 passed, 0 failed, 5 skippedswift-formatand SwiftLint clean on every touched fileTrunk vs this PR: no-sites site creation and the Jetpack app (pixel-identical below the status bar)
The no-sites account is the real test account behind a local proxy that answers
me/siteswith an empty list. The Reader screenshots are cropped to hide followed P2 names.