Disallow users from saying "Ship It!" to their own review requests.

 






 

 


  Review Request #14198
  —
  Created Oct. 11, 2024 and updated 

 




















 

  

   Information

  





 
 

  david
 









 
 

  Review Board
 









 
 

  release-7.x
 









 
 

  4984
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
We've always taken a pretty hands-off approach, preferring to encourage

conversation over policy. That said, one of the most requested bits of

policy is to make it so users can't mark their own review requests as

"Ship It". This is not only a reasonable policy ask (especially if

people are using approval workflows that can block pushes), but should

probably be the default.


This change implements that. The visibility of the "Ship It" action and

checkbox in the review dialog are now conditional on this flag, and

reviews will refuse to publish (either via the UI or API).


Because a big change like this may be controversial, I've added a toggle

for it in the admin settings, under a new page titled "Review Workflow".

 








  


 
 

  
    

  • Viewed a review request as both the owner and another user, and saw
    
  that the visibility of the action and review dialog checkbox was
    
  correct.
    • 

  • Toggled the setting in the admin and checked again, seeing that I
    
  could restore previous behavior.
    • 

  • Ran unit tests.
    • 


 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Disallow users from saying "Ship It!" to their own review requests.
      
We've always taken a pretty hands-off approach, preferring to encourage
conversation over policy. That said, one of the most requested bits of
policy is to make it so users can't mark their own review requests as
"Ship It". This is not only a reasonable policy ask (especially if
people are using approval workflows that can block pushes), but should
probably be the default.

This change implements that. The visibility of the "Ship It" action and
checkbox in the review dialog are now conditional on this flag, and
reviews will refuse to publish (either via the UI or API).

Because a big change like this may be controversial, I've added a toggle
for it in the admin settings, under a new page titled "Review Workflow".

Testing Done:
- Viewed a review request as both the owner and another user, and saw
  that the visibility of the action and review dialog checkbox was
  correct.
- Toggled the setting in the admin and checked again, seeing that I
  could restore previous behavior.
- Ran unit tests.

Fixes bug 4984.

     


    		
    a12cfc0820aaf92e1d6657c684756826e68fc7e3

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
continuation line missing indentation or outdented Column: 17 Error code: E122

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
Maybe better as JSONDict?

      		
      
       maubinmaubin
      
      
       
      
     




     

      
       
       
This can be on one line.

      		
      
       maubinmaubin
      
      
       
      
     




     

      
       
       
This can be on one line.

      		
      
       maubinmaubin
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 



 

  

   




david




   

   

  

 



 















 





 

  

   




maubin




   

   









  

 



 

  


   

   

    
 
maubin


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/actions/base.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Maybe better as JSONDict?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      This is django template context, not JSON.
      

      We probably should create a definition for that in typelets.
      
 
      

      2. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/tests/test_review.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    This can be on one line.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/webapi/tests/test_review.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    This can be on one line.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  5. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 




  

 
















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  















  

   

    Commits:
   

   

    





 

  
   


    

    Summary
    ID

   

  
  

   


    

    

     

      Disallow users from saying "Ship It!" to their own review requests.
      
We've always taken a pretty hands-off approach, preferring to encourage
conversation over policy. That said, one of the most requested bits of
policy is to make it so users can't mark their own review requests as
"Ship It". This is not only a reasonable policy ask (especially if
people are using approval workflows that can block pushes), but should
probably be the default.

This change implements that. The visibility of the "Ship It" action and
checkbox in the review dialog are now conditional on this flag, and
reviews will refuse to publish (either via the UI or API).

Because a big change like this may be controversial, I've added a toggle
for it in the admin settings, under a new page titled "Review Workflow".

Testing Done:
- Viewed a review request as both the owner and another user, and saw
  that the visibility of the action and review dialog checkbox was
  correct.
- Toggled the setting in the admin and checked again, seeing that I
  could restore previous behavior.
- Ran unit tests.

Fixes bug 4984.

     


    		
    ce192f063a2a7fdad383122f073119c038e04826

   




   


    

    

     

      Disallow users from saying "Ship It!" to their own review requests.
      
We've always taken a pretty hands-off approach, preferring to encourage
conversation over policy. That said, one of the most requested bits of
policy is to make it so users can't mark their own review requests as
"Ship It". This is not only a reasonable policy ask (especially if
people are using approval workflows that can block pushes), but should
probably be the default.

This change implements that. The visibility of the "Ship It" action and
checkbox in the review dialog are now conditional on this flag, and
reviews will refuse to publish (either via the UI or API).

Because a big change like this may be controversial, I've added a toggle
for it in the admin settings, under a new page titled "Review Workflow".

Testing Done:
- Viewed a review request as both the owner and another user, and saw
  that the visibility of the action and review dialog checkbox was
  correct.
- Toggled the setting in the admin and checked again, seeing that I
  could restore previous behavior.
- Ran unit tests.

Fixes bug 4984.

     


    		
    a12cfc0820aaf92e1d6657c684756826e68fc7e3

   



  
 







   

  





  

   

    Diff:
   

   

    
 Revision 3 (+468 -58)
Show changes
 
    reviewboard/accounts/templatetags/accounts.py 
    reviewboard/accounts/tests/test_template_tags.py 
    reviewboard/actions/base.py 
    reviewboard/actions/templatetags/actions.py 
    reviewboard/admin/siteconfig.py 
    reviewboard/admin/urls.py 
    reviewboard/admin/forms/review_settings.py 
    reviewboard/reviews/actions.py 
  5 more
 

   

  











 

  
Checks run (2 succeeded)



  

 flake8
 passed.
 





  

 JSHint
 passed.