HUE-8792: [notebook] Error while share documents(user can save document with only read permissions)

Review Request #13800 - Created April 5, 2019 and submitted

Information
Mykhailo Kysliuk
hue
Reviewers
hue
romain 
From f0d07aa9aad1fc6108b7d7838a49ce495b60c8b6 Mon Sep 17 00:00:00 2001

From: Mykhailo Kysliuk misha.kysliuk@gmail.com

Date: Fri, 5 Apr 2019 18:42:15 +0300

Subject: [PATCH] HUE-8792: [notebook] Error while share documents(user can

 save document with only read permissions)




https://issues.cloudera.org/browse/HUE-8792

https://github.com/cloudera/hue/pull/831
Manual on Chrome.
  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
Jean-Francois Desjeans Gauthier 
  1. Looks good to me.
  2. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   








   

   







  

 



 

  


   

   

    
 
Jean-Francois Desjeans Gauthier


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Looking back at this, is there a reason why we wouldn't just check for the presence of parentSavedQueryUuid instead of checking isHistory?
    

    e.g.
    
Document2.objects.get(uuid=notebook['parentSavedQueryUuid']) if notebook.get('parentSavedQueryUuid') else \
    
            Document2.objects.get(id=notebook['id'])
    

    

    Let's create a method for this check and reuse here:
    
https://github.com/cloudera/hue/blob/ca2ea6c5e3979b963b151931ac4a689f69131b54/desktop/libs/notebook/src/notebook/api.py#L336
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I have changed isHistory check to parentSavedQueryUuid.
      
Do you mean creating a method for one-line check? 
      

      def is_query_saved(notebook):
      
  return notebook.get('parentSavedQueryUuid')
      

      I think that something like this is redundant. But maybe I am missing something.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      My  concern was that we have a bug because we aren't reusing code, but let's get this in.
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   








   

   

  

 



 















 





 

  

   








   

   





  

 



 

  


   

   

    
 
Romain Rigaux


    
   


  

  

  



    




  1. 
 
    
  
    
   
    I would need to think a bit more as tricky but important, but the good behavior could be to offer the 'save as', like a read only query can be 'saved as'?
    

    Here is might be an issue with the save method too https://github.com/cloudera/hue/blob/master/desktop/libs/notebook/src/notebook/api.py#L336 (if not W perm on parent document, we go in the save as)
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      To answer your 2nd point, check_document_modify_permission will protect for api.py#L336 so we are good.
      
 
      

      2. 


 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   








   

   







  

 



 

  


   

   

    
 
Jean-Francois Desjeans Gauthier


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   








   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...