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.

Sign up for GitHub

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

Jump to bottom

Pixelkit invalid name fix #2751

Merged
merged 5 commits into from
May 8, 2024
Merged

Pixelkit invalid name fix #2751

merged 5 commits into from
May 8, 2024

Conversation

federicocappelli
Copy link
Member

@federicocappelli federicocappelli commented May 8, 2024 •
edited
Loading

Task/Issue URL: https://app.asana.com/0/1199230911884351/1207238594885159/f
CC: @ayoy @diegoreymendez @loremattei

Description:

Steps to test this PR:

Report broken site:
Report a site using the form and observe the console or set a breakpoint for the pixel, the name must be epbf_macos_desktop 

2024-05-08 07:52:41.621016+0100 DuckDuckGo[15640:12213726] 1 breakage history record removed
2024-05-08 07:52:41.621244+0100 DuckDuckGo[15640:12213726] Reporting website breakage for fcea18
2024-05-08 07:52:41.621296+0100 DuckDuckGo[15640:12213726] Broken site report sent on the 2024-04-08 for fcea18
2024-05-08 07:52:41.621714+0100 DuckDuckGo[15640:12213726] [PixelKit] 👾[Standard-Fired] epbf_macos_desktop ["blockedTrackers": "", "pixelSource": "browser-dmg", "userRefreshCount": "0", "appVersion": "1.87.0", "gpc": "true", "openerContext": "", "urlParametersRemoved": "false", "reportFlow": "menu", "surrogates": "", "protectionsState": "true", "category": "blocked", "lastSentDay": "2024-04-08", "upgradedHttps": "false", "vpnOn": "false", "ampUrl": "", "os": "14.4.1", "manufacturer": "Apple", "siteUrl": "https://federicocappelli.com/", "httpErrorCodes": "200", "description": "TEST IGNORE", "tds": "A_\"ef8ebcc98d8abccca793c7e04422b160", "jsPerformance": "745.0"]
2024-05-08 07:52:41.621956+0100 DuckDuckGo[15640:12213726] Website breakage reported for fcea18
Internal references:

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

@federicocappelli federicocappelli mentioned this pull request May 8, 2024
Merged
@federicocappelli federicocappelli changed the base branch from main to release/1.87.0 May 8, 2024 06:46
@federicocappelli federicocappelli mentioned this pull request May 8, 2024
Closed
@federicocappelli federicocappelli requested a review from ayoy May 8, 2024 07:03
ayoy
ayoy approved these changes May 8, 2024
View reviewed changes
Copy link
Collaborator

@ayoy ayoy left a comment

Choose a reason for hiding this comment

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

LGTM, I've tested broken site report and email pixels and they don't have the prefix now. Thanks also for reverting some other daily pixels naming to the old format!

federicocappelli added a commit to duckduckgo/BrowserServicesKit that referenced this pull request May 8, 2024
@federicocappelli
Pixels automatic naming prefixing fixed (#810)
f34b0a6 
Task/Issue URL: https://app.asana.com/0/1201037661562251/1207238594885159/f
macOS PR: duckduckgo/macos-browser#2751

*Description*

This PR contains few improvements:
- Added a check on every Pixel name, if the name contains a `.` an assertionError is fired (in Debug)
- Added a special pixel event called `NonStandardEvent` for those legacy Pixels non following the new PixelKit naming conventions, these kinds of pixels will maintain the name without automatic alterations.
- New unit tests for covering the new event type and Debug event
@federicocappelli federicocappelli merged commit 6cea0b6 into release/1.87.0 May 8, 2024
19 checks passed
@federicocappelli federicocappelli deleted the fcappelli/pixelkit_invalid_name_fix branch May 8, 2024 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayoy ayoy ayoy approved these changes

Assignees

@federicocappelli federicocappelli

Labels
DO NOT MERGE includes BSK changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@federicocappelli @ayoy @daxmobile