Rewrite WPAppAnalytics extensions in Swift #24316

kean · 2025-03-26T15:18:13Z

It is a requirement for Keystone.

To test:

Regression Notes

Potential unintended areas of impact
What I did to test those areas of impact (or what existing automated tests I relied on)
What automated tests I added (or what prevented me from doing so)

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

dangermattic · 2025-03-26T15:18:56Z

	3 Warnings
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️	This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️	This PR is assigned to the milestone 25.9. The due date for this milestone has already passed. Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

wpmobilebot · 2025-03-26T15:35:00Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr24316-7d00129
	Version	25.8
	Bundle ID	com.jetpack.alpha
	Commit	`7d00129`
	App Center Build	jetpack-installable-builds #10866

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2025-03-26T15:37:12Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr24316-7d00129
	Version	25.8
	Bundle ID	org.wordpress.alpha
	Commit	`7d00129`
	App Center Build	WPiOS - One-Offs #11843

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

crazytonyli · 2025-03-27T01:00:22Z

WordPress/Classes/Utility/Analytics/WPAppAnalytics.m

-        mutableProperties[WPAppAnalyticsKeyPostID] = postOrPage.postID;
-    }
-    mutableProperties[WPAppAnalyticsKeyHasGutenbergBlocks] = @([postOrPage containsGutenbergBlocks]);
-    mutableProperties[WPAppAnalyticsKeyHasStoriesBlocks] = @([postOrPage containsStoriesBlocks]);


This line appears to be removed in the translation.

I removed both WPAppAnalyticsKeyHasStoriesBlocks because stories have been removed from the app in general about a year ago or more.

crazytonyli · 2025-03-27T01:05:21Z

WordPress/Classes/ViewRelated/Reader/Comments/ReaderCommentsViewController.swift

-
-        if let railcar {
-            WPAppAnalytics.trackTrainTracksInteraction(.trainTracksInteract, withProperties: railcar)
-        }


I'm not sure what this "railcar" is, but do we just not care about it anymore?

It's something that was added nine years ago and appears to be unused. Posts don't have railcars, and I couldn't find any recent events on Tracks either. I think it's pretty safe to say it's dead code.

crazytonyli · 2025-03-27T01:06:24Z

WordPress/Classes/Utility/Analytics/WPAppAnalytics.h

 *  @brief      Timestamp of the app's opening time.
 */
-@property (nonatomic, strong, readwrite) NSDate* applicationOpenedTime;
+@property (nonatomic, strong, readwrite) NSDate* _Nullable applicationOpenedTime;


Nitpick: adding nonnull to the property attributes instead?

Oh yeah forgot it about. Updated. It's much nicer.

crazytonyli · 2025-03-27T01:08:16Z

WordPress/Classes/Utility/Analytics/WPAppAnalytics.m

-    } else {
-        [self setUserHasOptedOutValue:NO];
-    }
+    [self setUserHasOptedOutValue:NO];


Is it intentional to delete the code above that reads user settings?

I removed the migration code that I assume we no longer need. This runs once.

crazytonyli · 2025-03-27T01:10:15Z

...ess/Classes/ViewRelated/Blog/Site Management/SiteSettingsViewController+SiteManagement.swift

-        WPAppAnalytics.track(.siteSettingsExportSiteRequested, with: trackedBlog)
+        if let trackedBlog {
+            WPAppAnalytics.track(.siteSettingsExportSiteRequested, blog: trackedBlog)
+        }


I think we can add an else block which runs WPAppAnalytics.track(.siteSettingsExportSiteRequested) here? Same as the if let trackedBlog statements below?

Good point. I made blog optional in WPAppAnalytics for convenience and updated all of these method invocations.

crazytonyli · 2025-03-27T01:10:58Z

WordPress/Classes/ViewRelated/Comments/Controllers/CommentDetailViewController.swift

-                                                     withProperties: Constants.notificationDetailSource,
-                                                     withBlogID: notification?.metaSiteID) :
-                                CommentAnalytics.trackCommentUnApproved(comment: comment)
+        if isNotificationComment { WPAppAnalytics.track(.notificationsCommentUnapproved, properties: Constants.notificationDetailSource, blogID: notification?.metaSiteID)


This statement should be in a new line.

crazytonyli · 2025-03-27T01:12:05Z

WordPress/Classes/ViewRelated/Media/SiteMedia/SiteMediaViewController.swift

-            WPAppAnalytics.track(.mediaLibraryDeletedItems, withProperties: ["number_of_items_deleted": deletedItemsCount], with: self?.blog)
+            if let blog = self?.blog {
+                WPAppAnalytics.track(.mediaLibraryDeletedItems, properties: ["number_of_items_deleted": deletedItemsCount], blog: blog)
+            }


Adding an else block to call WPAppAnalytics.track(.mediaLibraryDeletedItems)?

crazytonyli · 2025-03-27T01:12:54Z

WordPress/Classes/ViewRelated/Themes/ThemeBrowserViewController.swift

-                WPAppAnalytics.track(.themesChangedTheme, withProperties: ["theme_id": theme?.themeId ?? ""], with: self?.blog)
+                if let blog = self?.blog {
+                    WPAppAnalytics.track(.themesChangedTheme, properties: ["theme_id": theme?.themeId ?? ""], blog: blog)
+                }


Similar to the above, adding an else block here?

wpmobilebot · 2025-03-27T22:18:03Z

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Configuration	Release-Alpha
	Build Number	`26819`
	Version	`PR #24316`
	Bundle ID	`com.jetpack.alpha`
	Commit	`3e72863`
	Installation URL	153no6gnvtqj8

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

wpmobilebot · 2025-03-27T22:20:43Z

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Configuration	Release-Alpha
	Build Number	`26819`
	Version	`PR #24316`
	Bundle ID	`org.wordpress.alpha`
	Commit	`3e72863`
	Installation URL	7a51v0d4ff4d0

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

crazytonyli · 2025-03-30T22:38:15Z

WordPress/Classes/ViewRelated/Comments/Controllers/CommentDetailViewController.swift

-                                                     withProperties: Constants.notificationDetailSource,
-                                                     withBlogID: notification?.metaSiteID) :
-                                CommentAnalytics.trackCommentApproved(comment: comment)
+        if isNotificationComment { WPAppAnalytics.track(.notificationsCommentApproved, properties: Constants.notificationDetailSource, blogID: notification?.metaSiteID)


Here needs a new line too.

crazytonyli · 2025-03-30T22:38:24Z

WordPress/Classes/ViewRelated/Comments/Controllers/CommentDetailViewController.swift

    func spamComment() {
-        isNotificationComment ? WPAppAnalytics.track(.notificationsCommentFlaggedAsSpam, withBlogID: notification?.metaSiteID) :
-                                CommentAnalytics.trackCommentSpammed(comment: comment)
+        if isNotificationComment { WPAppAnalytics.track(.notificationsCommentFlaggedAsSpam, blogID: notification?.metaSiteID)


crazytonyli · 2025-03-30T22:38:28Z

WordPress/Classes/ViewRelated/Comments/Controllers/CommentDetailViewController.swift

    func trashComment() {
-        isNotificationComment ? WPAppAnalytics.track(.notificationsCommentTrashed, withBlogID: notification?.metaSiteID) :
-                                CommentAnalytics.trackCommentTrashed(comment: comment)
+        if isNotificationComment { WPAppAnalytics.track(.notificationsCommentTrashed, blogID: notification?.metaSiteID)


kean added 13 commits March 26, 2025 08:15

Rewrite the session tracking parts

0edf08d

Convert to NSNumber

f1dab2e

Remove deprecated isTrackingUsage and the associated migration

49f558f

Remove unused imports

a9ca402

Convert Blog-related WPAppAnalytics to Swift

4a93c02

Optimize blog-related tracking to avoid constant blog lookup by ID

a230298

Update Post-related tracking (and remove legacy stories property)

67a7c69

Update remaing blog-related WPAppAnalytics calls

9388c93

Implement error tracking without the use of new APIs

d49d8ec

Redo how the WPAppAnalytics APIs are made available in Objective-C

e155a9e

Remove trackTrainTracksInteraction

76c24a3

Rewrite error reporting

f65e04e

Move remaining even tracking to Swift

d6a3300

kean added the General label Mar 26, 2025

kean added this to the 25.9 milestone Mar 26, 2025

Remove unused imports

7d00129

kean requested review from crazytonyli and mokagio and removed request for mokagio March 26, 2025 15:42

crazytonyli reviewed Mar 27, 2025

View reviewed changes

Base automatically changed from task/extract-wpanalytics to trunk March 27, 2025 01:22

kean added 2 commits March 27, 2025 12:05

Merge branch 'trunk' into task/wpanalytics-swift-2

4fe98ec

Make blog optional

b0f5276

kean requested a review from crazytonyli March 27, 2025 16:08

kean force-pushed the task/wpanalytics-swift-2 branch from b83d217 to 431857b Compare March 27, 2025 21:09

Use nullable

dc61d70

kean force-pushed the task/wpanalytics-swift-2 branch from 431857b to dc61d70 Compare March 27, 2025 21:11

Resolve merge conflicts

9423d45

Update tests

3e72863

kean enabled auto-merge March 28, 2025 21:25

crazytonyli approved these changes Mar 30, 2025

View reviewed changes

kean added this pull request to the merge queue Mar 30, 2025

Merged via the queue into trunk with commit 50cb14f Mar 30, 2025
23 checks passed

kean deleted the task/wpanalytics-swift-2 branch March 30, 2025 23:07

Rewrite WPAppAnalytics extensions in Swift #24316

Rewrite WPAppAnalytics extensions in Swift #24316

Uh oh!

Conversation

kean commented Mar 26, 2025

Regression Notes

Uh oh!

dangermattic commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Mar 26, 2025

Uh oh!

wpmobilebot commented Mar 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wpmobilebot commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dangermattic commented Mar 26, 2025 •

edited

Loading

wpmobilebot commented Mar 27, 2025 •

edited

Loading

wpmobilebot commented Mar 27, 2025 •

edited

Loading