From ef89b57fd15cb5b835ad542627d987430794e445 Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Mon, 24 Jul 2023 12:33:10 -0400 Subject: [PATCH 1/2] improve consistency redis cache key for comment count --- src/core/client/count/index.ts | 11 +-- src/core/client/count/injectJSONPCallback.ts | 91 ++++++++++--------- src/core/client/stream/classes.ts | 5 + .../client/ui/components/icons/RemoveIcon.tsx | 6 +- src/core/common/types/count.ts | 3 +- .../server/app/handlers/api/story/count.ts | 24 ++--- src/core/server/app/middleware/cache.ts | 5 +- src/core/server/locales/en-US/common.ftl | 2 - src/core/server/locales/nl-NL/common.ftl | 2 - 9 files changed, 73 insertions(+), 76 deletions(-) diff --git a/src/core/client/count/index.ts b/src/core/client/count/index.ts index c694728ae6..33d05faa65 100644 --- a/src/core/client/count/index.ts +++ b/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 = { id, url, - notext: notext ? "true" : "false", ref, }; diff --git a/src/core/client/count/injectJSONPCallback.ts b/src/core/client/count/injectJSONPCallback.ts index 452409aaa1..021f32c2fd 100644 --- a/src/core/client/count/injectJSONPCallback.ts +++ b/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. diff --git a/src/core/client/stream/classes.ts b/src/core/client/stream/classes.ts index 87316adc86..643ac93a89 100644 --- a/src/core/client/stream/classes.ts +++ b/src/core/client/stream/classes.ts @@ -413,6 +413,11 @@ const CLASSES = { * shareButton is the button that will show the permalink popover. */ shareButton: "coral coral-comment-shareButton", + /** + * goToConversationButton is the button that links to the permalink + * view for the comment in single comment embeds + */ + goToConversationButton: "coral coral-comment-goToConversationButton", /** * reportButton is the button that triggers the report feature. */ diff --git a/src/core/client/ui/components/icons/RemoveIcon.tsx b/src/core/client/ui/components/icons/RemoveIcon.tsx index b367b3da27..8c3f195ac1 100644 --- a/src/core/client/ui/components/icons/RemoveIcon.tsx +++ b/src/core/client/ui/components/icons/RemoveIcon.tsx @@ -1,7 +1,7 @@ import React, { FunctionComponent } from "react"; -const CloseIcon: FunctionComponent = () => { - // NOte: Using Streamline Remove icon here +const RemoveIcon: FunctionComponent = () => { + // https://www.streamlinehq.com/icons/streamline-regular/interface-essential/remove-add/remove return ( { ); }; -export default CloseIcon; +export default RemoveIcon; diff --git a/src/core/common/types/count.ts b/src/core/common/types/count.ts index bfabbbb38e..022a9bb687 100644 --- a/src/core/common/types/count.ts +++ b/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; } diff --git a/src/core/server/app/handlers/api/story/count.ts b/src/core/server/app/handlers/api/story/count.ts index f426cf1522..f754c84dab 100644 --- a/src/core/server/app/handlers/api/story/count.ts +++ b/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, storyMode: GQLSTORY_MODE | undefined | null, i18n: I18n, @@ -55,22 +53,20 @@ function getCountHTML( if (storyMode === GQLSTORY_MODE.RATINGS_AND_REVIEWS) { html = translate( bundle, - `${count} Ratings`, + `Ratings`, "comment-counts-ratings-and-reviews", { number: count, - numberClass: NUMBER_CLASS_NAME, textClass: TEXT_CLASS_NAME, } ); } else { html = translate( bundle, - `${count} Comments`, + `Comments`, "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 = `${count}`; - } else { - html = getCountHTML(tenant, story?.settings.mode, i18n, count); - } + const countHtml = `${count}`; + const textHtml = getTextHTML(tenant, story?.settings.mode, i18n, count); const data: CountJSONPData = { ref, - html, + countHtml, + textHtml, count, id: story?.id || null, }; diff --git a/src/core/server/app/middleware/cache.ts b/src/core/server/app/middleware/cache.ts index d2b43a9709..969f7753a6 100644 --- a/src/core/server/app/middleware/cache.ts +++ b/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. diff --git a/src/core/server/locales/en-US/common.ftl b/src/core/server/locales/en-US/common.ftl index 9d3f8c1156..1d5891bd36 100644 --- a/src/core/server/locales/en-US/common.ftl +++ b/src/core/server/locales/en-US/common.ftl @@ -6,14 +6,12 @@ reaction-labelActiveRespected = Respected reaction-sortLabelMostRespected = Most Respected comment-count = - { $number } { $number -> [one] Comment *[other] Comments } comment-counts-ratings-and-reviews = - { $number } { $number -> [one] Rating *[other] Ratings diff --git a/src/core/server/locales/nl-NL/common.ftl b/src/core/server/locales/nl-NL/common.ftl index 0da6222359..395cbf88d4 100644 --- a/src/core/server/locales/nl-NL/common.ftl +++ b/src/core/server/locales/nl-NL/common.ftl @@ -6,14 +6,12 @@ reaction-labelActiveRespected = Gerespecteerd reaction-sortLabelMostRespected = Meest Gerespecteerd comment-count = - { $number } { $number -> [one] Reactie *[other] Reacties } comment-counts-ratings-and-reviews = - { $number } { $number -> [one] Rating *[other] Ratings From c72a65f9700713cc8c4ae6b603600f3eea50c116 Mon Sep 17 00:00:00 2001 From: Kathryn Beaty Date: Mon, 24 Jul 2023 13:01:05 -0400 Subject: [PATCH 2/2] cleanup --- src/core/client/stream/classes.ts | 5 ----- src/core/client/ui/components/icons/RemoveIcon.tsx | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/core/client/stream/classes.ts b/src/core/client/stream/classes.ts index 643ac93a89..87316adc86 100644 --- a/src/core/client/stream/classes.ts +++ b/src/core/client/stream/classes.ts @@ -413,11 +413,6 @@ const CLASSES = { * shareButton is the button that will show the permalink popover. */ shareButton: "coral coral-comment-shareButton", - /** - * goToConversationButton is the button that links to the permalink - * view for the comment in single comment embeds - */ - goToConversationButton: "coral coral-comment-goToConversationButton", /** * reportButton is the button that triggers the report feature. */ diff --git a/src/core/client/ui/components/icons/RemoveIcon.tsx b/src/core/client/ui/components/icons/RemoveIcon.tsx index 8c3f195ac1..b367b3da27 100644 --- a/src/core/client/ui/components/icons/RemoveIcon.tsx +++ b/src/core/client/ui/components/icons/RemoveIcon.tsx @@ -1,7 +1,7 @@ import React, { FunctionComponent } from "react"; -const RemoveIcon: FunctionComponent = () => { - // https://www.streamlinehq.com/icons/streamline-regular/interface-essential/remove-add/remove +const CloseIcon: FunctionComponent = () => { + // NOte: Using Streamline Remove icon here return ( { ); }; -export default RemoveIcon; +export default CloseIcon;