Fix 500s from the diff context resource

Review Request #9065 - Created July 10, 2017 and submitted

Barret Rennie
Review Board
release-3.0.x
ce44bed...
reviewboard

The refactoring of views into mixins broke subtly broke the diff context
resource. It was passing in all arguments positionally and the new
CheckLocalSiteAccessViewMixin was pulling local_site_name out of both
kwargs and (mistakenly) from a caller passing *args. We now pass all
arguments in as keyword arguments to avoid this mess.

This patch also removes a bunch of dead imports I noticed in
reviews/views as a result of the aforementioned refactor.

  • Ran unit tests.
  • With this patch applied, there are no more 500s on the diff context
    resource when viewing interdiffs.
  • 0
  • 0
  • 6
  • 0
  • 6
Description From Last Updated
Barret Rennie
Barret Rennie
Christian Hammond
  1. 
      
  2. Looks good, but it would be nicer to remove the imports in a separate change, since otherwise that file isn't even being touched.

    1. Actually.. can you also add a unit test? Ideally this would have been caught by the tests :/

  3. 
      
Christian Hammond
  1. 
      
  2. reviewboard/webapi/tests/test_diff_context.py (Diff revision 2)
     
     
     
     

    No blank line.

  3. Wrong order.

  4. reviewboard/webapi/tests/test_diff_context.py (Diff revision 2)
     
     
     
     

    Did you mean to leave this in?

  5. reviewboard/webapi/tests/urls.py (Diff revision 2)
     
     
     
     

    Two blank lines.

  6. 
      
Barret Rennie
David Trowbridge
  1. 
      
  2. It doesn't look like you posted the right revision range...

  3. 
      
Barret Rennie
David Trowbridge
  1. Ship It!
  2. 
      
Christian Hammond
  1. Ship It!
  2. 
      
Barret Rennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (54c9d2f)
Loading...