Ratings and Reviews server

Merge lp:~michael.nelson/rnr-server/1564209-return-openid-for-username into lp:rnr-server

  1. 1564209-return-openid-for-username
  2. Merge into trunk
Proposed by Michael Nelson
Status: Merged
Approved by: Michael Nelson
Approved revision: 317
Merged at revision: 316
Proposed branch: lp:~michael.nelson/rnr-server/1564209-return-openid-for-username
Merge into: lp:rnr-server
Diff against target: 87 lines (+35/-3)
4 files modified
src/reviewsapp/api/handlers.py (+5/-3)
src/reviewsapp/models.py (+5/-0)
src/reviewsapp/tests/test_handlers.py (+12/-0)
src/reviewsapp/tests/test_models.py (+13/-0)
To merge this branch: bzr merge lp:~michael.nelson/rnr-server/1564209-return-openid-for-username
Reviewer Review Type Date Requested Status
Thomi Richards (community) Approve
Review via email: mp+290850@code.launchpad.net

Commit message

Return openid for username.

To post a comment you must log in.
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

This looks OK to me, but I have very little idea what I'm looking at. If this is important / security-vulnerable code we really ought to have someone familiar with the system check this out.

review: Approve
Revision history for this message
Michael Nelson (michael.nelson) wrote :

On Mon, Apr 4, 2016 at 2:31 PM Thomi Richards <email address hidden>
wrote:

> Review: Approve
>
> This looks OK to me, but I have very little idea what I'm looking at. If
> this is important / security-vulnerable code we really ought to have
> someone familiar with the system check this out.
>
>
Thanks Thomi. I'll land it now but have Fabian verify if it's OK or needs
more changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/reviewsapp/api/handlers.py'
--- src/reviewsapp/api/handlers.py 2016-02-19 17:11:09 +0000
+++ src/reviewsapp/api/handlers.py 2016-04-04 04:11:07 +0000
@@ -310,9 +310,11 @@
310 'version': version,310 'version': version,
311 }311 }
312 kwargs = dict(item for item in kwargs.items() if item[1] != 'any')312 kwargs = dict(item for item in kwargs.items() if item[1] != 'any')
313 return Review.objects.filter(313 return (Review.objects.prefetch_related('reviewer__useropenid_set')
314 softwareitem__package_name=package_name, hide=False,314 .select_related('softwareitem', 'reviewer', 'repository')
315 **kwargs).order_by("-version", sort_key)[batch_start:batch_end]315 .filter(softwareitem__package_name=package_name, hide=False,
316 **kwargs)
317 .order_by("-version", sort_key)[batch_start:batch_end])
316318
317319
318class ShowReviewsSinceHandler(ShowReviewsHandler):320class ShowReviewsSinceHandler(ShowReviewsHandler):
319321
=== modified file 'src/reviewsapp/models.py'
--- src/reviewsapp/models.py 2014-09-24 22:39:43 +0000
+++ src/reviewsapp/models.py 2016-04-04 04:11:07 +0000
@@ -272,6 +272,11 @@
272 return self.package_name()272 return self.package_name()
273273
274 def reviewer_username(self):274 def reviewer_username(self):
275 # Use .all() so that `prefetch_related` have effect.
276 openid_objs = list(self.reviewer.useropenid_set.all())
277 if openid_objs:
278 openid_obj = openid_objs[-1]
279 return openid_obj.claimed_id.split('/')[-1]
275 return self.reviewer.username280 return self.reviewer.username
276281
277 def reviewer_displayname(self):282 def reviewer_displayname(self):
278283
=== modified file 'src/reviewsapp/tests/test_handlers.py'
--- src/reviewsapp/tests/test_handlers.py 2016-02-19 17:11:09 +0000
+++ src/reviewsapp/tests/test_handlers.py 2016-04-04 04:11:07 +0000
@@ -56,6 +56,7 @@
56from piston.emitters import Emitter56from piston.emitters import Emitter
57from piston.handler import typemapper57from piston.handler import typemapper
5858
59from core.tests.helpers import assert_no_extra_queries_after
59from reviewsapp.api.handlers import (60from reviewsapp.api.handlers import (
60 Django13JSONEncoder,61 Django13JSONEncoder,
61 Django13JSONEmitter,62 Django13JSONEmitter,
@@ -841,6 +842,17 @@
841 response_decoded = simplejson.loads(response.content)842 response_decoded = simplejson.loads(response.content)
842 self.assertEqual(2, len(response_decoded))843 self.assertEqual(2, len(response_decoded))
843844
845 def test_no_extra_queries_for_openid(self):
846 # The username returns an openid identifier from a related
847 # model.
848 package = self.factory.makeSoftwareItem('foo')
849 self.factory.makeReview(software_item=package)
850
851 with patch_settings(CACHE_MIDDLEWARE_SECONDS=0):
852 assert_no_extra_queries_after(
853 lambda: self.factory.makeReview(software_item=package),
854 self.client.get, self._reviews_url_for_package('foo'))
855
844 def test_request_cached(self):856 def test_request_cached(self):
845 # Requests for reviews are cached.857 # Requests for reviews are cached.
846 url = self._reviews_url_for_package('package_foo')858 url = self._reviews_url_for_package('package_foo')
847859
=== modified file 'src/reviewsapp/tests/test_models.py'
--- src/reviewsapp/tests/test_models.py 2014-08-12 14:38:31 +0000
+++ src/reviewsapp/tests/test_models.py 2016-04-04 04:11:07 +0000
@@ -138,6 +138,19 @@
138 self.assertEqual(1, review.usefulness_favorable)138 self.assertEqual(1, review.usefulness_favorable)
139 self.assertEqual(50, review.usefulness_percentage)139 self.assertEqual(50, review.usefulness_percentage)
140140
141 def test_reviewer_username_returns_openid(self):
142 expected = 'theopenid'
143 user = self.factory.makeUser(
144 username='bogusfromlogin', open_id=expected)
145 review = self.factory.makeReview(reviewer=user)
146 self.assertEqual(review.reviewer_username(), expected)
147
148 def test_reviewer_username_fallbacks_to_username(self):
149 expected = 'bogusfromlogin'
150 user = self.factory.makeUser(username=expected, open_id=False)
151 review = self.factory.makeReview(reviewer=user)
152 self.assertEqual(review.reviewer_username(), expected)
153
141154
142class ReviewFlagTestCase(TestCaseWithFactory):155class ReviewFlagTestCase(TestCaseWithFactory):
143156

Subscribers

People subscribed via source and target branches