Skip to content

Conversation

@RafaelKayumov
Copy link
Contributor

@RafaelKayumov RafaelKayumov commented Aug 29, 2025

Part of WOOMOB-1189

Description

Implements an experimental feature toggle for "Application Passwords Experiment" feature

  • Adds isApplicationPasswordsSwitchEnabled property to GeneralAppSettings
  • Implements AppSettings actions to access the corresponding setting
  • Adds BetaFeature.applicationPasswords to be displayed in experimental features as a toggle.
  • Adds the settings_beta_features_application_passwords_toggled analytics event triggered by the feature toggle.
  • Adds ApplicationPasswordsExperimentAvailabilityChecker draft as an async wrapper around upcoming remote FF.
  • Stores presumable remote feature flag value in local cache for initial presentation.
  • Evaluates both feature state and availability to determine if it should be applied. ApplicationPasswordsExperimentState handles that.
  • Observes the beta feature enabled/disabled state and toggles the Application Passwords feature and updates RequestAuthenticator in runtime.
  • Adds a link to Application Passwords doc in beta feature section footer description.

Testing steps

  • Run the app on a site that supports application passwords.
  • Navigate to the app settings -> Experimental Features.
  • Make sure the "Application Passwords" entry is displayed. Make sure the corresponding description contains an embedded documentation link. Make sure the link is opened in Safari sheet.
  • Toggle the feature setting and restart the app. Make sure the state is persisted between app sessions.
  • Use Proxyman to check the behaviour:
    • With the toggle isON make sure confirm that supported Jetpack requests are made as direct REST requests with the remote sites.
    • In Experimental Features screen toggle the Application Passwords feature off.
    • (After a possible delay) Make sure JP requests are performed as usual.
    • Toggle the feature back ON.
    • (After a possible delay) Make sure requests are performed as direct REST requests.
    • Manually imitate the remote FF disabled state mocking the feature unavailability - in ApplicationPasswordsExperimentAvailabilityChecker set the mockResultValue to false. Make sure the setting entry disappears in "Experimental Features". Make sure JP requests are made as usual despite the prior toggle state.

Screenshots

Снимок экрана 2025-09-01 в 13 32 27
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@RafaelKayumov RafaelKayumov added this to the 23.2 milestone Aug 29, 2025
@RafaelKayumov RafaelKayumov added the type: task An internally driven task. label Aug 29, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Aug 29, 2025

2 Warnings
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️ This PR is assigned to the milestone 23.2. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.
1 Message
📖

This PR contains changes to Tracks-related logic. Please ensure (author and reviewer) the following are completed:

  • The tracks events must be validated in the Tracks system.
  • Verify the internal Tracks spreadsheet has also been updated.
  • Please consider registering any new events.
  • The PR must be assigned the category: tracks label.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Aug 29, 2025

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

App NameWooCommerce iOS Prototype
Build Numberpr16059-021e0d4
Version23.1
Bundle IDcom.automattic.alpha.woocommerce
Commit021e0d4
Installation URL4vuf6g4k0bmeo
Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@RafaelKayumov RafaelKayumov added the category: tracks Related to analytics, including Tracks Events. label Sep 1, 2025
@RafaelKayumov RafaelKayumov force-pushed the WOOMOB-1189-add-application-passwords-feature-toggle branch from cdb0d88 to 9194cd4 Compare September 1, 2025 12:20
@RafaelKayumov RafaelKayumov marked this pull request as ready for review September 1, 2025 13:08
@itsmeichigo itsmeichigo self-assigned this Sep 3, 2025
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

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

This works great 👍 I left some non-blocking comments and questions.

23.2
-----

- [*] Adds a toggle for Application Passwords feature in Experimental Features settings screen [https://github.com/woocommerce/woocommerce-ios/pull/16059]
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: since the feature is not enabled yet, this will not be available to users in version 23.2. Let's remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted in 9026d48

Section(
footer: VStack(
alignment: .leading,
spacing: Layout.spacing
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nit: is this spacing redundant? There seems to be only one item in the VStack in both cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Previously I attempted a solution with a separate Link element. The redundant spacing remained. I'm gonna remove VStack at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 4c6321a

Comment on lines +144 to +146
DispatchQueue.main.async {
self.checkApplicationPasswordExperimentFeatureState()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Should this be simplified:

Suggested change
DispatchQueue.main.async {
self.checkApplicationPasswordExperimentFeatureState()
}
Task { @MainActor in
await checkApplicationPasswordExperimentFeatureState()
}

Then make checkApplicationPasswordExperimentFeatureState async for better testability if we need to test it.

.betaFeatureEnabledPublisher(
.applicationPasswords
)
.dropFirst()
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ Why is it necessary to drop the initial value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added dropFirst because eventually the generalAppSettings.betaFeatureEnabledPublisher automatically emits the current toggle state value upon subscription. Without the dropFirst it would also trigger the checkApplicationPasswordExperimentFeatureState() here. But the initial checkApplicationPasswordExperimentFeatureState call happens in AuthenticatedState.init.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Open to improvement suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think dropFirst should be used with caution to avoid missing data. Since removeDuplicates is already in place, it should be sufficient for avoiding triggering checkApplicationPasswordExperimentFeatureState when unnecessary.

case hiddenStoreIDs

// Application passwords experiment remote FF cached value
case applicationPasswordsExperimentRemoteFFValue
Copy link
Contributor

Choose a reason for hiding this comment

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

Another suggestion: should we clear this flag in SessionManager.reset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in a085407

@RafaelKayumov RafaelKayumov merged commit fa8489e into trunk Sep 3, 2025
15 checks passed
@RafaelKayumov RafaelKayumov deleted the WOOMOB-1189-add-application-passwords-feature-toggle branch September 3, 2025 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: tracks Related to analytics, including Tracks Events. type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants