Create draft review request for Update > Add File
Review Request #10262 — Created Oct. 23, 2018 and submitted
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 usesreviewRequestEditor.createFileAttachment
instead ofreviewRequest.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 theUpdate > Add File
option:
- Open an already published review request.
- 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)- Upload file (with drag-and-drop or
Update > Add File
method)- 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? |
brennie | |
Rebinding this to that should not be necessary. The second argument to save is the context, so on success it … |
brennie | |
Mind updating this to the new documentation format while you're in here? |
david | |
This needs a period at the end. |
david | |
How about: On success, the dialog will be closed. Otherwise, on error, the dialog will display the errors. |
brennie |
-
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 1) This is no longer true. Also can you update this to our doc-comment format while you're here?
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 1) Rebinding
this
tothat
should not be necessary. The second argument tosave
is the context, so on success it will be called likesuccess.call(ctx, arg)
ie. it will get the properthis
.You can just use
this
.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+6 -5) |
Checks run (2 succeeded)
-
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 2) Mind updating this to the new documentation format while you're in here?
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 2) This needs a period at the end.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+10 -6) |
Checks run (2 succeeded)
-
Teeny tiny nitpick.
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 3) How about:
On success, the dialog will be closed. Otherwise, on error, the dialog will display the errors.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+10 -6) |