Fix text/rich text loading, rendering, and storage for custom fields.

 






 

 


  Review Request #10103
  —
  Created July 27, 2018 and submitted 

 


















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-3.0.x
 









 
 

  
 









 
 

  
 









 
 

  074a770...
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
The rewrite of custom fields on the JavaScript side broke several

aspects of text-based fields, causing rendering glitches and bad data

stored in extra_data. The following issues were found:


    

  1. 

    The registered field ID was often being used instead of the JSON
    
   field name, preventing some fields from being looked up in any
    
   payloads.
    

    2. 

  2. 

    Similarly, a <fieldID>RichText boolean was being looked up (and
    
   saved) for custom fields, instead of using the
    
<json_field_id>_text_type value.
    

    3. 

  3. 

    These values were always being pulled out of extra_data, which
    
   resulted in corrupted displays, as we were requesting HTML renders of
    
   the values. raw_text_fields should have been used instead.
    

    4. 

  4. 

    The new dirty handling wasn't noticing changes to the Enable Markdown
    
   checkbox, due to race conditions in calculation.
    

    5. 



Fields now have mandatory jsonFieldName and jsonTextTypeFieldName

(for text fields) attributes, which are guaranteed to be set. These are

used or provided wherever we may be working with extra_data or

raw_text_fields.


setDraftField() and getDraftField() now assert for required values.

getDraftField() also now accepts some options for working with

raw_text_fields.


Value loading for fields has been centralized in a _loadValue()

function, preventing some copy/paste for non-trivial calls. Same with

loading rich text booleans for text fields.


Unit tests have been introduced for custom fields to check all the

various states, preventing regressions in the future.

 








  


 
 

  
All unit tests pass.


Tested loading and saving both plain text and Markdown text for

all forms of built-in text fields (Close Description, Draft Change

Description, Description, Testing Done) and custom text fields.

Ensured that initial loads of both text types work, that saving

works, and that saving twice in one page session works.

 







 









 





  








 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Col: 25 'draft' is defined but never used.

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
Col: 41 Missing semicolon.

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
Col: 41 Missing semicolon.

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
Col: 41 Missing semicolon.

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
Col: 39 Unnecessary semicolon.

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
Col: 21 Do not use 'new' for side effects.

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
This looks like it can be const instead of let

      		
      
       daviddavid
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 passed.
 





 

 JSHint
 failed.
 
















 
JSHint

 


 







  

 





 














 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    This looks like it can be const instead of let
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  







  

   

    Status:
   

   


    Completed

   

  




  

   

    Change Summary:
   

   

    
Pushed to release-3.0.x (c1c4262)