From aa953817423fbb7845638c6d6ed419f54913eeb2 Mon Sep 17 00:00:00 2001 From: btangmu Date: Tue, 2 Apr 2024 12:15:16 -0400 Subject: [PATCH] CLDR-16900 Shorten server response if unchanged; bug fix, refactor -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 --- tools/cldr-apps/js/src/esm/cldrAnnounce.mjs | 77 ++++++++++++++----- .../cldr-apps/js/src/views/AnnouncePanel.vue | 23 ++---- tools/cldr-apps/js/test/Test.html | 10 ++- .../unicode/cldr/web/AnnouncementData.java | 17 +++- .../unicode/cldr/web/api/Announcements.java | 10 ++- 5 files changed, 94 insertions(+), 43 deletions(-) diff --git a/tools/cldr-apps/js/src/esm/cldrAnnounce.mjs b/tools/cldr-apps/js/src/esm/cldrAnnounce.mjs index ab472f85d35..7e9e2f9ce10 100644 --- a/tools/cldr-apps/js/src/esm/cldrAnnounce.mjs +++ b/tools/cldr-apps/js/src/esm/cldrAnnounce.mjs @@ -25,8 +25,11 @@ const schedule = new cldrSchedule.FetchSchedule( let thePosts = null; let callbackSetData = null; +let callbackSetCounts = null; let callbackSetUnread = null; +let alreadyGotId = 0; + /** * Get the number of unread announcements, to display in the main menu * @@ -36,29 +39,28 @@ async function getUnreadCount(setUnreadCount) { if (setUnreadCount) { callbackSetUnread = setUnreadCount; } - await refresh(callbackSetData); + await refresh(callbackSetData, callbackSetCounts); } /** * Refresh the Announcements page and/or unread count * * @param {Function} viewCallbackSetData the callback function for the Announcements page, or null + * @param {Function} viewCallbackSetCounts the callback function for the Announcements page, or null * * The callback function for setting the data may be null if the Announcements page isn't open and * we're only getting the number of unread announcements to display in the main header */ -async function refresh(viewCallbackSetData) { +async function refresh(viewCallbackSetData, viewCallbackSetCounts) { if (DISABLE_ANNOUNCEMENTS) { return; } if (viewCallbackSetData) { - 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(); - } callbackSetData = viewCallbackSetData; } + if (viewCallbackSetCounts) { + callbackSetCounts = viewCallbackSetCounts; + } if (!cldrStatus.getSurveyUser()) { if (viewCallbackSetData) { viewCallbackSetData(null); @@ -68,36 +70,64 @@ async function refresh(viewCallbackSetData) { if (schedule.tooSoon()) { return; } - const url = cldrAjax.makeApiUrl("announce", null); + let p = null; + if (alreadyGotId) { + p = new URLSearchParams(); + p.append("alreadyGotId", alreadyGotId); + } + const url = cldrAjax.makeApiUrl("announce", p); schedule.setRequestTime(); return await cldrAjax .doFetch(url) .then(cldrAjax.handleFetchErrors) .then((r) => r.json()) .then(setPosts) - .catch((e) => console.error(e)); + .catch(handleErrorOrNotModified); +} + +function handleErrorOrNotModified(e) { + if (e.message === "Not Modified") { + // 304 + if (CLDR_ANNOUNCE_DEBUG) { + console.log("cldrAnnounce got " + e.message); + } + } else { + console.error(e); + } } function setPosts(json) { thePosts = json; schedule.setResponseTime(); + setAlreadyGotId(thePosts); + const totalCount = thePosts.announcements?.length || 0; + let checkedCount = 0; + for (let announcement of thePosts.announcements) { + if (announcement.checked) { + ++checkedCount; + } + } + const unreadCount = totalCount - checkedCount; if (callbackSetData) { - callbackSetData(thePosts); + callbackSetData(thePosts); // AnnouncePanel.vue + } + if (callbackSetCounts) { + callbackSetCounts(unreadCount, totalCount); // AnnouncePanel.vue } if (callbackSetUnread) { - const totalCount = thePosts.announcements?.length || 0; - let checkedCount = 0; - for (let announcement of thePosts.announcements) { - if (announcement.checked) { - ++checkedCount; - } - } - const unreadCount = totalCount - checkedCount; - callbackSetUnread(unreadCount); + callbackSetUnread(unreadCount); // MainHeader.vue (balloon icon) } return thePosts; } +function setAlreadyGotId(thePosts) { + for (let announcement of thePosts.announcements) { + if (announcement.id > alreadyGotId) { + alreadyGotId = announcement.id; + } + } +} + function canAnnounce() { return cldrStatus.getPermissions()?.userIsManager || false; } @@ -107,7 +137,7 @@ function canChooseAllOrgs() { } async function compose(formState, viewCallbackComposeResult) { - schedule.reset(); + resetSchedule(); const init = cldrAjax.makePostData(formState); const url = cldrAjax.makeApiUrl("announce", null); return await cldrAjax @@ -119,6 +149,7 @@ async function compose(formState, viewCallbackComposeResult) { } async function saveCheckmark(checked, announcement) { + resetSchedule(); window.setTimeout(refreshCount, 1000); const init = cldrAjax.makePostData({ id: announcement.id, @@ -154,12 +185,18 @@ async function combineAndValidateLocales(locs, validateLocCallback) { .catch((e) => console.error(`Error: ${e} validating locales`)); } +function resetSchedule() { + alreadyGotId = 0; + schedule.reset(); +} + export { canAnnounce, canChooseAllOrgs, compose, getUnreadCount, refresh, + resetSchedule, saveCheckmark, combineAndValidateLocales, }; diff --git a/tools/cldr-apps/js/src/views/AnnouncePanel.vue b/tools/cldr-apps/js/src/views/AnnouncePanel.vue index 10e7162e8b5..c39576b2b27 100644 --- a/tools/cldr-apps/js/src/views/AnnouncePanel.vue +++ b/tools/cldr-apps/js/src/views/AnnouncePanel.vue @@ -83,7 +83,8 @@ export default { created() { this.canAnnounce = cldrAnnounce.canAnnounce(); this.canChooseAllOrgs = cldrAnnounce.canChooseAllOrgs(); - cldrAnnounce.refresh(this.setData); + cldrAnnounce.resetSchedule(); + cldrAnnounce.refresh(this.setData, this.setCounts); }, methods: { @@ -92,24 +93,16 @@ export default { this.pleaseLogIn = true; } else { this.announcementData = data; - this.updateCounts(); } }, - checkmarkChanged(event, announcement) { - cldrAnnounce.saveCheckmark(event.target.checked, announcement); - this.updateCounts(); + setCounts(unreadCount, totalCount) { + this.unreadCount = unreadCount; + this.totalCount = totalCount; }, - updateCounts() { - this.totalCount = this.announcementData.announcements.length; - let checkedCount = 0; - for (let announcement of this.announcementData.announcements) { - if (announcement.checked) { - ++checkedCount; - } - } - this.unreadCount = this.totalCount - checkedCount; + checkmarkChanged(event, announcement) { + cldrAnnounce.saveCheckmark(event.target.checked, announcement); }, startCompose() { @@ -137,7 +130,7 @@ export default { message: "Your announcement was not posted: " + errMessage, }); } - cldrAnnounce.refresh(this.setData); + cldrAnnounce.refresh(this.setData, this.setCounts); }, }, }; diff --git a/tools/cldr-apps/js/test/Test.html b/tools/cldr-apps/js/test/Test.html index d98bcbcb2c6..b49494b89ef 100644 --- a/tools/cldr-apps/js/test/Test.html +++ b/tools/cldr-apps/js/test/Test.html @@ -19,9 +19,13 @@

CLDR SurveyTool JavaScript Tests

References:

diff --git a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/AnnouncementData.java b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/AnnouncementData.java index b53960cc4ba..061a74b921b 100644 --- a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/AnnouncementData.java +++ b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/AnnouncementData.java @@ -15,6 +15,12 @@ public class AnnouncementData { static boolean dbIsSetUp = false; + static int mostRecentAnnouncementId = 0; + + public static int getMostRecentAnnouncementId() { + return mostRecentAnnouncementId; + } + public static void get( UserRegistry.User user, List announcementList) { makeSureDbSetup(); @@ -30,6 +36,9 @@ public static void get( if (aFilter.passes(a)) { a.setChecked(getChecked(a.id, user.id)); announcementList.add(a); + if (a.id > mostRecentAnnouncementId) { + mostRecentAnnouncementId = a.id; + } } } } finally { @@ -55,7 +64,6 @@ private static Announcements.Announcement makeAnnouncementFromDbObject(Object[] String audience = (String) objects[7]; String date = lastDate.toString(); - // long date_long = lastDate.getTime(); Announcements.Announcement a = new Announcements.Announcement(id, poster, date, subject, body); a.setFilters(locs, orgs, audience); @@ -69,10 +77,11 @@ public static void submit( throws SurveyException { makeSureDbSetup(); try { - int announcementId = savePostToDb(request, user); + final int announcementId = savePostToDb(request, user); logger.fine("Saved announcement, id = " + announcementId); response.id = announcementId; response.ok = true; + mostRecentAnnouncementId = announcementId; } catch (SurveyException e) { response.err = "An exception occured: " + e; logger.severe(e.getMessage()); @@ -117,7 +126,7 @@ private static PreparedStatement prepare_pAdd(Connection conn) throws SQLExcepti "INSERT INTO " + DBUtils.Table.ANNOUNCE + " (poster,subj,text,locs,orgs,audience)" - + " values (?,?,?,?,?,?)"; + + " VALUES (?,?,?,?,?,?)"; return DBUtils.prepareStatement(conn, "pAddAnnouncement", sql); } @@ -134,7 +143,7 @@ private static void emailNotify( + announcementId + " queueing:" + recipients.size()); - if (recipients.size() == 0) { + if (recipients.isEmpty()) { return; } String subject = "CLDR Survey Tool announcement: " + request.subject; diff --git a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/Announcements.java b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/Announcements.java index c0d4a217862..7e56caa7aba 100644 --- a/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/Announcements.java +++ b/tools/cldr-apps/src/main/java/org/unicode/cldr/web/api/Announcements.java @@ -59,7 +59,12 @@ public class Announcements { mediaType = "application/json", schema = @Schema(implementation = STError.class))), }) - public Response getAnnouncements(@HeaderParam(Auth.SESSION_HEADER) String sessionString) { + public Response getAnnouncements( + @HeaderParam(Auth.SESSION_HEADER) String sessionString, + @QueryParam("alreadyGotId") + @Schema(description = "The client already got this announcement ID") + @DefaultValue("0") + int alreadyGotId) { CookieSession session = Auth.getSession(sessionString); if (session == null) { return Auth.noSessionResponse(); @@ -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(); + } AnnouncementResponse response = new AnnouncementResponse(session.user); return Response.ok(response).build(); }