-
Notifications
You must be signed in to change notification settings - Fork 425
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
Subscription pro pixels #2531
Subscription pro pixels #2531
Conversation
# Conflicts: # DuckDuckGo/Subscription/Views/SubscriptionFlowView.swift # DuckDuckGo/Subscription/Views/SubscriptionRestoreView.swift
# Conflicts: # DuckDuckGo/Subscription/UserScripts/SubscriptionPagesUseSubscriptionFeature.swift # DuckDuckGo/Subscription/ViewModel/SubscriptionRestoreViewModel.swift
# Conflicts: # Core/PixelEvent.swift # DuckDuckGo.xcodeproj/project.pbxproj
Code looks fine, but I left a few questions about Daily pixels and some recommendations on moving some of the calls to the ViewModel. While I can run every feature scenario, I don't have enough context to understand which pixels should fire in every case and when.- While the code looks mostly fine to me, If you want me to test this in detail, I'll need detailed instructions on where in the process each pixel should be fired. (I don't necessarily have enough context on the project and discussions to infer this) |
@federicocappelli I updated the description to add the pixel list as a quick table so it's quick to review/validate. This may also be useful for the macOS part. |
@federicocappelli It seems some pixels are not correct when restoring a subscription. I did this:
I'm seeing duplicate pixels fired, as well as several failures that shouldn't be fired here. These pixels were fired. |
Cancelling the purchase, fires this twice.
|
# Conflicts: # Core/PixelEvent.swift # Core/UserAgentManager.swift # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo/SettingsSubscriptionView.swift
# Conflicts: # Core/PixelEvent.swift # submodules/privacy-reference-tests
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.
Code looks ok, but I still see some duplicate pixels and misfires. Let's get this merged to prevent more conflicts and iterate on that later
* main: Subscriptions: 20 - Subscription Caching (#2569) Subscriptions: 19. Error handling and minor updates (#2567) Use History on iOS (#2539) Release 7.112.0-1 (#2573) Report on toggle protections off (#2536) Release 7.112.0-0 (#2572) Update autoconsent to v10.2.0 (#2554) update metadata (#2571) Subscription pro pixels (#2531) Update SwiftSoup and Kingfisher versions (#2566) Move vpnFirstEnabled and networkPathChange out of VPNSettings (#2560) Fix VPN view model memory leak (#2570) Bump submodules/privacy-reference-tests from `40ce868` to `a603ff9` (#2500) 18. Subscription Entitlements caching (#2556) Release 7.111.0-2 (#2563) Revert "Report Apple Ad attribution using pixel (#2510)" (#2562) Vanilla browser integration (#2550) Fix blank space after URL bar hides (#2549) Release 7.111.0-1 (#2561)
…-lottie-animation * sam/vpn-ui-improvements: Subscriptions: 20 - Subscription Caching (#2569) Subscriptions: 19. Error handling and minor updates (#2567) Use History on iOS (#2539) Release 7.112.0-1 (#2573) Report on toggle protections off (#2536) Release 7.112.0-0 (#2572) Update autoconsent to v10.2.0 (#2554) update metadata (#2571) Subscription pro pixels (#2531) Update SwiftSoup and Kingfisher versions (#2566) Move vpnFirstEnabled and networkPathChange out of VPNSettings (#2560) Fix VPN view model memory leak (#2570) Bump submodules/privacy-reference-tests from `40ce868` to `a603ff9` (#2500) 18. Subscription Entitlements caching (#2556) Release 7.111.0-2 (#2563) Revert "Report Apple Ad attribution using pixel (#2510)" (#2562) Vanilla browser integration (#2550) Fix blank space after URL bar hides (#2549) Release 7.111.0-1 (#2561)
…n-ui-improvements-3-combine-notification-settings * sam/vpn-ui-improvements-2-lottie-animation: Subscriptions: 20 - Subscription Caching (#2569) Subscriptions: 19. Error handling and minor updates (#2567) Use History on iOS (#2539) Release 7.112.0-1 (#2573) Report on toggle protections off (#2536) Release 7.112.0-0 (#2572) Update autoconsent to v10.2.0 (#2554) update metadata (#2571) Subscription pro pixels (#2531) Update SwiftSoup and Kingfisher versions (#2566) Move vpnFirstEnabled and networkPathChange out of VPNSettings (#2560) Fix VPN view model memory leak (#2570) Bump submodules/privacy-reference-tests from `40ce868` to `a603ff9` (#2500) 18. Subscription Entitlements caching (#2556) Release 7.111.0-2 (#2563) Revert "Report Apple Ad attribution using pixel (#2510)" (#2562) Vanilla browser integration (#2550) Fix blank space after URL bar hides (#2549) Release 7.111.0-1 (#2561)
…' into sam/vpn-ui-improvements-4-location-setting-change # By Mariusz Śpiewak (3) and others # Via Sam Symons (4) and others * sam/vpn-ui-improvements-3-combine-notification-settings: Subscriptions: 20 - Subscription Caching (#2569) Subscriptions: 19. Error handling and minor updates (#2567) Use History on iOS (#2539) Release 7.112.0-1 (#2573) Report on toggle protections off (#2536) Release 7.112.0-0 (#2572) Update autoconsent to v10.2.0 (#2554) update metadata (#2571) Subscription pro pixels (#2531) Update SwiftSoup and Kingfisher versions (#2566) Move vpnFirstEnabled and networkPathChange out of VPNSettings (#2560) Fix VPN view model memory leak (#2570) Bump submodules/privacy-reference-tests from `40ce868` to `a603ff9` (#2500) 18. Subscription Entitlements caching (#2556) Release 7.111.0-2 (#2563) Revert "Report Apple Ad attribution using pixel (#2510)" (#2562) Vanilla browser integration (#2550) Fix blank space after URL bar hides (#2549) Release 7.111.0-1 (#2561) # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
Task/Issue URL: https://app.asana.com/0/1200019156869587/1205469290776415/f
Description:
Please hide whitespace changes when reviewing, this PR includes many swiftlint fixes to old code
Pixels added to all privacy pro subscription flows.
Steps to test this PR:
Please refer to https://app.asana.com/0/1205842942115003/1205469314884687/f for how to trigger the pixels
I re-run every scenario in the list above except the backend and Apple Store errors that I have no way to trigger.
--
New Pixels