Add a link to the latest diff revision in the review request resource

Review Request #8270 — Created July 6, 2016 and submitted

Information

Review Board
release-2.5.x
9c5f21c...

Reviewers

The review request resource now links to the latest diff (as
latest_diff in the links section of the response) when the
associated review request has diffs.

Ran unit tests.

Description From Last Updated

Col: 80 E501 line too long (95 > 79 characters)

reviewbotreviewbot

A useful method, but this is going to hit the database per item. Instead, I'd suggest updating get_queryset to do …

chipx86chipx86

We should also do a self.assertNumQueries with this. I'd expect the value to be (old value) + 1, with the …

chipx86chipx86

Too many blank lines.

chipx86chipx86

Col: 80 E501 line too long (95 > 79 characters)

reviewbotreviewbot

Just to verify, this isn't doing any extra queryies, because of the prefetch_related, right? Can we add a comment here …

chipx86chipx86

Can we put request on its own line? In this particular case, it helps the argument from getting a bit …

chipx86chipx86

Col: 80 E501 line too long (95 > 79 characters)

reviewbotreviewbot
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_review_request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_review_request.py
    
    
  2. Show all issues
    Col: 80
     E501 line too long (95 > 79 characters)
    
  3. 
      
chipx86
  1. 
      
  2. Show all issues

    A useful method, but this is going to hit the database per item. Instead, I'd suggest updating get_queryset to do a prefetch_related for DiffSet. (I wish we had the newer Prefetch stuff to limit fields and be smarter about queries, but alas). I think this would have to look something like:

    queryset = queryset.select_related('diffset_history').prefetch_related('diffsets')
    

    Then instead of get_latest_diffset, we'd need to grab the last diff in review_request.diffset_history.diffsets.all().

    Something like that.

  3. Show all issues

    We should also do a self.assertNumQueries with this. I'd expect the value to be (old value) + 1, with the above query.

  4. reviewboard/webapi/tests/test_review_request.py (Diff revision 1)
     
     
     
     
     
     
    Show all issues

    Too many blank lines.

  5. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_review_request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_review_request.py
    
    
  2. Show all issues
    Col: 80
     E501 line too long (95 > 79 characters)
    
  3. 
      
chipx86
  1. 
      
  2. reviewboard/webapi/resources/review_request.py (Diff revision 2)
     
     
     
     
     
    Show all issues

    Just to verify, this isn't doing any extra queryies, because of the prefetch_related, right? Can we add a comment here just mentioning that we already have this data and it's not a new query?

    Reason being that otherwise, diffsets.latest().only('revision') would be a better query, if we didn't already have the data.

  3. Show all issues

    Can we put request on its own line? In this particular case, it helps the argument from getting a bit lost in the other noise.

  4. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_review_request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_request.py
        reviewboard/webapi/tests/test_review_request.py
    
    
  2. Show all issues
    Col: 80
     E501 line too long (95 > 79 characters)
    
  3. 
      
david
  1. Ship It!
  2. 
      
brennie
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.5.x (6e19d96)