Merge pull request #4300 from coralproject/fix/CORL-2875-improve-coun…

…ts-callback-redis-cache-key

[CORL-2875]: Improve consistency of Redis cache key for comment count

src/core/client/count/index.ts

@@ -10,12 +10,11 @@ import injectJSONPCallback from "./injectJSONPCallback";
 interface CountQueryArgs {
   id?: string;
   url?: string;
-  notext: boolean;
 }
 
 /** createCountQueryRef creates a unique reference from the query args */
 function createCountQueryRef(args: CountQueryArgs) {
-  return btoa(`${args.notext ? "true" : "false"};${args.id || args.url}`);
+  return btoa(`${args.url}`);
 }
 
 interface DetectAndInjectArgs {
@@ -34,18 +33,17 @@ function detectAndInject(opts: DetectAndInjectArgs = {}) {
   const elements = document.querySelectorAll(COUNT_SELECTOR);
   Array.prototype.forEach.call(elements, (element: HTMLElement) => {
     const id = element.dataset.coralId;
-    const notext = element.dataset.coralNotext === "true";
 
     // If there is no URL or ID on the element, add one based on the story url
     // that we detected.
     let url = element.dataset.coralUrl;
-    if (!url && !id) {
+    if (!url) {
       url = STORY_URL;
       element.dataset.coralUrl = STORY_URL;
     }
 
     // Construct the args for generating the ref.
-    const args = { id, url, notext };
+    const args = { id, url };
 
     // Get or create a ref.
     let ref = element.dataset.coralRef;
@@ -68,13 +66,12 @@ function detectAndInject(opts: DetectAndInjectArgs = {}) {
 
   // Call server using JSONP.
   Object.keys(queryMap).forEach((ref) => {
-    const { url, id, notext } = queryMap[ref];
+    const { url, id } = queryMap[ref];
 
     // Compile the arguments used to generate the
     const args: Record<string, string | undefined> = {
       id,
       url,
-      notext: notext ? "true" : "false",
       ref,
     };
 

src/core/client/count/injectJSONPCallback.ts

@@ -2,8 +2,6 @@ import { CountJSONPData } from "coral-common/types/count";
 import { COUNT_SELECTOR } from "coral-framework/constants";
 import getPreviousCountStorageKey from "coral-framework/helpers/getPreviousCountStorageKey";
 
-const TEXT_CLASS_NAME = "coral-count-text";
-
 type GetCountFunction = (opts?: { reset?: boolean }) => void;
 
 /**
@@ -44,57 +42,62 @@ interface CountElementDataset {
 }
 
 function createCountElementEnhancer({
-  html,
+  countHtml,
+  textHtml,
   count: currentCount,
   id: storyID,
 }: CountJSONPData) {
-  // Get the dataset together for setting properties on the enhancer.
-  const dataset: CountElementDataset = {
-    coralCount: currentCount.toString(),
-  };
-
-  // Create the root element we're using for this.
-  const element = document.createElement("span");
-
-  const showText = html.includes(TEXT_CLASS_NAME);
-
-  // Update the innerHTML which contains the count and new value..
-  element.innerHTML = html;
-
-  if (storyID) {
-    const previousCount = getPreviousCount(storyID) ?? 0;
+  return (target: HTMLElement) => {
+    // Get the dataset together for setting properties on the enhancer.
+    const dataset: CountElementDataset = {
+      coralCount: currentCount.toString(),
+    };
 
-    // The new count is the current count subtracted from the previous
-    // count if the counts are different, otherwise, zero.
-    const newCount =
-      previousCount < currentCount ? currentCount - previousCount : 0;
+    // Create the root element we're using for this.
+    const element = document.createElement("span");
 
-    // Add the counts to the dataset so it can be targeted by CSS if you want.
-    dataset.coralPreviousCount = previousCount.toString();
-    dataset.coralNewCount = newCount.toString();
+    const showText = !(target.dataset.coralNotext === "true");
 
+    // Update the innerHTML which contains the count and new value..
     if (showText) {
-      // Insert the divider " / "
-      const dividerElement = document.createElement("span");
-      dividerElement.className = "coral-new-count-divider";
-      dividerElement.innerText = " / ";
-      element.appendChild(dividerElement);
-
-      // Add the number of new comments to that.
-      const newCountNumber = document.createElement("span");
-      newCountNumber.className = "coral-new-count-number";
-      newCountNumber.innerText = newCount.toString();
-      element.appendChild(newCountNumber);
-
-      // Add the number of new comments to that.
-      const newCountText = document.createElement("span");
-      newCountText.className = "coral-new-count-text";
-      newCountText.innerText = " New";
-      element.appendChild(newCountText);
+      element.innerHTML = `${countHtml} ${textHtml}`;
+    } else {
+      element.innerHTML = countHtml;
+    }
+
+    if (storyID) {
+      const previousCount = getPreviousCount(storyID) ?? 0;
+
+      // The new count is the current count subtracted from the previous
+      // count if the counts are different, otherwise, zero.
+      const newCount =
+        previousCount < currentCount ? currentCount - previousCount : 0;
+
+      // Add the counts to the dataset so it can be targeted by CSS if you want.
+      dataset.coralPreviousCount = previousCount.toString();
+      dataset.coralNewCount = newCount.toString();
+
+      if (showText) {
+        // Insert the divider " / "
+        const dividerElement = document.createElement("span");
+        dividerElement.className = "coral-new-count-divider";
+        dividerElement.innerText = " / ";
+        element.appendChild(dividerElement);
+
+        // Add the number of new comments to that.
+        const newCountNumber = document.createElement("span");
+        newCountNumber.className = "coral-new-count-number";
+        newCountNumber.innerText = newCount.toString();
+        element.appendChild(newCountNumber);
+
+        // Add the number of new comments to that.
+        const newCountText = document.createElement("span");
+        newCountText.className = "coral-new-count-text";
+        newCountText.innerText = " New";
+        element.appendChild(newCountText);
+      }
     }
-  }
 
-  return (target: HTMLElement) => {
     // Add the innerHTML from the element to the target element. This will
     // include any optional children that were appended related to new comment
     // counts.

src/core/common/types/count.ts

@@ -1,6 +1,7 @@
 export interface CountJSONPData {
   ref: string;
-  html: string;
+  countHtml: string;
+  textHtml: string;
   count: number;
   id?: string | null;
 }

src/core/server/app/handlers/api/story/count.ts

@@ -28,7 +28,6 @@ const StoryCountJSONPQuerySchema = Joi.object()
     callback: Joi.string().allow("").optional(),
     id: Joi.string().optional(),
     url: Joi.string().optional(),
-    notext: Joi.string().allow("true", "false").required(),
     ref: Joi.string().required(),
   })
   .or("id", "url");
@@ -37,11 +36,10 @@ interface StoryCountJSONPQuery {
   callback: string;
   id?: string;
   url?: string;
-  notext: "true" | "false";
   ref: string;
 }
 
-function getCountHTML(
+function getTextHTML(
   tenant: Readonly<Tenant>,
   storyMode: GQLSTORY_MODE | undefined | null,
   i18n: I18n,
@@ -55,22 +53,20 @@ function getCountHTML(
   if (storyMode === GQLSTORY_MODE.RATINGS_AND_REVIEWS) {
     html = translate(
       bundle,
-      `<span class="${NUMBER_CLASS_NAME}">${count}</span> <span class="${TEXT_CLASS_NAME}">Ratings</span>`,
+      `<span class="${TEXT_CLASS_NAME}">Ratings</span>`,
       "comment-counts-ratings-and-reviews",
       {
         number: count,
-        numberClass: NUMBER_CLASS_NAME,
         textClass: TEXT_CLASS_NAME,
       }
     );
   } else {
     html = translate(
       bundle,
-      `<span class="${NUMBER_CLASS_NAME}">${count}</span> <span class="${TEXT_CLASS_NAME}">Comments</span>`,
+      `<span class="${TEXT_CLASS_NAME}">Comments</span>`,
       "comment-count",
       {
         number: count,
-        numberClass: NUMBER_CLASS_NAME,
         textClass: TEXT_CLASS_NAME,
       }
     );
@@ -89,7 +85,7 @@ export const countJSONPHandler =
       const { tenant } = req.coral;
 
       // Ensure we have something to query with.
-      const { id, url, notext, ref }: StoryCountJSONPQuery = validate(
+      const { id, url, ref }: StoryCountJSONPQuery = validate(
         StoryCountJSONPQuerySchema,
         req.query
       );
@@ -102,17 +98,13 @@ export const countJSONPHandler =
 
       const count = story ? await calculateStoryCount(mongo, story) : 0;
 
-      let html = "";
-      if (notext === "true") {
-        // We only need the count without the text.
-        html = `<span class="${NUMBER_CLASS_NAME}">${count}</span>`;
-      } else {
-        html = getCountHTML(tenant, story?.settings.mode, i18n, count);
-      }
+      const countHtml = `<span class="${NUMBER_CLASS_NAME}">${count}</span>`;
+      const textHtml = getTextHTML(tenant, story?.settings.mode, i18n, count);
 
       const data: CountJSONPData = {
         ref,
-        html,
+        countHtml,
+        textHtml,
         count,
         id: story?.id || null,
       };

src/core/server/app/middleware/cache.ts

@@ -40,7 +40,10 @@ const cacheMiddleware =
   (redis: Redis, ttl: number): RequestHandler =>
   async (req, res, next) => {
     // Compute the cache key.
-    const key = `rmc:${req.hostname}:${req.originalUrl}`;
+    const url = new URL(`${req.hostname}:${req.originalUrl}`);
+    const params = new URLSearchParams(url.search);
+
+    const key = `rmc:CoralCount&url=${params.get("url")}`;
     const log = logger.child({ key }, true);
 
     // Try to lookup the entry in the cache.

src/core/server/locales/en-US/common.ftl

@@ -6,14 +6,12 @@ reaction-labelActiveRespected = Respected
 reaction-sortLabelMostRespected = Most Respected
 
 comment-count =
-  <span class="{ $numberClass }">{ $number }</span>
   <span class="{ $textClass }">{ $number  ->
     [one] Comment
     *[other] Comments
   }</span>
 
 comment-counts-ratings-and-reviews =
-  <span class="{ $numberClass }">{ $number }</span>
   <span class="{ $textClass }">{ $number  ->
     [one] Rating
     *[other] Ratings

src/core/server/locales/nl-NL/common.ftl

@@ -6,14 +6,12 @@ reaction-labelActiveRespected = Gerespecteerd
 reaction-sortLabelMostRespected = Meest Gerespecteerd
 
 comment-count =
-  <span class="{ $numberClass }">{ $number }</span>
   <span class="{ $textClass }">{ $number  ->
     [one] Reactie
     *[other] Reacties
   }</span>
 
 comment-counts-ratings-and-reviews =
-  <span class="{ $numberClass }">{ $number }</span>
   <span class="{ $textClass }">{ $number  ->
     [one] Rating
     *[other] Ratings