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

A listed version block does not set a Disabled status for the blocked version #7206

Closed
AlexandraMoga opened this issue Dec 17, 2019 · 15 comments · Fixed by mozilla/addons-server#15435

Comments

@AlexandraMoga
Copy link

Describe the problem and steps to reproduce it:

  1. Add a new block for an add-on with listed versions - see this example
  2. Add a min - max version range that will also block at least one listed version
  3. Verify the listed version status - in Rev Tools or DevHub - after the block is saved

What happened?

The listed version is Blocked but not Disabled - see here

What did you expect to happen?

The listed version should be disabled automatically when a block is submitted

Anything else we should know?

  • unlisted versions are automatically disabled when a block is submitted
  • reproduced on -dev and stage

listed version with status 'Approved' after a block was submitted
image

@eviljeff
Copy link
Member

Where is this functionality requested? (Issue/PRD)

@AlexandraMoga
Copy link
Author

@eviljeff it wasn't requested in Issues/PRD but, considering that we are setting unlisted versions as disabled when they are blocked, we should probably do the same for listed versions, for consistency.
I had a quick discussion on irc with @wagnerand about this and we concluded that this would be the expected flow.

@eviljeff
Copy link
Member

we are setting unlisted versions as disabled when they are blocked

That's not correct. Creating a Block doesn't make any change to the Addon or Versions - listed or unlisted. If you are thinking about #7158 then it's the action in the reviewer tools that disables the version.

@AlexandraMoga
Copy link
Author

Yes, you are right about the reviewer tools implication for unlisted. So, to put it more clearly, I was wondering if we could have something similar for listed reviews as well.
For example, when an admin reviewer uses the 'Block add-on' button from More actions section - which means that the block request originates in the reviewer tools - we could automatically disable the versions blocked in the admin. With the current implementation, admin reviewers are required to manually disable those versions afterwards.

I see how my initial request might have some inconvenient consequences in case of an accidental block, but I think it's safe to say that, when the block request originates from rev tools, we can rely on a more detailed analysis of the versions being blocked.

@eviljeff
Copy link
Member

@wagnerand / @jvillalobos to opine. #7158 was explicitly about unlisted versions.

@wagnerand
Copy link
Member

If feasible, I would appreciate if we could automatically disable all versions that are affected by a block. Otherwise, I will have to do it manually. In every case I can think of, a blocked version would also be disabled.

@eviljeff
Copy link
Member

If feasible, I would appreciate if we could automatically disable all versions that are affected by a block. Otherwise, I will have to do it manually. In every case I can think of, a blocked version would also be disabled.

It's not how the Blocklist Admin tool was specified - currently it purposely doesn't touch the Addon/Version/File instances at all. So this would be out of scope for the Blocklist projects and worked on separately/later.

@jvillalobos
Copy link

Yeah, the tool didn't contemplate status changes as designed. I can see it being useful to disable the versions once blocked, though it might get confusing if removing a block doesn't revert the status changes (and if we do that, then we would need to somehow store the previous state, etc).

@wagnerand
Copy link
Member

@AlexandraMoga, @eviljeff and I are a bit unclear on what this issue asks for. Is it one of the following?

  • Add a new action "Block multiple versions" to the reviewer tools page for listed versions
  • Disable the versions when you click the "Block add-on" button on the reviewer tools page
  • Disable the versions during blocking the add-on in the admin tools (regardless of whether the action originated from the reviewer tools or not)

@AlexandraMoga
Copy link
Author

@wagnerand my initial intent was to have what was mentioned in your second point:

  • Disable the versions when you click the "Block add-on" button on the reviewer tools page

However, if you have something more practical in mind we can go with that.

@wagnerand
Copy link
Member

Following up on this, we should:

Disable the versions during blocking the add-on in the admin tools (regardless of whether the action originated from the reviewer tools or not).

If specific versions or a version range that is not 0-* is blocked, then only the affected versions should be disabled.
If a version range of 0-* is blocked, all versions should be disabled, and the add-on should be disabled as well.

@eviljeff
Copy link
Member

If specific versions or a version range that is not 0-* is blocked, then only the affected versions should be disabled.

When should the versions be disabled? When the submission is signed off and the block is created? Or when the submission is created?

Also, it seems like the functionality for #7158 would be redundant as the versions would be disabled as part of the block flow rather than in the reviewer tools.

@wagnerand
Copy link
Member

If specific versions or a version range that is not 0-* is blocked, then only the affected versions should be disabled.

When should the versions be disabled? When the submission is signed off and the block is created? Or when the submission is created?

Ideally, when the submission gets signed and the block is created, but we can also do it during submission creation if turns out to be very complicated.

Also, it seems like the functionality for #7158 would be redundant as the versions would be disabled as part of the block flow rather than in the reviewer tools.

Only partly. The block flow in the reviewer tools is available to unlisted versions only.

@eviljeff eviljeff self-assigned this Aug 27, 2020
@eviljeff eviljeff added this to the 2020.09.10 milestone Sep 2, 2020
@wagnerand
Copy link
Member

wagnerand commented Sep 8, 2020

Side note, regardless of channels:

When the block submission gets signed (and the block is created), next to versions being disabled, we should also disable the add-on (set addon.status to STATUS_DISABLED) if the block range is 0-*.

@AlexandraMoga
Copy link
Author

I've tested block submissions on -dev and the Disabled status for add-ons and/or versions are applied as follows:

I'm marking this issue as verified fixed on -dev.

@KevinMind KevinMind transferred this issue from mozilla/addons-server May 4, 2024
@KevinMind KevinMind added the repository:addons-server Issue relating to addons-server label May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

5 participants