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

Filter Object: Addon #2104

Closed
jaredlockhart opened this issue Feb 13, 2020 · 3 comments · Fixed by #2136
Closed

Filter Object: Addon #2104

jaredlockhart opened this issue Feb 13, 2020 · 3 comments · Fixed by #2136
Assignees
Milestone

Comments

@jaredlockhart
Copy link
Collaborator

jaredlockhart commented Feb 13, 2020

We need a filter object for filtering for the presence of an addon by its addon id. I think if we want to exclude an addon we will have to combine this with #2105

@uhlissuh
Copy link
Contributor

uhlissuh commented Mar 9, 2020

@mythmon @rehandalal, I have a few questions about this one.

I see two different representations of addons on the filter expressions.

One looks like normandy.addons['uBlock0@raymondhill.net'].isActive and often times, several of these get strung together with ||.

The other I see is something like normandy.addons[\"@asdfjhsdfuhw\"] without the isActive part (also often strung together with ||s).

Also, the addon string looks different between these two things.

What is the difference between these two items?

@mythmon
Copy link
Contributor

mythmon commented Mar 9, 2020

If you are seeing \ in filter expressions in something like Delivery Console or NDT, those are probably bugs in the recipes. On the API though that might just be normal string escaping.

normandy.addons is an object where the keys are add-on IDs (which can be anything including uBlock0@raymondhill.net, @asdfjhsdfuhw, or even just UUIDs), and the values are an object describing details about that add-on. Here's an example from my own profile:

normandy.addons["{446900e4-71c2-419f-a6a7-df9c091e268b}"]
{
  "id": "{446900e4-71c2-419f-a6a7-df9c091e268b}",
  "isActive": true,
  "name": "Bitwarden - Free Password Manager",
  "type": "extension",
  "version": "1.42.2",
  "installDate": "2020-02-21T00:16:14.000Z"
}

A filter like normandy.addons[\"@asdfjhsdfuhw\"] asks "is this add-on installed". A filter like normandy.addons['uBlock0@raymondhill.net'].isActive asks "is this add-on installed and enabled", which is a stricter check. Both are useful in some cases.

For example, if we want to test out a new password manager feature, we might exclude users that already have a well known password manager. In this case we want to know if it is enabled, because if it is disabled they aren't using it. In this case we'd use normandy.addons[...].isActive.

Another example would be trying to mitigate the damage that a malware add-on has done. In this case, we would want to test if the add-on is present, even if it was disabled (since it could have done the damage and then disabled itself). In this case we'd just use normandy.addons[...]. If the add-on is installed, that will evaluate to a truthy value, and if the add-on is not installed, that will evaluate to undefined.

@uhlissuh
Copy link
Contributor

uhlissuh commented Mar 9, 2020

OK thanks for the clarification.

@uhlissuh uhlissuh self-assigned this Mar 13, 2020
@uhlissuh uhlissuh added this to the 1.33.0 milestone Mar 13, 2020
@jaredlockhart jaredlockhart modified the milestones: 1.133.0, 1.134.0 Mar 17, 2020
bors bot added a commit that referenced this issue Mar 30, 2020
2136: Add addon filter objects r=rehandalal a=uhlissuh

fixes #2104 

So, splitting these into two filter objects came from Rehan, which I thought was a good idea. He mentioned seeing about having them share some code, because there is duplication here, and I kinda looked at that for a while and arrived at wondering if we will really gain much here trying to have these two share code? There's only two that are shaped like this right now. There isn't a clear thing I could pull out, maybe someone sees something. Perhaps that's an argument for making them one filter? 

Co-authored-by: Alissa Sobo <alissasobo@gmail.com>
@bors bors bot closed this as completed in 7b912b6 Mar 30, 2020
@bors bors bot closed this as completed in #2136 Mar 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants