Skip to content
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

VPN Domain Exclusions pixel changes #3103

Merged
merged 9 commits into from
Aug 29, 2024

Conversation

diegoreymendez
Copy link
Contributor

@diegoreymendez diegoreymendez commented Aug 15, 2024

Task/Issue URL: https://app.asana.com/0/0/1208044141877984/f

BSK PR: duckduckgo/BrowserServicesKit#945
iOS PR: duckduckgo/iOS#3242

Description

Adds engagement pixels for VPN domain exclusions.

Testing

Test 1: Through the status view

  1. Open Console.app, start streaming and filter by 👾
  2. Enable PrivacyPro. You can use either a test account or use your production account if you have one.
  3. Start the VPN.
  4. Open duckduckgo.com (or any site actually)
  5. Go to the VPN status view in the main app's navigation bar and expand the "duckduckgo.com issues?" item.
  6. Select the option to exclude the site from the VPN.
Screenshot 2024-08-15 at 1 29 42 PM
  1. You should see this pixel in Console.app:
👾[Standard-Fired] m_mac_vpn_domain_exclusion_enabled ["pixelSource": "browser-dmg", "appVersion": "1.102.0"]
  1. You'll get an alert asking you if you want to report the site breakage, click "Report".
  2. You should see this pixel in Console.app:
👾[Standard-Fired] m_mac_vpn_report_site_issues ["appVersion": "1.102.0", "pixelSource": "browser-dmg", "domain": "digg.com"]
  1. Now repeat the process but select to route the site through the VPN.
  2. You should see this pixel in Console.app:
👾[Standard-Fired] m_mac_vpn_domain_exclusion_disabled ["appVersion": "1.102.0", "pixelSource": "browser-dmg"]
  1. Repeat step 6, but this time when the alert comes up select "Don't report". You should not see the m_mac_vpn_report_site_issues pixel.

Test 2: Exclusions through settings

This works like excluding from the status view, but you should never see m_mac_vpn_domain_exclusion_enabled and m_mac_vpn_domain_exclusion_disabled.

The only pixel you should see is the report one (m_mac_vpn_report_site_issues) if you select to report.

Definition of Done:


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

Comment on lines 89 to 90
let engagementPixel = DomainExclusionsEngagement(exclude: exclude, domain: domain)
pixelKit?.fire(engagementPixel)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're now tracking engagement

Comment on lines +41 to +43
func scopedPixelName(forUnscopedPixelName unscopedName: String) -> String {
"\(vpnPixelModulePrefix)_\(unscopedName)"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a convenience method to add the module prefix to the pixel name.

@diegoreymendez diegoreymendez requested a review from graeme August 15, 2024 11:43
@diegoreymendez diegoreymendez marked this pull request as ready for review August 15, 2024 11:48
Copy link
Collaborator

@graeme graeme left a comment

Choose a reason for hiding this comment

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

LGTM!

diegoreymendez added a commit to duckduckgo/BrowserServicesKit that referenced this pull request Aug 16, 2024
Task/Issue URL: https://app.asana.com/0/0/1208044141877984/f

iOS PR: duckduckgo/iOS#3242
macOS PR: duckduckgo/macos-browser#3103

What kind of version bump will this require?: Patch

## Description

Adds a domain pixel parameter to PixelKit.
Copy link
Contributor

This PR has been inactive for more than 7 days and will be automatically closed 7 days from now.

@github-actions github-actions bot added the stale label Aug 24, 2024
# By Dax the Duck (9) and others
# Via GitHub (6) and Dominik Kapusta (2)
* main: (44 commits)
  Allow using a production subscription with staging VPN environment (#3109)
  Add error pixels for Subscription keychain access errors (#3147)
  onboarding dax dialogs (#3149)
  Logging refactoring phase #2 (#3154)
  Move WireGuard dependency to VPN extensions (#3144)
  Unified feedback form for Privacy Pro (#3058)
  Fix state restoration after app termination (#3127)
  Bump Submodules/privacy-reference-tests from `afb4f61` to `6133e7d` (#3148)
  Bump version to 1.104.0 (250)
  Set marketing version to 1.104.0
  Update embedded files
  Freemium PIR - Add Desktop RMF Attribute (#3146)
  Migrate asana-extract-task-id GHA action to a fastlane plugin (#3145)
  Add missing secrets to publish_dmg_release.yml
  Migrate asana-extract-task-id GHA action to a fastlane plugin (#3145)
  Fix address bar queries when doing math expressions (#3130)
  Bump version to 1.103.0 (249)
  Bump version to 1.103.0 (248)
  Resolving automatic update edge cases (#3142)
  PIR Time-Based Pixel: 24 Opt-Out Request Success Rate (#2942)
  ...

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	LocalPackages/DataBrokerProtection/Package.swift
#	LocalPackages/NetworkProtectionMac/Package.swift
#	LocalPackages/SubscriptionUI/Package.swift
samsymons pushed a commit to duckduckgo/iOS that referenced this pull request Aug 29, 2024
Task/Issue URL: https://app.asana.com/0/0/1208044141877984/f

BSK PR: duckduckgo/BrowserServicesKit#945
macOS PR: duckduckgo/macos-browser#3103

Description

Removes a duplicated pixel and updates BSK to include the latest macOS changes.
@samsymons samsymons merged commit 3b474a0 into main Aug 29, 2024
18 checks passed
@samsymons samsymons deleted the diego/domain-exclusion-improvements branch August 29, 2024 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants