diff --git a/mkt/reviewers/api.py b/mkt/reviewers/api.py index d6064f40e4a..a2114026902 100644 --- a/mkt/reviewers/api.py +++ b/mkt/reviewers/api.py @@ -1,13 +1,23 @@ +import json + +from tastypie import http +from tower import ugettext as _ + +import amo +from access import acl from amo.urlresolvers import reverse from mkt.api.authentication import OAuthAuthentication from mkt.api.authorization import PermissionAuthorization -from mkt.api.base import MarketplaceResource +from mkt.api.base import MarketplaceResource, http_error from mkt.reviewers.utils import AppsReviewing +from mkt.reviewers.forms import ApiReviewersSearchForm +from mkt.search.api import SearchResource +from mkt.search.views import _get_query +from mkt.webapps.utils import update_with_reviewer_data class Wrapper(object): - def __init__(self, pk): self.pk = pk @@ -28,3 +38,57 @@ def get_resource_uri(self, bundle): def obj_get_list(self, request, **kwargs): return [Wrapper(r['app'].pk) for r in AppsReviewing(request).get_apps()] + + +class ReviewersSearchResource(SearchResource): + class Meta(SearchResource.Meta): + resource_name = 'search' + authorization = PermissionAuthorization('Apps', 'Review') + fields = ['device_types', 'id', 'is_packaged', 'latest_version', + 'name', 'premium_type', 'price', 'slug', 'status'] + + def get_search_data(self, request): + form = ApiReviewersSearchForm(request.GET if request else None) + if not form.is_valid(): + raise self.form_errors(form) + return form.cleaned_data + + def get_feature_profile(self, request): + # We don't want automatic feature profile filtering in the reviewers + # API. + return None + + def get_region(self, request): + # We don't want automatic region filtering in the reviewers API. + return None + + def apply_filters(self, request, qs, data=None): + qs = super(ReviewersSearchResource, self).apply_filters(request, qs, + data=data) + for k in ('is_privileged', 'has_info_request', 'has_editor_comment'): + if data.get(k, None) is not None: + qs = qs.filter(**{ + 'latest_version.%s' % k: data[k] + }) + if data.get('is_escalated', None) is not None: + qs = qs.filter(is_escalated=data['is_escalated']) + return qs + + def get_query(self, request, base_filters=None): + form_data = self.get_search_data(request) + status = form_data.get('status') + + if base_filters is None: + base_filters = {} + base_filters['status'] = status + + region = self.get_region(request) + return _get_query(request, region, gaia=None, mobile=None, tablet=None, + new_idx=True, filters=base_filters) + + def dehydrate(self, bundle): + bundle = super(ReviewersSearchResource, self).dehydrate(bundle) + bundle = update_with_reviewer_data(bundle, using_es=True) + # Filter out anything not present in Meta fields. + bundle.data = dict(((k, v) for k, v in bundle.data.items() if k in self._meta.fields)) + return bundle diff --git a/mkt/reviewers/forms.py b/mkt/reviewers/forms.py index 2633fced592..07985c7627e 100644 --- a/mkt/reviewers/forms.py +++ b/mkt/reviewers/forms.py @@ -12,7 +12,10 @@ from amo.utils import raise_required from editors.forms import NonValidatingChoiceField, ReviewLogForm from editors.models import CannedResponse +from mkt.api.forms import CustomNullBooleanSelect from mkt.reviewers.utils import ReviewHelper +from mkt.search.forms import ApiSearchForm + from .models import ThemeLock from .tasks import approve_rereview, reject_rereview, send_mail @@ -20,6 +23,13 @@ log = logging.getLogger('z.reviewers.forms') +# We set 'any' here since we need to default this field +# to PUBLIC if not specified for consumer pages. +STATUS_CHOICES = [('any', _lazy(u'Any Status'))] +for status in amo.WEBAPPS_UNLISTED_STATUSES + (amo.STATUS_PUBLIC,): + STATUS_CHOICES.append((amo.STATUS_CHOICES_API[status], + amo.STATUS_CHOICES[status])) + class ReviewAppAttachmentForm(happyforms.Form): attachment = forms.FileField(label=_lazy(u'Attachment:')) @@ -241,3 +251,30 @@ class ThemeSearchForm(forms.Form): required=False, label=_lazy(u'Search'), widget=forms.TextInput(attrs={'autocomplete': 'off', 'placeholder': _lazy(u'Search')})) + + +class ApiReviewersSearchForm(ApiSearchForm): + status = forms.ChoiceField(required=False, choices=STATUS_CHOICES, + label=_lazy(u'Status')) + is_privileged = forms.NullBooleanField(required=False, + label=_lazy(u'Privileged App'), + widget=CustomNullBooleanSelect) + has_editor_comment = forms.NullBooleanField( + required=False, + label=_lazy(u'Contains Editor Comment'), + widget=CustomNullBooleanSelect) + has_info_request = forms.NullBooleanField( + required=False, + label=_lazy(u'More Information Requested'), + widget=CustomNullBooleanSelect) + is_escalated = forms.NullBooleanField( + required=False, + label=_lazy(u'Escalated'), + widget=CustomNullBooleanSelect) + + def clean_status(self): + status = self.cleaned_data['status'] + if status == 'any': + return 'any' + + return amo.STATUS_CHOICES_API_LOOKUP.get(status, amo.STATUS_PENDING) diff --git a/mkt/reviewers/tests/test_api.py b/mkt/reviewers/tests/test_api.py index 49a359a697f..2d28aa4883b 100644 --- a/mkt/reviewers/tests/test_api.py +++ b/mkt/reviewers/tests/test_api.py @@ -1,23 +1,35 @@ +from datetime import datetime import json from django.conf import settings +from django.contrib.auth.models import User from django.core.cache import cache +from mock import patch from nose.tools import eq_ from test_utils import RequestFactory -from mkt.api.tests.test_oauth import BaseOAuth, get_absolute_url +import amo +import mkt.regions +from access.models import GroupUser +from addons.models import Category +from amo.tests import ESTestCase + +from mkt.api.tests.test_oauth import BaseOAuth, get_absolute_url, OAuthClient from mkt.api.base import get_url, list_url +from mkt.api.models import Access, generate +from mkt.constants.features import FeatureProfile from mkt.reviewers.utils import AppsReviewing from mkt.site.fixtures import fixture +from mkt.webapps.models import Webapp from users.models import UserProfile -class TestAccount(BaseOAuth): +class TestReviewing(BaseOAuth): fixtures = fixture('user_2519', 'webapp_337141') def setUp(self): - super(TestAccount, self).setUp(api_name='reviewers') + super(TestReviewing, self).setUp(api_name='reviewers') self.list_url = list_url('reviewing') self.user = UserProfile.objects.get(pk=2519) self.req = RequestFactory().get('/') @@ -54,3 +66,211 @@ def test_some(self): data = json.loads(res.content) eq_(data['objects'][0]['resource_uri'], get_absolute_url(get_url('app', '337141'), absolute=False)) + + +class TestApiReviewer(BaseOAuth, ESTestCase): + fixtures = fixture('webapp_337141', 'user_2519') + + def setUp(self, api_name='reviewers'): + super(TestApiReviewer, self).setUp(api_name=api_name) + self.user = User.objects.get(pk=2519) + self.profile = self.user.get_profile() + self.profile.update(read_dev_agreement=datetime.now()) + self.grant_permission(self.profile, 'Apps:Review') + + self.access = Access.objects.create( + key='test_oauth_key', secret=generate(), user=self.user) + self.client = OAuthClient(self.access, api_name=api_name) + self.url = list_url('search') + + self.webapp = Webapp.objects.get(pk=337141) + self.category = Category.objects.create(name='test', + type=amo.ADDON_WEBAPP) + + self.webapp.update(status=amo.STATUS_PENDING) + self.refresh('webapp') + + def test_anonymous_access(self): + res = self.anon.get(self.url) + eq_(res.status_code, 401) + + def test_non_reviewer_access(self): + GroupUser.objects.filter(group__rules='Apps:Review', + user=self.profile).delete() + res = self.client.get(self.url) + eq_(res.status_code, 401) + + def test_owner_still_non_reviewer_access(self): + user = Webapp.objects.get(pk=337141).authors.all()[0].user + access = Access.objects.create( + key='test_oauth_key_owner', secret=generate(), user=user) + client = OAuthClient(access, api_name='reviewers') + res = client.get(self.url) + eq_(res.status_code, 401) + + def test_status(self): + res = self.client.get(self.url) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'status': 'pending'},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'status': 'rejected'},)) + eq_(res.status_code, 200) + objs = res.json['objects'] + eq_(len(objs), 0) + + self.webapp.update(status=amo.STATUS_REJECTED) + self.refresh('webapp') + + res = self.client.get(self.url + ({'status': 'rejected'},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + self.webapp.update(status=amo.STATUS_PUBLIC) + self.refresh('webapp') + + res = self.client.get(self.url + ({'status': 'public'},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'status': 'any'},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'status': 'vindaloo'},)) + eq_(res.status_code, 400) + error = res.json['error_message'] + eq_(error.keys(), ['status']) + + def test_is_privileged(self): + res = self.client.get(self.url + ({'is_privileged': True},)) + eq_(res.status_code, 200) + objs = res.json['objects'] + eq_(len(objs), 0) + + res = self.client.get(self.url + ({'is_privileged': False},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'is_privileged': None},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + @patch('versions.models.Version.is_privileged', True) + def test_is_privileged_true(self): + self.webapp.save() + self.refresh('webapp') + + res = self.client.get(self.url + ({'is_privileged': False},)) + eq_(res.status_code, 200) + objs = res.json['objects'] + eq_(len(objs), 0) + + res = self.client.get(self.url + ({'is_privileged': True},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'is_privileged': None},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + def test_is_escalated(self): + res = self.client.get(self.url + ({'is_escalated': True},)) + eq_(res.status_code, 200) + objs = res.json['objects'] + eq_(len(objs), 0) + + res = self.client.get(self.url + ({'is_escalated': False},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'is_escalated': None},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + def test_has_editors_comment(self): + res = self.client.get(self.url + ({'has_editor_comment': True},)) + eq_(res.status_code, 200) + objs = res.json['objects'] + eq_(len(objs), 0) + + res = self.client.get(self.url + ({'has_editor_comment': False},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'has_editor_comment': None},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + def test_has_info_request(self): + res = self.client.get(self.url + ({'has_info_request': True},)) + eq_(res.status_code, 200) + objs = res.json['objects'] + eq_(len(objs), 0) + + res = self.client.get(self.url + ({'has_info_request': False},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'has_info_request': None},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + def test_addon_type(self): + res = self.client.get(self.url + ({'type': 'app'},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + res = self.client.get(self.url + ({'type': 'theme'},)) + eq_(res.status_code, 200) + objs = res.json['objects'] + eq_(len(objs), 0) + + res = self.client.get(self.url + ({'type': 'vindaloo'},)) + eq_(res.status_code, 400) + error = res.json['error_message'] + eq_(error.keys(), ['type']) + + def test_no_region_filtering(self): + self.webapp.addonexcludedregion.create(region=mkt.regions.BR.id) + self.webapp.save() + self.refresh('webapp') + + res = self.client.get(self.url + ({'region': 'br'},)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) + + def test_no_feature_profile_filtering(self): + self.create_switch('buchets') + feature_profile = FeatureProfile().to_signature() + qs = {'q': 'something', 'pro': feature_profile, 'dev': 'firefoxos'} + + # Enable an app feature that doesn't match one in our profile. + self.webapp.current_version.features.update(has_pay=True) + self.webapp.save() + self.refresh('webapp') + + res = self.client.get(self.url + (qs,)) + eq_(res.status_code, 200) + obj = res.json['objects'][0] + eq_(obj['slug'], self.webapp.app_slug) diff --git a/mkt/reviewers/urls.py b/mkt/reviewers/urls.py index 8a07e558e2c..d6b81701667 100644 --- a/mkt/reviewers/urls.py +++ b/mkt/reviewers/urls.py @@ -8,8 +8,9 @@ from . import api, views, views_themes -account = Api(api_name='reviewers') -account.register(api.ReviewingResource()) +reviewers_api = Api(api_name='reviewers') +reviewers_api.register(api.ReviewingResource()) +reviewers_api.register(api.ReviewersSearchResource()) # All URLs under /reviewers/. url_patterns = patterns('', @@ -76,5 +77,5 @@ ) api_patterns = patterns('', - url(r'^', include(account.urls)), # The API. + url(r'^', include(reviewers_api.urls)), # The API. ) diff --git a/mkt/reviewers/views.py b/mkt/reviewers/views.py index 78df5ad182a..dda26349d91 100644 --- a/mkt/reviewers/views.py +++ b/mkt/reviewers/views.py @@ -47,7 +47,7 @@ from mkt.reviewers.forms import DEFAULT_ACTION_VISIBILITY from mkt.reviewers.utils import (AppsReviewing, clean_sort_param, device_queue_search) -from mkt.search.forms import ApiSearchForm +from mkt.reviewers.forms import ApiReviewersSearchForm from mkt.site.helpers import product_as_dict from mkt.submit.forms import AppFeaturesForm from mkt.webapps.models import Webapp @@ -190,7 +190,7 @@ def context(request, **kw): ctx = dict(motd=unmemoized_get_config('mkt_reviewers_motd'), queue_counts=queue_counts(request), search_url=reverse('api_dispatch_list', kwargs={ - 'api_name': 'apps', 'resource_name': 'search'}), + 'api_name': 'reviewers', 'resource_name': 'search'}), statuses=statuses, point_types=amo.REVIEWED_MARKETPLACE) ctx.update(kw) return ctx @@ -541,10 +541,10 @@ def _queue_to_apps(request, queue_qs, date_field='created'): def _get_search_form(request): - form = ApiSearchForm() + form = ApiReviewersSearchForm() fields = [f.name for f in form.visible_fields() + form.hidden_fields()] get = dict((k, v) for k, v in request.GET.items() if k in fields) - return ApiSearchForm(get or None) + return ApiReviewersSearchForm(get or None) @permission_required('Apps', 'Review') diff --git a/mkt/search/api.py b/mkt/search/api.py index d7340abe314..e6058275cb1 100644 --- a/mkt/search/api.py +++ b/mkt/search/api.py @@ -1,26 +1,19 @@ -import json - from django.conf.urls import url -from tastypie import http from tastypie.authorization import ReadOnlyAuthorization from tastypie.throttle import BaseThrottle from tastypie.utils import trailing_slash -from tower import ugettext as _ - -import amo -from access import acl import mkt from mkt.api.authentication import (SharedSecretAuthentication, OptionalOAuthAuthentication) -from mkt.api.base import CORSResource, http_error, MarketplaceResource +from mkt.api.base import CORSResource, MarketplaceResource from mkt.api.resources import AppResource from mkt.constants.features import FeatureProfile from mkt.search.views import _filter_search, _get_query from mkt.search.forms import ApiSearchForm from mkt.webapps.models import Webapp -from mkt.webapps.utils import es_app_to_dict, update_with_reviewer_data +from mkt.webapps.utils import es_app_to_dict class SearchResource(CORSResource, MarketplaceResource): @@ -41,73 +34,65 @@ def get_resource_uri(self, bundle): # Link to the AppResource URI. return AppResource().get_resource_uri(bundle.obj) - def search_form(self, request): + def get_search_data(self, request): form = ApiSearchForm(request.GET if request else None) if not form.is_valid(): raise self.form_errors(form) return form.cleaned_data - def get_list(self, request=None, **kwargs): - form_data = self.search_form(request) - is_admin = acl.action_allowed(request, 'Admin', '%') - is_reviewer = acl.action_allowed(request, 'Apps', 'Review') - - # Pluck out status and addon type first since it forms part of the base - # query, but only for privileged users. - status = form_data['status'] - addon_type = form_data['type'] - - base_filters = { - 'type': addon_type, - } - - # Allow reviewers and admins to search by statuses other than PUBLIC. - if status and (status == 'any' or status != amo.STATUS_PUBLIC): - if is_admin or is_reviewer: - base_filters['status'] = status - else: - raise http_error(http.HttpUnauthorized, - _('Unauthorized to filter by status.')) - - # Only allow reviewers and admin to search by private fields or fields - # depending on the latest_version (which may or may not be public yet). - restricted_data = [form_data.get('is_privileged', None), - form_data.get('has_editor_comment', None), - form_data.get('has_info_request', None), - form_data.get('is_escalated', None)] - - if not (is_admin or is_reviewer) and any(f is not None - for f in restricted_data): - return http.HttpUnauthorized(content=json.dumps( - {'reason': _('Unauthorized to filter by private fields.')})) - - # Filter by device feature profile. + def get_feature_profile(self, request): profile = None if request.GET.get('dev') in ('firefoxos', 'android'): sig = request.GET.get('pro') if sig: profile = FeatureProfile.from_signature(sig) + return profile - # Filter by region. - region = getattr(request, 'REGION', mkt.regions.WORLDWIDE) + def get_region(self, request): + return getattr(request, 'REGION', mkt.regions.WORLDWIDE) + + def get_query(self, request, base_filters=None): + region = self.get_region(request) + return _get_query(request, region, gaia=request.GAIA, + mobile=request.MOBILE, tablet=request.TABLET, + new_idx=True, filters=base_filters) + + def apply_filters(self, request, qs, data=None): + # Build device features profile filter. + profile = self.get_feature_profile(request) - qs = _get_query(request, region, gaia=request.GAIA, - mobile=request.MOBILE, tablet=request.TABLET, - filters=base_filters, new_idx=True) - qs = _filter_search(request, qs, form_data, region=region, - profile=profile) + # Build region filter. + region = self.get_region(request) + + return _filter_search(request, qs, data, region=region, + profile=profile) + + def paginate_results(self, request, qs): paginator = self._meta.paginator_class(request.GET, qs, resource_uri=self.get_resource_list_uri(), limit=self._meta.limit) page = paginator.page() + page['objects'] = self.rehydrate_results(request, page['objects']) + return page + def rehydrate_results(self, request, qs): # Rehydrate the results as per tastypie. objs = [] - for obj in page['objects']: + for obj in qs: obj.pk = obj.id objs.append(self.build_bundle(obj=obj, request=request)) + return [self.full_dehydrate(bundle) for bundle in objs] + + def get_list(self, request=None, **kwargs): + form_data = self.get_search_data(request) + + base_filters = { + 'type': form_data['type'], + } - page['objects'] = [self.full_dehydrate(bundle) for bundle in objs] + qs = self.get_query(request, base_filters=base_filters) + qs = self.apply_filters(request, qs, data=form_data) + page = self.paginate_results(request, qs) # This isn't as quite a full as a full TastyPie meta object, # but at least it's namespaced that way and ready to expand. @@ -122,17 +107,14 @@ def dehydrate(self, bundle): profile=amo_user, request=bundle.request)) - # Add extra data for reviewers. Used in reviewer tool search. - bundle = update_with_reviewer_data(bundle, using_es=True) - return bundle def override_urls(self): return [ url(r'^(?P%s)/featured%s$' % (self._meta.resource_name, trailing_slash()), - self.wrap_view('with_featured'), name='api_with_featured') - ] + self.wrap_view('with_featured'), name='api_with_featured'), + ] def with_featured(self, request, **kwargs): return WithFeaturedResource().dispatch('list', request, **kwargs) @@ -150,18 +132,14 @@ class Meta(SearchResource.Meta): slug_lookup = None def alter_list_data_to_serialize(self, request, data): - form_data = self.search_form(request) + form_data = self.get_search_data(request) region = getattr(request, 'REGION', mkt.regions.WORLDWIDE) cat_slug = form_data.get('cat') if cat_slug: cat_slug = [cat_slug] # Filter by device feature profile. - profile = None - if request.GET.get('dev') in ('firefoxos', 'android'): - sig = request.GET.get('pro') - if sig: - profile = FeatureProfile.from_signature(sig) + profile = self.get_feature_profile(request) qs = Webapp.featured(cat=cat_slug, region=region, profile=profile) diff --git a/mkt/search/forms.py b/mkt/search/forms.py index 96887ceaef2..34ea48b9f33 100644 --- a/mkt/search/forms.py +++ b/mkt/search/forms.py @@ -5,18 +5,11 @@ from addons.models import Category import amo -from mkt.api.forms import CustomNullBooleanSelect, SluggableModelChoiceField +from mkt.api.forms import SluggableModelChoiceField ADDON_CHOICES = [(k, k) for k in amo.MKT_ADDON_TYPES_API.keys()] -# We set 'any' here since we need to default this field -# to PUBLIC if not specified for consumer pages. -STATUS_CHOICES = [('any', _lazy(u'Any Status'))] -for status in amo.WEBAPPS_UNLISTED_STATUSES + (amo.STATUS_PUBLIC,): - STATUS_CHOICES.append((amo.STATUS_CHOICES_API[status], - amo.STATUS_CHOICES[status])) - SORT_CHOICES = [ (None, _lazy(u'Relevance')), ('popularity', _lazy(u'Popularity')), @@ -91,8 +84,6 @@ class ApiSearchForm(forms.Form): 'placeholder': _lazy(u'Search')})) type = forms.ChoiceField(required=False, choices=ADDON_CHOICES, label=_lazy(u'Add-on type')) - status = forms.ChoiceField(required=False, choices=STATUS_CHOICES, - label=_lazy(u'Status')) cat = SluggableModelChoiceField( queryset=Category.objects.filter(type=amo.ADDON_WEBAPP), sluggable_to_field_name='slug', required=False) @@ -104,21 +95,6 @@ class ApiSearchForm(forms.Form): app_type = forms.ChoiceField(required=False, label=_lazy(u'App type'), choices=APP_TYPE_CHOICES) manifest_url = forms.CharField(required=False, label=_lazy('Manifest URL')) - is_privileged = forms.NullBooleanField(required=False, - label=_lazy(u'Privileged App'), - widget=CustomNullBooleanSelect) - has_editor_comment = forms.NullBooleanField( - required=False, - label=_lazy(u'Contains Editor Comment'), - widget=CustomNullBooleanSelect) - has_info_request = forms.NullBooleanField( - required=False, - label=_lazy(u'More Information Requested'), - widget=CustomNullBooleanSelect) - is_escalated = forms.NullBooleanField( - required=False, - label=_lazy(u'Escalated'), - widget=CustomNullBooleanSelect) sort = forms.MultipleChoiceField(required=False, choices=LISTING_SORT_CHOICES) # TODO: Drop this back to a reasonable value when we do pagination. @@ -143,15 +119,8 @@ def clean_cat(self): return self.cleaned_data['cat'].slug def clean_type(self): - return amo.MKT_ADDON_TYPES_API.get(self.cleaned_data['type'], - amo.ADDON_WEBAPP) - - def clean_status(self): - status = self.cleaned_data['status'] - if status == 'any': - return 'any' - - return amo.STATUS_CHOICES_API_LOOKUP.get(status, amo.STATUS_PUBLIC) + return amo.MKT_ADDON_TYPES_API.get(self.cleaned_data['type'], + amo.ADDON_WEBAPP) def clean_premium_types(self): pt_ids = [] diff --git a/mkt/search/tests/test_api.py b/mkt/search/tests/test_api.py index f7414f69388..63ad1632e58 100644 --- a/mkt/search/tests/test_api.py +++ b/mkt/search/tests/test_api.py @@ -1,7 +1,4 @@ import json -from datetime import datetime - -from django.contrib.auth.models import User from mock import Mock, patch from nose.tools import eq_, ok_ @@ -15,7 +12,6 @@ from users.models import UserProfile from mkt.api.base import list_url -from mkt.api.models import Access, generate from mkt.api.tests.test_oauth import BaseOAuth, OAuthClient from mkt.constants.features import FeatureProfile from mkt.search.forms import DEVICE_CHOICES_IDS @@ -171,6 +167,16 @@ def test_dehydrate_regions(self): ok_(mkt.regions.BR.slug not in [r['slug'] for r in regions]) eq_(len(regions), len(mkt.regions.ALL_REGION_IDS) - 1) + def test_region_filtering(self): + self.webapp.addonexcludedregion.create(region=mkt.regions.BR.id) + self.webapp.save() + self.refresh('webapp') + + res = self.client.get(self.url + ({'region': 'br'},)) + eq_(res.status_code, 200) + objs = res.json['objects'] + eq_(len(objs), 0) + def test_q(self): res = self.client.get(self.url + ({'q': 'something'},)) eq_(res.status_code, 200) @@ -257,91 +263,6 @@ def test_app_type_packaged(self): obj = res.json['objects'][0] eq_(obj['slug'], self.webapp.app_slug) - def test_status_anon(self): - res = self.client.get(self.url + ({'status': 'public'},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - res = self.client.get(self.url + ({'status': 'vindaloo'},)) - eq_(res.status_code, 400) - error = res.json['error_message'] - eq_(error.keys(), ['status']) - - res = self.client.get(self.url + ({'status': 'any'},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by status.') - - res = self.client.get(self.url + ({'status': 'rejected'},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by status.') - - def test_is_privileged_anon(self): - res = self.client.get(self.url + ({'is_privileged': True},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by private fields.') - - res = self.client.get(self.url + ({'is_privileged': False},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by private fields.') - - res = self.client.get(self.url + ({'is_privileged': None},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - def test_has_editor_comment_anon(self): - res = self.client.get(self.url + ({'has_editor_comment': True},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by private fields.') - - res = self.client.get(self.url + ({'has_editor_comment': False},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by private fields.') - - res = self.client.get(self.url + ({'has_editor_comment': None},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - def test_is_esclated_anon(self): - res = self.client.get(self.url + ({'is_escalated': True},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by private fields.') - - res = self.client.get(self.url + ({'is_escalated': False},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by private fields.') - - res = self.client.get(self.url + ({'is_escalated': None},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - def test_has_info_request_anon(self): - res = self.client.get(self.url + ({'has_info_request': True},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by private fields.') - - res = self.client.get(self.url + ({'has_info_request': False},)) - eq_(res.status_code, 401) - eq_(json.loads(res.content)['reason'], - 'Unauthorized to filter by private fields.') - - res = self.client.get(self.url + ({'has_info_request': None},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - def test_status_value_packaged(self): # When packaged and not a reviewer we exclude latest version status. self.webapp.update(is_packaged=True) @@ -478,137 +399,6 @@ def test_bad_profile_on_desktop(self): eq_(obj['slug'], self.webapp.app_slug) -class TestApiReviewer(BaseOAuth, ESTestCase): - fixtures = fixture('webapp_337141', 'user_2519') - - def setUp(self, api_name='apps'): - self.user = User.objects.get(pk=2519) - self.profile = self.user.get_profile() - self.profile.update(read_dev_agreement=datetime.now()) - self.grant_permission(self.profile, 'Apps:Review') - - self.access = Access.objects.create( - key='test_oauth_key', secret=generate(), user=self.user) - self.client = OAuthClient(self.access, api_name=api_name) - self.url = list_url('search') - - self.webapp = Webapp.objects.get(pk=337141) - self.category = Category.objects.create(name='test', - type=amo.ADDON_WEBAPP) - - self.webapp.save() - self.refresh('webapp') - - def test_status_reviewer(self): - res = self.client.get(self.url + ({'status': 'public'},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - res = self.client.get(self.url + ({'status': 'rejected'},)) - eq_(res.status_code, 200) - objs = res.json['objects'] - eq_(len(objs), 0) - - res = self.client.get(self.url + ({'status': 'any'},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - res = self.client.get(self.url + ({'status': 'vindaloo'},)) - eq_(res.status_code, 400) - error = res.json['error_message'] - eq_(error.keys(), ['status']) - - def test_is_privileged_reviewer(self): - res = self.client.get(self.url + ({'is_privileged': True},)) - eq_(res.status_code, 200) - objs = res.json['objects'] - eq_(len(objs), 0) - - res = self.client.get(self.url + ({'is_privileged': False},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - res = self.client.get(self.url + ({'is_privileged': None},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - def test_is_escalated_reviewer(self): - res = self.client.get(self.url + ({'is_escalated': True},)) - eq_(res.status_code, 200) - objs = res.json['objects'] - eq_(len(objs), 0) - - res = self.client.get(self.url + ({'is_escalated': False},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - res = self.client.get(self.url + ({'is_escalated': None},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - def test_has_editors_comment_reviewer(self): - res = self.client.get(self.url + ({'has_editor_comment': True},)) - eq_(res.status_code, 200) - objs = res.json['objects'] - eq_(len(objs), 0) - - res = self.client.get(self.url + ({'has_editor_comment': False},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - res = self.client.get(self.url + ({'has_editor_comment': None},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - def test_has_info_request_reviewer(self): - res = self.client.get(self.url + ({'has_info_request': True},)) - eq_(res.status_code, 200) - objs = res.json['objects'] - eq_(len(objs), 0) - - res = self.client.get(self.url + ({'has_info_request': False},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - res = self.client.get(self.url + ({'has_info_request': None},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - def test_status_value_packaged(self): - # When packaged we also include the latest version status. - self.webapp.update(is_packaged=True) - res = self.client.get(self.url) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['status'], amo.STATUS_PUBLIC) - - def test_addon_type_reviewer(self): - res = self.client.get(self.url + ({'type': 'app'},)) - eq_(res.status_code, 200) - obj = res.json['objects'][0] - eq_(obj['slug'], self.webapp.app_slug) - - res = self.client.get(self.url + ({'type': 'theme'},)) - eq_(res.status_code, 200) - objs = res.json['objects'] - eq_(len(objs), 0) - - res = self.client.get(self.url + ({'type': 'vindaloo'},)) - eq_(res.status_code, 400) - error = res.json['error_message'] - eq_(error.keys(), ['type']) - - class TestFeaturedNoCategories(BaseOAuth, ESTestCase): fixtures = fixture('user_2519', 'webapp_337141') list_url = list_url('search/featured') diff --git a/mkt/search/tests/test_filters.py b/mkt/search/tests/test_filters.py index 9f8e9571139..56eda365d1d 100644 --- a/mkt/search/tests/test_filters.py +++ b/mkt/search/tests/test_filters.py @@ -1,7 +1,6 @@ import json import test_utils -from mock import patch from nose.tools import ok_ from waffle.models import Flag @@ -14,6 +13,7 @@ from mkt import regions from mkt.api.tests.test_oauth import BaseOAuth from mkt.regions import set_region, UK +from mkt.reviewers.forms import ApiReviewersSearchForm from mkt.search.forms import ApiSearchForm, DEVICE_CHOICES_IDS from mkt.search.views import _filter_search from mkt.site.fixtures import fixture @@ -33,12 +33,14 @@ def setUp(self): # Pick a region that has relatively few filters. set_region(regions.UK.slug) + self.form_class = ApiSearchForm + def _grant(self, rules): self.grant_permission(self.profile, rules) self.req.groups = self.profile.groups.all() def _filter(self, req, filters, sorting=None, **kwargs): - form = ApiSearchForm(filters) + form = self.form_class(filters) if form.is_valid(): qs = Webapp.from_search(self.req, **kwargs) return _filter_search( @@ -74,6 +76,7 @@ def _status_check(self, query, expected=amo.STATUS_PUBLIC): 'Unexpected status. Expected: %s.' % expected) def test_status(self): + self.form_class = ApiReviewersSearchForm # Test all that should end up being public. # Note: Status permission can't be checked here b/c the acl check # happens in the view, not the _filter_search call. diff --git a/mkt/search/views.py b/mkt/search/views.py index 624804b0b01..4d176a45ea4 100644 --- a/mkt/search/views.py +++ b/mkt/search/views.py @@ -98,20 +98,6 @@ def _filter_search(request, qs, query, filters=None, sorting=None, qs = qs.filter(app_type=query['app_type']) if query.get('manifest_url'): qs = qs.filter(manifest_url=query['manifest_url']) - if query.get('is_privileged', None) is not None: - qs = qs.filter(**{ - 'latest_version.is_privileged': query['is_privileged'] - }) - if query.get('has_info_request', None) is not None: - qs = qs.filter(**{ - 'latest_version.has_info_request': query['has_info_request'] - }) - if query.get('has_editor_comment', None) is not None: - qs = qs.filter(**{ - 'latest_version.has_editor_comment': query['has_editor_comment'] - }) - if query.get('is_escalated', None) is not None: - qs = qs.filter(is_escalated=query['is_escalated']) if 'sort' in show: sort_by = [sorting[name] for name in query['sort'] if name in sorting]