CLDR-16900 Shorten server response if unchanged; bug fix, refactor #3596
Conversation
-Refactor (encapsulate) to calculate unread/total counts in only one place; new callbackSetCounts -On front end, remember the last announcement ID fetched (alreadyGotId) -On back end, respond with 304 Not Modified if alreadyGotId matches -Do not report 304 Not Modified response as error; log it to console only if debugging enabled -New front-end method resetSchedule prevents delay and prevents 304 response -Format Test.html (not specific to this ticket; unrelated PR left it unformatted) -Comments
| @@ -119,6 +149,7 @@ async function compose(formState, viewCallbackComposeResult) { | |||
| } | |||
|
|
|||
| async function saveCheckmark(checked, announcement) { | |||
| resetSchedule(); | |||
There was a problem hiding this comment.
this fixes a bug -- otherwise, the "unread" number by the balloon icon in the main menu took 60 seconds before it updated in response to adding a checkmark for "I have read this"
| if (!callbackSetData) { | ||
| // The Announcements page was just opened, so re-fetch the data immediately regardless | ||
| // of how recently it was fetched previously (for unread count) | ||
| schedule.reset(); |
There was a problem hiding this comment.
This schedule.reset isn't needed here anymore since now AnnouncePanel.vue "created" method calls cldrAnnounce.resetSchedule
| } | ||
|
|
||
| function handleErrorOrNotModified(e) { | ||
| if (e.message === "Not Modified") { |
There was a problem hiding this comment.
| if (e.message === "Not Modified") { | |
| if (e.status === 304) { |
should be able to check the status code directly - the message can be variable.
There was a problem hiding this comment.
That would require changes to cldrAjax -- currently cldrAjax.handleFetchErrors is as follows:
function handleFetchErrors(response) {
if (!response.ok) {
throw new Error(response.statusText);
}
return response;
}
The Error only contains response.statusText -- I'm not sure how to set e.status in a standard way
| @@ -71,6 +76,9 @@ public Response getAnnouncements(@HeaderParam(Auth.SESSION_HEADER) String sessio | |||
| if (SurveyMain.isBusted() || !SurveyMain.wasInitCalled() || !SurveyMain.triedToStartUp()) { | |||
| return STError.surveyNotQuiteReady(); | |||
| } | |||
| if (alreadyGotId != 0 && alreadyGotId == AnnouncementData.getMostRecentAnnouncementId()) { | |||
| return Response.notModified().build(); | |||
There was a problem hiding this comment.
I have some doubt whether this is a standard way to use notModified (304)...
Possible alternatives include (1) setting headers so that the browser delivers cached response for 304 (but that's maybe less efficient -- needless work processing the identical json); (2) return 200 and short json, e.g., just json.unchanged = true; (3) Etag...
| if (alreadyGotId != 0 && alreadyGotId == AnnouncementData.getMostRecentAnnouncementId()) { | ||
| return Response.notModified().build(); | ||
| } |
There was a problem hiding this comment.
might consider adding this here (see entry for 503, etc in the annotations on this function)
@APIResponse(responseCode = "304", description = "No further announcements (past alreadyGotId"),There was a problem hiding this comment.
good idea
if tests pass, maybe best to merge this as-is since it does include bug fix and performance improvement; but keep ticket open and consider protocol improvements under discussion
-Refactor (encapsulate) to calculate unread/total counts in only one place; new callbackSetCounts
-On front end, remember the last announcement ID fetched (alreadyGotId)
-On back end, respond with 304 Not Modified if alreadyGotId matches
-Do not report 304 Not Modified response as error; log it to console only if debugging enabled
-New front-end method resetSchedule prevents delay and prevents 304 response
-Format Test.html (not specific to this ticket; unrelated PR left it unformatted)
-Comments
CLDR-16900
ALLOW_MANY_COMMITS=true