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: Closed (submitted)

XU
  1. Ship It!
  2. 
      
Loading...