Search & Reviewer Search API cleanup and documentation (bug 899232)

Author: diox

docs/api/topics/reviewers.rst

@@ -9,6 +9,73 @@ Reviewer API provides access to the reviewer tools.
 Reviewing
 =========
 
+.. note:: Requires authentication and permission to review apps.
+
+.. http:get::  /api/v1/reviewers/search/
+
+    Performs a search just like the regular Search API, but customized with
+    extra parameters and different (smaller) apps objects returned, containing
+    only the information that is required for reviewer tools.
+
+    **Response**:
+
+    :param meta: :ref:`meta-response-label`.
+    :type meta: object
+    :param objects: A :ref:`listing <objects-response-label>` of :ref:`apps <reviewers-app-response-label>`.
+    :type objects: array
+
+    :status 200: successfully completed.
+
+    .. _reviewers-app-response-label:
+
+    Each app in the response will contain the following:
+
+    :param device_types: a list of the device types at least one of:
+        `desktop`, `mobile`, `tablet`, `firefoxos`. `mobile` and `tablet` both
+        refer to Android mobile and tablet. As opposed to Firefox OS.
+    :type device_types: array
+    :param id: the app's id.
+    :type id: int
+    :param is_escalated: a boolean indicating whether this app is currently
+        in the escalation queue or not.
+    :type is_escalated: boolean
+    :param is_packaged: a boolean indicating whether the app is packaged or
+        not.
+    :type is_packaged: boolean
+    :param latest_version: an array containing the following information about
+        the app's latest version:
+    :type latest_version: object
+    :param latest_version.has_editor_comment: a boolean indicathing whether
+        that version contains comments from a reviewer.
+    :type latest_version.has_editor_comment: boolean
+    :param latest_version.has_info_request: a boolean indicathing whether that
+        version contains an information request from a reviewer.
+    :type latest_version.has_info_request: boolean
+    :param latest_version.is_privileged: a boolean indicating whether this
+        version is a privileged app or not.
+    :type latest_version.is_privileged: boolean
+    :param latest_version.status: an int representing the version status. Can
+        be different from the app status, since the latest_version can be
+        different from the latest public one.
+    :type latest_version.status: int
+    :param name: the name of the app
+    :type name: string
+    :param premium_type: one of ``free``, ``premium``, ``free-inapp``,
+        ``premium-inapp``. If ``premium`` or ``premium-inapp`` the app should
+        be bought, check the ``price`` field to determine if it can.
+    :type premium_type: string
+    :param price: If it is a paid app this will be a string representing
+        the price in the currency calculated for the request. If ``0.00`` then
+        no payment is required, but the app requires a receipt. If ``null``, a
+        price cannot be calculated for the region and cannot be bought.
+        Example: 1.00
+    :type price: string|null
+    :param name: the URL slug for the app
+    :type name: string
+    :param status: an int representing the version status.
+    :type latest_version.status: int
+
+
 .. note:: Requires authentication and permission to review apps.
 
 .. warning:: Not available through CORS.

docs/api/topics/search.rst

@@ -47,16 +47,6 @@ Search
         'price', 'created', separated by commas. Sorts by relevance by default.
     :type sort: string
 
-    The following parameters requires an OAuth token by a user with App
-    Reviewer privileges:
-
-    :param optional status: Filters by app status. Default is 'public'. One
-        of 'pending', 'public', 'disabled', 'rejected', 'waiting'.
-    :type status: string
-    :param optional is_privileged: Filters by whether the latest version of the
-        app is privileged or not.
-    :type is_privileged: boolean
-
     **Response**
 
     :param meta: :ref:`meta-response-label`.

mkt/api/resources.py

@@ -45,7 +45,7 @@
 from mkt.submit.forms import AppDetailsBasicForm
 from mkt.webapps.models import get_excluded_in
 from mkt.webapps.tasks import _update_manifest
-from mkt.webapps.utils import app_to_dict, update_with_reviewer_data
+from mkt.webapps.utils import app_to_dict
 
 log = commonware.log.getLogger('z.api')
 
@@ -244,16 +244,13 @@ def dehydrate(self, bundle):
         bundle.data['privacy_policy'] = (
             PrivacyPolicyResource().get_resource_uri(bundle))
 
-        # Add extra data for reviewers. Used in reviewer tool search.
-        bundle = update_with_reviewer_data(bundle, using_es=False)
         self.dehydrate_extra(bundle)
         return bundle
 
     def dehydrate_extra(self, bundle):
         if bundle.obj.upsold:
             bundle.data['upsold'] = self.get_resource_uri(bundle.obj.upsold.free)
 
-
     def hydrate_premium_type(self, bundle):
         typ = amo.ADDON_PREMIUM_API_LOOKUP.get(bundle.data['premium_type'],
                                                None)

mkt/reviewers/api.py

@@ -14,7 +14,6 @@
 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):
@@ -45,8 +44,9 @@ 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']
+        fields = ['device_types', 'id', 'is_escalated', 'is_packaged',
+                  'latest_version', 'name', 'premium_type', 'price', 'slug',
+                  'status']
 
     def get_search_data(self, request):
         form = ApiReviewersSearchForm(request.GET if request else None)
@@ -88,8 +88,15 @@ def get_query(self, request, base_filters=None):
 
     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))
+
+        # Add reviewer-specific stuff that's not in the standard dehydrate.
+        bundle.data['latest_version'] = bundle.obj.latest_version
+        bundle.data['is_escalated'] = bundle.obj.is_escalated
+
+        # Throw away anything not in _meta.fields.
+        filtered_data = {}
+        for k in self._meta.fields:
+            filtered_data[k] = bundle.data[k]
+        bundle.data = filtered_data
+
         return bundle

mkt/reviewers/tests/test_api.py

@@ -19,6 +19,7 @@
 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.api import ReviewersSearchResource
 from mkt.reviewers.utils import AppsReviewing
 from mkt.site.fixtures import fixture
 from mkt.webapps.models import Webapp
@@ -90,6 +91,12 @@ def setUp(self, api_name='reviewers'):
         self.webapp.update(status=amo.STATUS_PENDING)
         self.refresh('webapp')
 
+    def test_fields(self):
+        res = self.client.get(self.url)
+        eq_(res.status_code, 200)
+        obj = res.json['objects'][0]
+        self.assertSetEqual(obj.keys(), ReviewersSearchResource._meta.fields)
+
     def test_anonymous_access(self):
         res = self.anon.get(self.url)
         eq_(res.status_code, 401)

mkt/webapps/utils.py

@@ -251,39 +251,3 @@ def es_app_to_dict(obj, region=None, profile=None, request=None):
         }
 
     return data
-
-
-def update_with_reviewer_data(bundle, using_es=False):
-    """Adds reviewer specific data to app response bundle."""
-    # TODO: Reviewer flags in ES (bug 848446)
-    from editors.models import EscalationQueue
-
-    if acl.action_allowed(bundle.request, 'Apps', 'Review'):
-        # Try bundle.obj._id first if it's coming from elasticsearch.
-        # Fallback to database results using `.id`.
-        addon_id = getattr(bundle.obj, '_id', bundle.obj.id)
-
-        if using_es and hasattr(bundle.obj, 'latest_version'):
-            # If we know we are using elasticsearch and we have latest_version,
-            # then we can directly return it in the results.
-            bundle.data['latest_version'] = bundle.obj.latest_version
-        else:
-            version = Version.objects.filter(addon_id=addon_id).latest()
-            try:
-                latest_version_status = version.statuses[0][1]
-            except IndexError:
-                latest_version_status = None
-            bundle.data['latest_version'] = {
-                'status': latest_version_status,
-                'is_privileged': version.is_privileged,
-                'has_editor_comment': version.has_editor_comment,
-                'has_info_request': version.has_info_request,
-            }
-        if using_es and hasattr(bundle.obj, 'is_escalated'):
-            bundle.data['is_escalated'] = bundle.obj.is_escalated
-        else:
-            escalated = EscalationQueue.objects.filter(
-                addon_id=addon_id).exists()
-            bundle.data['is_escalated'] = escalated
-
-    return bundle