flake8
passed.
JSHint
passed.
Fixed Upload File dialog ungracefully failing when failing to connect to the web-server.
Review Request #9200 — Created Sept. 22, 2017 and discarded
| Information | |
|---|---|
| nicho | |
| Review Board | |
| master | |
| 4467, 4592 | |
| Reviewers | |
| reviewboard, students | |
When failing to connect to the web-server, the server response message is parsed as a json object. If the server does not have a response message, the json object would be null, resulting in an error when trying to access members of the object.
Null checks were added for the json object, and if the object is null, the error message would default to "Unknown Error".
There were also issues with displaying the upload file dialog, where clicking Update -> Add File would not do anything.
The onUploadFileClicked() function was moved from reviewablePageView.es6.js to reviewRequestEditorView.es6.js, and the element's click function set to it.
Ran JSTests.
Tested manually by attempting to upload a file while the server was off. There were no javascript errors, and the displayed error message read "Unknown Error".
| Description | From | Last Updated |
|---|---|---|
|
It would be really, really nice to separate out the fix for the "Upload file" click bug into a different … |
 david
|
|
|
Can you wrap the description and testing done sections to 70 columns? It's also nice to put any code references … |
david
|
|
|
You've introduced some new blank lines here. Please remove them. |
david
|
|
|
Can we go back to defining the variable and assigning it in one step? |
david
|
-
-
It would be really, really nice to separate out the fix for the "Upload file" click bug into a different review request. Ideally we want each change to do one independent thing.
-
Can you wrap the description and testing done sections to 70 columns? It's also nice to put any code references and filenames in backticks (`) so they show up highlighted.
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 1) You've introduced some new blank lines here. Please remove them.
-
reviewboard/static/rb/js/views/uploadAttachmentView.js (Diff revision 1) Can we go back to defining the variable and assigning it in one step?