From 1d0d3569c4869c24fdcc792f94936cbfec9d0b49 Mon Sep 17 00:00:00 2001 From: DanielCliftonGuardian <110032454+DanielCliftonGuardian@users.noreply.github.com> Date: Wed, 7 Feb 2024 12:44:28 +0000 Subject: [PATCH 1/4] parse commentcontextresponse --- .../src/components/Discussion.tsx | 21 +++++++---- dotcom-rendering/src/lib/discussionApi.tsx | 35 +++++++++++++++++++ ...CommentContext.ts => discussionFilters.ts} | 34 ++---------------- dotcom-rendering/src/types/discussion.ts | 12 +++++++ 4 files changed, 64 insertions(+), 38 deletions(-) rename dotcom-rendering/src/lib/{getCommentContext.ts => discussionFilters.ts} (72%) diff --git a/dotcom-rendering/src/components/Discussion.tsx b/dotcom-rendering/src/components/Discussion.tsx index 5300b1b3d04..2b27dfde576 100644 --- a/dotcom-rendering/src/components/Discussion.tsx +++ b/dotcom-rendering/src/components/Discussion.tsx @@ -4,11 +4,12 @@ import { palette, space } from '@guardian/source-foundations'; import { Button, SvgPlus } from '@guardian/source-react-components'; import { useEffect, useReducer } from 'react'; import { assertUnreachable } from '../lib/assert-unreachable'; -import { getDiscussion, type reportAbuse } from '../lib/discussionApi'; import { getCommentContext, - initFiltersFromLocalStorage, -} from '../lib/getCommentContext'; + getDiscussion, + type reportAbuse, +} from '../lib/discussionApi'; +import { initFiltersFromLocalStorage } from '../lib/discussionFilters'; import { palette as themePalette } from '../palette'; import type { CommentForm, @@ -480,10 +481,16 @@ export const Discussion = ({ remapToValidFilters(filters, hashCommentId), ) .then((context) => { - dispatch({ - type: 'updateCommentPage', - commentPage: context.page, - }); + if (context.kind === 'ok') { + dispatch({ + type: 'updateCommentPage', + commentPage: context.value.page, + }); + } else { + console.error( + `getCommentContext - error: ${context.error}`, + ); + } }) .catch((e) => console.error(`getCommentContext - error: ${String(e)}`), diff --git a/dotcom-rendering/src/lib/discussionApi.tsx b/dotcom-rendering/src/lib/discussionApi.tsx index a4661089bd8..3b93b358a40 100644 --- a/dotcom-rendering/src/lib/discussionApi.tsx +++ b/dotcom-rendering/src/lib/discussionApi.tsx @@ -9,12 +9,15 @@ import type { } from '../types/discussion'; import { discussionApiResponseSchema, + getCommentContextResponseSchema, parseAbuseResponse, parseCommentRepliesResponse, parseCommentResponse, pickResponseSchema, postUsernameResponseSchema, } from '../types/discussion'; +import type { CommentContextType } from './discussionFilters'; +import { buildParams } from './discussionFilters'; import type { SignedInWithCookies, SignedInWithOkta } from './identity'; import { getOptionsHeadersWithOkta } from './identity'; import { fetchJSON } from './json'; @@ -436,3 +439,35 @@ export const getMoreResponses = async ( return parseCommentRepliesResponse(jsonResult.value); }; + +export const getCommentContext = async ( + ajaxUrl: string, + commentId: number, + filters: FilterOptions, +): Promise> => { + const url = joinUrl(ajaxUrl, 'comment', commentId.toString(), 'context'); + const params = buildParams(filters); + + const response = await fetch(url + '?' + params.toString()); + + if (!response.ok) { + const sentryError = new Error( + response.statusText || + `getCommentContext | An api call returned HTTP status ${response.status}`, + ); + window.guardian.modules.sentry.reportError( + sentryError, + 'get-comment-page', + ); + return error('ApiError'); + } + + const json = await response.json(); + const result = safeParse(getCommentContextResponseSchema, json); + + if (!result.success) { + return error('ParsingError'); + } + + return ok(result.output as CommentContextType); +}; diff --git a/dotcom-rendering/src/lib/getCommentContext.ts b/dotcom-rendering/src/lib/discussionFilters.ts similarity index 72% rename from dotcom-rendering/src/lib/getCommentContext.ts rename to dotcom-rendering/src/lib/discussionFilters.ts index 232cac29825..ea829c3f214 100644 --- a/dotcom-rendering/src/lib/getCommentContext.ts +++ b/dotcom-rendering/src/lib/discussionFilters.ts @@ -1,4 +1,4 @@ -import { isOneOf, isString, joinUrl, storage } from '@guardian/libs'; +import { isOneOf, isString, storage } from '@guardian/libs'; const orderByTypes = ['newest', 'oldest', 'recommendations'] as const; const threadTypes = ['collapsed', 'expanded', 'unthreaded'] as const; @@ -11,7 +11,7 @@ type PageSizeType = (typeof pageSizeTypes)[number]; /** * @see http://discussion.guardianapis.com/discussion-api/comment/3519111/context */ -type CommentContextType = { +export type CommentContextType = { status: 'ok' | 'error'; commentId: number; commentAncestorId: number; @@ -55,7 +55,7 @@ export const initFiltersFromLocalStorage = (): FilterOptions => { }; }; -const buildParams = (filters: FilterOptions) => { +export const buildParams = (filters: FilterOptions): URLSearchParams => { return new URLSearchParams({ // Frontend uses the 'recommendations' key to store this options but the api expects // 'mostRecommended' so we have to map here to support both @@ -69,31 +69,3 @@ const buildParams = (filters: FilterOptions) => { ), }); }; - -export const getCommentContext = async ( - ajaxUrl: string, - commentId: number, - filters: FilterOptions, -): Promise => { - const url = joinUrl(ajaxUrl, 'comment', commentId.toString(), 'context'); - - const params = buildParams(filters); - - return fetch(url + '?' + params.toString()) - .then((response) => { - if (!response.ok) { - throw Error( - response.statusText || - `getCommentContext | An api call returned HTTP status ${response.status}`, - ); - } - return response; - }) - .then((response) => response.json()) - .catch((error) => { - window.guardian.modules.sentry.reportError( - error, - 'get-comment-page', - ); - }); -}; diff --git a/dotcom-rendering/src/types/discussion.ts b/dotcom-rendering/src/types/discussion.ts index 529b0576e57..89016e2d4e7 100644 --- a/dotcom-rendering/src/types/discussion.ts +++ b/dotcom-rendering/src/types/discussion.ts @@ -389,3 +389,15 @@ export type CommentForm = { previewBody: string; body: string; }; + +export const getCommentContextResponseSchema = object({ + status: literal('ok'), + commentId: number(), + commentAncestorId: number(), + discussionKey: string(), + discussionWebUrl: string(), + discussionApiUrl: string(), + orderBy: string(), + pageSize: number(), + page: number(), +}); From e08ab7f57f9883f362a5ce61718afac3bc93702b Mon Sep 17 00:00:00 2001 From: DanielCliftonGuardian <110032454+DanielCliftonGuardian@users.noreply.github.com> Date: Thu, 8 Feb 2024 10:53:33 +0000 Subject: [PATCH 2/4] Refactors --- dotcom-rendering/src/lib/discussionApi.tsx | 38 +++++++++++-------- dotcom-rendering/src/lib/discussionFilters.ts | 15 -------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/dotcom-rendering/src/lib/discussionApi.tsx b/dotcom-rendering/src/lib/discussionApi.tsx index 3b93b358a40..9bd6dce7762 100644 --- a/dotcom-rendering/src/lib/discussionApi.tsx +++ b/dotcom-rendering/src/lib/discussionApi.tsx @@ -17,7 +17,6 @@ import { postUsernameResponseSchema, } from '../types/discussion'; import type { CommentContextType } from './discussionFilters'; -import { buildParams } from './discussionFilters'; import type { SignedInWithCookies, SignedInWithOkta } from './identity'; import { getOptionsHeadersWithOkta } from './identity'; import { fetchJSON } from './json'; @@ -440,6 +439,21 @@ export const getMoreResponses = async ( return parseCommentRepliesResponse(jsonResult.value); }; +const buildParams = (filters: FilterOptions): URLSearchParams => { + return new URLSearchParams({ + // Frontend uses the 'recommendations' key to store this options but the api expects + // 'mostRecommended' so we have to map here to support both + orderBy: + filters.orderBy === 'recommendations' + ? 'mostRecommended' + : filters.orderBy, + pageSize: String(filters.pageSize), + displayThreaded: String( + filters.threads === 'collapsed' || filters.threads === 'expanded', + ), + }); +}; + export const getCommentContext = async ( ajaxUrl: string, commentId: number, @@ -448,26 +462,18 @@ export const getCommentContext = async ( const url = joinUrl(ajaxUrl, 'comment', commentId.toString(), 'context'); const params = buildParams(filters); - const response = await fetch(url + '?' + params.toString()); - - if (!response.ok) { - const sentryError = new Error( - response.statusText || - `getCommentContext | An api call returned HTTP status ${response.status}`, - ); - window.guardian.modules.sentry.reportError( - sentryError, - 'get-comment-page', - ); - return error('ApiError'); - } + const jsonResult = await fetchJSON(url + '?' + params.toString()); - const json = await response.json(); - const result = safeParse(getCommentContextResponseSchema, json); + if (jsonResult.kind === 'error') return jsonResult; + + const result = safeParse(getCommentContextResponseSchema, jsonResult); if (!result.success) { return error('ParsingError'); } + if (result.output.status !== 'ok') { + return error('ApiError'); + } return ok(result.output as CommentContextType); }; diff --git a/dotcom-rendering/src/lib/discussionFilters.ts b/dotcom-rendering/src/lib/discussionFilters.ts index ea829c3f214..2f5e26a39b9 100644 --- a/dotcom-rendering/src/lib/discussionFilters.ts +++ b/dotcom-rendering/src/lib/discussionFilters.ts @@ -54,18 +54,3 @@ export const initFiltersFromLocalStorage = (): FilterOptions => { pageSize, }; }; - -export const buildParams = (filters: FilterOptions): URLSearchParams => { - return new URLSearchParams({ - // Frontend uses the 'recommendations' key to store this options but the api expects - // 'mostRecommended' so we have to map here to support both - orderBy: - filters.orderBy === 'recommendations' - ? 'mostRecommended' - : filters.orderBy, - pageSize: String(filters.pageSize), - displayThreaded: String( - filters.threads === 'collapsed' || filters.threads === 'expanded', - ), - }); -}; From 748ca12c44c61cc07f2669f58b46c7fa4407dad0 Mon Sep 17 00:00:00 2001 From: DanielCliftonGuardian <110032454+DanielCliftonGuardian@users.noreply.github.com> Date: Thu, 8 Feb 2024 13:53:46 +0000 Subject: [PATCH 3/4] Fix parse Co-Authored-By: Max Duval --- dotcom-rendering/package.json | 2 +- dotcom-rendering/src/lib/discussionApi.tsx | 4 ++-- dotcom-rendering/src/lib/discussionFilters.ts | 2 +- dotcom-rendering/src/types/discussion.ts | 4 ++-- pnpm-lock.yaml | 12 ++++++------ 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/dotcom-rendering/package.json b/dotcom-rendering/package.json index 20aee832450..1da6f5427ac 100644 --- a/dotcom-rendering/package.json +++ b/dotcom-rendering/package.json @@ -228,7 +228,7 @@ "typescript": "5.3.3", "typescript-json-schema": "0.58.1", "unified": "10.1.2", - "valibot": "0.26.0", + "valibot": "0.28.1", "web-vitals": "3.5.1", "webpack": "5.89.0", "webpack-assets-manifest": "5.1.0", diff --git a/dotcom-rendering/src/lib/discussionApi.tsx b/dotcom-rendering/src/lib/discussionApi.tsx index 9bd6dce7762..9a5505d1717 100644 --- a/dotcom-rendering/src/lib/discussionApi.tsx +++ b/dotcom-rendering/src/lib/discussionApi.tsx @@ -466,7 +466,7 @@ export const getCommentContext = async ( if (jsonResult.kind === 'error') return jsonResult; - const result = safeParse(getCommentContextResponseSchema, jsonResult); + const result = safeParse(getCommentContextResponseSchema, jsonResult.value); if (!result.success) { return error('ParsingError'); @@ -475,5 +475,5 @@ export const getCommentContext = async ( return error('ApiError'); } - return ok(result.output as CommentContextType); + return ok(result.output); }; diff --git a/dotcom-rendering/src/lib/discussionFilters.ts b/dotcom-rendering/src/lib/discussionFilters.ts index 2f5e26a39b9..25fd5920fb4 100644 --- a/dotcom-rendering/src/lib/discussionFilters.ts +++ b/dotcom-rendering/src/lib/discussionFilters.ts @@ -12,7 +12,7 @@ type PageSizeType = (typeof pageSizeTypes)[number]; * @see http://discussion.guardianapis.com/discussion-api/comment/3519111/context */ export type CommentContextType = { - status: 'ok' | 'error'; + status: 'ok'; commentId: number; commentAncestorId: number; discussionKey: string; diff --git a/dotcom-rendering/src/types/discussion.ts b/dotcom-rendering/src/types/discussion.ts index 89016e2d4e7..1817616cb84 100644 --- a/dotcom-rendering/src/types/discussion.ts +++ b/dotcom-rendering/src/types/discussion.ts @@ -397,7 +397,7 @@ export const getCommentContextResponseSchema = object({ discussionKey: string(), discussionWebUrl: string(), discussionApiUrl: string(), - orderBy: string(), - pageSize: number(), + orderBy: picklist(orderBy), + pageSize: picklist(pageSize), page: number(), }); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 12bd106e990..9444a30c177 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -346,7 +346,7 @@ importers: version: 7.0.1(tslib@2.6.2)(typescript@5.3.3) '@guardian/braze-components': specifier: 18.0.0 - version: 18.0.0(@emotion/react@11.11.1)(@guardian/libs@16.0.1)(@guardian/source-foundations@14.1.2)(@guardian/source-react-components-development-kitchen@16.0.0)(@guardian/source-react-components@18.0.0)(react@18.2.0) + version: 18.0.0(@emotion/react@11.11.1)(@guardian/libs@16.0.1)(@guardian/source-foundations@14.1.4)(@guardian/source-react-components-development-kitchen@16.0.0)(@guardian/source-react-components@18.0.0)(react@18.2.0) '@guardian/bridget': specifier: 2.6.0 version: 2.6.0 @@ -897,8 +897,8 @@ importers: specifier: 10.1.2 version: 10.1.2 valibot: - specifier: 0.26.0 - version: 0.26.0 + specifier: 0.28.1 + version: 0.28.1 web-vitals: specifier: 3.5.1 version: 3.5.1 @@ -4432,7 +4432,7 @@ packages: - utf-8-validate dev: false - /@guardian/braze-components@18.0.0(@emotion/react@11.11.1)(@guardian/libs@16.0.1)(@guardian/source-foundations@14.1.2)(@guardian/source-react-components-development-kitchen@16.0.0)(@guardian/source-react-components@18.0.0)(react@18.2.0): + /@guardian/braze-components@18.0.0(@emotion/react@11.11.1)(@guardian/libs@16.0.1)(@guardian/source-foundations@14.1.4)(@guardian/source-react-components-development-kitchen@16.0.0)(@guardian/source-react-components@18.0.0)(react@18.2.0): resolution: {integrity: sha512-tK0waGQj/dEwpssxEOlHw7/d9XXnkQqZai/2LFUSuHwDtbeTgZZfIsAd9DoX02qFHNZlzvRvbLlS48zKcPKHDw==} engines: {node: ^18.15 || ^20.9} peerDependencies: @@ -19925,8 +19925,8 @@ packages: convert-source-map: 2.0.0 dev: false - /valibot@0.26.0: - resolution: {integrity: sha512-cM8VM9jsJGx9P4qarZUBXd0I88OVI4ddtyZ1sAMiofC3Usj7Jxm2c8Xx5accdTrKc3CblO8yyytRaJ6lrsuA+Q==} + /valibot@0.28.1: + resolution: {integrity: sha512-zQnjwNJuXk6362Leu0+4eFa/SMwRom3/hEvH6s1EGf3oXIPbo2WFKDra9ymnbVh3clLRvd8hw4sKF5ruI2Lyvw==} dev: false /validate-npm-package-license@3.0.4: From 9ab3df424c0b741ed8eb7635bf8ca33913e0eadf Mon Sep 17 00:00:00 2001 From: DanielCliftonGuardian <110032454+DanielCliftonGuardian@users.noreply.github.com> Date: Fri, 9 Feb 2024 09:37:58 +0000 Subject: [PATCH 4/4] Update discussionApi.tsx --- dotcom-rendering/src/lib/discussionApi.tsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/dotcom-rendering/src/lib/discussionApi.tsx b/dotcom-rendering/src/lib/discussionApi.tsx index 9a5505d1717..e30f35c19ef 100644 --- a/dotcom-rendering/src/lib/discussionApi.tsx +++ b/dotcom-rendering/src/lib/discussionApi.tsx @@ -471,9 +471,6 @@ export const getCommentContext = async ( if (!result.success) { return error('ParsingError'); } - if (result.output.status !== 'ok') { - return error('ApiError'); - } return ok(result.output); };