Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

change to using fetch for ajax instead of jquery function #764

Merged
merged 12 commits into from
Feb 12, 2022
Merged

change to using fetch for ajax instead of jquery function #764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1e67346
change to using fetch for ajax instead of jquery function
70ray Jan 31, 2022
999d7e0
test for no error message on failure
70ray Feb 1, 2022
3322db2
add test for incorrect response type and catch fetch rejection
70ray Feb 1, 2022
220218b
catch instead of using 2nd param of then().
70ray Feb 1, 2022
7560865
added 'Accept': 'application/json' and method.toUpperCase()
70ray Feb 1, 2022
3125f61
added credetials
70ray Feb 1, 2022
172b8f3
use upperCaseMethod variable
70ray Feb 2, 2022
32a25b0
revise url construction
70ray Feb 2, 2022
cace700
revise code for error case
70ray Feb 2, 2022
ac7f34d
add unit tests
70ray Feb 3, 2022
32647c2
test for sent data
70ray Feb 4, 2022
89b7ffc
revise construction of search parameters
70ray Feb 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions SETUP/tests/jsTests/ajaxTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/* global QUnit ajax */
/* exported codeUrl */

let codeUrl = "https://www.dummy.org";

QUnit.module("Ajax test", function() {

QUnit.test("Return correct data", function (assert) {
function fetchPromise() {
const blob = new Blob([JSON.stringify({key: "value"})], {type : 'application/json'});
const init = {status: 200, headers: {'content-type': 'application/json'}};
return Promise.resolve(new Response(blob, init));
}

return ajax("GET", "myUrl", {}, {}, fetchPromise)
.then(function(data) {
assert.deepEqual(data, {key: "value"});
cpeel marked this conversation as resolved.
Show resolved Hide resolved
});
});

QUnit.test("Wrong type of data", function (assert) {
function fetchPromise() {
const blob = new Blob([JSON.stringify("mydata")], {type : 'application/json'});
const init = {status: 200, headers: {'content-type': 'text/html'}};
return Promise.resolve(new Response(blob, init));
}

return ajax("GET", "myUrl", {}, {}, fetchPromise)
.then(function() {}, function(data) {
assert.strictEqual(data, "Incorrect response type");
});
});

QUnit.test("Status 404", function (assert) {
function fetchPromise() {
const blob = new Blob([JSON.stringify({error: "not found"})], {type : 'application/json'});
const init = {status: 404, headers: {'content-type': 'application/json'}};
return Promise.resolve(new Response(blob, init));
}

return ajax("GET", "myUrl", {}, {}, fetchPromise)
.then(function() {}, function(data) {
assert.strictEqual(data, "not found");
});
});

QUnit.test("Unknown error", function (assert) {
function fetchPromise() {
const blob = new Blob([JSON.stringify("")], {type : 'application/json'});
const init = {status: 404, headers: {'content-type': 'application/json'}};
return Promise.resolve(new Response(blob, init));
}

return ajax("GET", "myUrl", {}, {}, fetchPromise)
.then(function() {}, function(data) {
assert.strictEqual(data, "Unknown error");
});
});

QUnit.test("Check sent data", function (assert) {
function fetchPromise(url, options) {
assert.strictEqual(url.href, "https://www.dummy.org/api/index.php?url=myUrl&querykey=queryvalue");
assert.strictEqual(options.method, "POST");
assert.strictEqual(options.headers['Content-Type'], 'application/json');
assert.strictEqual(options.headers["X-API-KEY"], "SESSION");
assert.strictEqual(options.headers.Accept, 'application/json');
assert.strictEqual(options.body, "{\"datakey\":\"datavalue\"}");
const blob = new Blob([JSON.stringify("mydata")], {type : 'application/json'});
const init = {status: 200, headers: {'content-type': 'application/json'}};
return Promise.resolve(new Response(blob, init));
}

return ajax("POST", "myUrl", {querykey: "queryvalue"}, {datakey: "datavalue"}, fetchPromise)
.then();
});
});
2 changes: 2 additions & 0 deletions SETUP/tests/qunit.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
<script src="../../pinc/3rdparty/xregexp/xregexp-all.js"></script>
<script src="../../tools/proofers/preview.js"></script>
<script src="../../scripts/splitControl.js"></script>
<script src="../../scripts/api.js"></script>
<script src="../../tools/proofers/srchrep.js"></script>
<script src="../../accounts/addproofer.js"></script>
<script src="../../scripts/character_test.js"></script>
Expand All @@ -28,5 +29,6 @@
<script src="./jsTests/addprooferTests.js"></script>
<script src="./jsTests/hieroTests.js"></script>
<script src="./jsTests/dp_proofTests.js"></script>
<script src="./jsTests/ajaxTests.js"></script>
</body>
</html>
53 changes: 40 additions & 13 deletions scripts/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,44 @@
/*global $ codeUrl */
/* exported makeApiAjaxSettings */
/*global codeUrl */
/* exported ajax */

function makeApiAjaxSettings(apiUrl, queryParams) {
let queryParamString = "";
if(queryParams) {
queryParamString = "&" + $.param(queryParams);
function ajax(method, apiUrl, queryParams = {}, data = {}, fetchPromise = fetch) {
let url = new URL("/api/index.php", codeUrl);
let searchParams = new URLSearchParams();
searchParams.append("url", apiUrl);
for (const key in queryParams) {
searchParams.append(key, queryParams[key]);
}
let settings = {
"url": codeUrl + "/api/index.php?url=" + apiUrl + queryParamString,
"dataType": "json",
"headers": {
"X-API-KEY": "SESSION"
}
url.search = searchParams;
let upperCaseMethod = method.toUpperCase();
let options = {
headers: {"X-API-KEY": "SESSION", 'Accept': 'application/json'},
credentials: "same-origin",
method: upperCaseMethod,
};
return settings;
if(upperCaseMethod !== "GET") {
// POST or PUT
options.headers['Content-Type'] = 'application/json';
options.body = JSON.stringify(data);
}
return new Promise(function(resolve, reject) {
fetchPromise(url, options)
.then(function(response) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not that it can happen often, but the fetch promise could need a catch if network interrupted or something so may want to catch the fetch promise too:
https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch

Copy link
Collaborator Author

@70ray 70ray Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put in a catch and tried issuing a request with the network disconnected but nothing ever happens. We could perhaps put in a timer to detect network disconnection.
If the url has a spurious character inserted at the beginning the error message that appears is "TypeError: NetworkError when attempting to fetch resource." so it seems to be working.
If you edit the tail of the url for example api -> apix then then an "incorrect response type" message appears which is correct because the html page for "file not found" is sent.

const contentType = response.headers.get('content-type');
if (!contentType || !contentType.includes('application/json')) {
reject("Incorrect response type");
} else if(response.ok) {
resolve(response.json());
} else {
response.json()
.then(function(data) {
70ray marked this conversation as resolved.
Show resolved Hide resolved
let message = data.error;
if(!message) {
message = "Unknown error";
}
reject(message);
});
}
})
.catch(reject);
});
}
32 changes: 13 additions & 19 deletions scripts/page_browse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*global $ proofIntData makeApiAjaxSettings splitControl makeImageWidget makeControlDiv */
/*global $ proofIntData ajax splitControl makeImageWidget makeControlDiv */
/* exported pageBrowse */

// Construct the font-face, font-size and wrap controls
Expand Down Expand Up @@ -322,11 +322,6 @@ function pageBrowse(params, storageKey, replaceUrl, mentorMode = false) {
// replace any previous content of topDiv
topDiv.html(fixHead);

// show error if ajax fails
function showError(jqxhr) {
alert(jqxhr.responseJSON.error);
}

let roundSelector = null;
// this allows rounds to be obtained from server only once when needed
// the roundSelector retains its selected item so we do not have to
Expand All @@ -336,16 +331,16 @@ function pageBrowse(params, storageKey, replaceUrl, mentorMode = false) {
resolve();
} else {
roundSelector = document.createElement("select");
$.ajax(makeApiAjaxSettings("v1/projects/pagerounds"))
.done(function(rounds) {
ajax("GET", "v1/projects/pagerounds")
.then(function(rounds) {
70ray marked this conversation as resolved.
Show resolved Hide resolved
rounds.forEach(function(round) {
let selected = (currentRound === round);
roundSelector.add(new Option(round, round, selected, selected));
});
resolve();
})
.fail(function(jqxhr) {
showError(jqxhr);
.catch(function(error) {
alert(error);
reject();
});
}
Expand Down Expand Up @@ -384,11 +379,10 @@ function pageBrowse(params, storageKey, replaceUrl, mentorMode = false) {
// by the shown (first) option
const round = roundSelector.value;
params.set("round_id", round);
$.ajax(makeApiAjaxSettings("v1/projects/" + projectId + "/pages/" + imageFileName + "/pagerounds/" + round))
.done(function(data) {
ajax("GET", `v1/projects/${projectId}/pages/${imageFileName}/pagerounds/${round}`)
70ray marked this conversation as resolved.
Show resolved Hide resolved
.then(function(data) {
textWidget.setText(data.text);
})
.fail(showError);
}, alert);
}
replaceUrl();
}
Expand Down Expand Up @@ -558,14 +552,14 @@ function pageBrowse(params, storageKey, replaceUrl, mentorMode = false) {
fixHead.append($("<p>").append($("<a>", {href: projectRef}).append(projectData.title)), resetButton);
}
// get pages
$.ajax(makeApiAjaxSettings(`v1/projects/${projectId}/pages`)).done(displayPages)
.fail(showError);
ajax("GET", `v1/projects/${projectId}/pages`)
.then(displayPages, alert);
}

getProjectData = function() {
$.ajax(makeApiAjaxSettings("v1/projects/" + projectId)).done(showProjectInfo)
.fail(function(jqxhr) {
showError(jqxhr);
ajax("GET", `v1/projects/${projectId}`)
.then(showProjectInfo, function(error) {
alert(error);
selectAProject();
});
};
Expand Down