-
Notifications
You must be signed in to change notification settings - Fork 136
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
[Analytics Audit] OPML import finished property update #2686
[Analytics Audit] OPML import finished property update #2686
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check my comment about the return being made on smaller OPML imports.
podcasts/OpmlImporter.swift
Outdated
@@ -78,7 +78,7 @@ class OpmlImporter: Operation, XMLParserDelegate { | |||
NotificationCenter.postOnMainThread(notification: Constants.Notifications.opmlImportCompleted) | |||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to remove this return
or else if we are doing a smaller import the event bellow will not be tracked.
@SergioEstevao I removed the return also for the failure. Have look and test to confirm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the update!
Fixes #2677
Update the property for the
opml_import_finished
event.To test
🔵 Tracked: opml_import_finished ["number_parsed": YOUR_NUMBER]
Checklist
CHANGELOG.md
if necessary.