Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Pagination thread count in Discussion #10477

Merged
merged 5 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import type { GetDiscussionSuccess } from '../../src/types/discussion';
export const discussionWithTwoComments = {
status: 'ok',
currentPage: 1,
pages: 0,
pages: 1,
pageSize: 100,
orderBy: 'oldest',
discussion: {
key: '/p/39f5x',
webUrl: 'https://www.theguardian.com/science/grrlscientist/2012/aug/07/3',
apiUrl: 'https://discussion.guardianapis.com/discussion-api/discussion//p/39f5x',
commentCount: 0,
topLevelCommentCount: 0,
commentCount: 2,
topLevelCommentCount: 2,
isClosedForComments: false,
isClosedForRecommendation: false,
isThreaded: true,
Expand Down
4 changes: 2 additions & 2 deletions dotcom-rendering/fixtures/manual/short-discussion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@ export const shortDiscussion = {
orderBy: 'oldest',
discussion: {
apiUrl: 'https://discussion.guardianapis.com/discussion-api/discussion//p/4v8kk',
commentCount: 7,
commentCount: 2,
isClosedForComments: true,
isClosedForRecommendation: true,
isThreaded: true,
key: '/p/4v8kk',
title: 'Stevie Nicks to release double album of songs from her past',
topLevelCommentCount: 6,
topLevelCommentCount: 2,
webUrl: 'https://www.theguardian.com/music/2014/jul/25/stevie-nicks-ro-release-double-album-of-songs-from-her-past',
comments: [
{
Expand Down
27 changes: 15 additions & 12 deletions dotcom-rendering/src/components/Discussion.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
getCommentContext,
initFiltersFromLocalStorage,
} from '../lib/getCommentContext';
import { useCommentCount } from '../lib/useCommentCount';
import { palette as themePalette } from '../palette';
import type {
CommentForm,
Expand Down Expand Up @@ -96,9 +95,10 @@ type State = {
isClosedForComments: boolean;
isExpanded: boolean;
commentPage: number;
commentCount: number | undefined;
topLevelCommentCount: number;
filters: FilterOptions;
hashCommentId: number | undefined;
totalPages: number;
loading: boolean;
topForm: CommentForm;
replyForm: CommentForm;
Expand All @@ -115,9 +115,10 @@ const initialState: State = {
isClosedForComments: false,
isExpanded: false,
commentPage: 1,
commentCount: undefined,
topLevelCommentCount: 0,
filters: initFiltersFromLocalStorage(),
hashCommentId: undefined,
totalPages: 0,
loading: true,
topForm: initialCommentFormState,
replyForm: initialCommentFormState,
Expand All @@ -129,7 +130,8 @@ type Action =
type: 'commentsLoaded';
comments: CommentType[];
isClosedForComments: boolean;
totalPages: number;
commentCount: number;
topLevelCommentCount: number;
}
| { type: 'expandComments' }
| { type: 'addComment'; comment: CommentType }
Expand All @@ -151,7 +153,8 @@ const reducer = (state: State, action: Action): State => {
...state,
comments: action.comments,
isClosedForComments: action.isClosedForComments,
totalPages: action.totalPages,
commentCount: action.commentCount,
topLevelCommentCount: action.topLevelCommentCount,
loading: false,
};
case 'addComment':
Expand Down Expand Up @@ -261,17 +264,16 @@ export const Discussion = ({
commentPage,
filters,
hashCommentId,
totalPages,
loading,
topForm,
replyForm,
bottomForm,
topLevelCommentCount,
commentCount,
},
dispatch,
] = useReducer(reducer, initialState);

const commentCount = useCommentCount(discussionApiUrl, shortUrlId);

useEffect(() => {
const newHashCommentId = commentIdFromUrl();
if (newHashCommentId !== undefined) {
Expand All @@ -291,13 +293,14 @@ export const Discussion = ({
return;
}

const { pages, discussion } = result.value;
const { discussion } = result.value;

dispatch({
type: 'commentsLoaded',
comments: discussion.comments,
isClosedForComments: discussion.isClosedForComments,
totalPages: pages,
topLevelCommentCount: discussion.topLevelCommentCount,
commentCount: discussion.commentCount,
});
})
.catch(() => {
Expand Down Expand Up @@ -351,6 +354,7 @@ export const Discussion = ({
}, []);

useEffect(() => {
console.log({ commentCount });
// There's no point showing the view more button if there isn't much more to view
if (commentCount === 0 || commentCount === 1 || commentCount === 2) {
dispatch({ type: 'expandComments' });
Expand Down Expand Up @@ -400,9 +404,8 @@ export const Discussion = ({
});
}}
filters={validFilters}
commentCount={commentCount ?? 0}
topLevelCommentCount={topLevelCommentCount}
loading={loading}
totalPages={totalPages}
comments={comments}
setComment={(comment: CommentType) => {
dispatch({ type: 'addComment', comment });
Expand Down
43 changes: 24 additions & 19 deletions dotcom-rendering/src/components/Discussion/Comments.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@ export const LoggedOutHiddenPicks = () => (
page={3}
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -123,9 +124,10 @@ export const InitialPage = () => (
page={1}
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -177,9 +179,10 @@ export const LoggedInHiddenNoPicks = () => {
page={3}
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -227,9 +230,10 @@ export const LoggedIn = () => {
page={3}
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -282,9 +286,10 @@ export const LoggedInShortDiscussion = () => {
page={3}
setPage={() => {}}
filters={filters}
commentCount={discussionWithTwoComments.discussion.commentCount}
topLevelCommentCount={
discussionWithTwoComments.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionWithTwoComments.pages}
comments={discussionWithTwoComments.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -328,9 +333,10 @@ export const LoggedOutHiddenNoPicks = () => (
page={3}
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={0}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -380,9 +386,10 @@ export const Closed = () => (
page={3}
setPage={() => {}}
filters={filters}
commentCount={discussionMock.discussion.commentCount}
topLevelCommentCount={
discussionMock.discussion.topLevelCommentCount
}
loading={false}
totalPages={discussionMock.pages}
comments={discussionMock.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -430,9 +437,8 @@ export const NoComments = () => (
page={3}
setPage={() => {}}
filters={filters}
commentCount={0}
topLevelCommentCount={0}
loading={false}
totalPages={0}
comments={[]}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down Expand Up @@ -477,14 +483,13 @@ export const LegacyDiscussion = () => (
onPermalinkClick={() => {}}
apiKey=""
idApiUrl="https://idapi.theguardian.com"
page={3}
page={2}
setPage={() => {}}
filters={filters}
commentCount={
topLevelCommentCount={
legacyDiscussionWithoutThreading.discussion.commentCount
}
loading={false}
totalPages={legacyDiscussionWithoutThreading.pages}
comments={legacyDiscussionWithoutThreading.discussion.comments}
setComment={() => {}}
handleFilterChange={() => {}}
Expand Down
23 changes: 9 additions & 14 deletions dotcom-rendering/src/components/Discussion/Comments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ type Props = {
page: number;
setPage: (page: number, shouldExpand: boolean) => void;
filters: FilterOptions;
commentCount: number;
topLevelCommentCount: number;
loading: boolean;
totalPages: number;
comments: CommentType[];
setComment: (comment: CommentType) => void;
handleFilterChange: (newFilters: FilterOptions, page?: number) => void;
Expand Down Expand Up @@ -121,9 +120,8 @@ export const Comments = ({
page,
setPage,
filters,
commentCount,
topLevelCommentCount,
loading,
totalPages,
comments,
setComment,
handleFilterChange,
Expand Down Expand Up @@ -197,7 +195,7 @@ export const Comments = ({
*/

const maxPagePossible = Math.ceil(
commentCount / newFilterObject.pageSize,
topLevelCommentCount / newFilterObject.pageSize,
);

if (page > maxPagePossible) {
Expand Down Expand Up @@ -236,7 +234,7 @@ export const Comments = ({

initialiseApi({ additionalHeaders, baseUrl, apiKey, idApiUrl });

const showPagination = totalPages > 1;
const showPagination = topLevelCommentCount > filters.pageSize;

if (!expanded && loading) {
return <span data-testid="loading-comments"></span>;
Expand All @@ -258,14 +256,13 @@ export const Comments = ({
<Filters
filters={filters}
onFilterChange={onFilterChange}
commentCount={commentCount}
topLevelCommentCount={topLevelCommentCount}
/>
{showPagination && (
<Pagination
totalPages={totalPages}
currentPage={page}
setCurrentPage={onPageChange}
commentCount={commentCount}
topLevelCommentCount={topLevelCommentCount}
filters={filters}
/>
)}
Expand Down Expand Up @@ -351,14 +348,13 @@ export const Comments = ({
<Filters
filters={filters}
onFilterChange={onFilterChange}
commentCount={commentCount}
topLevelCommentCount={topLevelCommentCount}
/>
{showPagination && (
<Pagination
totalPages={totalPages}
currentPage={page}
setCurrentPage={onPageChange}
commentCount={commentCount}
topLevelCommentCount={topLevelCommentCount}
filters={filters}
/>
)}
Expand Down Expand Up @@ -407,10 +403,9 @@ export const Comments = ({
{showPagination && (
<footer css={footerStyles}>
<Pagination
totalPages={totalPages}
currentPage={page}
setCurrentPage={onPageChange}
commentCount={commentCount}
topLevelCommentCount={topLevelCommentCount}
filters={filters}
/>
</footer>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const Default = () => {
<Filters
filters={filters}
onFilterChange={setFilters}
commentCount={74}
topLevelCommentCount={74}
/>
);
};
Expand Down
Loading
Loading