Merge lp://staging/~fgallina/rnr-server/clickpackage-moderation into lp://staging/rnr-server

Proposed by Fabián Ezequiel Gallina
Status: Merged
Approved by: Fabián Ezequiel Gallina
Approved revision: 286
Merged at revision: 276
Proposed branch: lp://staging/~fgallina/rnr-server/clickpackage-moderation
Merge into: lp://staging/rnr-server
Prerequisite: lp://staging/~fgallina/rnr-server/paginated-handler
Diff against target: 1018 lines (+793/-34)
9 files modified
src/clickreviews/api/handlers.py (+131/-6)
src/clickreviews/api/urls.py (+43/-0)
src/clickreviews/forms.py (+55/-1)
src/clickreviews/models.py (+4/-6)
src/clickreviews/tests/test_forms.py (+182/-3)
src/clickreviews/tests/test_handlers.py (+353/-1)
src/core/api/pagination.py (+0/-4)
src/core/models.py (+4/-13)
src/reviewsapp/models.py (+21/-0)
To merge this branch: bzr merge lp://staging/~fgallina/rnr-server/clickpackage-moderation
Reviewer Review Type Date Requested Status
Fabián Ezequiel Gallina (community) Approve
James Westby (community) Approve
Review via email: mp+232713@code.staging.launchpad.net

Commit message

Implementation of moderation endpoints for click packages

All API endpoints require that users have the moderator permissions
and read enpoints support pagination.

For setting a moderation status, a PATCH request is necessary, just
including moderation_text in its data.

Here are API endpoints available for click package moderations, click
package review moderations are omited as the endpoints are exactly the
same, with the exception that the base url path is
/click/api/1.0/review-moderations/.

GET /click/api/1.0/package-moderations/

Serves all click package moderations available of any status.
Supports pagination via the page and page_size GET params.

GET /click/api/1.0/package-moderations/{pending,approved,rejected}

Serves all click package moderations of pending, approved or rejected
status respectively. Supports pagination as well.

GET /click/api/1.0/package-moderations/1/

Returns a single package moderation instance with pk 1 (or 404 if missing).

GET /click/api/1.0/package-moderations/pending/1/

Returns a single pending package moderation instance with pk 1 (or 404 if missing).

PATCH /click/api/1.0/package-moderations/approved/1/

Sets moderation with pk 1 to approved if all validation checks pass.
For the request to work, moderation_text should be included in the
request data (e.g. '{"moderation_text": "sometext"}' using
'application/json' as the value for the Content-Type header).
Supported content types are those of Piston: JSON, YAML, XML and
Pickle.

Description of the change

Implementation of moderation endpoints for click packages

All API endpoints require that users have the moderator permissions
and read enpoints support pagination.

For setting a moderation status, a PATCH request is necessary, just
including moderation_text in its data.

Here are API endpoints available for click package moderations, click
package review moderations are omited as the endpoints are exactly the
same, with the exception that the base url path is
/click/api/1.0/review-moderations/.

GET /click/api/1.0/package-moderations/

Serves all click package moderations available of any status.
Supports pagination via the page and page_size GET params.

GET /click/api/1.0/package-moderations/{pending,approved,rejected}

Serves all click package moderations of pending, approved or rejected
status respectively. Supports pagination as well.

GET /click/api/1.0/package-moderations/1/

Returns a single package moderation instance with pk 1 (or 404 if missing).

GET /click/api/1.0/package-moderations/pending/1/

Returns a single pending package moderation instance with pk 1 (or 404 if missing).

PATCH /click/api/1.0/package-moderations/approved/1/

Sets moderation with pk 1 to approved if all validation checks pass.
For the request to work, moderation_text should be included in the
request data (e.g. '{"moderation_text": "sometext"}' using
'application/json' as the value for the Content-Type header).
Supported content types are those of Piston: JSON, YAML, XML and
Pickle.

To post a comment you must log in.
Revision history for this message
James Westby (james-w) :
review: Needs Fixing
Revision history for this message
Fabián Ezequiel Gallina (fgallina) :
Revision history for this message
James Westby (james-w) wrote :

Hi,

Thanks for making the changes, it all looks good now.

The pattern I like for testing when you add a .select_related() is the following.

def test_queries_dont_scale(self):
    expected_queries = 6
    thing = self.factory.makeThing()
    self.assertNumQueries(expected_queries, self.client.get, '/all-things')
    another_thing = self.factory.makeThing()
    self.assertNumQueries(expected_queries, self.client.get, '/all-things')

This is because for me the test isn't so much about the exact number of queries,
but about the fact that they don't increase with the number of things. Obviously
it asserts the number of queries too, but if there was a way for the first assertion
to be a getNumberOfQueries() or something then using that would be fine.

Overall a very nice branch, takes care of a lot of things.

Thanks,

James

review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

Revision history for this message
Fabián Ezequiel Gallina (fgallina) :
review: Approve

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