Merge lp:~sil/rnr-server/clickreview-api-more-parameters into lp:rnr-server

[Stuart Langridge]

Allow the click reviews API to be queried by package_name (as currently) OR reviewer_username, with an eye to possibly broadening it still further in the future.

[Natalia Bidart (nataliabidart)]

Hi!

There are some tests missing, for example for the changes in the forms.py and admin.py. Also the MP looks quite old, setting MP as Rejected to reduce numbers of landing candidates in @reviewlist. Change status again if this MP is still current.

Thanks!

Unmerged revisions

293. By Stuart Langridge on 2015-01-09

Allow the click reviews API to be queried by package_name (as currently) OR reviewer_username

=== added file 'src/clickreviews/admin.py'
--- src/clickreviews/admin.py	1970-01-01 00:00:00 +0000
+++ src/clickreviews/admin.py	2015-01-09 13:30:38 +0000
@@ -0,0 +1,14 @@
+from django.contrib import admin
+
+from clickreviews.models import (
+    ClickPackage, ClickPackageReview,
+    ClickPackageModeration, ClickPackageReviewModeration,
+    ClickPackageFlag, ClickPackageReviewFlag
+)
+
+admin.site.register(ClickPackage)
+admin.site.register(ClickPackageReview)
+admin.site.register(ClickPackageModeration)
+admin.site.register(ClickPackageReviewModeration)
+admin.site.register(ClickPackageFlag)
+admin.site.register(ClickPackageReviewFlag)
 
=== modified file 'src/clickreviews/api/handlers.py'
--- src/clickreviews/api/handlers.py	2014-10-14 16:08:08 +0000
+++ src/clickreviews/api/handlers.py	2015-01-09 13:30:38 +0000
@@ -54,9 +54,16 @@
         if not form.is_valid():
             return HttpResponseBadRequest(form.errors_json())
 
-        return ClickPackageReview.objects.filter(
-            click_package__package_name=form.cleaned_data['package_name'],
-            date_deleted=None).order_by(
+        reviews = ClickPackageReview.objects.filter(date_deleted=None)
+        if form.cleaned_data["package_name"]:
+            reviews = reviews.filter(
+                click_package__package_name=form.cleaned_data['package_name']
+            )
+        if form.cleaned_data["reviewer_username"]:
+            reviews = reviews.filter(
+                reviewer__username=form.cleaned_data['reviewer_username']
+            )
+        return reviews.order_by(
             '-usefulness_wilson_rating').select_related(
             'click_package', 'reviewer')
 
 
=== modified file 'src/clickreviews/forms.py'
--- src/clickreviews/forms.py	2014-10-07 19:04:20 +0000
+++ src/clickreviews/forms.py	2015-01-09 13:30:38 +0000
@@ -1,6 +1,7 @@
 from django import forms
 from django.conf import settings
 from django.core import validators
+from django.core.exceptions import ValidationError
 from django.utils.translation import ugettext as _
 
 from clickreviews.models import (
@@ -87,7 +88,16 @@
 
 
 class ClickPackageReviewFilterForm(forms.Form, JSONErrorMixin):
-    package_name = forms.CharField(max_length=255)
+    package_name = forms.CharField(max_length=255, required=False)
+    reviewer_username = forms.CharField(max_length=255, required=False)
+
+    def clean(self):
+        check = [self.cleaned_data.get(k) for k in self.fields.keys()]
+        if not any(check):
+            raise ValidationError('You must provide one of %(fns)s',
+                                  params={'fns':
+                                          ', '.join(self.fields.keys())})
+        return self.cleaned_data
 
 
 class BaseClickPackageFlagForm(forms.ModelForm, JSONErrorMixin):
 
=== modified file 'src/clickreviews/tests/test_handlers.py'
--- src/clickreviews/tests/test_handlers.py	2014-10-16 13:09:52 +0000
+++ src/clickreviews/tests/test_handlers.py	2015-01-09 13:30:38 +0000
@@ -194,11 +194,46 @@
             'usefulness_favorable': 0,
         }, result_review)
 
-    def test_must_filter_by_packagename(self):
+    def test_get_reviews_contains_correct_data_by_reviewer(self):
+        package = self.factory.makeClickPackage(
+            package_name="com.example.me.myapp")
+        review = self.factory.makeClickPackageReview(
+            click_package=package, language='en',
+            rating=5, summary='Best app', review_text='Unlimited fun!',
+            version='0.1.1', date_created=datetime(
+                2014, 1, 15, 11, 13, 55, 123456, tzinfo=pytz.UTC))
+
+        json_result = self._get_reviews(data={
+            'reviewer_username': review.reviewer_username(),
+        })
+        self.assertEqual(200, json_result.status_code)
+        result = json.loads(json_result.content)
+        self.assertEqual(1, len(result))
+        result_review = result[0]
+        DateTimeAwareJSONEncoder().encode(
+            review.date_created).strip('"')
+        self.assertEqual({
+            'id': review.id,
+            'package_name': 'com.example.me.myapp',
+            'language': 'en',
+            'date_created': '2014-01-15T11:13:55.123Z',
+            'rating': 5,
+            'reviewer_username': review.reviewer_username(),
+            'reviewer_displayname': review.reviewer_displayname(),
+            'summary': 'Best app',
+            'review_text': 'Unlimited fun!',
+            'hide': False,
+            'date_deleted': None,
+            'version': '0.1.1',
+            'usefulness_total': 0,
+            'usefulness_favorable': 0,
+        }, result_review)
+
+    def test_must_filter_by_something(self):
         result = self._get_reviews()
 
         self.assertEqual(400, result.status_code)
-        self.assertEqual(['package_name'],
+        self.assertEqual(['__all__'],
                          json.loads(result.content)['errors'].keys())
 
     def test_doesnt_include_deleted(self):