• 
  
    
 
    
 
    

    



    
    
    
    
     
    




    
 
    

  
    



    
 

 

  

  

 

    


  
    





    
 
    



  

    
 
 
    
  
    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)