• 
  
    
 
    
 
    

    



    
    
    
    
     
    




    
 
    

  
    



    
 

 

  

  

 

    


  
    





    
 
    



  

    
 
 
    
  
    Improve e-mail recipient logic, switch to Django's e-mail support, and add unit tests
    
 
    

    



 
    
 
    

  Review Request #44
  —
  Created June 1, 2007 and submitted

 
    


    




    




    




 
    
  
    
   Information

  
    



    
 
 
    
  chipx86
 
    

    





    
 
 
    
  Review Board SVN (deprecated)
 
    

    





    
 
 
    
  trunk
 
    

    





    
 
 
    
  
 
    

    





    
 
 
    
  
 
    

    



 
    



 
    
  
    
   Reviewers

  
    



    
 
 
    
  reviewboard
 
    

    





    
 
 
    
  
 
    

    



 
    



    


    
 
    




  

    
 
 
    
  
    Several improvements to the e-mail support here.

* Recipient logic is now improved. We always e-mail to the review request author and the target reviewers, but we also add to this in some cases:
  * Updates to a diff or review request go to every person who has been involved in a discussion on the review request.
  * A reply on a review goes to every person who has been involved in that review's discussion.

* We now use Django's e-mail support instead of our own, now that we can subclass their new EmailMessage object and inject the headers we need. This also gives us the next feature..
* Unit tests! We now unit test e-mail. We specifically check the subject and the To: list.
    
 
    

    





  

    
 
 
    
  
    Ran the unit tests. They worked.
    
 
    

    




 
    

    





    
 
    









 









 
    

    



  
    
 
    

    


    


 













    
 





 
    
  
    
   
    



david




   
    
   
    



    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
david


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      Mostly looks good.  Did you forget to include the fixture file?
      
  
      
 
      

 
      
  


      
 
      
 
        

  


      1. 
 
 
        
 
        
  
 
        
 
        
  
        Nope. It was just really big and ugly, so I left it out of the generated diff. It'll be committed.
        
 
        

        2. 


 
      


      

 
      

      2. 

















    2. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       /trunk/reviewboard/reviews/email.py
       


        (Diff revision 2)


       
      
     
      
          
    
     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    





    
      I'd prefer to see this called "ThreadedEmailMessage" or somesuch.  "Spiffy" just doesn't give me any information about what it does.

Alternatively, explain it with a docstring.
      
  
      
 
      

 
      
  


      
 
      
 
        

  


      1. 
 
 
        
 
        
  
 
        
 
        
  
        Yep. I couldn't come up with a name I liked, so I left it as-is, expecting that you'd come up with one :)
        
 
        

        2. 


 
      


      

 
      

      3. 






    3. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       /trunk/reviewboard/reviews/email.py
       


        (Diff revision 2)


       
      
     
      
          
    
     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    





    
      This could probably be slimmed down with a list comprehension, but not critical.
      
  
      
 
      

 
      
  


      
 
      
 
        

  


      1. 
 
 
        
 
        
  
 
        
 
        
  
        I was trying to figure out how to do that without ending up with a [[...], [...], ...].

If I did:

recipient_list += [harvest_people_from_review(reply) for reply in review.replies.all()]

I would get:

[["chipx86", "admin"], ["foo", "bar"]] and such.

I'd love to slim this down. Suggestions?
        
 
        

        2. 



      2. 
 
 
        
 
        
  
 
        
 
        
  
        I think [recipient_list += harvest_people_from_review(reply) for reply in review.replies.all()] will work.
        
 
        

        3. 



      3. 
 
 
        
 
        
  
 
        
 
        
  
        Found a solution, as we discussed in IRC. I'm quite happy by it, though it feels so backwards. Thanks Python.
        
 
        

        4. 


 
      


      

 
      

      4. 






    4. 
 
      
  
  
      
  
 
      
  
      
   
      
    
     
      
      
       /trunk/reviewboard/reviews/email.py
       


        (Diff revision 2)


       
      
     
      
          
    
     
       
      

     
       
      

          
   
      
  
      
 
      

   
      
    
    





    
      Same for this.
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      5. 




    5. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      6. 


    

  
    
 
    

    












    
 





 
    
  
    
   
    



david




   
    
   
    





    
  
    
 
    


 
    
  
    

   
    
   
    
    
     
david


    
    
   
    

  
    
  
    
  
    


      




    1. 
 
      
  
      
   
      Looks good.
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      2. 















    2. 
 
      
  
      
   
      
  
      
 
      

 
      
  


      
 
      
 
        

  

 
      


      

 
      

      3.