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

Allow a non-continuous range of versions to be added to the blocklist #9186

Closed
kplotnik opened this issue May 24, 2023 · 14 comments · Fixed by mozilla/addons-server#20829
Closed
Assignees
Labels
migration:2024 priority:p3 This priority level reflects our backlog. qa:extra repository:addons-server Issue relating to addons-server state:verified_fixed
Milestone

Comments

@kplotnik
Copy link

kplotnik commented May 24, 2023

┆Issue is synchronized with this Jira Task

@eviljeff eviljeff changed the title Non-continuous blocklist Allow a non-continuous range of versions to be added to the blocklist May 26, 2023
@eviljeff eviljeff added the priority:p3 This priority level reflects our backlog. label May 26, 2023
@eviljeff eviljeff added this to the 2023.06.08 milestone May 26, 2023
@wagnerand wagnerand modified the milestones: 2023.06.08, 2023.06.22 Jun 15, 2023
@diox diox modified the milestones: 2023.06.22, 2023.07.06 Jun 19, 2023
@eviljeff
Copy link
Member

eviljeff commented Jun 26, 2023

QA: this is a major rewrite of how Blocks and BlocklistSubmission objects work.

Some points:

  • min_version and max_version are replaced with a defined, finite, list of versions for each submission, and then for each block.
  • bloom filters should be produced exactly the same as before (i.e. blocking the same versions)
  • the admin for blocklistsubmissions has changed, so each version for a guid can be selected or deselected
  • the UI for a delete blocklistsubmission has changed - it is now used to unblock particular versions, and unblocking all versions for a guid will delete the block itself. (the checkboxes are perhaps counter-intuitive: for a delete submission if a version is checked the version will be un blocked)
  • you can't edit directly from the Block admin page currently, it was just a shortcut to creating a BlocklistSubmission anyway. I aim to restore that functionality in a follow-up [https://github.com/Restore edit functionality to Block admin page #9214]
  • viewing completed and pending BlocklistSubmission instances is a little broken last I checked - the checkboxes aren't accurate [https://github.com/correctly expose which versions are in blocklistsubmission #9223]

There are probably many bugs - the aim to have any major bugs fixed before the next tag to stage.

@ioanarusiczki
Copy link

@eviljeff

How can I check the bloomfilter on -dev , I mean I know this is for -stage:
https://firefox.settings.services.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records

but the URL with services-dev.allizom.org does not exist.

@ioanarusiczki
Copy link

Today's results:

I submitted new addons with multiple versions, listed, unlisted, mixed versions and tried to create blocks, partially or on all versions. With no accent on the layout of a block or it's editable fields such as url or reason. So far this is what I've noticed:

  • There's a "Disable add-on" option, the initial state is active (blue checkmark). When a partial block is done and disable add-on is checked the versions which have not been blocked are displayed as being approved on rev tools pages and in dev hub. The developer can no longer submit new versions and Force-enable state is available on the reviewer's pages which is expected when all versions are disabled.

Maybe the disable add-on should be available only when blocking the entire add-on (all versions).

An example: https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-listed/625710

Frontend throws a 404 at https://addons-dev.allizom.org/en-US/firefox/addon/sample-3-blocking1/ (with 2 versions still approved)
There's a blocked add-on URL -> https://addons-dev.allizom.org/en-US/firefox/blocked-addon/%7B0eff4a4f-1ace-4f07-91c1-ec7e7ad3e662%7D/

  • for unlisted versions when I initiate the block from "Block Multiple Versions" the versions are un-checked. If I try with Block/Update add-on block they're already checked (a blue check mark). It can be seen on https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-unlisted/625708

  • I edited a block by adding v2 to it (the only way an edit can be done yet, otherwise it throws an error). I've noticed that the reason I've added while blocking v2 is displayed in the review page for v1 too, maybe this should be displayed only once.
    Example: https://reviewers.addons-dev.allizom.org/en-US/reviewers/review-unlisted/625689

  • Edit Block url from admin -> the edit is not working right now. But I tried with Update Add-on Block from rev tools and I could continue editing only when I select a new version to block, and it worked for both listed/unlisted. Reading the instructions, that's going to be changed in future patches.

@ioanarusiczki
Copy link

@eviljeff P.S. I'll continue with other scenarios and the delete.

@eviljeff
Copy link
Member

@ioanarusiczki re: editing existing blocks - in theory you should be able to do everything you could before, but just by creating new blocklist submissions - if you wanted to edit the block to add a version, create an add blocklist submission (the default); if you wanted to edit the block to remove a version, create a delete blocklist submission; if you want to update the detail or url... well, you can at the same time as adding or removing a version, but otherwise currently that functionality is unavailable.

@eviljeff
Copy link
Member

  • There's a "Disable add-on" option, the initial state is active (blue checkmark). When a partial block is done and disable add-on is checked the versions which have not been blocked are displayed as being approved on rev tools pages and in dev hub. The developer can no longer submit new versions and Force-enable state is available on the reviewer's pages which is expected when all versions are disabled.

Maybe the disable add-on should be available only when blocking the entire add-on (all versions).

yes, I figured the default before was a 0 - * block that would disable the add-on, so to default to the same. But, what you said. Can you file a follow-up issue because I want to have Andreas weigh in on what he thinks should happen under different scenarios. (e.g. is it automatic if all the versions are blocked? Or just a different default? And what about when there are multiple submissions blocking different versions, so when/if all of the submissions complete all the versions will blocked, but individually not all versions are blocked)

@eviljeff
Copy link
Member

@eviljeff

How can I check the bloomfilter on -dev , I mean I know this is for -stage: https://firefox.settings.services.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records

but the URL with services-dev.allizom.org does not exist.

for dev it's https://remote-settings-dev.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records but it's currently empty. I'm investigating if this is because of a problem somewhere or just because it gets cleared regularly.

@eviljeff
Copy link
Member

@eviljeff
How can I check the bloomfilter on -dev , I mean I know this is for -stage: https://firefox.settings.services.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records
but the URL with services-dev.allizom.org does not exist.

for dev it's https://remote-settings-dev.allizom.org/v1/buckets/blocklists/collections/addons-bloomfilters/records but it's currently empty. I'm investigating if this is because of a problem somewhere or just because it gets cleared regularly.

It seems to have been broken for a few months - I just logged #1939 - so we will possibly have to test that on stage instead.

@ioanarusiczki
Copy link

ioanarusiczki commented Jun 28, 2023

@eviljeff I filed #9227 and #9228 and I'll continue testing around.

@ioanarusiczki
Copy link

@eviljeff

Because I'm not sure, it's better to ask:

  1. At partial blocks - when blocking the next version the version history look like below, is that expected?

at partial blocks

  1. There is no delete button for a blocked version while there are other versions delayed for approval or auto-sign off. Is this alright?
    An example:
    https://reviewers.addons-dev.allizom.org/en-US/reviewers/review/625740

There's also 2 buttons : view active blocklist submission and update add-on block -> this I think it should be alright
I can see all versions from View active blocklist submission -> those still pending have an Edit submission url where I can change the url and reason.

This is just a though:
3. When the number of versions > 3 -> the others will become visible by scrolling down, maybe versions could be exposed in a better way otherwise there's  the risk to leave a version checked or un-checked.

too narrow

@eviljeff
Copy link
Member

@eviljeff

Because I'm not sure, it's better to ask:

1. At partial blocks - when blocking the next version the version history look like below, is that expected?

This seems the same as #9228 - when I made the changes I replicated previous behaviour where logging is added to every version that is currently + previously affected by a block, but that may be overkill these days when we can target affected versions directly.

2. There is no [delete button](https://addons-internal.dev.mozaws.net/en-US/admin/models/blocklist/block/26358/change/) for a blocked version while there are other versions delayed for approval or auto-sign off. Is this alright?

no, can you file a follow-up.

This is just a though: 3. When the number of versions > 3 -> the others will become visible by scrolling down, maybe versions could be exposed in a better way otherwise there's the risk to leave a version checked or un-checked.

ux:needed 😛

@ioanarusiczki
Copy link

ioanarusiczki commented Jul 5, 2023

@eviljeff
addons-bloomfilter stage looks as expected right now

Signed versions blocked - approved or disabled/deleted at the time of the blocksubmission are present

I've tested with:
https://reviewers.addons.allizom.org/en-US/reviewers/review/1011797
https://reviewers.addons.allizom.org/en-US/reviewers/review-unlisted/1011798
https://reviewers.addons.allizom.org/en-US/reviewers/review/1011799
https://reviewers.addons.allizom.org/en-US/reviewers/review-unlisted/1011799

I'm going to try the unblock on a couple too.

@ioanarusiczki
Copy link

@eviljeff

Signed-unblocked versions are present too

signed-unblocked

This was yesterday's results on blocking

bloomfilter AMO stage

@KevinMind
Copy link
Contributor

@KevinMind KevinMind added repository:addons-server Issue relating to addons-server migration:2024 labels May 4, 2024
@KevinMind KevinMind transferred this issue from mozilla/addons-server May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration:2024 priority:p3 This priority level reflects our backlog. qa:extra repository:addons-server Issue relating to addons-server state:verified_fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants