Fixed Upload File dialog ungracefully failing when failing to connect to the web-server.

Review Request #9200 — Created Sept. 22, 2017 and discarded

Information

Review Board
master

Reviewers

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 …

daviddavid

Can you wrap the description and testing done sections to 70 columns? It's also nice to put any code references …

daviddavid

You've introduced some new blank lines here. Please remove them.

daviddavid

Can we go back to defining the variable and assigning it in one step?

daviddavid
NI
david
  1. 
      
  2. 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.

  3. 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.

  4. You've introduced some new blank lines here. Please remove them.

  5. Can we go back to defining the variable and assigning it in one step?

  6. 
      
NI
Review request changed

Status: Discarded

Change Summary:

See https://reviews.reviewboard.org/r/9208/ and https://reviews.reviewboard.org/r/9207/ for seperated bug fixes

Loading...