Review Board

beta

Convert dashboard to be a generic view

Updated 1 year, 7 months ago

David Trowbridge Reviewers
trunk reviewboard
75
None Review Board SVN 
We weren't using generic views in the dashboard because there's
a bug in Q objects that was preventing us from creating a query
for the "all incoming reviews" view.  We hacked around this by
doing the OR+distinct step ourselves using lists.

It turns out that with just a little coaxing, django's generic
object_list view can be used with a list instead of a QuerySet.
This change does that, and makes the dashboard into a simple
superset of object_list.  The major benefit of this is that we
get pagination basically for free.
Set pagination interval very low (for my meager number of
review requests in my test server) and tried a couple
different views in the dashboard.
Christian Hammond
Posted 1 year, 7 months ago (June 4th, 2007, 12:59 a.m.)
Awesome :) Only a couple things.
Loading diff fragment... 
Cute hack. I feel we should move it out of here, though, as it could be useful later.
  1. David Trowbridge 1 year, 7 months ago (June 4th, 2007, 1:19 a.m.) 
    Eh, I'd rather wait.  What we're doing in the dashboard for the incoming
view is pretty bad from a database performance standpoint (we're really
only saved from being totally crunked up by the fact that the db is
filtering for status='P' here).  If we really end up having cause to do
this same kind of hack elsewhere, we can move it to a common place.
  2. Christian Hammond 1 year, 7 months ago (June 4th, 2007, 11:36 a.m.) 
    Fair enough.
Loading diff fragment... 
Can we change object_list to review_request_list? We can name this by setting template_object_name to "review_request" when creating the view.
  1. David Trowbridge 1 year, 7 months ago (June 4th, 2007, 9:55 a.m.) 
    Done.
Loading diff fragment... 
There's a group= parameter that's passed to the page when looking at review requests for a certain group. We need to pass this as well. We can probably just unconditionally set it, since it'll be empty if we're not in group mode, but it would look nicer to make it conditional.
  1. David Trowbridge 1 year, 7 months ago (June 4th, 2007, 9:56 a.m.) 
    Done.  I don't think it's too terrible if it's always there,
since it'll only be in the URL if they're looking at pages
other than the first, which should hopefully never happen.
  2. Christian Hammond 1 year, 7 months ago (June 4th, 2007, 11:36 a.m.) 
    Yeah. It's fine to leave it.
Ship it!
Christian Hammond
Posted 1 year, 7 months ago (June 4th, 2007, 11:38 a.m.)
Looks good. Ship it.