New Datagrid Columns: People and Groups

Review Request #2290 — Created April 15, 2011 and submitted


Review Board


I extended and realized two new columns for Dashboard: 
Review Groups and Review People. In our case all requests are assigned 
to a team group. Therefore the People column helps my team to see 
which requests are not yet processed. 

Both columns are sortable. In order to allow sorting I had to disable optimization. This will be only done if People or Group is actually used for sorting.
It turned out the current implementation relies on optimization. I had to make sure that columns are always added - not only with the query is optimized.

    1. So, I'm actually thinking this needs to be simplified to just not allow sorting on these columns. I'm guessing that sorting is something you guys want, but we can't go this route. Turning off optimizations will actually cause the dashboard to totally swamp a server, with a large enough install. We've seen it happen before, and I really don't want to deal with those bugs again :)
      If we were to have any sort of advanced sorting, we'd need it to be optimization-friendly, cross-database, and it'd need to live entirely in the djblets datagrids backend code.
    2. First off, thanks for your feedback!
      I fully agree to you. While implementing the "sortable" feature I discovered some weaknesses / missing features in djblets. However, my intention was to keep changes local and not to change djblets. That's something for more experienced djblets users.
      One question about using my implementation: Sorting groups/people columns is used only occasionally by users. Might this already have a heavy impact on performance and stability? What do you think? 
      I updated a highly reduced diff. No raw SQL, no sorting, just the two new columns. 
  2. reviewboard/reviews/ (Diff revision 1)
    I'd need to see some testing done about this working with MySQL, PostgreSQL, and sqlite. Though I will say, I'd much prefer to minimize raw SQL here.
  3. reviewboard/reviews/ (Diff revision 1)
    This must match the rest of our style.
    1. OK. I will use something like enable_sorting_on_virtual_columns in future and maybe a shorter version as well ;-)
  4. reviewboard/reviews/ (Diff revision 1)
    Make sure all lines are < 80 columns.
    1. The new diff does not contain such long lines.
Review request changed

Change Summary:

simplified, non-sortable solution


Revision 2 (+28)

Show changes

  1. Pushed to master d88d045. Thanks!