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

CLDR-17248 Measure vote timing; log times to console; refactor to use fetch #3392

Merged
merged 1 commit into from
Nov 23, 2023
Merged

CLDR-17248 Measure vote timing; log times to console; refactor to use fetch #3392

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
269 changes: 140 additions & 129 deletions tools/cldr-apps/js/src/esm/cldrVote.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as cldrSurvey from "./cldrSurvey.mjs";
import * as cldrTable from "./cldrTable.mjs";
import * as cldrText from "./cldrText.mjs";

const CLDR_VOTE_DEBUG = false;
const CLDR_VOTE_DEBUG = true;

/**
* The special "vote level" selected by the user, or zero for default.
Expand All @@ -26,10 +26,11 @@ let voteLevelChanged = 0;
/**
* Wire up the button to perform a submit
*
* @param button
* @param tr
* @param theRow
* @param vHash
* @param {Element} button the GUI button
* @param {Element} tr the table row
* @param {Object} theRow object describing the table row
* @param {String} vHash hash of the value of the candidate item (cf. DataPage.getValueHash on back end),
* or empty string for newly submitted value
*/
function wireUpButton(button, tr, theRow, vHash) {
let vHashStr = vHash;
Expand Down Expand Up @@ -84,12 +85,10 @@ function wireUpButton(button, tr, theRow, vHash) {
* @param {Object} newValue the newly submitted value, or undefined if it's a vote for an already existing value (buttonb)
* @param {Element} button the GUI button
*
* TODO: shorten this function, using (non-nested) subroutines
*
* Called from cldrTable.addValueVote (for new value submission)
* as well as locally in cldrVote (vote for existing value or abstain)
*/
function handleWiredClick(tr, theRow, vHash, newValue, button) {
async function handleWiredClick(tr, theRow, vHash, newValue, button) {
Copy link
Member

Choose a reason for hiding this comment

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

💯

if (!tr || !theRow || tr.wait) {
return;
}
Expand Down Expand Up @@ -117,138 +116,134 @@ function handleWiredClick(tr, theRow, vHash, newValue, button) {
}
tr.myProposal = null; // mark any pending proposal as invalid.
}

var myUnDefer = function () {
tr.wait = false;
};
tr.wait = true;
cldrInfo.reset();
theRow.proposedResults = null;

if (CLDR_VOTE_DEBUG) {
console.log(
"Vote for " + tr.rowHash + " v='" + vHash + "', value='" + value + "'"
);
logVote(tr.rowHash, vHash, value);
}
const ourContent = {
value: valToShow,
voteLevelChanged: voteLevelChanged,
};
const ourUrl = getSubmitUrl(theRow.xpstrid);

var originalTrClassName = tr.className;
const init = cldrAjax.makePostData(ourContent);
const originalTrClassName = tr.className;
tr.className = "tr_checking1";

var loadHandler = function (json) {
oneMorePendingVote();
try {
const response = await cldrAjax.doFetch(ourUrl, init);
/*
* Restore tr.className, so it stops being 'tr_checking1' immediately on receiving
* any response. It may change again below, such as to 'tr_err' or 'tr_checking2'.
* any response. It may change again below to 'tr_err' or 'tr_checking2'.
*/
tr.className = originalTrClassName;
if (response.ok) {
const json = await response.json();
handleVoteOk(json, tr, theRow, button, valToShow);
} else {
const message = "Server response: " + response.statusText;
handleVoteErr(tr, message, button);
}
} catch (e) {
const message = e.name + " - " + e.message;
handleVoteErr(tr, message, button);
} finally {
tr.wait = false;
oneLessPendingVote();
try {
if (json.err && json.err.length > 0) {
tr.className = "tr_err";
tr.innerHTML =
"<td colspan='4'>" +
cldrStatus.stopIcon() +
" Could not check value. Try reloading the page.<br>" +
json.err +
"</td>";
myUnDefer();
cldrRetry.handleDisconnect("Error submitting a vote", json);
} else {
if (json.didVote) {
// if submitted..
tr.className = "tr_checking2";
cldrTable.refreshSingleRow(
tr,
theRow,
function (theRow) {
// submit went through. Now show the pop.
button.className = "ichoice-o";
button.checked = false;
cldrSurvey.hideLoader();
if (json.testResults && (json.testWarnings || json.testErrors)) {
// tried to submit, have errs or warnings.
showProposedItem(
tr.inputTd,
tr,
theRow,
valToShow,
json.testResults
);
}
myUnDefer();
},
function (err) {
myUnDefer();
cldrRetry.handleDisconnect(err, json);
}
); // end refresh-loaded-fcn
// end: async
} else {
// Did not submit. Show errors, etc
if (
(json.statusAction && json.statusAction != "ALLOW") ||
(json.testResults && (json.testWarnings || json.testErrors))
) {
showProposedItem(
tr.inputTd,
tr,
theRow,
valToShow,
json.testResults,
json
);
} // else no errors, not submitted. Nothing to do.
button.className = "ichoice-o";
button.checked = false;
cldrSurvey.hideLoader();
myUnDefer();
}
}
}

/**
* Handle an OK response. Here "OK" doesn't necessarily mean that the vote succeeded;
* it means that we got json, which might have json.err, etc.; json.didVote means
* the vote succeeded.
*
* @param {Object} json the server response
* @param {Element} tr the table row
* @param {Object} theRow object describing the table row
* @param {Element} button the GUI button
* @param {String} valToShow the value the user is voting for
*/
function handleVoteOk(json, tr, theRow, button, valToShow) {
if (json.err && json.err.length > 0) {
handleVoteErr(tr, json.err, button);
} else if (json.didVote) {
handleVoteSubmitted(json, tr, theRow, button, valToShow);
} else {
handleVoteNotSubmitted(json, tr, theRow, button, valToShow);
}
}

function handleVoteSubmitted(json, tr, theRow, button, valToShow) {
tr.className = "tr_checking2";
cldrTable.refreshSingleRow(
tr,
theRow,
function (theRow) {
// submit went through. Now show the pop.
button.className = "ichoice-o";
button.checked = false;
cldrSurvey.hideLoader();
if (json.testResults && (json.testWarnings || json.testErrors)) {
// tried to submit, have errs or warnings.
showProposedItem(tr.inputTd, tr, theRow, valToShow, json.testResults);
}
} catch (e) {
tr.className = "tr_err";
tr.innerHTML =
cldrStatus.stopIcon() +
" Could not check value. Try reloading the page.<br>" +
e.message;
console.log("Error in ajax post [handleWiredClick] ", e.message);
myUnDefer();
cldrRetry.handleDisconnect("handleWiredClick:" + e.message, json);
},
function (err) {
cldrRetry.handleDisconnect(err, json);
}
};
var errorHandler = function (err) {
/*
* Restore tr.className, so it stops being 'tr_checking1' immediately on receiving
* any response. It may change again below, such as to 'tr_err'.
*/
tr.className = originalTrClassName;
oneLessPendingVote();
// The err parameter is a string composed by cldrAjax.makeErrorMessage (legacy code),
// not user-friendly, still possibly useful for debugging if this actually occurs.
console.log("Error: " + err);
// To close the input pop-up window with minimal changes to this legacy code, call
// cldrLoad.reloadV when the user closes the notification. Postpone a better solution
// until cldrVote is modernized.
cldrNotify.errorWithCallback(
"Error submitting a vote",
err,
cldrLoad.reloadV
);
myUnDefer();
};
const xhrArgs = {
url: ourUrl,
handleAs: "json",
content: ourContent,
load: loadHandler,
error: errorHandler,
timeout: cldrAjax.mediumTimeout(),
};
oneMorePendingVote();
cldrAjax.sendXhr(xhrArgs);
);
}

function handleVoteNotSubmitted(json, tr, theRow, button, valToShow) {
if (
(json.statusAction && json.statusAction != "ALLOW") ||
(json.testResults && (json.testWarnings || json.testErrors))
) {
showProposedItem(tr.inputTd, tr, theRow, valToShow, json.testResults, json);
}
button.className = "ichoice-o";
button.checked = false;
cldrSurvey.hideLoader();
}

function handleVoteErr(tr, message, button) {
tr.className = "tr_err";
tr.innerHTML =
"<td colspan='4'>" +
cldrStatus.stopIcon() +
" Could not check value. Try reloading the page.<br>" +
Copy link
Member

Choose a reason for hiding this comment

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

should this be a cldrNotify instead? in the future

Copy link
Member

Choose a reason for hiding this comment

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

i guess we need to say something here too.

Copy link
Member Author

Choose a reason for hiding this comment

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

I actually tried using cldrNotify and got stuck on some problems with cleaning up the input window, row background color, etc., so I decided not to change too much with this PR

I did some testing by temporarily changing the back end to return Auth.noSessionResponse() or Response.Status.NOT_FOUND, as well as inputting values that triggered errors

The tr_err class shows the messages with black text on a deep red background, not ideal, but one has to pick one's battles :-)

makeSafe(message) +
"</td>";
Fixed Show fixed Hide fixed
cldrRetry.handleDisconnect("Error submitting a vote");
button.className = "ichoice-o";
button.checked = false;
cldrSurvey.hideLoader();
}

/**
* Avoid warning, "Directly writing error messages to a webpage without sanitization allows for a cross-site
* scripting vulnerability if parts of the error message can be influenced by a user."
*
* @param {String} s the raw string
* @returns the sanitized string
*/
function makeSafe(s) {
return s.replace(/[<>&]/g, '');
}

function logVote(rowHash, vHash, value) {
console.log(
"Vote for " +
rowHash +
" v='" +
vHash +
"', value='" +
value +
"' time = " +
Date.now()
);
}

function getSubmitUrl(xpstrid) {
Expand Down Expand Up @@ -375,7 +370,7 @@ function showProposedItem(inTd, tr, theRow, value, tests, json) {

if (!ourItem) {
var h3 = document.createElement("h3");
var span = appendItem(h3, value, "value");
appendItem(h3, value, "value");
h3.className = "span";
div3.appendChild(h3);
}
Expand Down Expand Up @@ -518,7 +513,12 @@ function oneMorePendingVote() {
++pendingVoteCount;
lastTimeChanged = Date.now();
if (CLDR_VOTE_DEBUG) {
console.log("oneMorePendingVote: ++pendingVoteCount = " + pendingVoteCount);
console.log(
"oneMorePendingVote: ++pendingVoteCount = " +
pendingVoteCount +
" time = " +
lastTimeChanged
);
}
}

Expand All @@ -533,7 +533,12 @@ function oneLessPendingVote() {
}
lastTimeChanged = Date.now();
if (CLDR_VOTE_DEBUG) {
console.log("oneLessPendingVote: --pendingVoteCount = " + pendingVoteCount);
console.log(
"oneLessPendingVote: --pendingVoteCount = " +
pendingVoteCount +
" time = " +
lastTimeChanged
);
}
if (pendingVoteCount == 0) {
cldrSurvey.expediteStatusUpdate();
Expand All @@ -553,18 +558,24 @@ function oneLessPendingVote() {
function isBusy() {
if (pendingVoteCount > 0) {
if (CLDR_VOTE_DEBUG) {
console.log("cldrVote busy: pendingVoteCount = " + pendingVoteCount);
console.log(
Copy link
Member

Choose a reason for hiding this comment

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

idea for future ; this would be better as:

const cldrVote = cldrDebug.getDebugLog(CLDR_VOTE_DEBUG, "cldrVote");
…
cldrVote.log(()=>`cldrVote busy: pendingVoteCount = ${pendingVoteCount} time=${Date.now())`);

where cldrVote is an object that handles logging. if logging is off, then the log function ()=> isn't called .

Copy link
Member Author

Choose a reason for hiding this comment

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

that sounds good!

"cldrVote busy: pendingVoteCount = " +
pendingVoteCount +
" time = " +
Date.now()
);
}
return true;
}
if (Date.now() - lastTimeChanged < 3000) {
const time = Date.now();
if (time - lastTimeChanged < 3000) {
if (CLDR_VOTE_DEBUG) {
console.log("cldrVote busy: less than 3 seconds elapsed");
console.log("cldrVote busy: less than 3 seconds elapsed; time = " + time);
}
return true;
}
if (CLDR_VOTE_DEBUG) {
console.log("cldrVote not busy");
console.log("cldrVote not busy; time = " + time);
Copy link
Member

Choose a reason for hiding this comment

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

a logging framework ( https://getpino.io/#/ is one i've used / contributed to though not in browser , or something homegrown) could collect timings automatically

}
return false;
}
Expand Down
Loading