Review Board

beta

Fix several issues with draft diffs

Submitted
Updated 8 months, 2 weeks ago

Christian Hammond Reviewers
trunk reviewboard
323
None Review Board SVN 
We had several issues with draft diffs.

1) They were automatically placed in the diffset history of a review request, making them publicly accessible. This meant they were not destroyed when destroying a draft. I'm unsure as to how long this has been broken.
2) The queries for grabbing a draft diff didn't take into account the user accessing the review request.
3) The diff viewer didn't really let you know you were looking at a draft diff, and didn't work when #1 was fixed.

We now keep diffs out of a review request's diffset history when creating them, and we assign them an appropriate revision number. We only check for the draft diffs if the requesting user is logged in and owns the review request. We also modify the diff viewer a bit to tell the user when they're looking at a draft diff.

This fixes bug 323, and should also fix bugs regarding revision numbers being skipped and default reviewers not being set (since that check queried if any diffsets existed for the review request, which they did due to #1 above).
All my manual tests showed that the correct things were being seen for a logged in user accessing a draft diff, and that other users and non-logged in users couldn't see it.

Verified that interdiffs work with the draft diffs and that discarding a draft deletes the diffset.

All unit tests pass (with a tweak to fix broken behavior).
Ship it!
David Trowbridge
Posted 8 months, 2 weeks ago (March 9th, 2008, 1:41 p.m.)
Just some trivial style things.  Thanks for doing this!
  1. Christian Hammond 8 months, 2 weeks ago (March 10th, 2008, 1:06 a.m.) 
    Changes made and committed. Thanks for the review.
/trunk/reviewboard/reviews/views.py (Diff revision 4) 

         215
         
    if revision != None:

       


      

     

    

    

     
This can just be "if revision:"

	 

  
    
   
  



    


    

     
     

      
       
       
       
       
      
      
       

        
         /trunk/reviewboard/webapi/json.py
		 

		  (Diff revision 4)

		 
        
       

      

      

       

         
         

         972
         
        # It would be nice to later consolidate this with the logic in

       


       

         
         

         973
         
        # DiffSet.save.

       


      

     

    

    

     
TODO please

	 

  
    
   
  



    


    

     
     

      
       
       
       
       
      
      
       

        
         /trunk/reviewboard/webapi/json.py
		 

		  (Diff revision 4)

		 
        
       

      

      

       

         
         

         976
         
        if public_diffsets.count() > 0:

       


      

     

    

    

     
You can write this as:
if public_diffsets:
   ...
else:
   ...