Issue 2766: Web api 'review-requests' not returning draft review requests

Review Request #6328 — Created Sept. 16, 2014 and submitted

Information

Review Board
master
9e9580a...

Reviewers

Added check for whether the user is admin in webapi/review_request.py. If so, the _query function for ReviewRequestManager (reviews/managers.py) will query for both public and NON public (draft) review requests, as opposed to just querying for public ones.

  • Verified that admin accounts see draft reviews through /api/review-requests/?status=all, but normal accounts do not.
  • This change only affects the webapi, and not any other module that might use the ReviewRequestManager. Verifiedthat the "All Review Requests" page is unaffected here.
  • webapi and reviewboard module unit tests run and passed
  • Added unit tests for private review request visibility in the reviewboard and the webapi unit tests
Description From Last Updated

Can we call this new parameter show_private?

daviddavid

In order to fix the line length issue, you can rewrap this as: queryset = self.model.objects.public( user=request.user, status=status, ...

daviddavid

Col: 80 E501 line too long (80 > 79 characters)

reviewbotreviewbot

Pretty close. We put the closing paren at the end of the last parameter.

daviddavid

We usually include the trailing comma on our list and dictionary literals.

SM smacleod

'P' is the default value for status, but to keep consistent can we add it to this call anyways?

SM smacleod
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
  2. Show all issues
    Col: 80
     E501 line too long (80 > 79 characters)
    
  3. 
      
david
  1. Looks like a good start. Can you link to the bug number in the "Bugs" field?

  2. reviewboard/reviews/managers.py (Diff revision 1)
     
     
    Show all issues

    Can we call this new parameter show_private?

  3. reviewboard/webapi/resources/review_request.py (Diff revision 1)
     
     
     
     
     
     
    Show all issues

    In order to fix the line length issue, you can rewrap this as:

    queryset = self.model.objects.public(
        user=request.user,
        status=status,
        ...
    
    1. I put the closing bracket on a newline for now.

  4. 
      
RM
RM
RM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
  2. 
      
david
  1. 
      
  2. Show all issues

    Pretty close. We put the closing paren at the end of the last parameter.

  3. 
      
RM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
  2. 
      
RM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/tests/test_review_request.py
        reviewboard/reviews/tests.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/tests/test_review_request.py
        reviewboard/reviews/tests.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
  2. 
      
SM
  1. This is looking good! Just a couple of style nits.

  2. reviewboard/reviews/tests.py (Diff revision 4)
     
     
    Show all issues

    We usually include the trailing comma on our list and dictionary literals.

  3. Show all issues

    'P' is the default value for status, but to keep consistent can we add it to this call anyways?

  4. 
      
RM
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/tests/test_review_request.py
        reviewboard/reviews/tests.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/tests/test_review_request.py
        reviewboard/reviews/tests.py
        reviewboard/webapi/resources/review_request.py
        reviewboard/reviews/managers.py
    
    
  2. 
      
ML
  1. What is the difference between filter_private and show_private in def _query?

    1. As we discussed...
      filter_private controls the visibilty for queries coming from NOT the currently logged in user (or an unauthenticated queries all together)
      show_private controls whether review requests with Public=False are returned

  2. 
      
david
  1. Ship It!

  2. 
      
RM
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.0.x (28dbc4a)