Lift comment state into discussion

dotcom-rendering/src/components/Discussion.tsx

@@ -1,15 +1,17 @@
 import { css } from '@emotion/react';
-import { joinUrl, storage } from '@guardian/libs';
+import { storage } from '@guardian/libs';
 import { palette, space } from '@guardian/source-foundations';
 import { Button, SvgPlus } from '@guardian/source-react-components';
 import { useEffect, useState } from 'react';
+import { getDiscussion } from '../lib/discussionApi';
 import {
 	getCommentContext,
 	initFiltersFromLocalStorage,
 } from '../lib/getCommentContext';
-import { useDiscussion } from '../lib/useDiscussion';
+import { useCommentCount } from '../lib/useCommentCount';
 import { palette as themePalette } from '../palette';
 import type {
+	CommentType,
 	FilterOptions,
 	SignedInUser,
 	ThreadsType,
@@ -90,22 +92,38 @@ export const Discussion = ({
 	user,
 	idApiUrl,
 }: Props) => {
-	const [commentPage, setCommentPage] = useState<number>(1);
+	const [commentPage, setCommentPage] = useState(1);
 	const [commentPageSize, setCommentPageSize] = useState<25 | 50 | 100>();
 	const [commentOrderBy, setCommentOrderBy] = useState<
 		'newest' | 'oldest' | 'recommendations'
 	>();
-	const [isExpanded, setIsExpanded] = useState<boolean>(false);
+	const [comments, setComments] = useState<CommentType[]>([]);
+	const [isClosedForComments, setIsClosedForComments] = useState(false);
+	const [isExpanded, setIsExpanded] = useState(false);
 	const [hashCommentId, setHashCommentId] = useState<number | undefined>(
 		commentIdFromUrl(),
 	);
 	const [filters, setFilters] = useState<FilterOptions>(
 		initFiltersFromLocalStorage(),
 	);
+	const [loading, setLoading] = useState(true);
+	const [totalPages, setTotalPages] = useState(0);
 
-	const { commentCount, isClosedForComments } = useDiscussion(
-		joinUrl(discussionApiUrl, 'discussion', shortUrlId),
-	);
+	const commentCount = useCommentCount(discussionApiUrl, shortUrlId);
+
+	useEffect(() => {
+		setLoading(true);
+		void getDiscussion(shortUrlId, { ...filters, page: commentPage })
+			.then((json) => {
+				setLoading(false);
+				if (json && json.status !== 'error') {
+					setComments(json.discussion.comments);
+					setIsClosedForComments(json.discussion.isClosedForComments);
+				}
+				if (json?.pages != null) setTotalPages(json.pages);
+			})
+			.catch((e) => console.error(`getDiscussion - error: ${String(e)}`));
+	}, [filters, commentPage, shortUrlId]);
 
 	useEffect(() => {
 		const orderByClosed = isClosedForComments ? 'oldest' : undefined;
@@ -168,6 +186,19 @@ export const Discussion = ({
 		}
 	}, [commentCount]);
 
+	useEffect(() => {
+		if (window.location.hash === '#comments') {
+			setIsExpanded(true);
+		}
+	}, []);
+
+	useEffect(() => {
+		// There's no point showing the view more button if there isn't much more to view
+		if (commentCount === 0 || commentCount === 1 || commentCount === 2) {
+			setIsExpanded(true);
+		}
+	}, [commentCount]);
+
 	return (
 		<>
 			<div css={[positionRelative, !isExpanded && fixHeight]}>
@@ -209,6 +240,11 @@ export const Discussion = ({
 					setPage={setCommentPage}
 					filters={filters}
 					setFilters={setFilters}
+					commentCount={commentCount ?? 0}
+					loading={loading}
+					totalPages={totalPages}
+					comments={comments}
+					setComments={setComments}
 				/>
 				{!isExpanded && (
 					<div id="discussion-overlay" css={overlayStyles} />

dotcom-rendering/src/components/Discussion/Comments.stories.tsx

@@ -2,6 +2,9 @@ import { css } from '@emotion/react';
 import { ArticleDesign, ArticleDisplay, Pillar } from '@guardian/libs';
 import { splitTheme } from '../../../.storybook/decorators/splitThemeDecorator';
 import { lightDecorator } from '../../../.storybook/decorators/themeDecorator';
+import { discussion as discussionMock } from '../../../fixtures/manual/discussion';
+import { discussionWithTwoComments } from '../../../fixtures/manual/discussionWithTwoComments';
+import { legacyDiscussionWithoutThreading } from '../../../fixtures/manual/legacyDiscussionWithoutThreading';
 import type { FilterOptions, SignedInUser } from '../../types/discussion';
 import { Comments } from './Comments';
 
@@ -45,7 +48,7 @@ export const LoggedOutHiddenPicks = () => (
 		`}
 	>
 		<Comments
-			shortUrl="p/39f5z"
+			shortUrl={discussionMock.discussion.key}
 			baseUrl="https://discussion.theguardian.com/discussion-api"
 			isClosedForComments={false}
 			additionalHeaders={{
@@ -61,6 +64,11 @@ export const LoggedOutHiddenPicks = () => (
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={discussionMock.discussion.commentCount}
+			loading={false}
+			totalPages={discussionMock.pages}
+			comments={discussionMock.discussion.comments}
+			setComments={() => {}}
 		/>
 	</div>
 );
@@ -82,7 +90,7 @@ export const InitialPage = () => (
 		`}
 	>
 		<Comments
-			shortUrl="p/39f5z"
+			shortUrl={discussionMock.discussion.key}
 			baseUrl="https://discussion.theguardian.com/discussion-api"
 			isClosedForComments={false}
 			additionalHeaders={{
@@ -94,14 +102,19 @@ export const InitialPage = () => (
 			onExpand={() => {}}
 			apiKey=""
 			idApiUrl="https://idapi.theguardian.com"
-			page={3}
+			page={1}
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={discussionMock.discussion.commentCount}
+			loading={false}
+			totalPages={discussionMock.pages}
+			comments={discussionMock.discussion.comments}
+			setComments={() => {}}
 		/>
 	</div>
 );
-InitialPage.storyName = 'with initial page set to 3';
+InitialPage.storyName = 'with initial page set to 1';
 InitialPage.decorators = [
 	splitTheme([
 		{
@@ -136,6 +149,11 @@ export const LoggedInHiddenNoPicks = () => (
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={discussionMock.discussion.commentCount}
+			loading={false}
+			totalPages={discussionMock.pages}
+			comments={discussionMock.discussion.comments}
+			setComments={() => {}}
 		/>
 	</div>
 );
@@ -168,6 +186,11 @@ export const LoggedIn = () => (
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={discussionMock.discussion.commentCount}
+			loading={false}
+			totalPages={discussionMock.pages}
+			comments={discussionMock.discussion.comments}
+			setComments={() => {}}
 		/>
 	</div>
 );
@@ -182,7 +205,7 @@ export const LoggedInShortDiscussion = () => (
 		`}
 	>
 		<Comments
-			shortUrl="p/39f5a" // Two comments"
+			shortUrl={discussionWithTwoComments.discussion.key} // Two comments"
 			isClosedForComments={false}
 			user={aUser}
 			baseUrl="https://discussion.theguardian.com/discussion-api"
@@ -199,6 +222,11 @@ export const LoggedInShortDiscussion = () => (
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={discussionWithTwoComments.discussion.commentCount}
+			loading={false}
+			totalPages={discussionWithTwoComments.pages}
+			comments={discussionWithTwoComments.discussion.comments}
+			setComments={() => {}}
 		/>
 	</div>
 );
@@ -229,6 +257,11 @@ export const LoggedOutHiddenNoPicks = () => (
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={discussionMock.discussion.commentCount}
+			loading={false}
+			totalPages={0}
+			comments={discussionMock.discussion.comments}
+			setComments={() => {}}
 		/>
 	</div>
 );
@@ -251,7 +284,7 @@ export const Closed = () => (
 		`}
 	>
 		<Comments
-			shortUrl="p/39f5z"
+			shortUrl={discussionMock.discussion.key}
 			baseUrl="https://discussion.theguardian.com/discussion-api"
 			isClosedForComments={true}
 			user={aUser}
@@ -268,6 +301,11 @@ export const Closed = () => (
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={discussionMock.discussion.commentCount}
+			loading={false}
+			totalPages={discussionMock.pages}
+			comments={discussionMock.discussion.comments}
+			setComments={() => {}}
 		/>
 	</div>
 );
@@ -305,6 +343,11 @@ export const NoComments = () => (
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={0}
+			loading={false}
+			totalPages={0}
+			comments={[]}
+			setComments={() => {}}
 		/>
 	</div>
 );
@@ -326,7 +369,7 @@ export const LegacyDiscussion = () => (
 		`}
 	>
 		<Comments
-			shortUrl="p/32255" // A 'legacy' discussion that doesn't allow threading
+			shortUrl={legacyDiscussionWithoutThreading.discussion.key} // A 'legacy' discussion that doesn't allow threading
 			baseUrl="https://discussion.theguardian.com/discussion-api"
 			isClosedForComments={false}
 			additionalHeaders={{
@@ -342,6 +385,13 @@ export const LegacyDiscussion = () => (
 			setPage={() => {}}
 			filters={filters}
 			setFilters={() => {}}
+			commentCount={
+				legacyDiscussionWithoutThreading.discussion.commentCount
+			}
+			loading={false}
+			totalPages={legacyDiscussionWithoutThreading.pages}
+			comments={legacyDiscussionWithoutThreading.discussion.comments}
+			setComments={() => {}}
 		/>
 	</div>
 );

dotcom-rendering/src/components/Discussion/Comments.tsx

@@ -6,11 +6,7 @@ import {
 	textSans,
 } from '@guardian/source-foundations';
 import { useEffect, useState } from 'react';
-import {
-	getDiscussion,
-	getPicks,
-	initialiseApi,
-} from '../../lib/discussionApi';
+import { getPicks, initialiseApi } from '../../lib/discussionApi';
 import type {
 	AdditionalHeadersType,
 	CommentResponse,
@@ -49,6 +45,11 @@ type Props = {
 	setPage: (page: number) => void;
 	filters: FilterOptions;
 	setFilters: (filters: FilterOptions) => void;
+	commentCount: number;
+	loading: boolean;
+	totalPages: number;
+	comments: CommentType[];
+	setComments: (comments: CommentType[]) => void;
 };
 
 const footerStyles = css`
@@ -121,16 +122,16 @@ export const Comments = ({
 	setPage,
 	filters,
 	setFilters,
+	commentCount,
+	loading,
+	totalPages,
+	comments,
+	setComments,
 }: Props) => {
-	const [loading, setLoading] = useState<boolean>(true);
-	const [totalPages, setTotalPages] = useState<number>(0);
 	const [picks, setPicks] = useState<CommentType[]>([]);
 	const [commentBeingRepliedTo, setCommentBeingRepliedTo] =
 		useState<CommentType>();
-	const [comments, setComments] = useState<CommentType[]>([]);
-	const [numberOfCommentsToShow, setNumberOfCommentsToShow] =
-		useState<number>(10);
-	const [commentCount, setCommentCount] = useState<number>(0);
+	const [numberOfCommentsToShow, setNumberOfCommentsToShow] = useState(10);
 	const [mutes, setMutes] = useState<string[]>(readMutes());
 
 	const loadingMore = !loading && comments.length !== numberOfCommentsToShow;
@@ -149,20 +150,6 @@ export const Comments = ({
 		} else return;
 	}, [expanded, comments.length]);
 
-	useEffect(() => {
-		setLoading(true);
-		//todo: come back and handle the error case
-		void getDiscussion(shortUrl, { ...filters, page }).then((json) => {
-			setLoading(false);
-			if (json && json.status !== 'error') {
-				setComments(json.discussion.comments);
-				setCommentCount(json.discussion.topLevelCommentCount);
-			}
-			//todo: come back and parse this properly (apologies for the horribleness)
-			setTotalPages(json?.pages as number);
-		});
-	}, [filters, page, shortUrl]);
-
 	//todo: parse this properly
 	useEffect(() => {
 		const fetchPicks = async () => {

dotcom-rendering/src/components/Discussion/Comments.test.tsx → dotcom-rendering/src/components/Discussion/Discussion.test.tsx

@@ -5,36 +5,21 @@ import {
 	waitForElementToBeRemoved,
 } from '@testing-library/react';
 import { mockRESTCalls } from '../../lib/mockRESTCalls';
-import { Comments } from './Comments';
+import { Discussion } from '../Discussion';
 
 mockRESTCalls();
 
 describe('App', () => {
 	it('should not render the comment form if user is logged out', async () => {
 		render(
-			<Comments
-				shortUrl="p/39f5z"
-				baseUrl="https://discussion.theguardian.com/discussion-api"
-				isClosedForComments={false}
-				additionalHeaders={{
-					'D2-X-UID': 'testD2Header',
-					'GU-Client': 'testClientHeader',
-				}}
-				expanded={false}
-				onPermalinkClick={() => {}}
-				onExpand={() => {
-					// do nothing
-				}}
-				apiKey=""
+			<Discussion
+				user={undefined}
+				discussionApiUrl="https://discussion.theguardian.com/discussion-api"
+				shortUrlId="p/39f5z"
+				discussionD2Uid="testD2Header"
+				discussionApiClientHeader="testClientHeader"
+				enableDiscussionSwitch={true}
 				idApiUrl="https://idapi.theguardian.com"
-				page={3}
-				setPage={() => {}}
-				filters={{
-					threads: 'collapsed',
-					pageSize: 25,
-					orderBy: 'newest',
-				}}
-				setFilters={() => {}}
 			/>,
 		);