630575 - Properly distinguish soft and hard blocked items in the update pings

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Townsend [:mossop]]

Currently in Firefox 3.5 and 3.6 we only include the "blocklisted" state if an item is hardblocked.
In Firefox 4.0 we include the "blocklisted" state if an item is soft or hard blocked.

This causes confusing data and for the branches makes it impossible to verify that blocklist changes are having any effect. We should split it and send "blocklisted" for hard blocks only and "softblocked" for soft blocked items.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Just for Firefox 4 for now this makes the changes. Turns out there were no tests for this so I added those. I think it's important to take this and bring us back into parity with 3.6 for stats.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 511242 [details] [diff] [review]
patch rev 1

Makes sense.

Comment 3

[Dave Townsend [:mossop]]

For drivers, this is really safe code that fixes a regression from 3.6 that generates us confusing stats when we try to evaluate the pickup rate of the blocklist.

Comment 4

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/53d06f07a4d2

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Dave, what's the best way to verify this fix at least for the softblocked items? In which query will the softblocked items be contained in?

Comment 6

[Dave Townsend [:mossop]]

(In reply to comment #5)
> Dave, what's the best way to verify this fix at least for the softblocked
> items? In which query will the softblocked items be contained in?

It shows up in the update ping to AMO or the %ITEM_STATUS% part of a custom updateURL

Comment 7

[Dave Townsend [:mossop]]

Attachment: branch patch

This adds the softblocked status to the 1.9.2 branch and adds the same tests.

Comment 8

[Dave Townsend [:mossop]]

Comment on attachment 517484 [details] [diff] [review]
branch patch

I'd like approval to land this on the branches to ensure that we get good feedback from changes to the blocklist. It is a pretty low risk patch and adds more tests than we had previously for this status reporting.

Comment 9

[Daniel Veditz [:dveditz]]

(In reply to comment #3)
> For drivers, this is really safe code that fixes a regression from 3.6 that

If it's a regression from 3.6 why do we need this patch in 3.6? Or if you meant a regression introduced _in_ 3.6 it's still not clear why you want this patch in 3.5.

Comment 10

[Dave Townsend [:mossop]]

(In reply to comment #9)
> (In reply to comment #3)
> > For drivers, this is really safe code that fixes a regression from 3.6 that
> 
> If it's a regression from 3.6 why do we need this patch in 3.6? Or if you meant
> a regression introduced _in_ 3.6 it's still not clear why you want this patch
> in 3.5.

Neither, that comment was for the 2.0 drivers where we changed the behaviour.

Firefox 3.6 and 3.5 tell AMO when they know an add-on that a user has installed is hard blocked but not when it is soft blocked (which 4.0 does do now). We have been using the stats from these pings in the past to verify the rollout of the blocklist updates and without the soft blocked information we can't verify that kind of block is rolling out as we expect.

Comment 11

[Daniel Veditz [:dveditz]]

Comment on attachment 517484 [details] [diff] [review]
branch patch

Approved for 1.9.2.16 and 1.9.1.18, a=dveditz for release-drivers

Comment 12

[Dave Townsend [:mossop]]

Landed:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/fabeec97235e
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/72a72241e47c