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
unicode-org · Apr 2, 2024 · aa95381 · aa95381
1 parent 4c5595e
commit aa95381
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 43 deletions.
diff --git 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
@@ -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
@@ -19,9 +19,13 @@ <h1>CLDR SurveyTool JavaScript Tests</h1>
       <ul>
         <li>in the 'js' directory, run <code>npm i</code> to install</li>
         <li>To rebuild, run <code>npm run build-test</code></li>
-        <li>If you want to continuously compile, keep <code>npm run watch-test</code> (control-C to cancel)</li>
-        <li>open the browser console to check
-        for errors/warnings that are otherwise invisible!
+        <li>
+          If you want to continuously compile, keep
+          <code>npm run watch-test</code> (control-C to cancel)
+        </li>
+        <li>
+          open the browser console to check for errors/warnings that are
+          otherwise invisible!
         </li>
       </ul>
       <p><em>References:</em></p>

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
@@ -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<Announcements.Announcement> 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
@@ -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();
     }