-
Notifications
You must be signed in to change notification settings - Fork 37
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
Malicious Site Protection - Make clientSideHit optional parameter #1199
Malicious Site Protection - Make clientSideHit optional parameter #1199
Conversation
private func fireErrorPageShown(threatKind: ThreatKind, clientSideHit: Bool) async { | ||
let filterSet = await dataManager.dataSet(for: .filterSet(threatKind: threatKind)) | ||
// https://app.asana.com/0/0/1209113403594297/1209141231997704/f | ||
let sanitisedClientSideHit = filterSet.filters.count < 100 ? clientSideHit : nil |
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.
it seems the Send Pixel clientSideHit parameter only if filterSet size is greater than 100
condition is inverted here
Also please add this as a comment in addition to the asana task URL for clarity
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.
🤦 good catch! I also updated the comment
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.
LGTM after resolving the inline comment
33c141b
to
e3520f7
Compare
407ba36
to
40910c9
Compare
d3a3e39
to
0e76b07
Compare
02091ea
into
alessandro/malicious-site-protection-ios
) **Required**: Task/Issue URL: https://app.asana.com/0/1206329551987282/1209242004458518/f iOS PR: duckduckgo/iOS#3891 macOS PR: duckduckgo/macos-browser#3796 What kind of version bump will this require?: Minor **Description**: This PR makes `MaliciousSiteProtection.Event.errorPageShown` `clientSideHit` parameter optional. See dicussion in [AsanaTask](https://app.asana.com/0/1206329551987282/1209242004458518/f)
) **Required**: Task/Issue URL: https://app.asana.com/0/1206329551987282/1209242004458518/f iOS PR: duckduckgo/iOS#3891 macOS PR: duckduckgo/macos-browser#3796 What kind of version bump will this require?: Minor **Description**: This PR makes `MaliciousSiteProtection.Event.errorPageShown` `clientSideHit` parameter optional. See dicussion in [AsanaTask](https://app.asana.com/0/1206329551987282/1209242004458518/f)
Please review the release process for BrowserServicesKit here.
Required:
Task/Issue URL: https://app.asana.com/0/1206329551987282/1209242004458518/f
iOS PR: duckduckgo/iOS#3891
macOS PR: duckduckgo/macos-browser#3796
What kind of version bump will this require?: Minor
Description:
This PR makes
MaliciousSiteProtection.Event.errorPageShown
clientSideHit
parameter optional. See dicussion in Asana TaskSteps to test this PR:
OS Testing:
Internal references:
Software Engineering Expectations
Technical Design Template