• 
      

    Make RB.FileAttachment inherit from BaseResource.

    Review Request #3996 — Created March 23, 2013 and submitted

    Information

    Review Board
    master

    Reviewers

    Make RB.FileAttachment inherit from BaseResource.
    
    RB.FileAttachment is now ported to Backbone, and is a BaseResource.
    
    Since RB.FileAttachment sends file data one of two ways (through a
    new-style file upload, with DnD, or a form upload), BaseResource had to
    be modified to handle this, the same way the old RB.FileAttachment had
    to.
    
    Now, BaseResources can mark that they have a field representing a file
    to upload. If, during save, that field has a suitable file associated,
    BaseResource will go down another path for the save operation. It will
    structure a payload to put up to the server that contains form data.
    
    (It's worth noting that there's new API out there for making this much
    nicer and easier to generate, but it's still a draft and not very
    supported.)
    
    If instead we're going through a formDlg (for a manual file upload, not
    DnD), then we have to save by doing an HTTP POST for an HTML form, using
    our form.ajaxSubmit code. (This is needed for older browsers.)
    BaseResource now supports this as well. Basically, if options.form
    exists, it will take precedence over all other fields and pass it down
    to RB.apiCall, which will do the right thing.
    
    Unit tests were added to this, and other code around file attachments
    were changed to properly work with the new class.
    Did a drag-and-drop and saw the file upload work and the thumbnail show.
    
    Tested a manual upload. Also worked.
    
    Unit tests passed.
    Description From Last Updated

    I think we can call destroy() directly without wrapping it in ready().

    daviddavid

    Since we don't use the args, we might as well remove them.

    daviddavid
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/settings.py
        Ignored Files:
          reviewboard/static/rb/js/models/tests/fileAttachmentModelTests.js
          reviewboard/static/rb/js/views/dndUploaderView.js
          reviewboard/static/rb/js/datastore.js
          reviewboard/static/rb/js/models/tests/baseResourceModelTests.js
          reviewboard/static/rb/js/models/baseResourceModel.js
          reviewboard/static/rb/js/models/fileAttachmentModel.js
          reviewboard/templates/reviews/review_request_dlgs.html
          reviewboard/static/rb/js/reviews.js
          reviewboard/static/rb/js/common.js
      
      
    2. 
        
    david
    1. 
        
    2. reviewboard/static/rb/js/reviews.js (Diff revision 1)
       
       
       
       
      Show all issues
      I think we can call destroy() directly without wrapping it in ready().
      1. I have an upcoming change that replaces this code and does the right thing.
    3. Show all issues
      Since we don't use the args, we might as well remove them.
    4. 
        
    chipx86
    reviewbot
    1. This is a review from Review Bot.
        Tool: PEP8 Style Checker
        Processed Files:
          reviewboard/settings.py
        Ignored Files:
          reviewboard/static/rb/js/models/tests/fileAttachmentModelTests.js
          reviewboard/static/rb/js/views/dndUploaderView.js
          reviewboard/static/rb/js/datastore.js
          reviewboard/static/rb/js/models/tests/baseResourceModelTests.js
          reviewboard/static/rb/js/models/baseResourceModel.js
          reviewboard/static/rb/js/models/fileAttachmentModel.js
          reviewboard/templates/reviews/review_request_dlgs.html
          reviewboard/static/rb/js/reviews.js
          reviewboard/static/rb/js/common.js
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    XU
    1. Ship It!
    2.