Transition remaining “bare” network calls to API module

[dondi]

There are some lingering $.getJSON and $.ajax function calls in the web app, all from the original GRNsight logic that did not need the database. Ideally, these should get folded into the grnsight-api.js module as well so that all such network calls are in one place

[Sarronnn]

I moved the function call of $.ajax from setup-load-and-import-handles.js into the grnsight-api.js file

• Functionality of local host is not maintained

• Question: when moving the $ajax function call, I have to move everything that calls $ajax ?

  • Same question for $.getJSON

[dondi]

The done function is what the loadGrn function does with the result of the network call, so what I would do here is:

1. Keep loadGrn where it is
2. The part that goes to the api.js file is lines 148-158. This becomes the abstracted function…can’t think of the right name offhand but it will look something like loadNetwork—maybe consultation with @ahmad00m or @Onariaginosa will be good here
3. The code in the done function stays in loadGrn as the callback to perform after loadNetwork finishes

[Sarronnn]

Wrote two functions inside grnsight-api.js :

• LOAD_AJAX
• GET_JSON

Then I called these functions inside setup-load-and-import-handlers.js

• Functionality maintained

[dondi]

Looks almost ready for a pull request; we can pre-empt some comments that would have gone into the code review by making just a few more changes:

• The all-caps-with-underscores naming convention generally represents constant values (this goes all the way back to C); as such, these functions should not use that naming convention because they aren’t constant values
• See if you can discover more descriptive/meaningful names for these; loadAjax and getJson are somewhat generic and do not communicate the specific functionality that is taking place

[Sarronnn]

Made few more changes to the functions:

[dondi]

Almost ready for PR; just need to move fullUrl calculation into the API functions, thus making the arguments into these functions be the true “input” that determines the behavior of the network request

[Sarronnn]

=> Functionality not maintained. => Question: Possible errors when moving function?

[dondi]

@Sarronnn yes, double-check the original versions. A key consideration when rearranging code is to ensure that you use the same code consistently with the initial

Something I can see right away is that in getWorkbookFromForm, the code you transplanted for computing fullUrl is not the same. Line 142 makes reference to queryURL but that variable is not in the original version (it is present in the second function, but not the first)

[dondi]

That’s just one thing that leaps out to me; there may be more

Also, keeping your Developer Tools console open while running the web app will help; it will show you any error messages encountered when executing the code

[Sarronnn]

=> What I did here is, I supplied uploadRoute as one of the arguments instead of having fullUrl as an argument.

• Then, I placed the const fullUrl inside the getWorkbookFromForm function.

  

• Error when doing this.

  

=> Possible solutions I tried:

• Tried setting uploadRoute=fullUrl, error persists
• I thought maybe the error was because fullUrl was a const function.
• I changed const to let, but same error persisted

[dondi]

404 means the URL didn’t correspond to anything that the server recognized; look at the Network tab to examine the request. Compare it to the one that master/beta sends out and see if there are differences. They should be identical

[dondi]

(well identical except for the hostname)

[dondi]

Conflicts and branch mixup have been resolved; after one last run of testing, @Sarronnn can commit, push, and issue a pull request for code review

[dondi]

@Sarronnn the epic pull request has been merged. Go ahead and merge beta into your branch, and retrace the changes you made, as we discussed earlier this week

[kdahlquist]

This is done and added to release notes.