Review Board

beta

Initial stab at reports

Updated 1 year, 4 months ago

David Trowbridge Reviewers
trunk reviewboard
None Review Board SVN
There's a lot unfinished here, but it's good enough for my purposes ;)

The report selector, in particular, needs a lot of love.  At the moment it
only allows selecting the report and a format.  The user and period fields
are wired up, but there's no UI.

 
Posted 1 year, 4 months ago (September 6th, 2007, 11:21 a.m.)
Looks like a good start, but I'd like to see such information accessible on a per-user basis. It'd be nice to just have the wiki auto-embed or auto-link to the generated status report per-user every week. The less w have to do for status reports, the better :)
  1. David Trowbridge 1 year, 4 months ago (September 6th, 2007, 11:27 a.m.)
    That's what the "user" query parameter is for ;)
  2. Christian Hammond 1 year, 4 months ago (September 6th, 2007, 11:42 a.m.)
    Oh, I missed that.
    
    In that case, can we change it to accept a URL of /status_report/<user>/ (or <user>/status_report/ might be better). Should make for a cleaner URL.
    
    I would actually really like to minimize query parameters, so something like /reports/<user>/status_report/<period>/ would be ideal, imho. More natural, easier to remember, and we can set up a matching JSON API path at some point to get the information.
  3. David Trowbridge 1 year, 4 months ago (September 8th, 2007, 2:28 a.m.)
    OK, I've updated it to be /reports/<user>/<report type>/<format>/
    
    Period is still a query string since I want to eventually rework it to be a lot more flexible (right now it's just "n days in the past from right now"), and I'm not yet sure what the final thing will look like.
Posted 1 year, 4 months ago (September 8th, 2007, 2:41 a.m.)

   

  
Loading diff fragment...
Out of curiosity, any reason why we don't rearrange reports to be a dictionary with 'report_review_requests', 'report_reviews', and 'report_status_report' keys?

I suppose it's for the template, though that should be able to work still and auto-sort alphabetically unless we want to preserve a specific sort order. Anyhow, food for thought.

On a similar note, it might be nice to use dictionaries instead of tuples for things like the template name and format and such, to improve code readability in the report function.
Ship it!
Posted 1 year, 4 months ago (September 8th, 2007, 2:31 p.m.)
Awesome, looks good!
Loading diff fragment...
We should at some point write some middleware to always output charset=UTF-8. Though I wonder if we get this already, since diffs and all show in UTF-8.
  1. David Trowbridge 1 year, 4 months ago (September 8th, 2007, 2:36 p.m.)
    I think it does *unless* you specify content-type by hand, like I'm doing here.
  2. Christian Hammond 1 year, 4 months ago (September 8th, 2007, 2:40 p.m.)
    Ohhh, okay. That makes sense.