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

Proposed by Stuart Langridge
Status: Rejected
Rejected by: Natalia Bidart
Proposed branch: lp:~sil/rnr-server/clickreview-api-more-parameters
Merge into: lp:rnr-server
Diff against target: 125 lines (+72/-6)
4 files modified
src/clickreviews/admin.py (+14/-0)
src/clickreviews/api/handlers.py (+10/-3)
src/clickreviews/forms.py (+11/-1)
src/clickreviews/tests/test_handlers.py (+37/-2)
To merge this branch: bzr merge lp:~sil/rnr-server/clickreview-api-more-parameters
Reviewer Review Type Date Requested Status
Ratings and Reviews Developers Pending
Review via email: mp+245958@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

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

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'src/clickreviews/admin.py'
2--- src/clickreviews/admin.py 1970-01-01 00:00:00 +0000
3+++ src/clickreviews/admin.py 2015-01-09 13:30:38 +0000
4@@ -0,0 +1,14 @@
5+from django.contrib import admin
6+
7+from clickreviews.models import (
8+ ClickPackage, ClickPackageReview,
9+ ClickPackageModeration, ClickPackageReviewModeration,
10+ ClickPackageFlag, ClickPackageReviewFlag
11+)
12+
13+admin.site.register(ClickPackage)
14+admin.site.register(ClickPackageReview)
15+admin.site.register(ClickPackageModeration)
16+admin.site.register(ClickPackageReviewModeration)
17+admin.site.register(ClickPackageFlag)
18+admin.site.register(ClickPackageReviewFlag)
19
20=== modified file 'src/clickreviews/api/handlers.py'
21--- src/clickreviews/api/handlers.py 2014-10-14 16:08:08 +0000
22+++ src/clickreviews/api/handlers.py 2015-01-09 13:30:38 +0000
23@@ -54,9 +54,16 @@
24 if not form.is_valid():
25 return HttpResponseBadRequest(form.errors_json())
26
27- return ClickPackageReview.objects.filter(
28- click_package__package_name=form.cleaned_data['package_name'],
29- date_deleted=None).order_by(
30+ reviews = ClickPackageReview.objects.filter(date_deleted=None)
31+ if form.cleaned_data["package_name"]:
32+ reviews = reviews.filter(
33+ click_package__package_name=form.cleaned_data['package_name']
34+ )
35+ if form.cleaned_data["reviewer_username"]:
36+ reviews = reviews.filter(
37+ reviewer__username=form.cleaned_data['reviewer_username']
38+ )
39+ return reviews.order_by(
40 '-usefulness_wilson_rating').select_related(
41 'click_package', 'reviewer')
42
43
44=== modified file 'src/clickreviews/forms.py'
45--- src/clickreviews/forms.py 2014-10-07 19:04:20 +0000
46+++ src/clickreviews/forms.py 2015-01-09 13:30:38 +0000
47@@ -1,6 +1,7 @@
48 from django import forms
49 from django.conf import settings
50 from django.core import validators
51+from django.core.exceptions import ValidationError
52 from django.utils.translation import ugettext as _
53
54 from clickreviews.models import (
55@@ -87,7 +88,16 @@
56
57
58 class ClickPackageReviewFilterForm(forms.Form, JSONErrorMixin):
59- package_name = forms.CharField(max_length=255)
60+ package_name = forms.CharField(max_length=255, required=False)
61+ reviewer_username = forms.CharField(max_length=255, required=False)
62+
63+ def clean(self):
64+ check = [self.cleaned_data.get(k) for k in self.fields.keys()]
65+ if not any(check):
66+ raise ValidationError('You must provide one of %(fns)s',
67+ params={'fns':
68+ ', '.join(self.fields.keys())})
69+ return self.cleaned_data
70
71
72 class BaseClickPackageFlagForm(forms.ModelForm, JSONErrorMixin):
73
74=== modified file 'src/clickreviews/tests/test_handlers.py'
75--- src/clickreviews/tests/test_handlers.py 2014-10-16 13:09:52 +0000
76+++ src/clickreviews/tests/test_handlers.py 2015-01-09 13:30:38 +0000
77@@ -194,11 +194,46 @@
78 'usefulness_favorable': 0,
79 }, result_review)
80
81- def test_must_filter_by_packagename(self):
82+ def test_get_reviews_contains_correct_data_by_reviewer(self):
83+ package = self.factory.makeClickPackage(
84+ package_name="com.example.me.myapp")
85+ review = self.factory.makeClickPackageReview(
86+ click_package=package, language='en',
87+ rating=5, summary='Best app', review_text='Unlimited fun!',
88+ version='0.1.1', date_created=datetime(
89+ 2014, 1, 15, 11, 13, 55, 123456, tzinfo=pytz.UTC))
90+
91+ json_result = self._get_reviews(data={
92+ 'reviewer_username': review.reviewer_username(),
93+ })
94+ self.assertEqual(200, json_result.status_code)
95+ result = json.loads(json_result.content)
96+ self.assertEqual(1, len(result))
97+ result_review = result[0]
98+ DateTimeAwareJSONEncoder().encode(
99+ review.date_created).strip('"')
100+ self.assertEqual({
101+ 'id': review.id,
102+ 'package_name': 'com.example.me.myapp',
103+ 'language': 'en',
104+ 'date_created': '2014-01-15T11:13:55.123Z',
105+ 'rating': 5,
106+ 'reviewer_username': review.reviewer_username(),
107+ 'reviewer_displayname': review.reviewer_displayname(),
108+ 'summary': 'Best app',
109+ 'review_text': 'Unlimited fun!',
110+ 'hide': False,
111+ 'date_deleted': None,
112+ 'version': '0.1.1',
113+ 'usefulness_total': 0,
114+ 'usefulness_favorable': 0,
115+ }, result_review)
116+
117+ def test_must_filter_by_something(self):
118 result = self._get_reviews()
119
120 self.assertEqual(400, result.status_code)
121- self.assertEqual(['package_name'],
122+ self.assertEqual(['__all__'],
123 json.loads(result.content)['errors'].keys())
124
125 def test_doesnt_include_deleted(self):

Subscribers

People subscribed via source and target branches