Review Board

beta

Modify a draft when using update_from_changenum

Updated 11 months, 2 weeks ago

Christian Hammond Reviewers
trunk reviewboard
326
None Review Board SVN 
Our code for updating a review request by changenum through the API was still operating on the review request and not the draft. This meant that a subsequent call to post-review would touch the underlying review request that everyone saw, but not the draft that the user was modifying. Now we update the draft.
Tested this by creating a review request with a changenum. Saw that the initial review request creation still worked and the changeset info was still being pulled. I then updated the content and ran post-review again. The draft content had changed to match.
David Trowbridge
Posted 11 months, 2 weeks ago (January 26th, 2008, 12:27 a.m.)

   

  

   
    
    
   




   



    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
It seems like we're collecting these sorts of functions (either as separate utility functions or duplicated code).  Maybe we should put them all in one place at some point?

     

  

   
    
    

   
  1. 
    
    
     
    
      
      Christian Hammond
      11 months, 2 weeks ago (January 26th, 2008, 12:31 a.m.)
      
     
    
     
    Yeah, I wasn't thrilled with it. Another option is just to keep this in the draft and instead of populating the initial review request based on the changenum, auto-create the draft and populate that. We really create one anyway but through the uploaded diff.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Where do we handle the review-request status if it's discarded?

     

  

   
    
    

   
  1. 
    
    
     
    
      
      Christian Hammond
      11 months, 2 weeks ago (January 26th, 2008, 12:37 a.m.)
      
     
    
     
    With the draft banners how they are now, they will click "Reopen for Review" on that page, which will set the status appropriately. This isn't really the correct place for it.

We could put this code in ReviewRequestDraft.save_draft(), though.
    
    
    
   
    2. 



   
  2. 
    
    
     
    
      
      David Trowbridge
      11 months, 2 weeks ago (January 26th, 2008, 3:08 p.m.)
      
     
    
     
    Seems to me if someone runs post-review on a changeset which has previously been discarded, it should replace the data and make in pending+private.  Maybe that should be a separate API function.
    
    
    
   
    3. 



   
  3. 
    
    
     
    
      
      Christian Hammond
      11 months, 2 weeks ago (January 28th, 2008, 12:23 a.m.)
      
     
    
     
    I'll bring this back to what we had for now. We can revisit it later.
    
    
    
   
    4. 


   



    

    

   


  
 




  

 


















 

  




 

 

  
Ship it!

  
David Trowbridge

  
Posted 11 months, 2 weeks ago (January 28th, 2008, 12:31 a.m.)

 

 

   
Looks great.