Ratings and Reviews server

Merge lp://staging/~michael.nelson/rnr-server/709172-reviews-per-distroseries-2 into lp://staging/rnr-server

  1. 709172-reviews-per-distroseries-2
  2. Merge into trunk
Proposed by Michael Nelson
Status: Merged
Approved by: Łukasz Czyżykowski
Approved revision: 161
Merged at revision: 155
Proposed branch: lp://staging/~michael.nelson/rnr-server/709172-reviews-per-distroseries-2
Merge into: lp://staging/rnr-server
Diff against target: 413 lines (+191/-34)
8 files modified
src/reviewsapp/api/handlers.py (+32/-5)
src/reviewsapp/api/urls.py (+10/-0)
src/reviewsapp/management/commands/update_stats.py (+2/-2)
src/reviewsapp/models/reviews.py (+22/-2)
src/reviewsapp/tests/factory.py (+1/-1)
src/reviewsapp/tests/test_command_update_stats.py (+2/-2)
src/reviewsapp/tests/test_handlers.py (+94/-21)
src/reviewsapp/tests/test_models.py (+28/-1)
To merge this branch: bzr merge lp://staging/~michael.nelson/rnr-server/709172-reviews-per-distroseries-2
Reviewer Review Type Date Requested Status
Danny Tamez (community) Approve
Review via email: mp+53609@code.staging.launchpad.net

Commit message

[r=zematynnad,lukasz][bug=709172] Enable per origin or repository stats via the api.

Description of the change

Overview
========

This branch continues on from:

https://code.edge.launchpad.net/~michael.nelson/rnr-server/709172-reviews-per-distroseries/+merge/53450

It:
1) Ensures that when a review is added/hidden that all aggregates are updated.
2) Enables the rnrclient to request stats for all software items (irrespective of origin/distroseries), or for a specific origin, or for a specific origin/distroseries.
3) Updates the update_stats management command to update SoftwareItemInRepository (which updates the others in turn.

The following diff for rnrclient is necessary to land this branch (but is backwards compatible):

http://pastebin.ubuntu.com/581047/

To post a comment you must log in.
Revision history for this message
Danny Tamez (zematynnad) wrote :

Looks good.

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

Conversation with achuni related to r160:
14:46 < achuni> noodles: about your MP
14:47 < achuni> are the extra fields for the filters you do below, or for the encoded json to have the right keys?
14:47 <@noodles> To get the correct keys in the encoded json without iterating the result.
14:48 < achuni> noodles: iirc piston lets you do that with the right kind word...
14:48 * achuni checks the code
14:49 <@noodles> They just ensure that 'package_name' and 'app_name' are valid targets of the sql select query (as values() doesn't take kwargs, like aggregate() does for the same reason)
14:49 <@noodles> achuni: ah, that'd be perfect... although it would need to cope with both options ('package_name' and 'softwareitem__package_name')
14:49 <@noodles> (as the queryset is on a different model depending on the filtering options)
14:51 <@noodles> If piston lets us do that conditionally, that'd be perfect, otherwise, the current solution works, I just don't like having sql in the code.
14:54 < achuni> ah, hm, so we don't use fields at all, I see...
14:55 <@noodles> Yeah, this result set is just values, not a result set of items.
15:25 < zematynnad> noodles: on your mp line 42ff (I think the answer is no but just checking..) there isn't ever a case where origin is 'any' but distroseries is specified, correct?
15:27 <@noodles> zematynnad: correct
15:27 < zematynnad> cool thanks
15:45 < achuni> noodles: re: your MP, it might make sense to leave the old calls in place, behaving as /any/any/ for backwards compatibility until the client has switched over
15:46 <@noodles> achuni: the rnrclient change is backwards compatible, ah but right, so that the current rnrclient will still work too. Ack.
15:46 < achuni> yup
15:47 < achuni> noodles: and, nope, I can't think of a better workaround than the one in your mp

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches