• 
      

    Create draft review request for Update > Add File

    Review Request #10262 — Created Oct. 23, 2018 and submitted

    Information

    Review Board
    release-4.0.x
    d493ae3...

    Reviewers

    When a user adds a file through drag and drop, it creates a draft review
    request, as expected. However, when a user adds a file through the menu
    option Update > Add File, there is no draft review request created.

    This bug fix addresses this issue by removing the window reload when the
    user uploads a file and uses reviewRequestEditor.createFileAttachment
    instead of reviewRequest.createFileAttachment to create a draft review
    and closing the "Add File" dialog manually.

    Ran JS tests (all of which pass).

    Tested behaviour in Chrome and Firefox by first running through the
    following steps with the drag-and-drop file upload (which was
    expected to work, as a baseline) and then running through the steps
    again by uploading a file with the Update > Add File option:

    1. Open an already published review request.
    2. In the console, run the following commands:
      RB.PageManager.getPage().reviewRequest.draft.id (expected & observed
      output: undefined)
      RB.PageManager.getPage().model.reviewRequestEditor.get('hasDraft')
      (expected & observed output: false)
    3. Upload file (with drag-and-drop or Update > Add File method)
    4. Observe the file attachment in the Files section of the review
      request and the appearance of the draft review banner. Run the commands
      again:
      RB.PageManager.getPage().reviewRequest.draft.id (expected & observed
      output: defined (some id))
      RB.PageManager.getPage().model.reviewRequestEditor.get('hasDraft')
      (expected output: true)

    Description From Last Updated

    This is no longer true. Also can you update this to our doc-comment format while you're here?

    brenniebrennie

    Rebinding this to that should not be necessary. The second argument to save is the context, so on success it …

    brenniebrennie

    Mind updating this to the new documentation format while you're in here?

    daviddavid

    This needs a period at the end.

    daviddavid

    How about: On success, the dialog will be closed. Otherwise, on error, the dialog will display the errors.

    brenniebrennie
    brennie
    1. 
        
    2. Show all issues

      This is no longer true. Also can you update this to our doc-comment format while you're here?

    3. reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 1)
       
       
       
       
       
       
       
       
      Show all issues

      Rebinding this to that should not be necessary. The second argument to save is the context, so on success it will be called like success.call(ctx, arg) ie. it will get the proper this.

      You can just use this.

    4. 
        
    shoven
    david
    1. 
        
    2. Show all issues

      Mind updating this to the new documentation format while you're in here?

      1. Can you confirm that I did this properly? (I updated based on what I believe is the new documentation format)

      2. Looks good!

    3. Show all issues

      This needs a period at the end.

    4. 
        
    shoven
    brennie
    1. Teeny tiny nitpick.

    2. Show all issues

      How about:

      On success, the dialog will be closed. Otherwise, on error, the dialog will display the errors.
      
    3. 
        
    shoven
    david
    1. Ship It!
    2. 
        
    shoven
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-4.0.x (e56c8f7)