Split New Theme submission from the rest (#20968)
* Split New Theme Submission from the rest For simplicity new version creation are not modified in this commit, because we're already able to serve different content for them, or gate them however we'd like, as we already have the add-on available. * Display error message when trying to use theme submission flow to upload a non-theme Note: this is not intended as a security measure, it's only to be nice to users who accidentally ended up in the wrong flow.
This commit is contained in:
Родитель
711c11faf4
Коммит
117dd81743
|
@ -69,7 +69,7 @@ def global_settings(request):
|
|||
tools_links.append(
|
||||
{
|
||||
'text': gettext('Submit a New Theme'),
|
||||
'href': reverse('devhub.submit.agreement'),
|
||||
'href': reverse('devhub.submit.theme.agreement'),
|
||||
}
|
||||
)
|
||||
tools_links.append(
|
||||
|
|
|
@ -154,7 +154,7 @@ class TestCommon(TestCase):
|
|||
expected = [
|
||||
('Tools', '#'),
|
||||
('Submit a New Add-on', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.theme.agreement')),
|
||||
('Developer Hub', reverse('devhub.index')),
|
||||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
]
|
||||
|
@ -176,7 +176,7 @@ class TestCommon(TestCase):
|
|||
('Tools', '#'),
|
||||
('Manage My Submissions', reverse('devhub.addons')),
|
||||
('Submit a New Add-on', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.theme.agreement')),
|
||||
('Developer Hub', reverse('devhub.index')),
|
||||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
]
|
||||
|
@ -193,7 +193,7 @@ class TestCommon(TestCase):
|
|||
expected = [
|
||||
('Tools', '#'),
|
||||
('Submit a New Add-on', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.theme.agreement')),
|
||||
('Developer Hub', reverse('devhub.index')),
|
||||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
('Reviewer Tools', reverse('reviewers.dashboard')),
|
||||
|
@ -215,7 +215,7 @@ class TestCommon(TestCase):
|
|||
('Tools', '#'),
|
||||
('Manage My Submissions', reverse('devhub.addons')),
|
||||
('Submit a New Add-on', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.theme.agreement')),
|
||||
('Developer Hub', reverse('devhub.index')),
|
||||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
('Reviewer Tools', reverse('reviewers.dashboard')),
|
||||
|
@ -236,7 +236,7 @@ class TestCommon(TestCase):
|
|||
expected = [
|
||||
('Tools', '#'),
|
||||
('Submit a New Add-on', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.theme.agreement')),
|
||||
('Developer Hub', reverse('devhub.index')),
|
||||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
('Reviewer Tools', reverse('reviewers.dashboard')),
|
||||
|
@ -262,7 +262,7 @@ class TestCommon(TestCase):
|
|||
('Tools', '#'),
|
||||
('Manage My Submissions', reverse('devhub.addons')),
|
||||
('Submit a New Add-on', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.agreement')),
|
||||
('Submit a New Theme', reverse('devhub.submit.theme.agreement')),
|
||||
('Developer Hub', reverse('devhub.index')),
|
||||
('Manage API Keys', reverse('devhub.api_key')),
|
||||
('Reviewer Tools', reverse('reviewers.dashboard')),
|
||||
|
|
|
@ -1019,6 +1019,7 @@ class NewUploadForm(CheckThrottlesMixin, forms.Form):
|
|||
widget=CompatAppSelectWidget(),
|
||||
error_messages={'required': _('Need to select at least one application.')},
|
||||
)
|
||||
theme_specific = forms.BooleanField(required=False, widget=forms.HiddenInput)
|
||||
|
||||
def __init__(self, *args, **kw):
|
||||
self.request = kw.pop('request')
|
||||
|
|
|
@ -51,7 +51,7 @@ from olympia.files.utils import (
|
|||
log = olympia.core.logger.getLogger('z.devhub.task')
|
||||
|
||||
|
||||
def validate(file_, listed=None, final_task=None):
|
||||
def validate(file_, *, final_task=None, theme_specific=False):
|
||||
"""Run the validator on the given File or FileUpload object. If a task has
|
||||
already begun for this file, instead return an AsyncResult object for that
|
||||
task.
|
||||
|
@ -66,7 +66,7 @@ def validate(file_, listed=None, final_task=None):
|
|||
# Import loop.
|
||||
from .utils import Validator
|
||||
|
||||
validator = Validator(file_, listed=listed, final_task=final_task)
|
||||
validator = Validator(file_, theme_specific=theme_specific, final_task=final_task)
|
||||
|
||||
task_id = cache.get(validator.cache_key)
|
||||
|
||||
|
@ -78,23 +78,23 @@ def validate(file_, listed=None, final_task=None):
|
|||
return result
|
||||
|
||||
|
||||
def validate_and_submit(addon, file_, channel):
|
||||
def validate_and_submit(addon, file_, *, theme_specific=False):
|
||||
return validate(
|
||||
file_,
|
||||
listed=(channel == amo.CHANNEL_LISTED),
|
||||
final_task=submit_file.si(addon.pk, file_.pk, channel),
|
||||
theme_specific=theme_specific,
|
||||
final_task=submit_file.si(addon.pk, file_.pk),
|
||||
)
|
||||
|
||||
|
||||
@task
|
||||
@use_primary_db
|
||||
def submit_file(addon_pk, upload_pk, channel):
|
||||
def submit_file(addon_pk, upload_pk):
|
||||
from olympia.devhub.utils import create_version_for_upload
|
||||
|
||||
addon = Addon.unfiltered.get(pk=addon_pk)
|
||||
upload = FileUpload.objects.get(pk=upload_pk)
|
||||
if upload.passed_all_validations:
|
||||
create_version_for_upload(addon, upload, channel)
|
||||
create_version_for_upload(addon, upload, upload.channel)
|
||||
else:
|
||||
log.info(
|
||||
'Skipping version creation for {upload_uuid} that failed '
|
||||
|
@ -195,12 +195,12 @@ def validation_task(fn):
|
|||
|
||||
|
||||
@validation_task
|
||||
def validate_upload(results, upload_pk, channel):
|
||||
def validate_upload(results, upload_pk):
|
||||
"""Validate a FileUpload instance.
|
||||
|
||||
Should only be called directly by Validator."""
|
||||
upload = FileUpload.objects.get(pk=upload_pk)
|
||||
data = validate_file_path(upload.file_path, channel)
|
||||
data = validate_file_path(upload.file_path, upload.channel)
|
||||
return {**results, **json.loads(force_str(data))}
|
||||
|
||||
|
||||
|
@ -271,7 +271,7 @@ def forward_linter_results(results, upload_pk):
|
|||
|
||||
@task
|
||||
@use_primary_db
|
||||
def handle_upload_validation_result(results, upload_pk, channel, is_mozilla_signed):
|
||||
def handle_upload_validation_result(results, upload_pk, is_mozilla_signed):
|
||||
"""Save a set of validation results to a FileUpload instance corresponding
|
||||
to the given upload_pk."""
|
||||
upload = FileUpload.objects.get(pk=upload_pk)
|
||||
|
|
|
@ -24,7 +24,7 @@
|
|||
class="button prominent">{{ _('Submit a New Add-on') }}</a>
|
||||
</p>
|
||||
<p class="submit-theme submit-cta">
|
||||
<a href="{{ url('devhub.submit.agreement') }}"
|
||||
<a href="{{ url('devhub.submit.theme.agreement') }}"
|
||||
class="button prominent">{{ _('Submit a New Theme') }}</a>
|
||||
</p>
|
||||
<div class="item recent-activity">
|
||||
|
|
|
@ -2,6 +2,8 @@
|
|||
|
||||
{% if submit_page == 'version' %}
|
||||
{% set title = _('Submit a New Version') %}
|
||||
{% elif theme_specific %}
|
||||
{% set title = _('Submit a New Theme') %}
|
||||
{% else %}
|
||||
{% set title = _('Submit a New Add-on') %}
|
||||
{% endif %}
|
||||
|
|
|
@ -48,7 +48,7 @@
|
|||
{% endif %}
|
||||
{% endif %}
|
||||
>
|
||||
|
||||
{{ new_addon_form.theme_specific }}
|
||||
{{ new_addon_form.non_field_errors() }}
|
||||
|
||||
<div class="compatible-apps">
|
||||
|
@ -69,11 +69,6 @@
|
|||
</div>
|
||||
{% endif %}
|
||||
|
||||
{% if addon and addon.type == amo.ADDON_STATICTHEME %}
|
||||
{% set wizard_url = url('devhub.submit.version.wizard', addon.slug, channel_param) %}
|
||||
{% elif not addon %}
|
||||
{% set wizard_url = url('devhub.submit.wizard', channel_param) %}
|
||||
{% endif %}
|
||||
{% if wizard_url %}
|
||||
<div class="submission-buttons addon-create-theme-section">
|
||||
<p></p>
|
||||
|
|
|
@ -111,7 +111,7 @@
|
|||
{% if addon %}
|
||||
formaction="{{ url('devhub.submit.version.upload', addon.slug, channel_param) }}"
|
||||
{% else %}
|
||||
formaction="{{ url('devhub.submit.upload', channel_param) }}"
|
||||
formaction="{{ url('devhub.submit.theme.upload', channel_param) }}"
|
||||
{% endif %}
|
||||
>{{ _('Back') }}
|
||||
</button>
|
||||
|
|
|
@ -25,6 +25,8 @@
|
|||
{% endfor %}
|
||||
<li><em><a href="{{ url('devhub.submit.agreement') }}">
|
||||
{{ _('Submit a New Add-on') }}</a></em></li>
|
||||
<li><em><a href="{{ url('devhub.submit.theme.agreement') }}">
|
||||
{{ _('Submit a New Theme') }}</a></em></li>
|
||||
</ul>
|
||||
</li>
|
||||
{% endif %}
|
||||
|
|
|
@ -95,7 +95,7 @@
|
|||
<a class="DevHub-MyAddons-item-buttons-all" href="{{ url('devhub.addons') }}">{{ _('View All Submissions') }}</a>
|
||||
<div class="DevHub-MyAddons-item-buttons-submit">
|
||||
<a href="{{ url('devhub.submit.agreement') }}" class="Button">{{ _('Submit a New Add-on') }}</a>
|
||||
<a href="{{ url('devhub.submit.agreement') }}" class="Button">{{ _('Submit a New Theme') }}</a>
|
||||
<a href="{{ url('devhub.submit.theme.agreement') }}" class="Button">{{ _('Submit a New Theme') }}</a>
|
||||
</div>
|
||||
</div>
|
||||
{% else %}
|
||||
|
@ -107,7 +107,7 @@
|
|||
{% endtrans %}
|
||||
</p>
|
||||
<a href="{{ url('devhub.submit.agreement') }}" class="Button">{{ _('Submit Your First Add-on') }}</a>
|
||||
<a href="{{ url('devhub.submit.agreement') }}" class="Button">{{ _('Submit Your First Theme') }}</a>
|
||||
<a href="{{ url('devhub.submit.theme.agreement') }}" class="Button">{{ _('Submit Your First Theme') }}</a>
|
||||
{% endif %}
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
@ -144,9 +144,9 @@ class TestMeasureValidationTime(UploadMixin, TestCase):
|
|||
calculated_ms + fuzz
|
||||
)
|
||||
|
||||
def handle_upload_validation_result(self, channel=amo.CHANNEL_LISTED):
|
||||
def handle_upload_validation_result(self):
|
||||
results = amo.VALIDATOR_SKELETON_RESULTS.copy()
|
||||
tasks.handle_upload_validation_result(results, self.upload.pk, channel, False)
|
||||
tasks.handle_upload_validation_result(results, self.upload.pk, False)
|
||||
|
||||
def test_track_upload_validation_results_time(self):
|
||||
with self.statsd_timing_mock() as statsd_calls:
|
||||
|
@ -282,21 +282,21 @@ class TestRunAddonsLinter(UploadMixin, ValidatorTestCase):
|
|||
def test_pass_validation(self, _mock):
|
||||
_mock.return_value = {'errors': 0}
|
||||
upload = self.get_upload(abspath=self.valid_path, with_validation=False)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
assert upload.reload().valid
|
||||
|
||||
@mock.patch('olympia.devhub.tasks.run_addons_linter')
|
||||
def test_fail_validation(self, _mock):
|
||||
_mock.return_value = {'errors': 2}
|
||||
upload = self.get_upload(abspath=self.valid_path, with_validation=False)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
assert not upload.reload().valid
|
||||
|
||||
@mock.patch('olympia.devhub.tasks.run_addons_linter')
|
||||
def test_validation_error(self, _mock):
|
||||
_mock.side_effect = Exception
|
||||
upload = self.get_upload(abspath=self.valid_path, with_validation=False)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
upload.reload()
|
||||
validation = upload.processed_validation
|
||||
assert validation
|
||||
|
@ -309,7 +309,7 @@ class TestRunAddonsLinter(UploadMixin, ValidatorTestCase):
|
|||
"""If we sign addons, warn on signed addon submission."""
|
||||
_mock.return_value = self.mock_sign_addon_warning
|
||||
upload = self.get_upload(abspath=self.valid_path, with_validation=False)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
upload.reload()
|
||||
validation = json.loads(upload.validation)
|
||||
assert validation['warnings'] == 1
|
||||
|
@ -318,7 +318,7 @@ class TestRunAddonsLinter(UploadMixin, ValidatorTestCase):
|
|||
@mock.patch('olympia.devhub.tasks.statsd.incr')
|
||||
def test_track_validation_stats(self, mock_statsd_incr):
|
||||
upload = self.get_upload(abspath=self.valid_path, with_validation=False)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
mock_statsd_incr.assert_has_calls(
|
||||
(
|
||||
mock.call('devhub.linter.results.all.success'),
|
||||
|
@ -341,13 +341,13 @@ class TestRunAddonsLinter(UploadMixin, ValidatorTestCase):
|
|||
run_addons_linter_mock.return_value = {'errors': 0}
|
||||
upload = self.get_upload(abspath=self.valid_path, with_validation=False)
|
||||
assert not upload.valid
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
upload.reload()
|
||||
assert upload.valid, upload.validation
|
||||
|
||||
def test_run_linter_fail(self):
|
||||
upload = self.get_upload(abspath=self.invalid_path, with_validation=False)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
upload.reload()
|
||||
assert not upload.valid
|
||||
|
||||
|
@ -670,16 +670,27 @@ class TestSubmitFile(UploadMixin, TestCase):
|
|||
def test_file_passed_all_validations(self):
|
||||
file_ = get_addon_file('valid_webextension.xpi')
|
||||
upload = self.get_upload(abspath=file_, addon=self.addon, version='1.0')
|
||||
tasks.submit_file(self.addon.pk, upload.pk, amo.CHANNEL_LISTED)
|
||||
tasks.submit_file(self.addon.pk, upload.pk)
|
||||
self.create_version_for_upload.assert_called_with(
|
||||
self.addon, upload, amo.CHANNEL_LISTED
|
||||
)
|
||||
|
||||
@mock.patch('olympia.devhub.tasks.FileUpload.passed_all_validations', True)
|
||||
def test_file_passed_all_validations_unlisted(self):
|
||||
file_ = get_addon_file('valid_webextension.xpi')
|
||||
upload = self.get_upload(
|
||||
abspath=file_, addon=self.addon, version='1.0', channel=amo.CHANNEL_UNLISTED
|
||||
)
|
||||
tasks.submit_file(self.addon.pk, upload.pk)
|
||||
self.create_version_for_upload.assert_called_with(
|
||||
self.addon, upload, amo.CHANNEL_UNLISTED
|
||||
)
|
||||
|
||||
@mock.patch('olympia.devhub.tasks.FileUpload.passed_all_validations', False)
|
||||
def test_file_not_passed_all_validations(self):
|
||||
file_ = get_addon_file('valid_webextension.xpi')
|
||||
upload = self.get_upload(abspath=file_, addon=self.addon, version='1.0')
|
||||
tasks.submit_file(self.addon.pk, upload.pk, amo.CHANNEL_LISTED)
|
||||
tasks.submit_file(self.addon.pk, upload.pk)
|
||||
assert not self.create_version_for_upload.called
|
||||
|
||||
|
||||
|
@ -701,7 +712,7 @@ class TestAPIKeyInSubmission(UploadMixin, TestCase):
|
|||
upload = self.get_upload(
|
||||
abspath=self.file, with_validation=False, addon=self.addon, user=self.user
|
||||
)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
|
||||
upload.refresh_from_db()
|
||||
|
||||
|
@ -728,7 +739,7 @@ class TestAPIKeyInSubmission(UploadMixin, TestCase):
|
|||
upload = self.get_upload(
|
||||
abspath=self.file, with_validation=False, addon=self.addon, user=self.user
|
||||
)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
|
||||
upload.refresh_from_db()
|
||||
|
||||
|
@ -758,7 +769,7 @@ class TestAPIKeyInSubmission(UploadMixin, TestCase):
|
|||
upload = self.get_upload(
|
||||
abspath=self.file, with_validation=False, addon=self.addon, user=coauthor
|
||||
)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
|
||||
upload.refresh_from_db()
|
||||
|
||||
|
@ -806,7 +817,7 @@ class TestAPIKeyInSubmission(UploadMixin, TestCase):
|
|||
upload = self.get_upload(
|
||||
abspath=self.file, with_validation=False, user=self.user
|
||||
)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
upload.refresh_from_db()
|
||||
mock_revoke.apply_async.assert_called_with(
|
||||
kwargs={'key_id': self.key.id}, countdown=120
|
||||
|
@ -819,7 +830,7 @@ class TestAPIKeyInSubmission(UploadMixin, TestCase):
|
|||
upload = self.get_upload(
|
||||
abspath=self.file, with_validation=False, user=different_author
|
||||
)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
|
||||
upload.refresh_from_db()
|
||||
|
||||
|
@ -829,7 +840,7 @@ class TestAPIKeyInSubmission(UploadMixin, TestCase):
|
|||
def test_does_not_revoke_safe_webextension(self):
|
||||
file_ = get_addon_file('valid_webextension.xpi')
|
||||
upload = self.get_upload(abspath=file_, with_validation=False, user=self.user)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
|
||||
upload.refresh_from_db()
|
||||
|
||||
|
@ -840,7 +851,7 @@ class TestAPIKeyInSubmission(UploadMixin, TestCase):
|
|||
def test_validation_finishes_if_containing_binary_content(self):
|
||||
file_ = get_addon_file('webextension_containing_binary_files.xpi')
|
||||
upload = self.get_upload(abspath=file_, with_validation=False, user=self.user)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
|
||||
upload.refresh_from_db()
|
||||
|
||||
|
@ -851,7 +862,7 @@ class TestAPIKeyInSubmission(UploadMixin, TestCase):
|
|||
def test_validation_finishes_if_containing_invalid_filename(self):
|
||||
file_ = get_addon_file('invalid_webextension.xpi')
|
||||
upload = self.get_upload(abspath=file_, with_validation=False, user=self.user)
|
||||
tasks.validate(upload, listed=True)
|
||||
tasks.validate(upload)
|
||||
|
||||
upload.refresh_from_db()
|
||||
|
||||
|
|
|
@ -52,34 +52,32 @@ class TestAddonsLinterListed(UploadMixin, TestCase):
|
|||
self.addCleanup(patcher.stop)
|
||||
return patcher.start()
|
||||
|
||||
def check_upload(self, file_upload, listed=True):
|
||||
def test_check_upload(self):
|
||||
"""Check that the given new file upload is validated properly."""
|
||||
# Run validator.
|
||||
utils.Validator(file_upload, listed=listed)
|
||||
|
||||
channel = amo.CHANNEL_LISTED if listed else amo.CHANNEL_UNLISTED
|
||||
utils.Validator(self.file_upload)
|
||||
|
||||
# Make sure we setup the correct validation task.
|
||||
self.mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(file_upload.pk),
|
||||
tasks.validate_upload.s(file_upload.pk, channel),
|
||||
tasks.check_for_api_keys_in_file.s(file_upload.pk),
|
||||
repack_fileupload.s(self.file_upload.pk),
|
||||
tasks.validate_upload.s(self.file_upload.pk),
|
||||
tasks.check_for_api_keys_in_file.s(self.file_upload.pk),
|
||||
chord(
|
||||
[tasks.forward_linter_results.s(file_upload.pk)],
|
||||
call_mad_api.s(file_upload.pk),
|
||||
[tasks.forward_linter_results.s(self.file_upload.pk)],
|
||||
call_mad_api.s(self.file_upload.pk),
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, channel, False),
|
||||
tasks.handle_upload_validation_result.s(self.file_upload.pk, False),
|
||||
)
|
||||
|
||||
def check_file(self, file_):
|
||||
def test_check_file(self):
|
||||
"""Check that the given file is validated properly."""
|
||||
# Mock tasks that we should not execute.
|
||||
repack_fileupload = self.patch('olympia.files.tasks.repack_fileupload')
|
||||
validate_upload = self.patch('olympia.devhub.tasks.validate_upload')
|
||||
|
||||
# Run validator.
|
||||
utils.Validator(file_)
|
||||
utils.Validator(self.file)
|
||||
|
||||
# We shouldn't be attempting to call the `validate_upload` tasks when
|
||||
# dealing with a file.
|
||||
|
@ -89,10 +87,19 @@ class TestAddonsLinterListed(UploadMixin, TestCase):
|
|||
# Make sure we setup the correct validation task.
|
||||
self.mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
tasks.validate_file.s(file_.pk),
|
||||
tasks.handle_file_validation_result.s(file_.pk),
|
||||
tasks.validate_file.s(self.file.pk),
|
||||
tasks.handle_file_validation_result.s(self.file.pk),
|
||||
)
|
||||
|
||||
def test_validate_non_theme_passing_theme_specific_arg(self):
|
||||
repack_fileupload = self.patch('olympia.files.tasks.repack_fileupload')
|
||||
validate_upload = self.patch('olympia.devhub.tasks.validate_upload')
|
||||
|
||||
with self.assertRaises(utils.InvalidAddonType):
|
||||
utils.Validator(self.file_upload, theme_specific=True)
|
||||
assert not repack_fileupload.called
|
||||
assert not validate_upload.called
|
||||
|
||||
@mock.patch.object(utils.Validator, 'get_task')
|
||||
def test_run_once_per_file(self, get_task_mock):
|
||||
"""Tests that only a single validation task is run for a given file."""
|
||||
|
@ -114,10 +121,10 @@ class TestAddonsLinterListed(UploadMixin, TestCase):
|
|||
upload."""
|
||||
get_task_mock.return_value.delay.return_value = mock.Mock(task_id='42')
|
||||
|
||||
assert isinstance(tasks.validate(self.file_upload, listed=True), mock.Mock)
|
||||
assert isinstance(tasks.validate(self.file_upload), mock.Mock)
|
||||
assert get_task_mock.return_value.delay.call_count == 1
|
||||
|
||||
assert isinstance(tasks.validate(self.file_upload, listed=True), AsyncResult)
|
||||
assert isinstance(tasks.validate(self.file_upload), AsyncResult)
|
||||
assert get_task_mock.return_value.delay.call_count == 1
|
||||
|
||||
def test_cache_key(self):
|
||||
|
@ -125,12 +132,19 @@ class TestAddonsLinterListed(UploadMixin, TestCase):
|
|||
|
||||
assert (
|
||||
utils.Validator(self.file).cache_key
|
||||
== f'validation-task:files.File:{self.file.pk}:None'
|
||||
== f'validation-task:files.File:{self.file.pk}:2'
|
||||
)
|
||||
|
||||
assert utils.Validator(
|
||||
self.file_upload, listed=False
|
||||
).cache_key == 'validation-task:files.FileUpload:{}:False'.format(
|
||||
self.file_upload
|
||||
).cache_key == 'validation-task:files.FileUpload:{}:2'.format(
|
||||
self.file_upload.pk
|
||||
)
|
||||
|
||||
self.file_upload.update(channel=amo.CHANNEL_UNLISTED)
|
||||
assert utils.Validator(
|
||||
self.file_upload
|
||||
).cache_key == 'validation-task:files.FileUpload:{}:1'.format(
|
||||
self.file_upload.pk
|
||||
)
|
||||
|
||||
|
@ -316,20 +330,19 @@ class TestValidator(UploadMixin, TestCase):
|
|||
def test_appends_final_task_for_file_uploads(self, mock_chain):
|
||||
final_task = mock.Mock()
|
||||
file_upload = self.get_upload('webextension.xpi', with_validation=False)
|
||||
channel = amo.CHANNEL_LISTED
|
||||
|
||||
utils.Validator(file_upload, listed=True, final_task=final_task)
|
||||
utils.Validator(file_upload, final_task=final_task)
|
||||
|
||||
mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(file_upload.pk),
|
||||
tasks.validate_upload.s(file_upload.pk, channel),
|
||||
tasks.validate_upload.s(file_upload.pk),
|
||||
tasks.check_for_api_keys_in_file.s(file_upload.pk),
|
||||
chord(
|
||||
[tasks.forward_linter_results.s(file_upload.pk)],
|
||||
call_mad_api.s(file_upload.pk),
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, channel, False),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, False),
|
||||
final_task,
|
||||
)
|
||||
|
||||
|
@ -351,14 +364,13 @@ class TestValidator(UploadMixin, TestCase):
|
|||
def test_adds_run_yara_when_enabled(self, mock_chain):
|
||||
self.create_switch('enable-yara', active=True)
|
||||
file_upload = self.get_upload('webextension.xpi', with_validation=False)
|
||||
channel = amo.CHANNEL_LISTED
|
||||
|
||||
utils.Validator(file_upload, listed=True)
|
||||
utils.Validator(file_upload)
|
||||
|
||||
mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(file_upload.pk),
|
||||
tasks.validate_upload.s(file_upload.pk, channel),
|
||||
tasks.validate_upload.s(file_upload.pk),
|
||||
tasks.check_for_api_keys_in_file.s(file_upload.pk),
|
||||
chord(
|
||||
[
|
||||
|
@ -367,41 +379,39 @@ class TestValidator(UploadMixin, TestCase):
|
|||
],
|
||||
call_mad_api.s(file_upload.pk),
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, channel, False),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, False),
|
||||
)
|
||||
|
||||
@mock.patch('olympia.devhub.utils.chain')
|
||||
def test_does_not_add_run_yara_when_disabled(self, mock_chain):
|
||||
self.create_switch('enable-yara', active=False)
|
||||
file_upload = self.get_upload('webextension.xpi', with_validation=False)
|
||||
channel = amo.CHANNEL_LISTED
|
||||
|
||||
utils.Validator(file_upload, listed=True)
|
||||
utils.Validator(file_upload)
|
||||
|
||||
mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(file_upload.pk),
|
||||
tasks.validate_upload.s(file_upload.pk, channel),
|
||||
tasks.validate_upload.s(file_upload.pk),
|
||||
tasks.check_for_api_keys_in_file.s(file_upload.pk),
|
||||
chord(
|
||||
[tasks.forward_linter_results.s(file_upload.pk)],
|
||||
call_mad_api.s(file_upload.pk),
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, channel, False),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, False),
|
||||
)
|
||||
|
||||
@mock.patch('olympia.devhub.utils.chain')
|
||||
def test_adds_run_customs_when_enabled(self, mock_chain):
|
||||
self.create_switch('enable-customs', active=True)
|
||||
file_upload = self.get_upload('webextension.xpi', with_validation=False)
|
||||
channel = amo.CHANNEL_LISTED
|
||||
|
||||
utils.Validator(file_upload, listed=True)
|
||||
utils.Validator(file_upload)
|
||||
|
||||
mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(file_upload.pk),
|
||||
tasks.validate_upload.s(file_upload.pk, channel),
|
||||
tasks.validate_upload.s(file_upload.pk),
|
||||
tasks.check_for_api_keys_in_file.s(file_upload.pk),
|
||||
chord(
|
||||
[
|
||||
|
@ -410,27 +420,26 @@ class TestValidator(UploadMixin, TestCase):
|
|||
],
|
||||
call_mad_api.s(file_upload.pk),
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, channel, False),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, False),
|
||||
)
|
||||
|
||||
@mock.patch('olympia.devhub.utils.chain')
|
||||
def test_does_not_add_run_customs_when_disabled(self, mock_chain):
|
||||
self.create_switch('enable-customs', active=False)
|
||||
file_upload = self.get_upload('webextension.xpi', with_validation=False)
|
||||
channel = amo.CHANNEL_LISTED
|
||||
|
||||
utils.Validator(file_upload, listed=True)
|
||||
utils.Validator(file_upload)
|
||||
|
||||
mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(file_upload.pk),
|
||||
tasks.validate_upload.s(file_upload.pk, channel),
|
||||
tasks.validate_upload.s(file_upload.pk),
|
||||
tasks.check_for_api_keys_in_file.s(file_upload.pk),
|
||||
chord(
|
||||
[tasks.forward_linter_results.s(file_upload.pk)],
|
||||
call_mad_api.s(file_upload.pk),
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, channel, False),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, False),
|
||||
)
|
||||
|
||||
@mock.patch('olympia.devhub.utils.chain')
|
||||
|
@ -438,14 +447,13 @@ class TestValidator(UploadMixin, TestCase):
|
|||
self.create_switch('enable-customs', active=True)
|
||||
self.create_switch('enable-yara', active=True)
|
||||
file_upload = self.get_upload('webextension.xpi', with_validation=False)
|
||||
channel = amo.CHANNEL_LISTED
|
||||
|
||||
utils.Validator(file_upload, listed=True)
|
||||
utils.Validator(file_upload)
|
||||
|
||||
mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(file_upload.pk),
|
||||
tasks.validate_upload.s(file_upload.pk, channel),
|
||||
tasks.validate_upload.s(file_upload.pk),
|
||||
tasks.check_for_api_keys_in_file.s(file_upload.pk),
|
||||
chord(
|
||||
[
|
||||
|
@ -455,7 +463,7 @@ class TestValidator(UploadMixin, TestCase):
|
|||
],
|
||||
call_mad_api.s(file_upload.pk),
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, channel, False),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, False),
|
||||
)
|
||||
|
||||
@mock.patch('olympia.devhub.utils.chain')
|
||||
|
@ -463,14 +471,13 @@ class TestValidator(UploadMixin, TestCase):
|
|||
self.create_switch('enable-customs', active=True)
|
||||
self.create_switch('enable-yara', active=True)
|
||||
file_upload = self.get_upload('webextension.xpi', with_validation=False)
|
||||
channel = amo.CHANNEL_LISTED
|
||||
|
||||
utils.Validator(file_upload, listed=True)
|
||||
utils.Validator(file_upload)
|
||||
|
||||
mock_chain.assert_called_once_with(
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(file_upload.pk),
|
||||
tasks.validate_upload.s(file_upload.pk, channel),
|
||||
tasks.validate_upload.s(file_upload.pk),
|
||||
tasks.check_for_api_keys_in_file.s(file_upload.pk),
|
||||
chord(
|
||||
[
|
||||
|
@ -480,18 +487,17 @@ class TestValidator(UploadMixin, TestCase):
|
|||
],
|
||||
call_mad_api.s(file_upload.pk),
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, channel, False),
|
||||
tasks.handle_upload_validation_result.s(file_upload.pk, False),
|
||||
)
|
||||
|
||||
def test_create_file_upload_tasks(self):
|
||||
self.create_switch('enable-customs', active=True)
|
||||
self.create_switch('enable-yara', active=True)
|
||||
file_upload = self.get_upload('webextension.xpi', with_validation=False)
|
||||
channel = amo.CHANNEL_LISTED
|
||||
validator = utils.Validator(file_upload, listed=True)
|
||||
validator = utils.Validator(file_upload)
|
||||
|
||||
tasks = validator.create_file_upload_tasks(
|
||||
upload_pk=file_upload.pk, channel=channel, is_mozilla_signed=False
|
||||
upload_pk=file_upload.pk, is_mozilla_signed=False
|
||||
)
|
||||
|
||||
assert isinstance(tasks, list)
|
||||
|
|
|
@ -1,12 +1,12 @@
|
|||
import json
|
||||
import os
|
||||
import zipfile
|
||||
from datetime import datetime, timedelta
|
||||
from unittest import mock
|
||||
from urllib.parse import quote, urlencode
|
||||
|
||||
from django.conf import settings
|
||||
from django.core import mail
|
||||
from django.core.files.storage import default_storage as storage
|
||||
from django.test import RequestFactory
|
||||
from django.urls import reverse
|
||||
from django.utils.encoding import force_str
|
||||
|
@ -1097,12 +1097,14 @@ class TestUpload(UploadMixin, TestCase):
|
|||
super().setUp()
|
||||
self.client.force_login(UserProfile.objects.get(email='regular@mozilla.com'))
|
||||
self.url = reverse('devhub.upload')
|
||||
self.image_path = get_image_path('animated.png')
|
||||
self.xpi_path = self.file_path('webextension_no_id.xpi')
|
||||
|
||||
def post(self, **kwargs):
|
||||
# Has to be a binary, non xpi file.
|
||||
data = open(self.image_path, 'rb')
|
||||
return self.client.post(self.url, {'upload': data}, **kwargs)
|
||||
def post(self, theme_specific=False, **kwargs):
|
||||
data = {
|
||||
'upload': open(self.xpi_path, 'rb'),
|
||||
'theme_specific': 'True' if theme_specific else 'False',
|
||||
}
|
||||
return self.client.post(self.url, data, **kwargs)
|
||||
|
||||
def test_login_required(self):
|
||||
self.client.logout()
|
||||
|
@ -1113,9 +1115,17 @@ class TestUpload(UploadMixin, TestCase):
|
|||
self.post()
|
||||
|
||||
upload = FileUpload.objects.filter().order_by('-created').first()
|
||||
assert 'animated.png' in upload.name
|
||||
data = open(self.image_path, 'rb').read()
|
||||
assert storage.open(upload.file_path).read() == data
|
||||
assert 'webextension_no_id.xpi' in upload.name
|
||||
assert upload.file_path.endswith('.zip') # and not .xpi!
|
||||
# Can't compare the data bit by bit because we're repacking, look
|
||||
# inside to check it contains the same thing.
|
||||
manifest_data = json.loads(
|
||||
zipfile.ZipFile(upload.file_path).read('manifest.json')
|
||||
)
|
||||
original_manifest_data = json.loads(
|
||||
zipfile.ZipFile(self.xpi_path).read('manifest.json')
|
||||
)
|
||||
assert manifest_data == original_manifest_data
|
||||
|
||||
def test_fileupload_metadata(self):
|
||||
user = UserProfile.objects.get(email='regular@mozilla.com')
|
||||
|
@ -1126,7 +1136,8 @@ class TestUpload(UploadMixin, TestCase):
|
|||
assert upload.source == amo.UPLOAD_SOURCE_DEVHUB
|
||||
assert upload.ip_address == '4.8.15.16.23.42'
|
||||
|
||||
def test_fileupload_validation(self):
|
||||
def test_fileupload_validation_not_a_xpi_file(self):
|
||||
self.xpi_path = get_image_path('animated.png') # not a xpi file.
|
||||
self.post()
|
||||
upload = FileUpload.objects.filter().order_by('-created').first()
|
||||
assert upload.validation
|
||||
|
@ -1148,6 +1159,7 @@ class TestUpload(UploadMixin, TestCase):
|
|||
def test_redirect(self):
|
||||
response = self.post()
|
||||
upload = FileUpload.objects.get()
|
||||
assert upload.channel == amo.CHANNEL_LISTED
|
||||
url = reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])
|
||||
self.assert3xx(response, url)
|
||||
|
||||
|
@ -1161,9 +1173,43 @@ class TestUpload(UploadMixin, TestCase):
|
|||
"""Unlisted addons are validated as "self hosted" addons."""
|
||||
validate_mock.return_value = json.dumps(amo.VALIDATOR_SKELETON_RESULTS)
|
||||
self.url = reverse('devhub.upload_unlisted')
|
||||
self.post()
|
||||
# Make sure it was called with listed=False.
|
||||
assert not validate_mock.call_args[1]['listed']
|
||||
response = self.post()
|
||||
upload = FileUpload.objects.get()
|
||||
assert upload.channel == amo.CHANNEL_UNLISTED
|
||||
url = reverse('devhub.upload_detail', args=[upload.uuid.hex, 'json'])
|
||||
self.assert3xx(response, url)
|
||||
|
||||
def test_upload_extension_in_theme_specific_flow(self):
|
||||
self.url = reverse('devhub.upload_unlisted')
|
||||
response = self.post(theme_specific=True)
|
||||
assert response.status_code == 400
|
||||
assert response.json() == {
|
||||
'validation': {
|
||||
'errors': 1,
|
||||
'warnings': 0,
|
||||
'notices': 0,
|
||||
'success': False,
|
||||
'compatibility_summary': {'notices': 0, 'errors': 0, 'warnings': 0},
|
||||
'metadata': {'listed': True},
|
||||
'messages': [
|
||||
{
|
||||
'tier': 1,
|
||||
'type': 'error',
|
||||
'id': ['validation', 'messages', ''],
|
||||
'message': (
|
||||
'This add-on is not a theme. Use the <a href="http://test'
|
||||
'server/en-US/developers/addon/submit/upload-unlisted">'
|
||||
'Submit a New Add-on</a> page to submit extensions.'
|
||||
),
|
||||
'description': [],
|
||||
'compatibility_type': None,
|
||||
'extra': True,
|
||||
}
|
||||
],
|
||||
'message_tree': {},
|
||||
'ending_tier': 5,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
class TestUploadDetail(UploadMixin, TestCase):
|
||||
|
@ -1689,10 +1735,10 @@ class TestRedirects(TestCase):
|
|||
response = self.client.get(url, follow=True)
|
||||
self.assert3xx(response, reverse('devhub.addons.versions', args=['a3615']), 301)
|
||||
|
||||
def test_lwt_submit_redirects_to_addon_submit(self):
|
||||
url = reverse('devhub.themes.submit')
|
||||
def test_lwt_submit_redirects_to_theme_submit(self):
|
||||
url = reverse('devhub.submit.theme.old_lwt_flow')
|
||||
response = self.client.get(url, follow=True)
|
||||
self.assert3xx(response, reverse('devhub.submit.distribution'), 302)
|
||||
self.assert3xx(response, reverse('devhub.submit.theme.distribution'), 302)
|
||||
|
||||
|
||||
class TestHasCompleteMetadataRedirects(TestCase):
|
||||
|
|
|
@ -113,6 +113,20 @@ class TestAddonSubmitAgreement(TestSubmitBase):
|
|||
},
|
||||
)
|
||||
assert response.status_code == 302
|
||||
self.assert3xx(response, reverse('devhub.submit.distribution'))
|
||||
self.user.reload()
|
||||
self.assertCloseToNow(self.user.read_dev_agreement)
|
||||
|
||||
def test_set_read_dev_agreement_theme(self):
|
||||
response = self.client.post(
|
||||
reverse('devhub.submit.theme.agreement'),
|
||||
{
|
||||
'distribution_agreement': 'on',
|
||||
'review_policy': 'on',
|
||||
},
|
||||
)
|
||||
assert response.status_code == 302
|
||||
self.assert3xx(response, reverse('devhub.submit.theme.distribution'))
|
||||
self.user.reload()
|
||||
self.assertCloseToNow(self.user.read_dev_agreement)
|
||||
|
||||
|
@ -396,6 +410,24 @@ class TestAddonSubmitDistribution(TestCase):
|
|||
response = self.client.get(reverse('devhub.submit.distribution'), follow=True)
|
||||
self.assert3xx(response, reverse('devhub.submit.agreement'))
|
||||
|
||||
def test_redirect_back_to_agreement_theme(self):
|
||||
self.user.update(read_dev_agreement=None)
|
||||
|
||||
response = self.client.get(
|
||||
reverse('devhub.submit.theme.distribution'), follow=True
|
||||
)
|
||||
self.assert3xx(response, reverse('devhub.submit.theme.agreement'))
|
||||
|
||||
# read_dev_agreement needs to be a more recent date than
|
||||
# the setting.
|
||||
set_config('last_dev_agreement_change_date', '2019-06-10 00:00')
|
||||
before_agreement_last_changed = datetime(2019, 6, 10) - timedelta(days=1)
|
||||
self.user.update(read_dev_agreement=before_agreement_last_changed)
|
||||
response = self.client.get(
|
||||
reverse('devhub.submit.theme.distribution'), follow=True
|
||||
)
|
||||
self.assert3xx(response, reverse('devhub.submit.theme.agreement'))
|
||||
|
||||
def test_redirect_back_to_agreement_if_restricted(self):
|
||||
IPNetworkUserRestriction.objects.create(network='127.0.0.1/32')
|
||||
response = self.client.get(reverse('devhub.submit.distribution'), follow=True)
|
||||
|
@ -413,6 +445,12 @@ class TestAddonSubmitDistribution(TestCase):
|
|||
)
|
||||
self.assert3xx(response, reverse('devhub.submit.upload', args=['unlisted']))
|
||||
|
||||
def test_listed_redirects_to_next_step_theme(self):
|
||||
response = self.client.post(
|
||||
reverse('devhub.submit.theme.distribution'), {'channel': 'listed'}
|
||||
)
|
||||
self.assert3xx(response, reverse('devhub.submit.theme.upload', args=['listed']))
|
||||
|
||||
def test_channel_selection_error_shown(self):
|
||||
url = reverse('devhub.submit.distribution')
|
||||
# First load should have no error
|
||||
|
@ -441,7 +479,6 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
self.client.force_login(UserProfile.objects.get(email='regular@mozilla.com'))
|
||||
self.user = UserProfile.objects.get(email='regular@mozilla.com')
|
||||
self.user.update(last_login_ip='192.168.1.1')
|
||||
self.client.post(reverse('devhub.submit.agreement'))
|
||||
self.upload = self.get_upload('webextension_no_id.xpi', user=self.user)
|
||||
self.statsd_incr_mock = self.patch('olympia.devhub.views.statsd.incr')
|
||||
|
||||
|
@ -452,6 +489,7 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
listed=True,
|
||||
status_code=200,
|
||||
url=None,
|
||||
theme=False,
|
||||
extra_kwargs=None,
|
||||
):
|
||||
if compatible_apps is None:
|
||||
|
@ -460,9 +498,8 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
'upload': self.upload.uuid.hex,
|
||||
'compatible_apps': [p.id for p in compatible_apps],
|
||||
}
|
||||
url = url or reverse(
|
||||
'devhub.submit.upload', args=['listed' if listed else 'unlisted']
|
||||
)
|
||||
urlname = 'devhub.submit.upload' if not theme else 'devhub.submit.theme.upload'
|
||||
url = url or reverse(urlname, args=['listed' if listed else 'unlisted'])
|
||||
response = self.client.post(url, data, follow=True, **(extra_kwargs or {}))
|
||||
assert response.status_code == status_code
|
||||
if not expect_errors:
|
||||
|
@ -530,7 +567,7 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
# We're not passing `expected_errors=True`, so if there was any errors
|
||||
# like "This name is already in use. Please choose another one", the
|
||||
# test would fail.
|
||||
response = self.post()
|
||||
response = self.post(theme=True)
|
||||
# Kind of redundant with the `self.post()` above: we just want to make
|
||||
# really sure there's no errors raised by posting an add-on with a name
|
||||
# that is already used by an unlisted add-on.
|
||||
|
@ -619,9 +656,9 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
== amo.DEFAULT_WEBEXT_MIN_VERSION_GECKO_ANDROID
|
||||
)
|
||||
|
||||
def test_static_theme_wizard_button_shown(self):
|
||||
def test_theme_variant_has_theme_stuff_visible(self):
|
||||
response = self.client.get(
|
||||
reverse('devhub.submit.upload', args=['listed']), follow=True
|
||||
reverse('devhub.submit.theme.upload', args=['listed']), follow=True
|
||||
)
|
||||
assert response.status_code == 200
|
||||
doc = pq(response.content)
|
||||
|
@ -629,9 +666,10 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
assert doc('#wizardlink').attr('href') == (
|
||||
reverse('devhub.submit.wizard', args=['listed'])
|
||||
)
|
||||
assert doc('#id_theme_specific').attr('value') == 'True'
|
||||
|
||||
response = self.client.get(
|
||||
reverse('devhub.submit.upload', args=['unlisted']), follow=True
|
||||
reverse('devhub.submit.theme.upload', args=['unlisted']), follow=True
|
||||
)
|
||||
assert response.status_code == 200
|
||||
doc = pq(response.content)
|
||||
|
@ -639,6 +677,24 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
assert doc('#wizardlink').attr('href') == (
|
||||
reverse('devhub.submit.wizard', args=['unlisted'])
|
||||
)
|
||||
assert doc('#id_theme_specific').attr('value') == 'True'
|
||||
|
||||
def test_non_theme_variant_has_theme_stuff_hidden(self):
|
||||
response = self.client.get(
|
||||
reverse('devhub.submit.upload', args=['listed']), follow=True
|
||||
)
|
||||
assert response.status_code == 200
|
||||
doc = pq(response.content)
|
||||
assert not doc('#wizardlink')
|
||||
assert doc('#id_theme_specific').attr('value') == 'False'
|
||||
|
||||
response = self.client.get(
|
||||
reverse('devhub.submit.upload', args=['unlisted']), follow=True
|
||||
)
|
||||
assert response.status_code == 200
|
||||
doc = pq(response.content)
|
||||
assert not doc('#wizardlink')
|
||||
assert doc('#id_theme_specific').attr('value') == 'False'
|
||||
|
||||
def test_static_theme_submit_listed(self):
|
||||
assert Addon.objects.count() == 0
|
||||
|
@ -646,7 +702,7 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
settings.ROOT, 'src/olympia/devhub/tests/addons/static_theme.zip'
|
||||
)
|
||||
self.upload = self.get_upload(abspath=path, user=self.user)
|
||||
response = self.post()
|
||||
response = self.post(theme=True)
|
||||
addon = Addon.objects.get()
|
||||
self.assert3xx(response, reverse('devhub.submit.details', args=[addon.slug]))
|
||||
assert addon.current_version.file.file.name.endswith(
|
||||
|
@ -664,7 +720,7 @@ class TestAddonSubmitUpload(UploadMixin, TestCase):
|
|||
settings.ROOT, 'src/olympia/devhub/tests/addons/static_theme.zip'
|
||||
)
|
||||
self.upload = self.get_upload(abspath=path, user=self.user)
|
||||
response = self.post(listed=False)
|
||||
response = self.post(listed=False, theme=True)
|
||||
addon = Addon.unfiltered.get()
|
||||
latest_version = addon.find_latest_version(channel=amo.CHANNEL_UNLISTED)
|
||||
self.assert3xx(response, reverse('devhub.submit.finish', args=[addon.slug]))
|
||||
|
|
|
@ -183,6 +183,22 @@ urlpatterns = decorate(
|
|||
views.submit_addon_upload,
|
||||
name='devhub.submit.upload',
|
||||
),
|
||||
# Theme-specific submission pages
|
||||
re_path(
|
||||
r'^addon/submit/theme/agreement$',
|
||||
views.submit_theme,
|
||||
name='devhub.submit.theme.agreement',
|
||||
),
|
||||
re_path(
|
||||
r'^addon/submit/theme/distribution$',
|
||||
views.submit_theme_distribution,
|
||||
name='devhub.submit.theme.distribution',
|
||||
),
|
||||
re_path(
|
||||
r'^addon/submit/theme/upload-(?P<channel>listed|unlisted)$',
|
||||
views.submit_theme_upload,
|
||||
name='devhub.submit.theme.upload',
|
||||
),
|
||||
re_path(
|
||||
r'^addon/submit/wizard-(?P<channel>listed|unlisted)$',
|
||||
views.submit_addon_theme_wizard,
|
||||
|
@ -241,8 +257,8 @@ urlpatterns = decorate(
|
|||
# Old LWT Theme submission.
|
||||
re_path(
|
||||
r'^theme/submit/?$',
|
||||
lambda r: redirect('devhub.submit.agreement'),
|
||||
name='devhub.themes.submit',
|
||||
lambda r: redirect('devhub.submit.theme.agreement'),
|
||||
name='devhub.submit.theme.old_lwt_flow',
|
||||
),
|
||||
# Add-on SDK page
|
||||
re_path(r'builder$', lambda r: redirect(views.MDN_BASE)),
|
||||
|
|
|
@ -3,6 +3,7 @@ import uuid
|
|||
from django.conf import settings
|
||||
from django.db import transaction
|
||||
from django.forms import ValidationError
|
||||
from django.urls import reverse
|
||||
from django.utils.translation import gettext
|
||||
|
||||
import waffle
|
||||
|
@ -11,6 +12,7 @@ from django_statsd.clients import statsd
|
|||
|
||||
import olympia.core.logger
|
||||
from olympia import amo, core
|
||||
from olympia.amo.templatetags.jinja_helpers import absolutify
|
||||
from olympia.amo.urlresolvers import linkify_and_clean
|
||||
from olympia.files.models import File, FileUpload
|
||||
from olympia.files.tasks import repack_fileupload
|
||||
|
@ -169,24 +171,23 @@ def fix_addons_linter_output(validation, channel):
|
|||
}
|
||||
|
||||
|
||||
class InvalidAddonType(ValidationError):
|
||||
pass
|
||||
|
||||
|
||||
class Validator:
|
||||
"""
|
||||
Class which handles creating or fetching validation results for File
|
||||
Class which handles creating and running validation tasks for File
|
||||
and FileUpload instances.
|
||||
|
||||
It forwards the actual validation to `devhub.tasks:validate_upload`
|
||||
and `devhub.tasks:validate_file` but implements shortcuts for
|
||||
legacy add-ons and search plugins to avoid running the linter.
|
||||
"""
|
||||
|
||||
def __init__(self, file_, addon=None, listed=None, final_task=None):
|
||||
def __init__(self, file_, *, addon=None, theme_specific=False, final_task=None):
|
||||
self.addon = addon
|
||||
self.file = None
|
||||
self.prev_file = None
|
||||
|
||||
if isinstance(file_, FileUpload):
|
||||
assert listed is not None
|
||||
channel = amo.CHANNEL_LISTED if listed else amo.CHANNEL_UNLISTED
|
||||
channel = file_.channel
|
||||
is_mozilla_signed = False
|
||||
|
||||
# We're dealing with a bare file upload. Try to extract the
|
||||
|
@ -195,6 +196,29 @@ class Validator:
|
|||
try:
|
||||
addon_data = parse_addon(file_, minimal=True)
|
||||
is_mozilla_signed = addon_data.get('is_mozilla_signed_extension', False)
|
||||
# If trying to upload a non-theme in the theme specific flow,
|
||||
# raise an error immediately and don't validate. We don't care
|
||||
# about the opposite: if a developer tries to upload a theme
|
||||
# using the "non-theme" flow, that works.
|
||||
if theme_specific and addon_data['type'] != amo.ADDON_STATICTHEME:
|
||||
channel_text = amo.CHANNEL_CHOICES_API[channel]
|
||||
raise InvalidAddonType(
|
||||
gettext(
|
||||
'This add-on is not a theme. '
|
||||
'Use the <a href="{link}">Submit a New Add-on</a> '
|
||||
'page to submit extensions.'
|
||||
).format(
|
||||
link=absolutify(
|
||||
reverse('devhub.submit.upload', args=[channel_text])
|
||||
)
|
||||
),
|
||||
)
|
||||
except InvalidAddonType:
|
||||
log.error(
|
||||
'Tried to validate non-theme upload %s using theme specific flow',
|
||||
file_.uuid,
|
||||
)
|
||||
raise
|
||||
except ValidationError as form_error:
|
||||
log.info(
|
||||
'could not parse addon for upload {}: {}'.format(
|
||||
|
@ -208,14 +232,9 @@ class Validator:
|
|||
assert not file_.validation
|
||||
|
||||
validation_tasks = self.create_file_upload_tasks(
|
||||
upload_pk=file_.pk, channel=channel, is_mozilla_signed=is_mozilla_signed
|
||||
upload_pk=file_.pk, is_mozilla_signed=is_mozilla_signed
|
||||
)
|
||||
elif isinstance(file_, File):
|
||||
# The listed flag for a File object should always come from
|
||||
# the status of its owner Addon. If the caller tries to override
|
||||
# this, something is wrong.
|
||||
assert listed is None
|
||||
|
||||
channel = file_.version.channel
|
||||
is_mozilla_signed = file_.is_mozilla_signed_extension
|
||||
|
||||
|
@ -240,14 +259,14 @@ class Validator:
|
|||
# same object do not result in duplicate tasks.
|
||||
opts = file_._meta
|
||||
self.cache_key = 'validation-task:{}.{}:{}:{}'.format(
|
||||
opts.app_label, opts.object_name, file_.pk, listed
|
||||
opts.app_label, opts.object_name, file_.pk, channel
|
||||
)
|
||||
|
||||
def get_task(self):
|
||||
"""Return task chain to execute to trigger validation."""
|
||||
return self.task
|
||||
|
||||
def create_file_upload_tasks(self, upload_pk, channel, is_mozilla_signed):
|
||||
def create_file_upload_tasks(self, upload_pk, is_mozilla_signed):
|
||||
"""
|
||||
This method creates the validation chain used during the submission
|
||||
process, combining tasks in parallel (chord) with tasks chained
|
||||
|
@ -264,12 +283,10 @@ class Validator:
|
|||
return [
|
||||
tasks.create_initial_validation_results.si(),
|
||||
repack_fileupload.s(upload_pk),
|
||||
tasks.validate_upload.s(upload_pk, channel),
|
||||
tasks.validate_upload.s(upload_pk),
|
||||
tasks.check_for_api_keys_in_file.s(upload_pk),
|
||||
chord(tasks_in_parallel, call_mad_api.s(upload_pk)),
|
||||
tasks.handle_upload_validation_result.s(
|
||||
upload_pk, channel, is_mozilla_signed
|
||||
),
|
||||
tasks.handle_upload_validation_result.s(upload_pk, is_mozilla_signed),
|
||||
]
|
||||
|
||||
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
import datetime
|
||||
import os
|
||||
import time
|
||||
from copy import deepcopy
|
||||
from urllib.parse import quote
|
||||
from uuid import UUID, uuid4
|
||||
|
||||
|
@ -49,6 +50,7 @@ from olympia.amo.utils import (
|
|||
)
|
||||
from olympia.api.models import APIKey, APIKeyConfirmation
|
||||
from olympia.devhub.decorators import dev_required, no_admin_disabled
|
||||
from olympia.devhub.file_validation_annotations import insert_validation_message
|
||||
from olympia.devhub.models import BlogPost, RssKey
|
||||
from olympia.devhub.utils import (
|
||||
extract_theme_properties,
|
||||
|
@ -553,6 +555,7 @@ def validate_addon(request):
|
|||
|
||||
|
||||
def handle_upload(
|
||||
*,
|
||||
filedata,
|
||||
request,
|
||||
channel,
|
||||
|
@ -560,6 +563,7 @@ def handle_upload(
|
|||
is_standalone=False,
|
||||
submit=False,
|
||||
source=amo.UPLOAD_SOURCE_DEVHUB,
|
||||
theme_specific=False,
|
||||
):
|
||||
upload = FileUpload.from_post(
|
||||
filedata,
|
||||
|
@ -570,12 +574,10 @@ def handle_upload(
|
|||
source=source,
|
||||
user=request.user,
|
||||
)
|
||||
|
||||
if submit:
|
||||
tasks.validate_and_submit(addon, upload, channel=channel)
|
||||
tasks.validate_and_submit(addon, upload, theme_specific=theme_specific)
|
||||
else:
|
||||
tasks.validate(upload, listed=(channel == amo.CHANNEL_LISTED))
|
||||
|
||||
tasks.validate(upload, theme_specific=theme_specific)
|
||||
return upload
|
||||
|
||||
|
||||
|
@ -584,13 +586,25 @@ def handle_upload(
|
|||
def upload(request, channel='listed', addon=None, is_standalone=False):
|
||||
channel = amo.CHANNEL_CHOICES_LOOKUP[channel]
|
||||
filedata = request.FILES['upload']
|
||||
upload = handle_upload(
|
||||
filedata=filedata,
|
||||
request=request,
|
||||
addon=addon,
|
||||
is_standalone=is_standalone,
|
||||
channel=channel,
|
||||
theme_specific = django_forms.BooleanField().to_python(
|
||||
request.POST.get('theme_specific')
|
||||
)
|
||||
try:
|
||||
upload = handle_upload(
|
||||
filedata=filedata,
|
||||
request=request,
|
||||
addon=addon,
|
||||
is_standalone=is_standalone,
|
||||
channel=channel,
|
||||
theme_specific=theme_specific,
|
||||
)
|
||||
except django_forms.ValidationError as exc:
|
||||
# handle_upload() should be firing tasks to do validation. If it raised
|
||||
# a ValidationError, that means we failed before even reaching those
|
||||
# tasks, and need to return an error response immediately.
|
||||
results = deepcopy(amo.VALIDATOR_SKELETON_RESULTS)
|
||||
insert_validation_message(results, message=exc.message)
|
||||
return JsonResponse({'validation': results}, status=400)
|
||||
if addon:
|
||||
return redirect('devhub.upload_detail_for_version', addon.slug, upload.uuid.hex)
|
||||
elif is_standalone:
|
||||
|
@ -1239,6 +1253,15 @@ def submit_addon(request):
|
|||
)
|
||||
|
||||
|
||||
@login_required
|
||||
def submit_theme(request):
|
||||
return render_agreement(
|
||||
request=request,
|
||||
template='devhub/addons/submit/start.html',
|
||||
next_step='devhub.submit.theme.distribution',
|
||||
)
|
||||
|
||||
|
||||
@dev_required
|
||||
def submit_version_agreement(request, addon_id, addon):
|
||||
return render_agreement(
|
||||
|
@ -1285,6 +1308,13 @@ def submit_addon_distribution(request):
|
|||
return _submit_distribution(request, None, 'devhub.submit.upload')
|
||||
|
||||
|
||||
@login_required
|
||||
def submit_theme_distribution(request):
|
||||
if not RestrictionChecker(request=request).is_submission_allowed():
|
||||
return redirect('devhub.submit.theme.agreement')
|
||||
return _submit_distribution(request, None, 'devhub.submit.theme.upload')
|
||||
|
||||
|
||||
@dev_required(submitting=True)
|
||||
def submit_version_distribution(request, addon_id, addon):
|
||||
if not RestrictionChecker(request=request).is_submission_allowed():
|
||||
|
@ -1363,7 +1393,9 @@ WIZARD_COLOR_FIELDS = [
|
|||
|
||||
|
||||
@transaction.atomic
|
||||
def _submit_upload(request, addon, channel, next_view, wizard=False):
|
||||
def _submit_upload(
|
||||
request, addon, channel, next_view, wizard=False, theme_specific=False
|
||||
):
|
||||
"""If this is a new addon upload `addon` will be None.
|
||||
|
||||
next_view is the view that will be redirected to.
|
||||
|
@ -1375,6 +1407,7 @@ def _submit_upload(request, addon, channel, next_view, wizard=False):
|
|||
form = forms.NewUploadForm(
|
||||
request.POST or None, request.FILES or None, addon=addon, request=request
|
||||
)
|
||||
form.fields['theme_specific'].initial = theme_specific
|
||||
channel_text = amo.CHANNEL_CHOICES_API[channel]
|
||||
if request.method == 'POST' and form.is_valid():
|
||||
data = form.cleaned_data
|
||||
|
@ -1442,21 +1475,31 @@ def _submit_upload(request, addon, channel, next_view, wizard=False):
|
|||
submit_notification_warning = get_config(
|
||||
'submit_notification_warning_pre_review'
|
||||
)
|
||||
if addon and addon.type == amo.ADDON_STATICTHEME:
|
||||
wizard_url = reverse(
|
||||
'devhub.submit.version.wizard', args=[addon.slug, channel_text]
|
||||
)
|
||||
elif not addon and theme_specific:
|
||||
wizard_url = reverse('devhub.submit.wizard', args=[channel_text])
|
||||
else:
|
||||
wizard_url = None
|
||||
return TemplateResponse(
|
||||
request,
|
||||
template,
|
||||
context={
|
||||
'new_addon_form': form,
|
||||
'is_admin': is_admin,
|
||||
'addon': addon,
|
||||
'submit_notification_warning': submit_notification_warning,
|
||||
'submit_page': submit_page,
|
||||
'channel': channel,
|
||||
'channel_choice_text': channel_choice_text,
|
||||
'existing_properties': existing_properties,
|
||||
'colors': WIZARD_COLOR_FIELDS,
|
||||
'existing_properties': existing_properties,
|
||||
'is_admin': is_admin,
|
||||
'new_addon_form': form,
|
||||
'submit_notification_warning': submit_notification_warning,
|
||||
'submit_page': submit_page,
|
||||
'theme_specific': theme_specific,
|
||||
'unsupported_properties': unsupported_properties,
|
||||
'version_number': get_next_version_number(addon) if wizard else None,
|
||||
'wizard_url': wizard_url,
|
||||
},
|
||||
)
|
||||
|
||||
|
@ -1469,6 +1512,16 @@ def submit_addon_upload(request, channel):
|
|||
return _submit_upload(request, None, channel_id, 'devhub.submit.source')
|
||||
|
||||
|
||||
@login_required
|
||||
def submit_theme_upload(request, channel):
|
||||
if not RestrictionChecker(request=request).is_submission_allowed():
|
||||
return redirect('devhub.submit.theme.agreement')
|
||||
channel_id = amo.CHANNEL_CHOICES_LOOKUP[channel]
|
||||
return _submit_upload(
|
||||
request, None, channel_id, 'devhub.submit.source', theme_specific=True
|
||||
)
|
||||
|
||||
|
||||
@dev_required(submitting=True)
|
||||
@no_admin_disabled
|
||||
def submit_version_upload(request, addon_id, addon, channel):
|
||||
|
|
|
@ -96,7 +96,7 @@ class FileUploadViewSet(CreateModelMixin, ReadOnlyModelViewSet):
|
|||
user=request.user,
|
||||
)
|
||||
|
||||
devhub_tasks.validate(upload, listed=(channel == amo.CHANNEL_LISTED))
|
||||
devhub_tasks.validate(upload)
|
||||
headers = self.get_success_headers({})
|
||||
data = self.get_serializer(instance=upload).data
|
||||
return Response(data, status=status.HTTP_201_CREATED, headers=headers)
|
||||
|
|
|
@ -32,7 +32,7 @@
|
|||
|
||||
$.fn.addonUploader = function (options) {
|
||||
var settings = {
|
||||
filetypes: ['zip', 'xpi', 'crx', 'xml'],
|
||||
filetypes: ['zip', 'xpi', 'crx'],
|
||||
getErrors: getErrors,
|
||||
cancel: $(),
|
||||
maxSize: 200 * 1024 * 1024, // 200M
|
||||
|
|
|
@ -44,15 +44,19 @@ $(document).ready(function () {
|
|||
var $uploadAddon = $('#upload-addon');
|
||||
if ($('#upload-addon').length) {
|
||||
var opt = { cancel: $('.upload-file-cancel') };
|
||||
if ($('#addon-compat-upload').length) {
|
||||
opt.appendFormData = function (formData) {
|
||||
opt.appendFormData = function (formData) {
|
||||
if ($('#addon-compat-upload').length) {
|
||||
formData.append('app_id', $('#id_application option:selected').val());
|
||||
formData.append(
|
||||
'version_id',
|
||||
$('#id_app_version option:selected').val(),
|
||||
);
|
||||
};
|
||||
}
|
||||
}
|
||||
// theme_specific is a django BooleanField, so the value will be the
|
||||
// litteral string "True" or "False". That's what the upload() view
|
||||
// expects.
|
||||
formData.append('theme_specific', $('#id_theme_specific').val());
|
||||
};
|
||||
$uploadAddon.addonUploader(opt);
|
||||
}
|
||||
|
||||
|
|
|
@ -216,6 +216,7 @@ $(document).ready(function () {
|
|||
function (blob) {
|
||||
var formData = new FormData();
|
||||
formData.append('upload', blob, 'upload.zip');
|
||||
formData.append('theme_specific', true);
|
||||
$.ajax({
|
||||
type: 'POST',
|
||||
url: $button.attr('formaction'),
|
||||
|
|
Загрузка…
Ссылка в новой задаче