• 
      

    Index private review requests for search, and limit their results.

    Review Request #7067 — Created March 16, 2015 and submitted

    Information

    Review Board
    release-2.0.x
    c57f183...

    Reviewers

    Previously, only publicly-accessible review requests were returned in
    the search results. Teams using private repositories or groups wouldn't
    get much benefit from our search support.
    
    Now, we index all the information needed with a review request to limit
    results based on the user's access. This works just like all our
    existing accessible_by functions.
    
    The index stores a private flag (indicating whether the review request
    is inaccessible in general to users), and then a series of IDs for
    invite-only groups, private repositories, and the list of users targeted
    for review.
    
    The search query is then constructed to compare the user's
    currently-accessible groups/repositories against the results of these
    lists. The search results shown are therefore personalized to the user's
    access.
    
    Unit tests were added for covering all the access control checks, along
    with some of the existing search functionality (general searches and
    filters). These tests are based on the equivalent tests for the review
    request accessibility filtering in the database.

    Unit tests pass.

    I created a bunch of review requests with the different combinations of
    access-restricted objects (private repos, invite-only groups), with and
    without my user having permission on those objects. I saw all the results
    I expected.

    I tested this on local sites as well. I also got the results I expected.

    Description From Last Updated

    'RBSearchView' imported but unused

    reviewbotreviewbot

    local variable 'review_request' is assigned to but never used

    reviewbotreviewbot

    Col: 25 E131 continuation line unaligned for hanging indent

    reviewbotreviewbot

    Maybe reformat this a little using a variable? users = review_request.target_people.values_list('pk', flat=True) return list(users) or [0]

    daviddavid

    We should make manager functions for these so they're not duplicated.

    daviddavid

    'Q' imported but unused

    reviewbotreviewbot
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/search/tests.py
          reviewboard/reviews/models/group.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/search_indexes.py
          reviewboard/reviews/managers.py
          reviewboard/search/views.py
          reviewboard/test.py
      
      
      
      Tool: Pyflakes
      Processed Files:
          reviewboard/search/tests.py
          reviewboard/reviews/models/group.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/search_indexes.py
          reviewboard/reviews/managers.py
          reviewboard/search/views.py
          reviewboard/test.py
      
      
    2. reviewboard/search/tests.py (Diff revision 1)
       
       
      Show all issues
       'RBSearchView' imported but unused
      
    3. reviewboard/search/tests.py (Diff revision 1)
       
       
      Show all issues
       local variable 'review_request' is assigned to but never used
      
    4. reviewboard/search/views.py (Diff revision 1)
       
       
      Show all issues
      Col: 25
       E131 continuation line unaligned for hanging indent
      
    5. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/search/tests.py
          reviewboard/reviews/models/group.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/search_indexes.py
          reviewboard/reviews/managers.py
          reviewboard/search/views.py
          reviewboard/test.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/search/tests.py
          reviewboard/reviews/models/group.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/search_indexes.py
          reviewboard/reviews/managers.py
          reviewboard/search/views.py
          reviewboard/test.py
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/reviews/search_indexes.py (Diff revision 2)
       
       
       
       
      Show all issues

      Maybe reformat this a little using a variable?

      users = review_request.target_people.values_list('pk', flat=True)
      return list(users) or [0]
      
    3. reviewboard/search/views.py (Diff revision 2)
       
       
       
       
       
       
       
       
       
      Show all issues

      We should make manager functions for these so they're not duplicated.

      1. These queries are different from all the other variants. There's a RepositoryManager.accessible, for instance, that checks for some things that we don't care about either here or in ReviewRequestManager.accessible, and we have a check here (for the public and invite_only flags) that we don't use in the others.

        I'll see about combining these in a less-than-ugly way.

      2. Okay, guess it doesn't hurt to fetch a few more IDs we know we won't care about. I'll just use those.

    4. 
        
    chipx86
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/scmtools/managers.py
          reviewboard/search/tests.py
          reviewboard/reviews/models/group.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/search_indexes.py
          reviewboard/reviews/managers.py
          reviewboard/search/views.py
          reviewboard/test.py
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/scmtools/managers.py
          reviewboard/search/tests.py
          reviewboard/reviews/models/group.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/search_indexes.py
          reviewboard/reviews/managers.py
          reviewboard/search/views.py
          reviewboard/test.py
      
      
    2. reviewboard/search/views.py (Diff revision 3)
       
       
      Show all issues
       'Q' imported but unused
      
    3. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-2.0.x (f322bb8)