Skip to content

Application password login add analytic events #21886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

adalpari
Copy link
Contributor

@adalpari adalpari commented May 16, 2025

Description

This PR is adding a simple event to know about the successful Application Password login.

Testing instructions

  1. Set the variable ApplicationPasswordViewModelSlice.buildCard to true
    Screenshot 2025-05-16 at 09 23 53

  2. Log in with a self-hosted site

  3. Open MySite screen, wait for the Application Password card to appear

  4. Log in using the Application Password flow

  • Verify the event jp_android_application_password_login is sent (you can see it in logcat)

WordPress-STATS com.jetpack.android.beta I 🔵 Tracked: jp_android_application_password_login, Properties: {"success":"true","url":"https://vanilla.wpmt.co"}

@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 16, 2025

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr21886-376b3ec
Commit376b3ec
Direct Downloadjetpack-prototype-build-pr21886-376b3ec.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 16, 2025

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr21886-376b3ec
Commit376b3ec
Direct Downloadwordpress-prototype-build-pr21886-376b3ec.apk
Note: Google Login is not supported on these builds.

Copy link

codecov bot commented May 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 39.58%. Comparing base (13cb7b6) to head (376b3ec).
Report is 1 commits behind head on feat/CMM-335-Offer-the-Application-Password-login-inside-a-card.

Additional details and impacted files
@@                                       Coverage Diff                                        @@
##           feat/CMM-335-Offer-the-Application-Password-login-inside-a-card   #21886   +/-   ##
================================================================================================
  Coverage                                                            39.58%   39.58%           
================================================================================================
  Files                                                                 2130     2130           
  Lines                                                                99558    99558           
  Branches                                                             15350    15350           
================================================================================================
  Hits                                                                 39406    39406           
  Misses                                                               56669    56669           
  Partials                                                              3483     3483           

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

…e-a-card' into feat/CMM-334-Application-Password-login-add-analytic-events
@adalpari adalpari changed the base branch from trunk to feat/CMM-335-Offer-the-Application-Password-login-inside-a-card May 19, 2025 12:17
@adalpari adalpari requested a review from Copilot May 19, 2025 12:23
@adalpari adalpari marked this pull request as ready for review May 19, 2025 12:23
@adalpari adalpari requested a review from nbradbury May 19, 2025 12:23
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds analytics event tracking for successful Application Password logins, distinguishing between Jetpack and WordPress app flavors.

  • Introduces two new Stat entries for application password login in the analytics tracker
  • Updates ApplicationPasswordLoginHelper to inject BuildConfigWrapper and call a new trackSuccessful method on login success
  • Adjusts the unit test to supply the new BuildConfigWrapper dependency

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
libs/analytics/src/main/java/org/wordpress/android/analytics/AnalyticsTracker.java Added WP_ANDROID_APPLICATION_PASSWORD_LOGIN and JP_ANDROID_APPLICATION_PASSWORD_LOGIN enum entries
WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt Injected BuildConfigWrapper, implemented trackSuccessful to send analytics events
WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt Updated helper constructor in test to include BuildConfigWrapper
Comments suppressed due to low confidence (2)

WordPress/src/test/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelperTest.kt:41

  • Consider adding unit tests to verify that AnalyticsTracker.track is called with the correct Stat and properties when a login succeeds for both Jetpack and WordPress app flavors.
helper = ApplicationPasswordLoginHelper(testDispatcher(), siteSqlUtils, uriLoginWrapper, buildConfigWrapper)

WordPress/src/main/java/org/wordpress/android/ui/accounts/login/ApplicationPasswordLoginHelper.kt:15

  • [nitpick] Consider renaming URL_TAG and SUCCESS_TAG to more descriptive names like PROPERTY_URL and PROPERTY_SUCCESS to clarify their use as analytics property keys.
private const val URL_TAG = "url"

},
properties
)
Log.d("WP_RS", "Saved application password credentials for: $siteUrl")
Copy link
Preview

Copilot AI May 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Extract the log tag "WP_RS" into a constant (e.g., in a companion object) to avoid magic strings and ensure consistency across the class.

Suggested change
Log.d("WP_RS", "Saved application password credentials for: $siteUrl")
Log.d(LOG_TAG, "Saved application password credentials for: $siteUrl")

Copilot uses AI. Check for mistakes.

@nbradbury nbradbury self-assigned this May 19, 2025
Copy link
Contributor

@nbradbury nbradbury left a comment

Choose a reason for hiding this comment

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

@adalpari I'm unable to test this due to the problem I reported in the previous PR, where I always see this in the log:

E  WP_RS: Error during API discovery for https://nbradbury10.wordpress.com - FailureFindApiRoot

However, the logic here is sound so lets :shipit:

@adalpari adalpari merged commit 661a58d into feat/CMM-335-Offer-the-Application-Password-login-inside-a-card May 19, 2025
31 checks passed
@adalpari adalpari deleted the feat/CMM-334-Application-Password-login-add-analytic-events branch May 19, 2025 14:17
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