• 
      

    Add server-side filtering for repositories on "New Review Request"

    Review Request #12140 — Created March 11, 2022 and submitted

    Information

    Review Board
    release-3.0.x
    95b4c81...

    Reviewers

    The "New Review Request" page was designed for smaller servers in mind,
    since in our heads, most people on large servers are (correctly) using
    RBTools. To wit, it was loading the entirety of the repository list into
    the sidebar of the page, and then providing client-side search of the
    names. On servers with very large numbers of repositories, this would
    cause major delays and even timeouts.

    This change fixes all that up to be much better. Rather than loading all
    repositories, we load only the first 25. Instead of the special "File
    attachments only" repository being something that is done in the view,
    it happens entirely client-side. And the "Filter" search box will now
    make queries to the API to regenerate the collection of repositories,
    rather than just filtering the full collection on the client.

    This required adding a few new data fields to the repository API, as
    well as the new "q" query parameter to the list API.

    Created a bunch of different repositories on my local system. Typed some
    stuff in the repository filter field and saw appropriate network
    requests and that the repository collection was reloaded with the
    returned results.

    Description From Last Updated

    Can you add a Version Added?

    chipx86chipx86

    This is just a suggestion and does not need to be done, but we could move this to the pattern …

    chipx86chipx86

    I don't know that we're going to need to do any further 3.0.x releases, aside from the backport we know …

    chipx86chipx86

    If we do __contains, we won't benefit from any indexes (and we've seen what that can do). Maybe just a …

    chipx86chipx86

    Maybe cleaner as: queryset = queryset.filter(q).distinct() if 'q' in request.GET: queryset = queryset.order_by('name') return queryset

    chipx86chipx86
    chipx86
    1. 
        
    2. Show all issues

      Can you add a Version Added?

    3. reviewboard/templates/reviews/new_review_request.html (Diff revision 1)
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      This is just a suggestion and does not need to be done, but we could move this to the pattern of building these attributes up-front in the Django view, then pass the result as {{..|json_dumps}} here, simplifying this greatly.

    4. Show all issues

      I don't know that we're going to need to do any further 3.0.x releases, aside from the backport we know of. Let's just target for 4.0.7. It'll be a little less confusing in the docs that way.

      1. Because of reasons, I'm going to keep this on release-3.0.x while it's going through review. I'll port over to 4.0.x once it's ready.

    5. reviewboard/webapi/resources/repository.py (Diff revision 1)
       
       
       
       
      Show all issues

      If we do __contains, we won't benefit from any indexes (and we've seen what that can do). Maybe just a __startswith?

      I do like the idea of searching by path/mirror_path, but I also wonder if we can get by with just the name, given that.

    6. reviewboard/webapi/resources/repository.py (Diff revision 1)
       
       
       
       
       
       
       
      Show all issues

      Maybe cleaner as:

      queryset = queryset.filter(q).distinct()
      
      if 'q' in request.GET:
          queryset = queryset.order_by('name')
      
      return queryset
      
    7. 
        
    david
    david
    chipx86
    1. Ship It!
    2. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-4.0.x (81374e2)