From d7e7f9cf946ec5a00d96de8d7e5378ef02c00c79 Mon Sep 17 00:00:00 2001 From: Max Duval Date: Wed, 24 Jan 2024 16:12:01 +0000 Subject: [PATCH] feat(discussionApi): parse CommentResponse reduce the complexity of the possible responses --- .../components/Discussion/AbuseReportForm.tsx | 4 +- .../src/components/Discussion/Comment.tsx | 8 +- .../Discussion/CommentContainer.tsx | 10 +- .../src/components/Discussion/CommentForm.tsx | 134 +++++++++--------- .../src/components/Discussion/Comments.tsx | 10 +- dotcom-rendering/src/lib/discussionApi.tsx | 90 +++++++++--- dotcom-rendering/src/types/discussion.ts | 63 +++++++- 7 files changed, 209 insertions(+), 110 deletions(-) diff --git a/dotcom-rendering/src/components/Discussion/AbuseReportForm.tsx b/dotcom-rendering/src/components/Discussion/AbuseReportForm.tsx index 7c3938f7280..25e7902990a 100644 --- a/dotcom-rendering/src/components/Discussion/AbuseReportForm.tsx +++ b/dotcom-rendering/src/components/Discussion/AbuseReportForm.tsx @@ -167,9 +167,9 @@ export const AbuseReportForm = ({ authStatus, }) .then((response) => { - if (response.status !== 'ok') { + if (response.kind === 'error') { // Fallback to errors returned from the API - setErrors({ ...errors, response: response.message }); + setErrors({ ...errors, response: response.error.message }); } else { setSuccessMessage('Report submitted'); } diff --git a/dotcom-rendering/src/components/Discussion/Comment.tsx b/dotcom-rendering/src/components/Discussion/Comment.tsx index f816f8c74aa..cafa5bd9e56 100644 --- a/dotcom-rendering/src/components/Discussion/Comment.tsx +++ b/dotcom-rendering/src/components/Discussion/Comment.tsx @@ -314,8 +314,8 @@ export const Comment = ({ setError(''); const response = await pickComment(staffUser.authStatus, comment.id); - if (response.status === 'error') { - setError(response.message); + if (response.kind === 'error') { + setError(response.error.message); } else { setIsHighlighted(true); } @@ -324,8 +324,8 @@ export const Comment = ({ const unPick = async (staffUser: SignedInUser) => { setError(''); const response = await unPickComment(staffUser.authStatus, comment.id); - if (response.status === 'error') { - setError(response.message); + if (response.kind === 'error') { + setError(response.error.message); } else { setIsHighlighted(false); } diff --git a/dotcom-rendering/src/components/Discussion/CommentContainer.tsx b/dotcom-rendering/src/components/Discussion/CommentContainer.tsx index b2a240511a7..1c5ff8d72f3 100644 --- a/dotcom-rendering/src/components/Discussion/CommentContainer.tsx +++ b/dotcom-rendering/src/components/Discussion/CommentContainer.tsx @@ -2,9 +2,9 @@ import { css } from '@emotion/react'; import { palette as sourcePalette, space } from '@guardian/source-foundations'; import { SvgPlus } from '@guardian/source-react-components'; import { useEffect, useState } from 'react'; +import type { comment, reply } from '../../lib/discussionApi'; import { getMoreResponses } from '../../lib/discussionApi'; import type { - CommentResponse, CommentType, SignedInUser, ThreadsType, @@ -27,12 +27,8 @@ type Props = { toggleMuteStatus: (userId: string) => void; onPermalinkClick: (commentId: number) => void; onRecommend?: (commentId: number) => Promise; - onComment?: (shortUrl: string, body: string) => Promise; - onReply?: ( - shortUrl: string, - body: string, - parentCommentId: number, - ) => Promise; + onComment?: ReturnType; + onReply?: ReturnType; onPreview?: (body: string) => Promise; showPreview: boolean; setShowPreview: (showPreview: boolean) => void; diff --git a/dotcom-rendering/src/components/Discussion/CommentForm.tsx b/dotcom-rendering/src/components/Discussion/CommentForm.tsx index 7735fc11057..279383cb5fc 100644 --- a/dotcom-rendering/src/components/Discussion/CommentForm.tsx +++ b/dotcom-rendering/src/components/Discussion/CommentForm.tsx @@ -13,11 +13,7 @@ import { reply as defaultReply, } from '../../lib/discussionApi'; import { palette as schemedPalette } from '../../palette'; -import type { - CommentResponse, - CommentType, - SignedInUser, -} from '../../types/discussion'; +import type { CommentType, SignedInUser } from '../../types/discussion'; import { FirstCommentWelcome } from './FirstCommentWelcome'; import { PillarButton } from './PillarButton'; import { Preview } from './Preview'; @@ -29,12 +25,8 @@ type Props = { onAddComment: (response: CommentType) => void; setCommentBeingRepliedTo?: () => void; commentBeingRepliedTo?: CommentType; - onComment?: (shortUrl: string, body: string) => Promise; - onReply?: ( - shortUrl: string, - body: string, - parentCommentId: number, - ) => Promise; + onComment?: ReturnType; + onReply?: ReturnType; onPreview?: (body: string) => Promise; showPreview: boolean; setShowPreview: (showPreview: boolean) => void; @@ -325,88 +317,92 @@ export const CommentForm = ({ if (body) { const comment = onComment ?? defaultComment(user.authStatus); const reply = onReply ?? defaultReply(user.authStatus); - const response: CommentResponse = commentBeingRepliedTo + const response = commentBeingRepliedTo ? await reply(shortUrl, body, commentBeingRepliedTo.id) : await comment(shortUrl, body); // Check response message for error states - if (response.errorCode === 'USERNAME_MISSING') { - // Reader has never posted before and needs to choose a username - setUserNameMissing(true); - } else if (response.errorCode === 'EMPTY_COMMENT_BODY') { - setError('Please write a comment.'); - } else if (response.errorCode === 'COMMENT_TOO_LONG') { - setError( - 'Your comment must be fewer than 5000 characters long.', - ); - } else if (response.errorCode === 'USER_BANNED') { - setError( - 'Commenting has been disabled for this account (why?).', - ); - } else if (response.errorCode === 'IP_THROTTLED') { - setError( - 'Commenting has been temporarily blocked for this IP address (why?).', - ); - } else if (response.errorCode === 'DISCUSSION_CLOSED') { - setError( - 'Sorry your comment can not be published as the discussion is now closed for comments.', - ); - } else if (response.errorCode === 'PARENT_COMMENT_MODERATED') { - setError( - 'Sorry the comment can not be published as the comment you replied to has been moderated since.', - ); - } else if (response.errorCode === 'COMMENT_RATE_LIMIT_EXCEEDED') { - setError( - 'You can only post one comment every minute. Please try again in a moment.', - ); - } else if (response.errorCode === 'INVALID_PROTOCOL') { - setError(`Sorry your comment can not be published as it was not sent over + if (response.kind === 'error') { + if (response.error.code === 'USERNAME_MISSING') { + // Reader has never posted before and needs to choose a username + setUserNameMissing(true); + } else if (response.error.code === 'EMPTY_COMMENT_BODY') { + setError('Please write a comment.'); + } else if (response.error.code === 'COMMENT_TOO_LONG') { + setError( + 'Your comment must be fewer than 5000 characters long.', + ); + } else if (response.error.code === 'USER_BANNED') { + setError( + 'Commenting has been disabled for this account (why?).', + ); + } else if (response.error.code === 'IP_THROTTLED') { + setError( + 'Commenting has been temporarily blocked for this IP address (why?).', + ); + } else if (response.error.code === 'DISCUSSION_CLOSED') { + setError( + 'Sorry your comment can not be published as the discussion is now closed for comments.', + ); + } else if (response.error.code === 'PARENT_COMMENT_MODERATED') { + setError( + 'Sorry the comment can not be published as the comment you replied to has been moderated since.', + ); + } else if ( + response.error.code === 'COMMENT_RATE_LIMIT_EXCEEDED' + ) { + setError( + 'You can only post one comment every minute. Please try again in a moment.', + ); + } else if (response.error.code === 'INVALID_PROTOCOL') { + setError(`Sorry your comment can not be published as it was not sent over a secure channel. Please report us this issue using the technical issue link in the page footer.`); - } else if (response.errorCode === 'AUTH_COOKIE_INVALID') { - setError( - 'Sorry, your comment was not published as you are no longer signed in. Please sign in and try again.', - ); - } else if (response.errorCode === 'READ-ONLY-MODE') { - setError(`Sorry your comment can not currently be published as + } else if (response.error.code === 'AUTH_COOKIE_INVALID') { + setError( + 'Sorry, your comment was not published as you are no longer signed in. Please sign in and try again.', + ); + } else if (response.error.code === 'READ-ONLY-MODE') { + setError(`Sorry your comment can not currently be published as commenting is undergoing maintenance but will be back shortly. Please try again in a moment.`); - } else if (response.errorCode === 'API_CORS_BLOCKED') { - setError(`Could not post due to your internet settings, which might be + } else if (response.error.code === 'API_CORS_BLOCKED') { + setError(`Could not post due to your internet settings, which might be controlled by your provider. Please contact your administrator or disable any proxy servers or VPNs and try again.`); - } else if (response.errorCode === 'API_ERROR') { - setError(`Sorry, there was a problem posting your comment. Please try + } else if (response.error.code === 'API_ERROR') { + setError(`Sorry, there was a problem posting your comment. Please try another browser or network connection. Reference code `); - } else if (response.errorCode === 'EMAIL_VERIFIED') { - setInfo( - 'Sent. Please check your email to verify your email address. Once verified post your comment.', - ); - } else if (response.errorCode === 'EMAIL_VERIFIED_FAIL') { - // TODO: Support resending verification email - setError(`We are having technical difficulties. Please try again later or + } else if (response.error.code === 'EMAIL_VERIFIED') { + setInfo( + 'Sent. Please check your email to verify your email address. Once verified post your comment.', + ); + } else if (response.error.code === 'EMAIL_VERIFIED_FAIL') { + // TODO: Support resending verification email + setError(`We are having technical difficulties. Please try again later or resend the verification.`); - } else if (response.errorCode === 'EMAIL_NOT_VALIDATED') { - // TODO: Support resending verification email - setError(`Please confirm your email address to comment.
+ } else if (response.error.code === 'EMAIL_NOT_VALIDATED') { + // TODO: Support resending verification email + setError(`Please confirm your email address to comment.
If you can't find the email, we can resend the verification email to your email address.`); - } else if (response.status === 'ok') { + } else { + setError( + 'Sorry, there was a problem posting your comment.', + ); + } + } else { onAddComment( simulateNewComment( - // response.errorCode is the id of the comment that was created on the server - // it is returned as a string, so we need to cast to an number to be compatable - parseInt(response.message), + response.value, body, user.profile, commentBeingRepliedTo, ), ); resetForm(); - } else { - setError('Sorry, there was a problem posting your comment.'); } } }; diff --git a/dotcom-rendering/src/components/Discussion/Comments.tsx b/dotcom-rendering/src/components/Discussion/Comments.tsx index 75242513248..b25a3045889 100644 --- a/dotcom-rendering/src/components/Discussion/Comments.tsx +++ b/dotcom-rendering/src/components/Discussion/Comments.tsx @@ -6,10 +6,10 @@ import { textSans, } from '@guardian/source-foundations'; import { useEffect, useState } from 'react'; +import type { comment, reply } from '../../lib/discussionApi'; import { getPicks, initialiseApi } from '../../lib/discussionApi'; import type { AdditionalHeadersType, - CommentResponse, CommentType, FilterOptions, SignedInUser, @@ -32,12 +32,8 @@ type Props = { onPermalinkClick: (commentId: number) => void; apiKey: string; onRecommend?: (commentId: number) => Promise; - onComment?: (shortUrl: string, body: string) => Promise; - onReply?: ( - shortUrl: string, - body: string, - parentCommentId: number, - ) => Promise; + onComment?: ReturnType; + onReply?: ReturnType; onPreview?: (body: string) => Promise; onExpand: () => void; idApiUrl: string; diff --git a/dotcom-rendering/src/lib/discussionApi.tsx b/dotcom-rendering/src/lib/discussionApi.tsx index 7d48706126f..64fa10a705c 100644 --- a/dotcom-rendering/src/lib/discussionApi.tsx +++ b/dotcom-rendering/src/lib/discussionApi.tsx @@ -1,8 +1,8 @@ -import { joinUrl } from '@guardian/libs'; +import { isObject, isString, joinUrl } from '@guardian/libs'; import { safeParse } from 'valibot'; import type { AdditionalHeadersType, - CommentResponse, + CommentResponseErrorCodes, CommentType, DiscussionOptions, GetDiscussionSuccess, @@ -10,7 +10,10 @@ import type { ThreadsType, UserNameResponse, } from '../types/discussion'; -import { discussionApiResponseSchema } from '../types/discussion'; +import { + discussionApiResponseSchema, + parseCommentResponse, +} from '../types/discussion'; import type { SignedInWithCookies, SignedInWithOkta } from './identity'; import { getOptionsHeadersWithOkta } from './identity'; import { fetchJSON } from './json'; @@ -135,12 +138,17 @@ export const preview = async (body: string): Promise => { ...options.headers, }, }); - const json = await resp.json(); - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access -- YOLO - return json.commentBody; + const json = (await resp.json()) as unknown; + + return isObject(json) && isString(json.commentBody) ? json.commentBody : ''; }; +type CommentResponse = Result< + { code: CommentResponseErrorCodes | GetDiscussionError; message: string }, + number +>; + export const comment = (authStatus: SignedInWithCookies | SignedInWithOkta) => async (shortUrl: string, body: string): Promise => { @@ -152,7 +160,7 @@ export const comment = const authOptions = getOptionsHeadersWithOkta(authStatus); - const resp = await fetch(url, { + const jsonResult = await fetchJSON(url, { method: 'POST', body: data.toString(), headers: { @@ -163,7 +171,17 @@ export const comment = credentials: authOptions.credentials, }); - return resp.json(); + if (jsonResult.kind === 'error') { + return { + kind: 'error', + error: { + code: jsonResult.error, + message: 'Could not retrieve the comment', + }, + }; + } + + return parseCommentResponse(jsonResult.value); }; export const reply = @@ -186,7 +204,7 @@ export const reply = data.append('body', body); const authOptions = getOptionsHeadersWithOkta(authStatus); - const resp = await fetch(url, { + const jsonResult = await fetchJSON(url, { method: 'POST', body: data.toString(), headers: { @@ -197,7 +215,17 @@ export const reply = credentials: authOptions.credentials, }); - return resp.json(); + if (jsonResult.kind === 'error') { + return { + kind: 'error', + error: { + code: jsonResult.error, + message: 'Could not retrieve the comment', + }, + }; + } + + return parseCommentResponse(jsonResult.value); }; //todo: come back and parse the response properly and set a proper return type for the error case @@ -250,7 +278,7 @@ export const reportAbuse = async ({ ? getOptionsHeadersWithOkta(authStatus) : undefined; - const resp = await fetch(url, { + const jsonResult = await fetchJSON(url, { method: 'POST', body: data.toString(), headers: { @@ -261,7 +289,17 @@ export const reportAbuse = async ({ credentials: authOptions?.credentials, }); - return resp.json(); + if (jsonResult.kind === 'error') { + return { + kind: 'error', + error: { + code: jsonResult.error, + message: 'Could not retrieve the comment', + }, + }; + } + + return parseCommentResponse(jsonResult.value); }; export const recommend = @@ -323,7 +361,7 @@ export const pickComment = async ( objAsParams(defaultParams); const authOptions = getOptionsHeadersWithOkta(authStatus); - const resp = await fetch(url, { + const jsonResult = await fetchJSON(url, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', @@ -333,7 +371,17 @@ export const pickComment = async ( credentials: authOptions.credentials, }); - return resp.json(); + if (jsonResult.kind === 'error') { + return { + kind: 'error', + error: { + code: jsonResult.error, + message: 'Could not retrieve the comment', + }, + }; + } + + return parseCommentResponse(jsonResult.value); }; export const unPickComment = async ( @@ -349,7 +397,7 @@ export const unPickComment = async ( ) + objAsParams(defaultParams); const authOptions = getOptionsHeadersWithOkta(authStatus); - const resp = await fetch(url, { + const jsonResult = await fetchJSON(url, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', @@ -359,7 +407,17 @@ export const unPickComment = async ( credentials: authOptions.credentials, }); - return resp.json(); + if (jsonResult.kind === 'error') { + return { + kind: 'error', + error: { + code: jsonResult.error, + message: 'Could not retrieve the comment', + }, + }; + } + + return parseCommentResponse(jsonResult.value); }; export const getMoreResponses = async ( diff --git a/dotcom-rendering/src/types/discussion.ts b/dotcom-rendering/src/types/discussion.ts index e13cbe017c8..d3d64b20166 100644 --- a/dotcom-rendering/src/types/discussion.ts +++ b/dotcom-rendering/src/types/discussion.ts @@ -6,14 +6,18 @@ import { number, object, optional, + picklist, recursive, + safeParse, string, + transform, unknown, variant, } from 'valibot'; import type { Guard } from '../lib/guard'; import { guard } from '../lib/guard'; import type { SignedInWithCookies, SignedInWithOkta } from '../lib/identity'; +import type { Result } from '../lib/result'; export type CAPIPillar = | 'news' @@ -141,11 +145,60 @@ export interface CommentType { }; } -export type CommentResponse = { - status: 'ok' | 'error'; - statusCode: number; - message: string; - errorCode?: string; +export type CommentResponseErrorCodes = (typeof errorCodes)[number]; +const errorCodes = [ + 'USERNAME_MISSING', + 'EMPTY_COMMENT_BODY', + 'COMMENT_TOO_LONG', + 'USER_BANNED', + 'IP_THROTTLED', + 'DISCUSSION_CLOSED', + 'PARENT_COMMENT_MODERATED', + 'COMMENT_RATE_LIMIT_EXCEEDED', + 'INVALID_PROTOCOL', + 'AUTH_COOKIE_INVALID', + 'READ-ONLY-MODE', + 'API_CORS_BLOCKED', + 'API_ERROR', + 'EMAIL_VERIFIED', + 'EMAIL_VERIFIED_FAIL', + 'EMAIL_NOT_VALIDATED', +] as const; + +const commentErrorSchema = object({ + status: literal('error'), + errorCode: picklist(errorCodes), + message: string(), +}); + +const commentResponseSchema = variant('status', [ + commentErrorSchema, + object({ + status: literal('ok'), + // response.errorCode is the id of the comment that was created on the server + // it is returned as a string, so we need to cast to an number to be compatible + message: transform(string(), (input) => parseInt(input, 10)), + }), +]); + +export const parseCommentResponse = ( + data: unknown, +): Result< + { code: CommentResponseErrorCodes | 'ParsingError'; message: string }, + number +> => { + const { output, success } = safeParse(commentResponseSchema, data); + if (!success) + return { + kind: 'error', + error: { code: 'ParsingError', message: 'An error occured' }, + }; + return output.status === 'error' + ? { + kind: 'error', + error: { code: output.errorCode, message: output.message }, + } + : { kind: 'ok', value: output.message }; }; type UserNameError = {