Remove editor_upload_media_paused analytics event #19972
Conversation
|
 |
App Name |  WordPress |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr19972-61034fc | |
| Commit | 61034fc | |
| Direct Download | wordpress-prototype-build-pr19972-61034fc.apk |
|
 |
App Name |  Jetpack |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr19972-61034fc | |
| Commit | 61034fc | |
| Direct Download | jetpack-prototype-build-pr19972-61034fc.apk |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #19972 +/- ##
=======================================
Coverage 40.45% 40.45%
=======================================
Files 1440 1440
Lines 66513 66506 -7
Branches 10957 10957
=======================================
Hits 26905 26905
+ Misses 37121 37114 -7
Partials 2487 2487 ☔ View full report in Codecov by Sentry. |
| val properties: Map<String, Any?> = withContext(bgDispatcher) { | ||
| analyticsUtilsWrapper | ||
| .getMediaProperties(media.isVideo, null, media.filePath) | ||
| .also { | ||
| it["error_type"] = error.type.name | ||
| } | ||
| } | ||
|
|
||
| analyticsTrackerWrapper.track(EDITOR_UPLOAD_MEDIA_PAUSED, site, properties) |
There was a problem hiding this comment.
Now that I review the required changes, it makes me pause and wonder if retaining this event is worthwhile to retain a "balance" in total number of editor_upload analytic events. I.e., with these changes the overall project work will now reduce the number of editor_upload events because we introduced a scenario where we no longer track the editor_upload error event.1
I still think the EDITOR_UPLOAD_MEDIA_PAUSED is unnecessary in that it duplicates media_service failure events associated with NO_CONNECTION, but that is true of EDITOR_UPLOAD_MEDIA_FAILED as well.
From looking at the origins of the events in 5201e74 and a88901e, their additions appear to be over a year apart with no mention of one another. My guess is the overlapping events are merely oversight.
All that to say: the proposed changes look sound for removing the event, but if you'd prefer to close this PR instead of merging, I'd support that as well.
Footnotes
-
I will note that this "balancing" argument is irrelevant to iOS where we seemingly do not trigger any
editor_uploadevents (p1704979060599929-slack-C063XEQAM8X?thread_ts=1704979060.599929&cid=C063XEQAM8X). ↩
|
Deferring to 24.2 milestone to evaluate this event further. |
|
It seems this PR was approved a while back, but it's not being merged until further evaluation. Instead of pushing the milestone every release, I am going to remove the milestone from it. Please do set it once it's decided which release it's going to go in. |
Generated by 🚫 Danger |
|
As discussed elsewhere, we've decided to close older PRs that have non-trivial merge conflicts and/or have non-trivial CI conflicts. |
Removes instances of the (unreleased)
editor_upload_media_pausedanalytics event in favor of tracking these as part ofmedia_service_upload_response_errorin tandem withCONNECTION_ERROR.See also:
To Test:
editor_upload_media_pausedshould no longer appear in analytics events.Regression Notes
Potential unintended areas of impact
Analytics events
What I did to test those areas of impact (or what existing automated tests I relied on)
Manual Testing in Superset/Looker
What automated tests I added (or what prevented me from doing so)
N/A
PR Submission Checklist:
RELEASE-NOTES.txtif necessary.