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. reviewboard/templates/reviews/new_review_request.html (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    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.

  3. 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.

  4. reviewboard/webapi/resources/repository.py (Diff revision 1)
     
     
     
     

    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.

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

    Maybe cleaner as:

    queryset = queryset.filter(q).distinct()
    
    if 'q' in request.GET:
        queryset = queryset.order_by('name')
    
    return queryset
    
  6. 
      
david
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (81374e2)
Loading...