From e899a5de650703509a91b6da3950e1c18bf678c5 Mon Sep 17 00:00:00 2001 From: infinite-persistence Date: Wed, 4 Aug 2021 21:40:16 +0800 Subject: [PATCH 1/2] Comments: include Commentron blocklists when filtering. ## Issue When you block Channel-X, Channel-X's comments will still be visible on someone else's content. This feels odd. ## Change In addition to the blacklist, filter-list and muted-list, we now include the Commentron blocklists (personal, admin, moderator) when filtering out comments. ## Specifics `makeSelectCommentsForUri`, `makeSelectTopLevelCommentsForUri` and `makeSelectRepliesForParentId` all perform the same filtering code. Factor that out to `makeSelectFilteredComments` and add the Commentron lists into the mix. ## Downsides This probably adds to the already-high CPU usage in rendering comments. --- ui/redux/selectors/comments.js | 199 ++++++++++++--------------------- 1 file changed, 74 insertions(+), 125 deletions(-) diff --git a/ui/redux/selectors/comments.js b/ui/redux/selectors/comments.js index e1f53738c..1fe9be599 100644 --- a/ui/redux/selectors/comments.js +++ b/ui/redux/selectors/comments.js @@ -204,18 +204,82 @@ export const selectFetchingBlockedWords = createSelector(selectState, (state) => export const makeSelectCommentsForUri = (uri: string) => createSelector( + (state) => state, selectCommentsByClaimId, selectCommentsByUri, + (state, byClaimId, byUri) => { + const claimId = byUri[uri]; + const comments = byClaimId && byClaimId[claimId]; + return makeSelectFilteredComments(comments)(state); + } + ); + +export const makeSelectTopLevelCommentsForUri = (uri: string) => + createSelector( + (state) => state, + selectTopLevelCommentsByClaimId, + selectCommentsByUri, + (state, byClaimId, byUri) => { + const claimId = byUri[uri]; + const comments = byClaimId && byClaimId[claimId]; + return makeSelectFilteredComments(comments)(state); + } + ); + +export const makeSelectTopLevelTotalCommentsForUri = (uri: string) => + createSelector(selectState, selectCommentsByUri, (state, byUri) => { + const claimId = byUri[uri]; + return state.topLevelTotalCommentsById[claimId] || 0; + }); + +export const makeSelectTopLevelTotalPagesForUri = (uri: string) => + createSelector(selectState, selectCommentsByUri, (state, byUri) => { + const claimId = byUri[uri]; + return state.topLevelTotalPagesById[claimId] || 0; + }); + +export const makeSelectRepliesForParentId = (id: string) => + createSelector( + (state) => state, + selectCommentsById, + (state, commentsById) => { + // const claimId = byUri[uri]; // just parentId (id) + const replyIdsByParentId = state.comments.repliesByParentId; + const replyIdsForParent = replyIdsByParentId[id] || []; + if (!replyIdsForParent.length) return null; + + const comments = []; + replyIdsForParent.forEach((cid) => { + comments.push(commentsById[cid]); + }); + // const comments = byParentId && byParentId[id]; + + return makeSelectFilteredComments(comments)(state); + } + ); + +const makeSelectFilteredComments = (comments: Array) => + createSelector( selectClaimsById, selectMyActiveClaims, selectMutedChannels, + selectModerationBlockList, + selectAdminBlockList, + selectModeratorBlockList, selectBlacklistedOutpointMap, selectFilteredOutpointMap, selectShowMatureContent, - (byClaimId, byUri, claimsById, myClaims, blockedChannels, blacklistedMap, filteredMap, showMatureContent) => { - const claimId = byUri[uri]; - const comments = byClaimId && byClaimId[claimId]; - + ( + claimsById, + myClaims, + mutedChannels, + personalBlockList, + adminBlockList, + moderatorBlockList, + blacklistedMap, + filteredMap, + showMatureContent + ) => { return comments ? comments.filter((comment) => { if (!comment) { @@ -247,127 +311,12 @@ export const makeSelectCommentsForUri = (uri: string) => } } - return !blockedChannels.includes(comment.channel_url); - }) - : []; - } - ); - -export const makeSelectTopLevelCommentsForUri = (uri: string) => - createSelector( - selectTopLevelCommentsByClaimId, - selectCommentsByUri, - selectClaimsById, - selectMyActiveClaims, - selectMutedChannels, - selectBlacklistedOutpointMap, - selectFilteredOutpointMap, - selectShowMatureContent, - (byClaimId, byUri, claimsById, myClaims, blockedChannels, blacklistedMap, filteredMap, showMatureContent) => { - const claimId = byUri[uri]; - const comments = byClaimId && byClaimId[claimId]; - - return comments - ? comments.filter((comment) => { - if (!comment) { - return false; - } - - const channelClaim = claimsById[comment.channel_id]; - - // Return comment if `channelClaim` doesn't exist so the component knows to resolve the author - if (channelClaim) { - if (myClaims && myClaims.size > 0) { - const claimIsMine = channelClaim.is_my_output || myClaims.has(channelClaim.claim_id); - if (claimIsMine) { - return true; - } - } - - const outpoint = `${channelClaim.txid}:${channelClaim.nout}`; - if (blacklistedMap[outpoint] || filteredMap[outpoint]) { - return false; - } - - if (!showMatureContent) { - const claimIsMature = isClaimNsfw(channelClaim); - if (claimIsMature) { - return false; - } - } - } - - return !blockedChannels.includes(comment.channel_url); - }) - : []; - } - ); - -export const makeSelectTopLevelTotalCommentsForUri = (uri: string) => - createSelector(selectState, selectCommentsByUri, (state, byUri) => { - const claimId = byUri[uri]; - return state.topLevelTotalCommentsById[claimId] || 0; - }); - -export const makeSelectTopLevelTotalPagesForUri = (uri: string) => - createSelector(selectState, selectCommentsByUri, (state, byUri) => { - const claimId = byUri[uri]; - return state.topLevelTotalPagesById[claimId] || 0; - }); - -export const makeSelectRepliesForParentId = (id: string) => - createSelector( - selectState, // no selectRepliesByParentId - selectCommentsById, - selectClaimsById, - selectMyActiveClaims, - selectMutedChannels, - selectBlacklistedOutpointMap, - selectFilteredOutpointMap, - selectShowMatureContent, - (state, commentsById, claimsById, myClaims, blockedChannels, blacklistedMap, filteredMap, showMatureContent) => { - // const claimId = byUri[uri]; // just parentId (id) - const replyIdsByParentId = state.repliesByParentId; - const replyIdsForParent = replyIdsByParentId[id] || []; - if (!replyIdsForParent.length) return null; - - const comments = []; - replyIdsForParent.forEach((cid) => { - comments.push(commentsById[cid]); - }); - // const comments = byParentId && byParentId[id]; - - return comments - ? comments.filter((comment) => { - if (!comment) { - return false; - } - - const channelClaim = claimsById[comment.channel_id]; - - // Return comment if `channelClaim` doesn't exist so the component knows to resolve the author - if (channelClaim) { - if (myClaims && myClaims.size > 0) { - const claimIsMine = channelClaim.is_my_output || myClaims.has(channelClaim.claim_id); - if (claimIsMine) { - return true; - } - } - - const outpoint = `${channelClaim.txid}:${channelClaim.nout}`; - if (blacklistedMap[outpoint] || filteredMap[outpoint]) { - return false; - } - - if (!showMatureContent) { - const claimIsMature = isClaimNsfw(channelClaim); - if (claimIsMature) { - return false; - } - } - } - - return !blockedChannels.includes(comment.channel_url); + return !( + mutedChannels.includes(comment.channel_url) || + personalBlockList.includes(comment.channel_url) || + adminBlockList.includes(comment.channel_url) || + moderatorBlockList.includes(comment.channel_url) + ); }) : []; } From 74986a8b3aca960acf6d33d0976815fdceba3e3d Mon Sep 17 00:00:00 2001 From: infinite-persistence Date: Wed, 4 Aug 2021 23:01:31 +0800 Subject: [PATCH 2/2] Comments: simplify blocked replies display Previously, we decide when to display "Show More" based on the current fetched reply count vs. total replies in Commentron. It was already troublesome as `comment.List` and `comment.replies` give different values (one includes blocked content, while another does not). Now, we are further filtering the list with Commentron blocklists (personal, admin, moderator), so it is not feasible to run the logic based on reply count. ## Solution - Keep track of number of remaining pages instead and use that to determine when to display "Show More". - While it doesn't solve the "Show N replies" mismatch (YT has this problem too), it prevents the button from lingering. - In the event that all replies are blocked, just show an empty space (same as YT). I didn't like the previous version that cluttered the space with "comment(s) blocked". --- flow-typed/Comment.js | 2 +- ui/component/comment/index.js | 7 ++++++- ui/component/comment/view.jsx | 3 +++ ui/component/commentsReplies/index.js | 7 +------ ui/component/commentsReplies/view.jsx | 26 ++++---------------------- ui/redux/reducers/comments.js | 22 ++++------------------ ui/redux/selectors/comments.js | 4 ++-- 7 files changed, 21 insertions(+), 50 deletions(-) diff --git a/flow-typed/Comment.js b/flow-typed/Comment.js index 412052a39..ffda63876 100644 --- a/flow-typed/Comment.js +++ b/flow-typed/Comment.js @@ -35,7 +35,7 @@ declare type CommentsState = { byId: { [string]: Array }, // ClaimID -> list of fetched comment IDs. totalCommentsById: {}, // ClaimId -> ultimate total (including replies) in commentron. repliesByParentId: { [string]: Array }, // ParentCommentID -> list of fetched replies. - totalRepliesByParentId: {}, // ParentCommentID -> total replies in commentron. + repliesTotalPagesByParentId: {}, // ParentCommentID -> total number of reply pages for a parentId in commentron. topLevelCommentsById: { [string]: Array }, // ClaimID -> list of fetched top level comments. topLevelTotalPagesById: { [string]: number }, // ClaimID -> total number of top-level pages in commentron. Based on COMMENT_PAGE_SIZE_TOP_LEVEL. topLevelTotalCommentsById: { [string]: number }, // ClaimID -> total top level comments in commentron. diff --git a/ui/component/comment/index.js b/ui/component/comment/index.js index 26e604e64..901b8cd3b 100644 --- a/ui/component/comment/index.js +++ b/ui/component/comment/index.js @@ -10,7 +10,11 @@ import { makeSelectChannelIsMuted } from 'redux/selectors/blocked'; import { doToast } from 'redux/actions/notifications'; import { doSetPlayingUri } from 'redux/actions/content'; import { selectUserVerifiedEmail } from 'redux/selectors/user'; -import { selectLinkedCommentAncestors, makeSelectOthersReactionsForComment } from 'redux/selectors/comments'; +import { + selectLinkedCommentAncestors, + makeSelectOthersReactionsForComment, + makeSelectTotalReplyPagesForParentId, +} from 'redux/selectors/comments'; import { selectActiveChannelClaim } from 'redux/selectors/app'; import { selectPlayingUri } from 'redux/selectors/content'; import Comment from './view'; @@ -31,6 +35,7 @@ const select = (state, props) => { playingUri: selectPlayingUri(state), stakedLevel: makeSelectStakedLevelForChannelUri(props.authorUri)(state), linkedCommentAncestors: selectLinkedCommentAncestors(state), + totalReplyPages: makeSelectTotalReplyPagesForParentId(props.commentId)(state), }; }; diff --git a/ui/component/comment/view.jsx b/ui/component/comment/view.jsx index 6d3c6dc62..bc8222db7 100644 --- a/ui/component/comment/view.jsx +++ b/ui/component/comment/view.jsx @@ -41,6 +41,7 @@ type Props = { commentIsMine: boolean, // if this comment was signed by an owned channel updateComment: (string, string) => void, fetchReplies: (string, string, number, number, number) => void, + totalReplyPages: number, commentModBlock: (string) => void, linkedCommentId?: string, linkedCommentAncestors: { [string]: Array }, @@ -82,6 +83,7 @@ function Comment(props: Props) { commentId, updateComment, fetchReplies, + totalReplyPages, linkedCommentId, linkedCommentAncestors, commentingEnabled, @@ -417,6 +419,7 @@ function Comment(props: Props) { linkedCommentId={linkedCommentId} numDirectReplies={numDirectReplies} onShowMore={() => setPage(page + 1)} + hasMore={page < totalReplyPages} /> )} diff --git a/ui/component/commentsReplies/index.js b/ui/component/commentsReplies/index.js index 0bb4aba85..8008d5535 100644 --- a/ui/component/commentsReplies/index.js +++ b/ui/component/commentsReplies/index.js @@ -1,16 +1,11 @@ import { connect } from 'react-redux'; import { makeSelectClaimIsMine, selectMyChannelClaims } from 'lbry-redux'; -import { - selectIsFetchingCommentsByParentId, - makeSelectRepliesForParentId, - makeSelectTotalRepliesForParentId, -} from 'redux/selectors/comments'; +import { selectIsFetchingCommentsByParentId, makeSelectRepliesForParentId } from 'redux/selectors/comments'; import { selectUserVerifiedEmail } from 'redux/selectors/user'; import CommentsReplies from './view'; const select = (state, props) => ({ fetchedReplies: makeSelectRepliesForParentId(props.parentId)(state), - totalReplies: makeSelectTotalRepliesForParentId(props.parentId)(state), claimIsMine: makeSelectClaimIsMine(props.uri)(state), commentingEnabled: IS_WEB ? Boolean(selectUserVerifiedEmail(state)) : true, myChannels: selectMyChannelClaims(state), diff --git a/ui/component/commentsReplies/view.jsx b/ui/component/commentsReplies/view.jsx index 6e78e7dac..42d0e451a 100644 --- a/ui/component/commentsReplies/view.jsx +++ b/ui/component/commentsReplies/view.jsx @@ -4,11 +4,9 @@ import React from 'react'; import Comment from 'component/comment'; import Button from 'component/button'; import Spinner from 'component/spinner'; -import ChannelThumbnail from 'component/channelThumbnail'; type Props = { fetchedReplies: Array, - totalReplies: number, uri: string, parentId: string, claimIsMine: boolean, @@ -16,9 +14,10 @@ type Props = { linkedCommentId?: string, commentingEnabled: boolean, threadDepth: number, - numDirectReplies: number, + numDirectReplies: number, // Total replies for parentId as reported by 'comment[replies]'. Includes blocked items. isFetchingByParentId: { [string]: boolean }, onShowMore?: () => void, + hasMore: boolean, }; function CommentsReplies(props: Props) { @@ -26,7 +25,6 @@ function CommentsReplies(props: Props) { uri, parentId, fetchedReplies, - totalReplies, claimIsMine, myChannels, linkedCommentId, @@ -35,6 +33,7 @@ function CommentsReplies(props: Props) { numDirectReplies, isFetchingByParentId, onShowMore, + hasMore, } = props; const [isExpanded, setExpanded] = React.useState(true); @@ -102,28 +101,11 @@ function CommentsReplies(props: Props) { /> ); })} - {!isFetchingByParentId[parentId] && totalReplies < numDirectReplies && ( -
  • -
    -
    - -
    -
    -
    - --- -
    -
    - {__('Comment(s) blocked.')} -
    -
    -
    -
    • - )} )} - {isExpanded && fetchedReplies && displayedComments.length < totalReplies && ( + {isExpanded && fetchedReplies && hasMore && (
    diff --git a/ui/redux/reducers/comments.js b/ui/redux/reducers/comments.js index ac344c8c7..815025698 100644 --- a/ui/redux/reducers/comments.js +++ b/ui/redux/reducers/comments.js @@ -9,7 +9,7 @@ const defaultState: CommentsState = { byId: {}, // ClaimID -> list of fetched comment IDs. totalCommentsById: {}, // ClaimId -> ultimate total (including replies) in commentron. repliesByParentId: {}, // ParentCommentID -> list of fetched replies. - totalRepliesByParentId: {}, // ParentCommentID -> total replies in commentron. + repliesTotalPagesByParentId: {}, // ParentCommentID -> total number of reply pages for a parentId in commentron. topLevelCommentsById: {}, // ClaimID -> list of fetched top level comments. topLevelTotalPagesById: {}, // ClaimID -> total number of top-level pages in commentron. Based on COMMENT_PAGE_SIZE_TOP_LEVEL. topLevelTotalCommentsById: {}, // ClaimID -> total top level comments in commentron. @@ -79,7 +79,6 @@ export default handleActions( const totalCommentsById = Object.assign({}, state.totalCommentsById); const topLevelCommentsById = Object.assign({}, state.topLevelCommentsById); // was byId {ClaimId -> [commentIds...]} const repliesByParentId = Object.assign({}, state.repliesByParentId); // {ParentCommentID -> [commentIds...] } list of reply comments - const totalRepliesByParentId = Object.assign({}, state.totalRepliesByParentId); const commentsByUri = Object.assign({}, state.commentsByUri); const comments = byId[claimId] || []; const newCommentIds = comments.slice(); @@ -104,12 +103,6 @@ export default handleActions( repliesByParentId[comment.parent_id].unshift(comment.comment_id); } - if (!totalRepliesByParentId[comment.parent_id]) { - totalRepliesByParentId[comment.parent_id] = 1; - } else { - totalRepliesByParentId[comment.parent_id] += 1; - } - // Update the parent's "replies" value if (commentById[comment.parent_id]) { commentById[comment.parent_id].replies = (commentById[comment.parent_id].replies || 0) + 1; @@ -128,7 +121,6 @@ export default handleActions( ...state, topLevelCommentsById, repliesByParentId, - totalRepliesByParentId, commentById, byId, totalCommentsById, @@ -262,7 +254,7 @@ export default handleActions( const repliesByParentId = Object.assign({}, state.repliesByParentId); const totalCommentsById = Object.assign({}, state.totalCommentsById); const pinnedCommentsById = Object.assign({}, state.pinnedCommentsById); - const totalRepliesByParentId = Object.assign({}, state.totalRepliesByParentId); + const repliesTotalPagesByParentId = Object.assign({}, state.repliesTotalPagesByParentId); const isLoadingByParentId = Object.assign({}, state.isLoadingByParentId); const settingsByChannelId = Object.assign({}, state.settingsByChannelId); @@ -277,7 +269,7 @@ export default handleActions( if (!disabled) { if (parentId) { - totalRepliesByParentId[parentId] = totalFilteredItems; + repliesTotalPagesByParentId[parentId] = totalPages; } else { totalCommentsById[claimId] = totalItems; topLevelTotalCommentsById[claimId] = totalFilteredItems; @@ -330,7 +322,7 @@ export default handleActions( repliesByParentId, totalCommentsById, pinnedCommentsById, - totalRepliesByParentId, + repliesTotalPagesByParentId, byId, commentById, commentsByUri, @@ -548,7 +540,6 @@ export default handleActions( const commentById = Object.assign({}, state.commentById); const byId = Object.assign({}, state.byId); const repliesByParentId = Object.assign({}, state.repliesByParentId); // {ParentCommentID -> [commentIds...] } list of reply comments - const totalRepliesByParentId = Object.assign({}, state.totalRepliesByParentId); const totalCommentsById = Object.assign({}, state.totalCommentsById); const comment = commentById[comment_id]; @@ -571,10 +562,6 @@ export default handleActions( if (commentById[comment.parent_id]) { commentById[comment.parent_id].replies = Math.max(0, (commentById[comment.parent_id].replies || 0) - 1); } - - if (totalRepliesByParentId[comment.parent_id]) { - totalRepliesByParentId[comment.parent_id] = Math.max(0, totalRepliesByParentId[comment.parent_id] - 1); - } } } @@ -590,7 +577,6 @@ export default handleActions( byId, totalCommentsById, repliesByParentId, - totalRepliesByParentId, isLoading: false, }; }, diff --git a/ui/redux/selectors/comments.js b/ui/redux/selectors/comments.js index 1fe9be599..e4c21ff54 100644 --- a/ui/redux/selectors/comments.js +++ b/ui/redux/selectors/comments.js @@ -322,9 +322,9 @@ const makeSelectFilteredComments = (comments: Array) => } ); -export const makeSelectTotalRepliesForParentId = (parentId: string) => +export const makeSelectTotalReplyPagesForParentId = (parentId: string) => createSelector(selectState, (state) => { - return state.totalRepliesByParentId[parentId] || 0; + return state.repliesTotalPagesByParentId[parentId] || 0; }); export const makeSelectTotalCommentsCountForUri = (uri: string) =>