Closed Bug 1172035 Opened 9 years ago Closed 9 years ago

Give rejected beta versions option to submit to unlisted preliminary review queue

Categories

(addons.mozilla.org Graveyard :: Developer Pages, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
2015-06

People

(Reporter: jorgev, Assigned: magopian)

References

Details

(Whiteboard: [june-launch])

Attachments

(1 file)

Since betas that don't pass validation has very quickly become a recurring problem and not just an edge case, we'll need to better deal with this case.

Beta versions will need to be treated like unlisted versions. If they don't pass automatic review, developers will be given the option to submit the file for manual review. If they do, the file will be added to the unlisted preliminary review queue.

We should eventually make it visually clear which files in that queue are betas, but I suppose that can be deal with in a follow up bug.
Whiteboard: [june-launch]
There's three ways to fix this:


The workaround
==============

It was discussed on IRC that we might prefer accepting all beta files, and signing them in any case. We would then need a list/log of those accepted and signed beta files that didn't pass automatic validation, to be able to go back to them and validate that they're indeed safe and not malwares.

Security wise, it shouldn't be that much of an issue because beta files can only be submitted on already fully reviewed addons.


The proper solution
===================

1/ refactor the current code that deals with beta versions to get rid of the STATUS_BETA status on files, and instead store it in a File.is_beta field. The status of the file would then be STATUS_UNREVIEWED and then STATUS_LITE when reviewed (either automatically or manually)
2/ maybe create a new review queue specifically for beta files (no need for "full", "prelim", "fast track" and "update" queues, as beta files would all be in the same "prelim" queue).


The hackish solution
====================

1/ Modify the code to list beta files (with the STATUS_BETA status) that aren't signed yet in the "unlisted preliminary review queue" (or in a new queue, as in the "proper solution")
2/ when a beta file is reviewed, if it's accepted, it gets signed, and the beta file is thus visible on the website and available through the beta channel. If it's rejected, it's disabled by mozilla, as for non beta files.


Of the three solutions, only the first one could be implemented quickly I believe. It also has my vote for another reason: it's the way it's always been for beta files, they've never been reviewed. Also, having a manual review on beta files kind of defeats the purpose of having a beta channel. If we want to manually review beta files, maybe we should simply remove the beta channel altogether?

Jorge, Kris, thoughts?
Blocks: 1172690
This is pretty urgent, so we should go with whatever is quickest, at least for now. I'm okay with the first solution, even if it's only temporary.
PR: https://github.com/mozilla/olympia/pull/591

This implements the first solution from comment 2.

As explained in the PR, if this lands, it will revert most (if not all) of what is done to fix bug 1172690 and bug 1160593.

If beta files are always signed, we don't need the special "admin override" anymore.


Each time a beta file is signed, a new log is added. Those logs are in a new "Signed Beta Files Log" menu, in the editor tools, and can be filtered to only show files that passed or failed validation.
Assignee: nobody → mathieu
Blocks: 1070153
As an add-on developer I strongly feel that the solution listed as a "workaround" is in fact the only "proper" solution and I look forward to it being implemented urgently, even if a final decision needs to take longer to allow for further discussion.
Commits pushed to master at https://github.com/mozilla/olympia

https://github.com/mozilla/olympia/commit/2fe776aee7fdcb7700c6d4050653d06ac27a0d3f
Auto sign beta files and log them (bug 1172035)

https://github.com/mozilla/olympia/commit/004a74f14b4fb6c8c3402b4dd19d6464772359c4
Merge pull request #591 from magopian/1172035-auto-sign-beta-files

Auto sign beta files and log them (bug 1172035)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
My bug 1184324 was resolved a dupe of this, but I don't see it's central point covered: upon submission AMO said that "Your add-on was validated with no errors ..." but then continued to complain that it "didn't pass automatic validation".  Which is it?

And as a developer I pretty desperately need more information about _why_ it didn't pass validation, if indeed it did not, so that I can address whatever's wrong.

It's bad enough that I can't do my nightly builds anymore, but now there's also apparently an expected review delay before I can even push a beta build to users?  My extension is complicated, it's terribly useful to get real users to test real code and confirm fixes, before pushing it to everyone.
(In reply to Anthony Lieuallen from comment #8)
> My bug 1184324 was resolved a dupe of this, but I don't see it's central
> point covered: upon submission AMO said that "Your add-on was validated with
> no errors ..." but then continued to complain that it "didn't pass automatic
> validation".  Which is it?

That's bug 1172030. Validations with signing severity above "trivial" block unlisted submissions from being automatically approved. The same applied to betas, but this bug changes that behavior.

> It's bad enough that I can't do my nightly builds anymore, but now there's
> also apparently an expected review delay before I can even push a beta build
> to users?  My extension is complicated, it's terribly useful to get real
> users to test real code and confirm fixes, before pushing it to everyone.

No, once this is pushed to production, all beta submissions will be automatically accepted. If we discover any serious issues after the fact, they may be taken down, but I wouldn't expect that to happen in your case.
Not linked to the previous comments: I saw some intermittent test failures lately on travis, and traced them back to an issue in the fix I made in comment 4.

PR to fixup: https://github.com/mozilla/olympia/pull/608
Commits pushed to master at https://github.com/mozilla/olympia

https://github.com/mozilla/olympia/commit/9a78b5cdd9ffb456416a8cba137ffe7a850ee910
Fixup following #591: LOG id was already used (bug 1172035)

https://github.com/mozilla/olympia/commit/f64ea0471840161b3e0d264a5a8de804ada571d0
Merge pull request #608 from magopian/1172035-fixup-already-used-id

Fixup following #591: LOG id was already used (bug 1172035)
Pushed, built, but betas still getting rejected?
Hello Ian,

it should be deployed to production by the end of this week hopefully.
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.