From 96e7fda26aa2a7c92006d7f5c32277b335452643 Mon Sep 17 00:00:00 2001 From: Rafael Date: Wed, 2 Mar 2022 09:10:52 -0300 Subject: [PATCH] Batch-resolve channels on doCommentList fetch --- flow-typed/Comment.js | 2 +- ui/component/commentsList/index.js | 22 +------- ui/component/commentsList/view.jsx | 41 +++----------- ui/component/commentsReplies/index.js | 22 ++------ ui/component/commentsReplies/view.jsx | 71 +++++++---------------- ui/redux/actions/comments.js | 81 ++++++++++++--------------- ui/redux/selectors/comments.js | 4 +- 7 files changed, 79 insertions(+), 164 deletions(-) diff --git a/flow-typed/Comment.js b/flow-typed/Comment.js index b88277968..adbdccd28 100644 --- a/flow-typed/Comment.js +++ b/flow-typed/Comment.js @@ -148,7 +148,7 @@ declare type CommentListParams = { channel_name?: string, // signing channel name of claim (enables 'commentsEnabled' check) channel_id?: string, // signing channel claim id of claim (enables 'commentsEnabled' check) author_claim_id?: string, // filters comments to just this author - parent_id?: string, // filters comments to those under this thread + parent_id?: ?string, // filters comments to those under this thread top_level?: boolean, // filters to only top level comments hidden?: boolean, // if true, will show hidden comments as well sort_by?: number, // @see: ui/constants/comments.js::SORT_BY diff --git a/ui/component/commentsList/index.js b/ui/component/commentsList/index.js index f8755219d..e378556c0 100644 --- a/ui/component/commentsList/index.js +++ b/ui/component/commentsList/index.js @@ -1,11 +1,5 @@ import { connect } from 'react-redux'; -import { doResolveUris } from 'redux/actions/claims'; -import { - selectClaimForUri, - makeSelectClaimForUri, - selectClaimIsMine, - selectFetchingMyChannels, -} from 'redux/selectors/claims'; +import { selectClaimForUri, selectClaimIsMine, selectFetchingMyChannels } from 'redux/selectors/claims'; import { selectTopLevelCommentsForUri, makeSelectTopLevelTotalPagesForUri, @@ -28,25 +22,16 @@ const select = (state, props) => { const { uri } = props; const claim = selectClaimForUri(state, uri); - const channelId = getChannelIdFromClaim(claim); - const activeChannelClaim = selectActiveChannelClaim(state); - const topLevelComments = selectTopLevelCommentsForUri(state, uri); - - const resolvedComments = - topLevelComments && topLevelComments.length > 0 - ? topLevelComments.filter(({ channel_url }) => makeSelectClaimForUri(channel_url)(state) !== undefined) - : []; return { - topLevelComments, - resolvedComments, + topLevelComments: selectTopLevelCommentsForUri(state, uri), allCommentIds: selectCommentIdsForUri(state, uri), pinnedComments: selectPinnedCommentsForUri(state, uri), topLevelTotalPages: makeSelectTopLevelTotalPagesForUri(uri)(state), totalComments: makeSelectTotalCommentsCountForUri(uri)(state), claimId: claim && claim.claim_id, - channelId, + channelId: getChannelIdFromClaim(claim), claimIsMine: selectClaimIsMine(state, claim), isFetchingComments: selectIsFetchingComments(state), isFetchingCommentsById: selectIsFetchingCommentsById(state), @@ -64,7 +49,6 @@ const perform = { fetchComment: doCommentById, fetchReacts: doCommentReactList, resetComments: doCommentReset, - doResolveUris, }; export default connect(select, perform)(CommentsList); diff --git a/ui/component/commentsList/view.jsx b/ui/component/commentsList/view.jsx index 6f9a6eed8..58e841407 100644 --- a/ui/component/commentsList/view.jsx +++ b/ui/component/commentsList/view.jsx @@ -30,7 +30,6 @@ type Props = { allCommentIds: any, pinnedComments: Array, topLevelComments: Array, - resolvedComments: Array, topLevelTotalPages: number, uri: string, claimId?: string, @@ -47,11 +46,10 @@ type Props = { activeChannelId: ?string, settingsByChannelId: { [channelId: string]: PerChannelSettings }, commentsAreExpanded?: boolean, - fetchTopLevelComments: (uri: string, parentId: string, page: number, pageSize: number, sortBy: number) => void, + fetchTopLevelComments: (uri: string, parentId: ?string, page: number, pageSize: number, sortBy: number) => void, fetchComment: (commentId: string) => void, fetchReacts: (commentIds: Array) => Promise, resetComments: (claimId: string) => void, - doResolveUris: (uris: Array, returnCachedClaims: boolean) => void, }; export default function CommentList(props: Props) { @@ -60,7 +58,6 @@ export default function CommentList(props: Props) { uri, pinnedComments, topLevelComments, - resolvedComments, topLevelTotalPages, claimId, channelId, @@ -79,7 +76,6 @@ export default function CommentList(props: Props) { fetchComment, fetchReacts, resetComments, - doResolveUris, } = props; const isMobile = useIsMobile(); @@ -89,7 +85,6 @@ export default function CommentList(props: Props) { const DEFAULT_SORT = ENABLE_COMMENT_REACTIONS ? SORT_BY.POPULARITY : SORT_BY.NEWEST; const [sort, setSort] = usePersistedState('comment-sort-by', DEFAULT_SORT); const [page, setPage] = React.useState(0); - const [commentsToDisplay, setCommentsToDisplay] = React.useState(topLevelComments); const [didInitialPageFetch, setInitialPageFetch] = React.useState(false); const hasDefaultExpansion = commentsAreExpanded || !isMediumScreen || isMobile; const [expandedComments, setExpandedComments] = React.useState(hasDefaultExpansion); @@ -97,9 +92,6 @@ export default function CommentList(props: Props) { const totalFetchedComments = allCommentIds ? allCommentIds.length : 0; const channelSettings = channelId ? settingsByChannelId[channelId] : undefined; const moreBelow = page < topLevelTotalPages; - const isResolvingComments = topLevelComments && resolvedComments.length !== topLevelComments.length; - const alreadyResolved = !isResolvingComments && resolvedComments.length !== 0; - const canDisplayComments = commentsToDisplay && commentsToDisplay.length === topLevelComments.length; const title = getCommentsListTitle(totalComments); // Display comments immediately if not fetching reactions @@ -123,8 +115,7 @@ export default function CommentList(props: Props) { } setPage(1); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [page, uri, resetComments]); // 'claim' is derived from 'uri' + }, [page, claimId, resetComments]); // Fetch top-level comments useEffect(() => { @@ -133,7 +124,7 @@ export default function CommentList(props: Props) { fetchComment(linkedCommentId); } - fetchTopLevelComments(uri, '', page, COMMENT_PAGE_SIZE_TOP_LEVEL, sort); + fetchTopLevelComments(uri, undefined, page, COMMENT_PAGE_SIZE_TOP_LEVEL, sort); } }, [fetchComment, fetchTopLevelComments, linkedCommentId, page, sort, uri]); @@ -182,6 +173,8 @@ export default function CommentList(props: Props) { // Infinite scroll useEffect(() => { + if (topLevelComments.length === 0) return; + function shouldFetchNextPage(page, topLevelTotalPages, yPrefetchPx = 1000) { if (!spinnerRef || !spinnerRef.current) return false; @@ -216,7 +209,7 @@ export default function CommentList(props: Props) { setInitialPageFetch(true); } - if (hasDefaultExpansion && !isFetchingComments && canDisplayComments && readyToDisplayComments && moreBelow) { + if (hasDefaultExpansion && !isFetchingComments && readyToDisplayComments && moreBelow) { const commentsInDrawer = Boolean(document.querySelector('.MuiDrawer-root .card--enable-overflow')); const scrollingElement = commentsInDrawer ? document.querySelector('.card--enable-overflow') : window; @@ -227,7 +220,7 @@ export default function CommentList(props: Props) { } } }, [ - canDisplayComments, + topLevelComments, hasDefaultExpansion, didInitialPageFetch, isFetchingComments, @@ -238,22 +231,6 @@ export default function CommentList(props: Props) { topLevelTotalPages, ]); - // Wait to only display topLevelComments after resolved or else - // other components will try to resolve again, like channelThumbnail - useEffect(() => { - if (!isResolvingComments) setCommentsToDisplay(topLevelComments); - }, [isResolvingComments, topLevelComments]); - - // Batch resolve comment channel urls - useEffect(() => { - if (!topLevelComments || alreadyResolved) return; - - const urisToResolve = []; - topLevelComments.map(({ channel_url }) => channel_url !== undefined && urisToResolve.push(channel_url)); - - if (urisToResolve.length > 0) doResolveUris(urisToResolve, true); - }, [alreadyResolved, doResolveUris, topLevelComments]); - const commentProps = { isTopLevel: true, threadDepth: 3, uri, claimIsMine, linkedCommentId }; const actionButtonsProps = { totalComments, sort, changeSort, setPage }; @@ -282,7 +259,7 @@ export default function CommentList(props: Props) { <> {pinnedComments && } - {commentsToDisplay && } + )} @@ -308,7 +285,7 @@ export default function CommentList(props: Props) { )} - {(isFetchingComments || (hasDefaultExpansion && moreBelow) || !canDisplayComments) && ( + {(isFetchingComments || (hasDefaultExpansion && moreBelow)) && (
diff --git a/ui/component/commentsReplies/index.js b/ui/component/commentsReplies/index.js index b287420dd..4a19cafe2 100644 --- a/ui/component/commentsReplies/index.js +++ b/ui/component/commentsReplies/index.js @@ -1,26 +1,16 @@ import { connect } from 'react-redux'; -import { doResolveUris } from 'redux/actions/claims'; -import { selectClaimIsMineForUri, makeSelectClaimForUri } from 'redux/selectors/claims'; +import { selectClaimIsMineForUri } from 'redux/selectors/claims'; import { selectIsFetchingCommentsByParentId, selectRepliesForParentId } from 'redux/selectors/comments'; -import { selectUserVerifiedEmail } from 'redux/selectors/user'; import CommentsReplies from './view'; const select = (state, props) => { - const fetchedReplies = selectRepliesForParentId(state, props.parentId); - const resolvedReplies = - fetchedReplies && fetchedReplies.length > 0 - ? fetchedReplies.filter(({ channel_url }) => makeSelectClaimForUri(channel_url)(state) !== undefined) - : []; + const { uri, parentId } = props; return { - fetchedReplies, - resolvedReplies, - claimIsMine: selectClaimIsMineForUri(state, props.uri), - userCanComment: IS_WEB ? Boolean(selectUserVerifiedEmail(state)) : true, - isFetchingByParentId: selectIsFetchingCommentsByParentId(state), + fetchedReplies: selectRepliesForParentId(state, parentId), + claimIsMine: selectClaimIsMineForUri(state, uri), + isFetching: selectIsFetchingCommentsByParentId(state, parentId), }; }; -const perform = (dispatch) => ({ doResolveUris: (uris) => dispatch(doResolveUris(uris, true)) }); - -export default connect(select, perform)(CommentsReplies); +export default connect(select)(CommentsReplies); diff --git a/ui/component/commentsReplies/view.jsx b/ui/component/commentsReplies/view.jsx index d1e0e0923..20808310c 100644 --- a/ui/component/commentsReplies/view.jsx +++ b/ui/component/commentsReplies/view.jsx @@ -6,61 +6,34 @@ import React from 'react'; import Spinner from 'component/spinner'; type Props = { - fetchedReplies: Array, - resolvedReplies: Array, uri: string, - parentId: string, - claimIsMine: boolean, linkedCommentId?: string, - userCanComment: boolean, threadDepth: number, numDirectReplies: number, // Total replies for parentId as reported by 'comment[replies]'. Includes blocked items. - isFetchingByParentId: { [string]: boolean }, hasMore: boolean, supportDisabled: boolean, - doResolveUris: (Array) => void, onShowMore?: () => void, + // redux + fetchedReplies: Array, + claimIsMine: boolean, + isFetching: boolean, }; -function CommentsReplies(props: Props) { +export default function CommentsReplies(props: Props) { const { uri, - parentId, fetchedReplies, - resolvedReplies, claimIsMine, linkedCommentId, - userCanComment, threadDepth, numDirectReplies, - isFetchingByParentId, + isFetching, hasMore, supportDisabled, - doResolveUris, onShowMore, } = props; const [isExpanded, setExpanded] = React.useState(true); - const [commentsToDisplay, setCommentsToDisplay] = React.useState(fetchedReplies); - const isResolvingReplies = fetchedReplies && resolvedReplies.length !== fetchedReplies.length; - const alreadyResolved = !isResolvingReplies && resolvedReplies.length !== 0; - const canDisplayComments = commentsToDisplay && commentsToDisplay.length === fetchedReplies.length; - - // Batch resolve comment channel urls - React.useEffect(() => { - if (!fetchedReplies || alreadyResolved) return; - - const urisToResolve = []; - fetchedReplies.map(({ channel_url }) => channel_url !== undefined && urisToResolve.push(channel_url)); - - if (urisToResolve.length > 0) doResolveUris(urisToResolve); - }, [alreadyResolved, doResolveUris, fetchedReplies]); - - // Wait to only display topLevelComments after resolved or else - // other components will try to resolve again, like channelThumbnail - React.useEffect(() => { - if (!isResolvingReplies) setCommentsToDisplay(fetchedReplies); - }, [isResolvingReplies, fetchedReplies]); return !numDirectReplies ? null : (
@@ -78,24 +51,21 @@ function CommentsReplies(props: Props) {
)} + {isExpanded && fetchedReplies && hasMore && (
)} - {(isFetchingByParentId[parentId] || isResolvingReplies || !canDisplayComments) && ( + + {isFetching && (
@@ -116,5 +87,3 @@ function CommentsReplies(props: Props) {
); } - -export default CommentsReplies; diff --git a/ui/redux/actions/comments.js b/ui/redux/actions/comments.js index 44c2584fe..f17f32b2c 100644 --- a/ui/redux/actions/comments.js +++ b/ui/redux/actions/comments.js @@ -19,6 +19,7 @@ import { import { makeSelectNotificationForCommentId } from 'redux/selectors/notifications'; import { selectActiveChannelClaim } from 'redux/selectors/app'; import { toHex } from 'util/hex'; +import { getChannelFromClaim } from 'util/claim'; import Comments from 'comments'; import { selectPrefsReady } from 'redux/selectors/sync'; import { doAlertWaitingForSync } from 'redux/actions/app'; @@ -30,47 +31,41 @@ const MENTION_REGEX = /(?:^| |\n)@[^\s=&#$@%?:;/"<>%{}|^~[]*(?::[\w]+)?/gm; export function doCommentList( uri: string, - parentId: string, + parentId: ?string, page: number = 1, pageSize: number = 99999, sortBy: number = SORT_BY.NEWEST ) { return (dispatch: Dispatch, getState: GetState) => { const state = getState(); - const claim = selectClaimsByUri(state)[uri]; - const claimId = claim ? claim.claim_id : null; + const claim = selectClaimForUri(state, uri); + const { claim_id: claimId } = claim || {}; if (!claimId) { - dispatch({ - type: ACTIONS.COMMENT_LIST_FAILED, - data: 'unable to find claim for uri', - }); - return; + return dispatch({ type: ACTIONS.COMMENT_LIST_FAILED, data: 'unable to find claim for uri' }); } - dispatch({ - type: ACTIONS.COMMENT_LIST_STARTED, - data: { - parentId, - }, - }); + dispatch({ type: ACTIONS.COMMENT_LIST_STARTED, data: { parentId } }); // Adding 'channel_id' and 'channel_name' enables "CreatorSettings > commentsEnabled". - const creatorChannelClaim = claim.value_type === 'channel' ? claim : claim.signing_channel; + const creatorChannelClaim = getChannelFromClaim(claim); + const { claim_id: creatorClaimId, name: channelName } = creatorChannelClaim || {}; return Comments.comment_list({ page, claim_id: claimId, page_size: pageSize, - parent_id: parentId || undefined, + parent_id: parentId, top_level: !parentId, - channel_id: creatorChannelClaim ? creatorChannelClaim.claim_id : undefined, - channel_name: creatorChannelClaim ? creatorChannelClaim.name : undefined, + channel_id: creatorClaimId, + channel_name: channelName, sort_by: sortBy, }) .then((result: CommentListResponse) => { const { items: comments, total_items, total_filtered_items, total_pages } = result; - dispatch({ + + const commentChannelUrls = comments && comments.map((comment) => comment.channel_url || ''); + const dispatchData = { type: ACTIONS.COMMENT_LIST_COMPLETED, data: { comments, @@ -78,38 +73,36 @@ export function doCommentList( totalItems: total_items, totalFilteredItems: total_filtered_items, totalPages: total_pages, - claimId: claimId, - creatorClaimId: creatorChannelClaim ? creatorChannelClaim.claim_id : undefined, - uri: uri, + claimId, + creatorClaimId, + uri, }, - }); + }; - return result; + // Batch resolve comment channel urls + if (commentChannelUrls) { + return dispatch(async () => await doResolveUris(commentChannelUrls, true)).then(() => { + dispatch({ ...dispatchData }); + + return result; + }); + } else { + dispatch({ ...dispatchData }); + + return result; + } }) .catch((error) => { - switch (error.message) { + const { message } = error; + + switch (message) { case 'comments are disabled by the creator': - dispatch({ - type: ACTIONS.COMMENT_LIST_COMPLETED, - data: { - creatorClaimId: creatorChannelClaim ? creatorChannelClaim.claim_id : undefined, - disabled: true, - }, - }); - break; - + return dispatch({ type: ACTIONS.COMMENT_LIST_COMPLETED, data: { creatorClaimId, disabled: true } }); case FETCH_API_FAILED_TO_FETCH: - dispatch( - doToast({ - isError: true, - message: __('Failed to fetch comments.'), - }) - ); - dispatch({ type: ACTIONS.COMMENT_LIST_FAILED, data: error }); - break; - + dispatch(doToast({ isError: true, message: __('Failed to fetch comments.') })); + return dispatch({ type: ACTIONS.COMMENT_LIST_FAILED, data: error }); default: - dispatch(doToast({ isError: true, message: `${error.message}` })); + dispatch(doToast({ isError: true, message: `${message}` })); dispatch({ type: ACTIONS.COMMENT_LIST_FAILED, data: error }); } }); diff --git a/ui/redux/selectors/comments.js b/ui/redux/selectors/comments.js index c54f03b63..d36c54d8e 100644 --- a/ui/redux/selectors/comments.js +++ b/ui/redux/selectors/comments.js @@ -24,7 +24,6 @@ export const selectCommentsById = (state: State) => selectState(state).commentBy export const selectCommentIdsByClaimId = (state: State) => selectState(state).byId; export const selectIsFetchingComments = (state: State) => selectState(state).isLoading; export const selectIsFetchingCommentsById = (state: State) => selectState(state).isLoadingById; -export const selectIsFetchingCommentsByParentId = (state: State) => selectState(state).isLoadingByParentId; export const selectIsFetchingReacts = (state: State) => selectState(state).isFetchingReacts; export const selectMyReacts = (state: State) => state.comments.myReactsByCommentId; @@ -32,6 +31,9 @@ export const selectMyReactsForComment = (state: State, commentIdChannelId: strin // @commentIdChannelId: Format = 'commentId:MyChannelId' return state.comments.myReactsByCommentId && state.comments.myReactsByCommentId[commentIdChannelId]; }; +export const selectIsFetchingCommentsByParentId = (state: State, parentId: string) => { + return selectState(state).isLoadingByParentId[parentId]; +}; export const selectOthersReacts = (state: State) => state.comments.othersReactsByCommentId; export const selectOthersReactsForComment = (state: State, id: string) => {