Merge pull request #4084 from eviljeff/addons-de-is-listed

clean is_listed uses in addons
This commit is contained in:
Andrew Williamson 2016-11-29 12:59:04 +00:00 коммит произвёл GitHub
Родитель f46641ec8c 9c9f9c0b1b
Коммит 5845604091
19 изменённых файлов: 101 добавлений и 88 удалений

Просмотреть файл

@ -39,7 +39,8 @@ def addon_view(f, qs=Addon.objects.all):
addon = get_object_or_404(qs(), slug=addon_id)
# If the addon is unlisted it needs either an owner/viewer/dev/support,
# or an unlisted addon reviewer.
if not (addon.is_listed or owner_or_unlisted_reviewer(request, addon)):
if not (addon.has_listed_versions() or
owner_or_unlisted_reviewer(request, addon)):
raise http.Http404
return f(request, addon, *args, **kw)
return wrapper

Просмотреть файл

@ -485,7 +485,7 @@ class Addon(OnChangeMixin, ModelBase):
return True
@classmethod
def initialize_addon_from_upload(cls, data, upload, is_listed=True):
def initialize_addon_from_upload(cls, data, upload, channel):
fields = cls._meta.get_all_field_names()
guid = data.get('guid')
old_guid_addon = None
@ -510,7 +510,7 @@ class Addon(OnChangeMixin, ModelBase):
addon = Addon(**dict((k, v) for k, v in data.items() if k in fields))
addon.status = amo.STATUS_NULL
addon.is_listed = is_listed
addon.is_listed = channel == amo.RELEASE_CHANNEL_LISTED
locale_is_set = (addon.default_locale and
addon.default_locale in (
settings.AMO_LANGUAGES +
@ -527,25 +527,24 @@ class Addon(OnChangeMixin, ModelBase):
return addon
@classmethod
def create_addon_from_upload_data(cls, data, upload, user=None, **kwargs):
addon = cls.initialize_addon_from_upload(data, upload=upload, **kwargs)
def create_addon_from_upload_data(cls, data, upload, channel, user=None,
**kwargs):
addon = cls.initialize_addon_from_upload(data, upload, channel,
**kwargs)
AddonUser(addon=addon, user=user).save()
return addon
@classmethod
def from_upload(cls, upload, platforms, source=None, is_listed=True,
data=None):
# TODO: change is_listed arg to channel
def from_upload(cls, upload, platforms, source=None,
channel=amo.RELEASE_CHANNEL_LISTED, data=None):
if not data:
data = parse_addon(upload)
addon = cls.initialize_addon_from_upload(
is_listed=is_listed, data=data, upload=upload)
data=data, upload=upload, channel=channel)
if upload.validation_timeout:
addon.update(admin_review=True)
channel = (amo.RELEASE_CHANNEL_LISTED if is_listed else
amo.RELEASE_CHANNEL_UNLISTED)
Version.from_upload(upload, addon, platforms, source=source,
channel=channel)
@ -583,7 +582,7 @@ class Addon(OnChangeMixin, ModelBase):
return data
def get_url_path(self, more=False, add_prefix=True):
if not self.is_listed: # Not listed? Doesn't have a public page.
if not self.current_version:
return ''
# If more=True you get the link to the ajax'd middle chunk of the
# detail page.
@ -632,13 +631,6 @@ class Addon(OnChangeMixin, ModelBase):
def share_url(self):
return reverse('addons.share', args=[self.slug])
@property
def automated_signing(self):
# We allow automated signing for add-ons which are not listed.
# Beta versions are a special case for listed add-ons, and are dealt
# with on a file-by-file basis.
return not self.is_listed
@amo.cached_property(writable=True)
def listed_authors(self):
return UserProfile.objects.filter(

Просмотреть файл

@ -54,18 +54,18 @@
<td>{{ version.compatible_apps[APP] }}</td>
</tr>
{% endif %}
{% if addon.has_listed_versions() %}
<tr>
<th>{{ _('Visibility') }}</th>
<td>
{% if not addon.is_disabled and addon.is_listed %}
{{ _('Listed') }}
{% elif addon.is_disabled and addon.is_listed %}
{{ _('Hidden') }}
{% elif not addon.is_listed %}
{{ _('Unlisted') }}
{% if not addon.is_disabled %}
{{ _('Visible') }}
{% elif addon.is_disabled %}
{{ _('Invisible') }}
{% endif %}
</td>
</tr>
{% endif %}
{% set deps = addon.all_dependencies %}
{% if deps %}
<tr class="addon-dependencies">
@ -114,9 +114,10 @@
<tr class="meta-abuse">
<th>{{ _('Abuse Reports') }}</th>
<td>
{% if addon.is_listed %}<a href="{{ url('editors.abuse_reports', addon.slug) }}">{% endif %}
{% if addon.has_listed_versions() %}
<a href="{{ url('editors.abuse_reports', addon.slug) }}">{% endif %}
<strong>{{ addon.abuse_reports.count()|numberfmt }}</strong>
{% if addon.is_listed %}</a>{% endif %}
{% if addon.has_listed_versions() %}</a>{% endif %}
</td>
</tr>
{% if not show_actions %}

Просмотреть файл

@ -207,8 +207,10 @@ def test_process_addons_invalid_task():
@pytest.mark.django_db
def test_process_addons_update_current_version_for_unlisted():
addon1 = addon_factory(is_listed=False)
addon2 = addon_factory(is_listed=False)
addon1 = addon_factory(
version_kw={'channel': amo.RELEASE_CHANNEL_UNLISTED})
addon2 = addon_factory(
version_kw={'channel': amo.RELEASE_CHANNEL_UNLISTED})
listed_addon = addon_factory()
# Manually set a current version on the unlisted addons to mimic the state

Просмотреть файл

@ -1,8 +1,10 @@
import urllib
from django import http
import mock
from olympia.amo.tests import TestCase
from olympia.amo.tests import addon_factory, TestCase
from olympia.addons import decorators as dec
from olympia.addons.models import Addon
@ -11,14 +13,15 @@ class TestAddonView(TestCase):
def setUp(self):
super(TestAddonView, self).setUp()
self.addon = Addon.objects.create(slug='x', type=1)
self.addon = addon_factory()
self.func = mock.Mock()
self.func.return_value = mock.sentinel.OK
self.func.__name__ = 'mock_function'
self.view = dec.addon_view(self.func)
self.request = mock.Mock()
self.slug_path = '/addon/%s/reviews' % self.addon.slug
self.request.path = self.id_path = '/addon/%s/reviews' % self.addon.id
self.slug_path = urllib.quote(
('/addon/%s/reviews' % self.addon.slug).encode('utf8'))
self.request.path = self.id_path = u'/addon/%s/reviews' % self.addon.id
self.request.GET = {}
def test_301_by_id(self):
@ -26,11 +29,13 @@ class TestAddonView(TestCase):
self.assert3xx(res, self.slug_path, 301)
def test_slug_replace_no_conflict(self):
self.request.path = '/addon/{id}/reviews/{id}345/path'.format(
self.request.path = u'/addon/{id}/reviews/{id}345/path'.format(
id=self.addon.id)
res = self.view(self.request, str(self.addon.id))
self.assert3xx(res, '/addon/{slug}/reviews/{id}345/path'.format(
id=self.addon.id, slug=self.addon.slug), 301)
redirection = urllib.quote(
u'/addon/{slug}/reviews/{id}345/path'.format(
id=self.addon.id, slug=self.addon.slug).encode('utf8'))
self.assert3xx(res, redirection, 301)
def test_301_with_querystring(self):
self.request.GET = mock.Mock()
@ -83,17 +88,17 @@ class TestAddonView(TestCase):
view(self.request, self.addon.slug)
def test_addon_no_slug(self):
app = Addon.objects.create(type=1, name='xxxx')
res = self.view(self.request, app.slug)
addon = addon_factory(slug=None)
res = self.view(self.request, addon.slug)
assert res == mock.sentinel.OK
def test_slug_isdigit(self):
app = Addon.objects.create(type=1, name='xxxx')
app.update(slug=str(app.id))
r = self.view(self.request, app.slug)
addon = addon_factory()
addon.update(slug=str(addon.id))
r = self.view(self.request, addon.slug)
assert r == mock.sentinel.OK
request, addon = self.func.call_args[0]
assert addon == app
request, addon_ = self.func.call_args[0]
assert addon_ == addon
class TestAddonViewWithUnlisted(TestAddonView):
@ -109,7 +114,7 @@ class TestAddonViewWithUnlisted(TestAddonView):
lambda *args, **kwargs: False)
def test_unlisted_addon(self):
"""Return a 404 for non authorized access."""
self.addon.update(is_listed=False)
self.make_addon_unlisted(self.addon)
with self.assertRaises(http.Http404):
self.view(self.request, self.addon.slug)
@ -119,7 +124,7 @@ class TestAddonViewWithUnlisted(TestAddonView):
lambda *args, **kwargs: True)
def test_unlisted_addon_owner(self):
"""Addon owners have access."""
self.addon.update(is_listed=False)
self.make_addon_unlisted(self.addon)
assert self.view(self.request, self.addon.slug) == mock.sentinel.OK
request, addon = self.func.call_args[0]
assert addon == self.addon
@ -130,7 +135,7 @@ class TestAddonViewWithUnlisted(TestAddonView):
lambda *args, **kwargs: False)
def test_unlisted_addon_unlisted_admin(self):
"""Unlisted addon reviewers have access."""
self.addon.update(is_listed=False)
self.make_addon_unlisted(self.addon)
assert self.view(self.request, self.addon.slug) == mock.sentinel.OK
request, addon = self.func.call_args[0]
assert addon == self.addon

Просмотреть файл

@ -69,7 +69,7 @@ class FormsTest(TestCase):
def test_update_addon_existing_name_used_by_unlisted(self):
"""An add-on edit can change the name to an existing name used by an
unlisted add-on."""
Addon.objects.get(pk=3615).update(is_listed=False)
self.make_addon_unlisted(Addon.objects.get(pk=3615))
addon = addon_factory(name='some name')
form = forms.AddonFormBasic(dict(name=self.existing_name),
request=self.request, instance=addon)
@ -79,7 +79,9 @@ class FormsTest(TestCase):
def test_update_addon_existing_name_used_by_listed(self):
"""An unlisted add-on edit can change the name to an existing name used
by an listed add-on."""
addon = addon_factory(name='some name', is_listed=False)
addon = addon_factory(
name='some name',
version_kw={'channel': amo.RELEASE_CHANNEL_UNLISTED})
form = forms.AddonFormBasic(dict(name=self.existing_name),
request=self.request, instance=addon)
form.is_valid()

Просмотреть файл

@ -1454,8 +1454,7 @@ class TestAddonModels(TestCase):
def test_unlisted_has_complete_metadata(self):
addon = Addon.objects.get(id=3615)
addon.update(is_listed=False)
addon.versions.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
self.make_addon_unlisted(addon)
assert addon.has_complete_metadata() # Confirm complete already.
# Clear everything
@ -1747,16 +1746,17 @@ class TestGetVersion(TestCase):
class TestAddonGetURLPath(TestCase):
def test_get_url_path(self):
addon = Addon(slug='woo')
addon = addon_factory(slug='woo')
assert addon.get_url_path() == '/en-US/firefox/addon/woo/'
def test_get_url_path_more(self):
addon = Addon(slug='yeah')
addon = addon_factory(slug='yeah')
assert addon.get_url_path(more=True) == (
'/en-US/firefox/addon/yeah/more')
def test_unlisted_addon_get_url_path(self):
addon = Addon(slug='woo', is_listed=False)
addon = addon_factory(
slug='woo', version_kw={'channel': amo.RELEASE_CHANNEL_UNLISTED})
assert addon.get_url_path() == ''
@ -1804,7 +1804,7 @@ class TestBackupVersion(TestCase):
self.addon.update(_current_version=None)
assert self.addon.current_version is None
def test_current_version_is_listed_only(self):
def test_current_version_listed_only(self):
version = self.addon.current_version
version.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
# The call above should have triggerred update_version().
@ -2242,7 +2242,8 @@ class TestAddonFromUpload(UploadTest):
def test_is_not_listed(self):
# An addon can be explicitly unlisted.
addon = Addon.from_upload(self.get_upload('extension.xpi'),
[self.platform], is_listed=False)
[self.platform],
channel=amo.RELEASE_CHANNEL_UNLISTED)
assert not addon.is_listed
def test_validation_completes(self):
@ -2588,14 +2589,12 @@ class TestAddonWatchDeveloperNotes(TestCase):
class TestTrackAddonStatusChange(TestCase):
def create_addon(self, **kwargs):
kwargs.setdefault('type', amo.ADDON_EXTENSION)
addon = Addon(**kwargs)
addon.save()
return addon
return addon_factory(kwargs.pop('status', amo.STATUS_NULL), **kwargs)
def test_increment_new_status(self):
with patch('olympia.addons.models.track_addon_status_change') as mock_:
addon = self.create_addon()
addon = Addon()
addon.save()
mock_.assert_called_with(addon)
def test_increment_updated_status(self):
@ -2623,7 +2622,8 @@ class TestTrackAddonStatusChange(TestCase):
)
def test_increment_listed_addon_statuses(self):
addon = self.create_addon(is_listed=True)
addon = self.create_addon(
version_kw={'channel': amo.RELEASE_CHANNEL_LISTED})
with patch('olympia.addons.models.statsd.incr') as mock_incr:
track_addon_status_change(addon)
mock_incr.assert_any_call(
@ -2631,7 +2631,8 @@ class TestTrackAddonStatusChange(TestCase):
)
def test_increment_unlisted_addon_statuses(self):
addon = self.create_addon(is_listed=False)
addon = self.create_addon(
version_kw={'channel': amo.RELEASE_CHANNEL_UNLISTED})
with patch('olympia.addons.models.statsd.incr') as mock_incr:
track_addon_status_change(addon)
mock_incr.assert_any_call(

Просмотреть файл

@ -94,8 +94,7 @@ class TestDataValidate(VersionCheckMixin, TestCase):
"""Add-ons with only unlisted versions are valid, they just don't
receive any updates (See TestLookup.test_no_unlisted below)."""
addon = Addon.objects.get(pk=3615)
addon.update(is_listed=False)
addon.versions.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
self.make_addon_unlisted(addon)
up = self.get(self.good_data)
assert up.is_valid()

Просмотреть файл

@ -890,7 +890,7 @@ class TestDetailPage(TestCase):
def test_unlisted_addon_returns_404(self):
"""Unlisted addons are not listed and return 404."""
self.addon.update(is_listed=False)
self.make_addon_unlisted(self.addon)
assert self.client.get(self.url).status_code == 404
def test_fx_ios_addons_message(self):
@ -2359,7 +2359,7 @@ class TestAddonSearchView(ESTestCase):
addon_factory(slug='my-disabled-addon', name=u'My disabled Addôn',
status=amo.STATUS_DISABLED)
addon_factory(slug='my-unlisted-addon', name=u'My unlisted Addôn',
is_listed=False)
version_kw={'channel': amo.RELEASE_CHANNEL_UNLISTED})
addon_factory(slug='my-disabled-by-user-addon',
name=u'My disabled by user Addôn',
disabled_by_user=True)

Просмотреть файл

@ -591,6 +591,13 @@ class TestCase(PatchMixin, InitializeSessionMixin, MockEsMixin,
setattr(request, '_messages', messages)
return request
def make_addon_unlisted(self, addon, listed=False):
addon.update(is_listed=listed)
channel = (amo.RELEASE_CHANNEL_LISTED if listed else
amo.RELEASE_CHANNEL_UNLISTED)
for version in addon.versions.all():
version.update(channel=channel)
class AMOPaths(object):
"""Mixin for getting common AMO Paths."""
@ -663,6 +670,8 @@ def addon_factory(
'created': when,
'last_updated': when,
}
if 'is_listed' not in kw and 'channel' in version_kw:
kw['is_listed'] = version_kw['channel'] == amo.RELEASE_CHANNEL_LISTED
kwargs.update(kw)
# Save 1.

Просмотреть файл

@ -817,7 +817,7 @@ class TestSubmitFile(TestCase):
def create_upload(self, version='1.0'):
return FileUpload.objects.create(
addon=self.addon, version=version, validation='{"errors":0}',
automated_signing=self.addon.automated_signing)
automated_signing=False)
@mock.patch('olympia.devhub.tasks.FileUpload.passed_all_validations', True)
def test_file_passed_all_validations(self):
@ -849,7 +849,7 @@ class TestCreateVersionForUpload(TestCase):
def create_upload(self, version='1.0'):
return FileUpload.objects.create(
addon=self.addon, version=version, validation='{"errors":0}',
automated_signing=self.addon.automated_signing)
automated_signing=False)
def test_file_passed_all_validations_not_most_recent(self):
upload = self.create_upload()

Просмотреть файл

@ -968,7 +968,8 @@ class TestVersionEditFiles(TestVersionEditBase):
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 '
u'<a href="/en-US/firefox/addon/a3615/">Delicious Bookmarks</a>.')
# no url because no current version becomes none.
u'<a href="">Delicious Bookmarks</a>.')
assert log.to_string() == log_string
assert r.status_code == 302
assert self.version.files.count() == 0

Просмотреть файл

@ -1532,7 +1532,7 @@ def _submit_upload(request, addon, channel, next_listed, next_unlisted,
upload=data['upload'],
platforms=data.get('supported_platforms', []),
source=data['source'],
is_listed=channel == amo.RELEASE_CHANNEL_LISTED)
channel=channel)
version = addon.find_latest_version(channel=channel)
AddonUser(addon=addon, user=request.user).save()
url_args = [addon.slug]

Просмотреть файл

@ -1621,7 +1621,7 @@ class TestReview(ReviewBase):
def test_needs_unlisted_reviewer_for_only_unlisted(self):
self.addon.versions.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
assert self.client.head(self.url).status_code == 403
assert self.client.head(self.url).status_code == 404
self.login_as_senior_editor()
assert self.client.head(self.url).status_code == 200
@ -2101,8 +2101,8 @@ class TestReview(ReviewBase):
status_code=302)
self.version.delete()
# Regular reviewer has no permission, gets a 403.
assert self.client.get(self.url).status_code == 403
# Regular reviewer has no permission, gets a 404.
assert self.client.get(self.url).status_code == 404
self.login_as_senior_editor()
# Reviewer with more powers can look.
assert self.client.get(self.url).status_code == 200
@ -2845,4 +2845,4 @@ class TestLimitedReviewerReview(ReviewBase, LimitedReviewerBase):
channel=amo.RELEASE_CHANNEL_LISTED)
version.delete()
response = self.client.get(self.url)
assert response.status_code == 403
assert response.status_code == 404

Просмотреть файл

@ -106,7 +106,7 @@ class File(OnChangeMixin, ModelBase):
"""True if this file is eligible for automated signing. This currently
means that either its add-on is eligible for automated signing, or
this file is a beta version."""
return (self.version.addon.automated_signing or
return (self.version.channel == amo.RELEASE_CHANNEL_UNLISTED or
self.status == amo.STATUS_BETA)
@property

Просмотреть файл

@ -1,7 +1,7 @@
import jingo
from pyquery import PyQuery as pq
from olympia.amo.tests import TestCase
from olympia.amo.tests import addon_factory, TestCase
from olympia.addons.models import Addon
from olympia.amo.urlresolvers import reverse
from olympia.reviews.models import ReviewFlag
@ -35,7 +35,7 @@ class HelpersTest(TestCase):
assert doc.attr('class') == 'stars stars-5'
def test_reviews_link(self):
a = Addon(average_rating=4, total_reviews=37, id=1, type=1, slug='xx')
a = addon_factory(average_rating=4, total_reviews=37, id=1, slug='xx')
s = self.render('{{ reviews_link(myaddon) }}', {'myaddon': a})
assert pq(s)('strong').text() == '37 reviews'
@ -60,7 +60,7 @@ class HelpersTest(TestCase):
assert pq(s)('a').attr('href') == u
def test_impala_reviews_link(self):
a = Addon(average_rating=4, total_reviews=37, id=1, type=1, slug='xx')
a = addon_factory(average_rating=4, total_reviews=37, id=1, slug='xx')
s = self.render('{{ impala_reviews_link(myaddon) }}', {'myaddon': a})
assert pq(s)('a').text() == '(37)'

Просмотреть файл

@ -156,10 +156,10 @@ class VersionView(APIView):
status.HTTP_400_BAD_REQUEST)
if addon is None:
addon = Addon.create_addon_from_upload_data(
data=pkg, user=request.user, upload=filedata, is_listed=False)
created = True
channel = amo.RELEASE_CHANNEL_UNLISTED
addon = Addon.create_addon_from_upload_data(
data=pkg, user=request.user, upload=filedata, channel=channel)
created = True
else:
created = False
last_version = addon.find_latest_version_including_rejected()

Просмотреть файл

@ -12,7 +12,7 @@ import mock
from pyquery import PyQuery as pq
from olympia import amo
from olympia.amo.tests import TestCase
from olympia.amo.tests import TestCase, version_factory
from olympia.amo.urlresolvers import reverse
from olympia.access.models import Group, GroupUser
from olympia.addons.models import Addon, AddonUser
@ -37,6 +37,9 @@ class StatsTest(TestCase):
self.url_args = {'start': '20090601', 'end': '20090930', 'addon_id': 4}
self.url_args_theme = {'start': '20090601', 'end': '20090930',
'addon_id': 6}
version_factory(addon=Addon.objects.get(pk=4))
version_factory(addon=Addon.objects.get(pk=5))
version_factory(addon=Addon.objects.get(pk=6))
# Most tests don't care about permissions.
self.login_as_admin()
@ -88,7 +91,9 @@ class TestUnlistedAddons(StatsTest):
def setUp(self):
super(TestUnlistedAddons, self).setUp()
Addon.objects.get(pk=4).update(is_listed=False)
addon = Addon.objects.get(pk=4)
addon.update(is_listed=False)
addon.versions.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
def test_no_stats_for_unlisted_addon(self):
"""All the views for the stats return 404 for unlisted addons."""

Просмотреть файл

@ -664,11 +664,6 @@ class TestViews(TestCase):
url = reverse('addons.versions', args=[self.addon.slug])
assert self.client.get(url).status_code == 404
def test_version_list_does_not_return_unlisted_versions(self):
self.addon.versions.update(channel=amo.RELEASE_CHANNEL_UNLISTED)
doc = self.get_content()
assert len(doc('.version')) == 0
def test_version_detail_does_not_return_unlisted_versions(self):
version_number = self.addon.current_version.version
self.addon.versions.update(channel=amo.RELEASE_CHANNEL_UNLISTED)