Prevent deleting files of a version entirely
This is causing problems for reviewers; Instead developers should delete entire versions to remove files.
This commit is contained in:
Родитель
c2f77248d6
Коммит
0defb8cc47
|
@ -456,7 +456,7 @@ class BaseCompatFormSet(BaseModelFormSet):
|
|||
[{'application': a.id} for a in apps])
|
||||
self.extra = len(amo.APP_GUIDS) - len(self.forms)
|
||||
|
||||
# After these changes, the foms need to be rebuilt. `forms`
|
||||
# After these changes, the forms need to be rebuilt. `forms`
|
||||
# is a cached property, so we delete the existing cache and
|
||||
# ask for a new one to be built.
|
||||
del self.forms
|
||||
|
@ -644,26 +644,13 @@ class FileForm(happyforms.ModelForm):
|
|||
plats.append([pid, amo.PLATFORMS[pid].name])
|
||||
self.fields['platform'].choices = plats
|
||||
|
||||
def clean_DELETE(self):
|
||||
if any(self.errors):
|
||||
return
|
||||
delete = self.cleaned_data['DELETE']
|
||||
|
||||
if (delete and not self.instance.version.is_all_unreviewed):
|
||||
error = _('You cannot delete a file once the review process has '
|
||||
'started. You must delete the whole version.')
|
||||
raise forms.ValidationError(error)
|
||||
|
||||
return delete
|
||||
|
||||
|
||||
class BaseFileFormSet(BaseModelFormSet):
|
||||
|
||||
def clean(self):
|
||||
if any(self.errors):
|
||||
return
|
||||
files = [f.cleaned_data for f in self.forms
|
||||
if not f.cleaned_data.get('DELETE', False)]
|
||||
files = [f.cleaned_data for f in self.forms]
|
||||
|
||||
if self.forms and 'platform' in self.forms[0].fields:
|
||||
platforms = [f['platform'] for f in files]
|
||||
|
@ -679,7 +666,7 @@ class BaseFileFormSet(BaseModelFormSet):
|
|||
|
||||
|
||||
FileFormSet = modelformset_factory(File, formset=BaseFileFormSet,
|
||||
form=FileForm, can_delete=True, extra=0)
|
||||
form=FileForm, can_delete=False, extra=0)
|
||||
|
||||
|
||||
class DescribeForm(AddonFormBase):
|
||||
|
|
|
@ -7,36 +7,5 @@
|
|||
<td>{{ file.size|filesizeformat(binary=True) }}</td>
|
||||
<td>{{ form.platform }}</td>
|
||||
<td>{{ choices[file.status] }}</td>
|
||||
<td>
|
||||
<span class="js-hidden">
|
||||
<span class="delete">{{ form.DELETE }}</span>
|
||||
{{ form.id }}
|
||||
</span>
|
||||
{# Don't let them delete an individual file any of the files have been reviewed. #}
|
||||
{% if version and version.is_all_unreviewed: %}
|
||||
<a class="remove" href="#">x</a>
|
||||
{% else: %}
|
||||
<span class="remove tooltip" href="#"
|
||||
title="{% trans -%}
|
||||
You cannot remove an individual file after the review process has begun.
|
||||
You must delete the entire version.
|
||||
{%- endtrans %}">x</span>
|
||||
{% endif %}
|
||||
</td>
|
||||
</tr>
|
||||
{% if form.DELETE.errors %}
|
||||
<tr>
|
||||
<td colspan="4">
|
||||
{{ form.DELETE.errors }}
|
||||
</td>
|
||||
</tr>
|
||||
{% endif %}
|
||||
<tr class="hide">
|
||||
<td colspan="4">
|
||||
<span class="delete-msg error_message">{% trans %}This file will be deleted on save.{% endtrans %}</span>
|
||||
</td>
|
||||
<td>
|
||||
<a class="undo" href="#">{{ _("Undo") }}</a>
|
||||
</td>
|
||||
</tr>
|
||||
{% endwith %}
|
||||
|
|
|
@ -820,32 +820,6 @@ class TestVersionEditFiles(TestVersionEditBase):
|
|||
compat.update(kw)
|
||||
return super(TestVersionEditFiles, self).formset(*args, **compat)
|
||||
|
||||
def test_delete_file(self):
|
||||
version = self.addon.current_version
|
||||
version.files.all()[0].update(status=amo.STATUS_AWAITING_REVIEW)
|
||||
|
||||
assert self.version.files.count() == 1
|
||||
forms = map(initial,
|
||||
self.client.get(self.url).context['file_form'].forms)
|
||||
forms[0]['DELETE'] = True
|
||||
assert ActivityLog.objects.count() == 0
|
||||
response = self.client.post(
|
||||
self.url, self.formset(*forms, prefix='files'))
|
||||
|
||||
assert ActivityLog.objects.count() == 2
|
||||
log = ActivityLog.objects.order_by('created')[1]
|
||||
log_string = (
|
||||
u'File delicious_bookmarks-2.1.072-fx.xpi deleted from '
|
||||
u'<a href="/en-US/firefox/addon/a3615/versions/2.1.072">'
|
||||
u'Version 2.1.072</a> of '
|
||||
# no url because no current version becomes none.
|
||||
u'<a href="">Delicious Bookmarks</a>.')
|
||||
assert log.to_string() == log_string
|
||||
assert response.status_code == 302
|
||||
assert self.version.files.count() == 0
|
||||
response = self.client.get(self.url)
|
||||
assert response.status_code == 200
|
||||
|
||||
def test_unique_platforms(self):
|
||||
# Move the existing file to Linux.
|
||||
file_ = self.version.files.get()
|
||||
|
@ -878,19 +852,6 @@ class TestVersionEditFiles(TestVersionEditBase):
|
|||
assert response.context['file_form'].non_form_errors()[0] == (
|
||||
'The platform All cannot be combined with specific platforms.')
|
||||
|
||||
def test_all_platforms_and_delete(self):
|
||||
version = self.addon.current_version
|
||||
version.files.all()[0].update(status=amo.STATUS_AWAITING_REVIEW)
|
||||
|
||||
File.objects.create(
|
||||
version=self.version, platform=amo.PLATFORM_MAC.id)
|
||||
forms = self.client.get(self.url).context['file_form'].forms
|
||||
forms = map(initial, forms)
|
||||
# A test that we don't check the platform for deleted files.
|
||||
forms[1]['DELETE'] = 1
|
||||
self.client.post(self.url, self.formset(*forms, prefix='files'))
|
||||
assert self.version.files.count() == 1
|
||||
|
||||
def add_in_bsd(self):
|
||||
file_ = self.version.files.get()
|
||||
# The default file is All, which prevents the addition of more files.
|
||||
|
|
|
@ -1099,11 +1099,6 @@ def version_edit(request, addon_id, addon, version_id):
|
|||
data['version_form'].save()
|
||||
data['file_form'].save()
|
||||
|
||||
for deleted in data['file_form'].deleted_forms:
|
||||
file = deleted.cleaned_data['id']
|
||||
amo.log(amo.LOG.DELETE_FILE_FROM_VERSION,
|
||||
file.filename, file.version, addon)
|
||||
|
||||
if 'compat_form' in data:
|
||||
for compat in data['compat_form'].save(commit=False):
|
||||
compat.version = version
|
||||
|
|
Загрузка…
Ссылка в новой задаче