Skip to content

Commit

Permalink
[C-5821] Recent comments final QA (#11514)
Browse files Browse the repository at this point in the history
  • Loading branch information
dylanjeffers authored Mar 5, 2025
1 parent c2815ae commit b796369
Show file tree
Hide file tree
Showing 13 changed files with 192 additions and 45 deletions.
1 change: 0 additions & 1 deletion packages/common/src/api/tan-query/comments.ts

This file was deleted.

30 changes: 27 additions & 3 deletions packages/common/src/api/tan-query/comments/useCommentReplies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ export const useCommentReplies = (
return replies.map((reply) => reply.id)
},
select: (data) => data.pages.flat(),
staleTime: Infinity,
gcTime: 1,
...options
})

Expand All @@ -87,7 +89,29 @@ export const useCommentReplies = (
}
}, [error, dispatch, reportToSentry])

const { data: replies } = useComments(replyIds)

return { ...queryRes, data: replies }
const commentsQuery = useComments(replyIds)
const { data: replies } = commentsQuery

// Merge the loading states from both queries
const isLoading = queryRes.isLoading || commentsQuery.isLoading
const isPending = queryRes.isPending || commentsQuery.isPending
const isFetching = queryRes.isFetching || commentsQuery.isFetching
const isSuccess = queryRes.isSuccess && commentsQuery.isSuccess

// Determine the overall status based on both queries
let status = queryRes.status
if (queryRes.status === 'success' && commentsQuery.status !== 'success') {
status = commentsQuery.status
}

return {
...queryRes,
data: replies,
replyIds,
isLoading,
isPending,
isFetching,
isSuccess,
status
}
}
19 changes: 17 additions & 2 deletions packages/common/src/api/tan-query/comments/useComments.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useMemo } from 'react'
import { useEffect, useMemo, useState } from 'react'

import { useQueries, useQueryClient } from '@tanstack/react-query'
import { keyBy } from 'lodash'
Expand All @@ -21,6 +21,7 @@ export const useComments = (
options?: QueryOptions
) => {
const queryClient = useQueryClient()
const [hasInitialized, setHasInitialized] = useState(false)

const { data: comments, ...queryResults } = useQueries({
queries: (commentIds ?? []).map((commentId) => ({
Expand All @@ -35,6 +36,18 @@ export const useComments = (
combine: combineQueryResults<CommentOrReply[]>
})

useEffect(() => {
if (commentIds?.length) {
setHasInitialized(true)
}
}, [commentIds?.length])

const isPending =
!hasInitialized || commentIds?.length === 0 || queryResults.isPending

const isLoading =
!hasInitialized || commentIds?.length === 0 || queryResults.isLoading

const byId = useMemo(() => {
const byId = keyBy(comments, 'id')
return byId
Expand All @@ -43,6 +56,8 @@ export const useComments = (
return {
data: comments,
byId,
...queryResults
...queryResults,
isPending,
isLoading
}
}
32 changes: 28 additions & 4 deletions packages/common/src/api/tan-query/comments/useTrackComments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,13 @@ export const useTrackComments = (
return commentList.map((comment) => comment.id)
},
select: (data) => data.pages.flat(),
staleTime: Infinity, // Stale time is set to infinity so that we never reload data thats currently shown on screen (because sorting could have changed)
gcTime: 0, // Cache time is set to 1 so that the data is cleared any time we leave the page viewing it or change sorts
...options,
enabled: isMutating === 0 && options?.enabled !== false
enabled: isMutating === 0 && options?.enabled !== false && !!trackId
})

const { error, data: commentIds } = queryRes
const { error, data: commentIds = [] } = queryRes

useEffect(() => {
if (error) {
Expand All @@ -90,7 +92,29 @@ export const useTrackComments = (
}
}, [error, dispatch, reportToSentry])

const { data: comments } = useComments(commentIds)
const commentsQuery = useComments(commentIds)
const { data: comments } = commentsQuery

return { ...queryRes, data: comments }
// Merge the loading states from both queries
const isLoading = queryRes.isLoading || commentsQuery.isLoading
const isPending = queryRes.isPending || commentsQuery.isPending
const isFetching = queryRes.isFetching || commentsQuery.isFetching
const isSuccess = queryRes.isSuccess && commentsQuery.isSuccess

// Determine the overall status based on both queries
let status = queryRes.status
if (queryRes.status === 'success' && commentsQuery.status !== 'success') {
status = commentsQuery.status
}

return {
...queryRes,
data: comments,
commentIds,
isLoading,
isPending,
isFetching,
isSuccess,
status
}
}
28 changes: 25 additions & 3 deletions packages/common/src/api/tan-query/comments/useUserComments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const useUserComments = (
},
select: (data) => data.pages.flat(),
...options,
enabled: isMutating === 0 && options?.enabled !== false
enabled: isMutating === 0 && options?.enabled !== false && !!userId
})

const { error, data: commentIds } = queryRes
Expand All @@ -84,7 +84,29 @@ export const useUserComments = (
}
}, [error, dispatch, reportToSentry])

const { data: comments } = useComments(commentIds)
const commentsQuery = useComments(commentIds)
const { data: comments } = commentsQuery

return { ...queryRes, data: comments }
// Merge the loading states from both queries
const isLoading = queryRes.isLoading || commentsQuery.isLoading
const isPending = queryRes.isPending || commentsQuery.isPending
const isFetching = queryRes.isFetching || commentsQuery.isFetching
const isSuccess = queryRes.isSuccess && commentsQuery.isSuccess

// Determine the overall status based on both queries
let status = queryRes.status
if (queryRes.status === 'success' && commentsQuery.status !== 'success') {
status = commentsQuery.status
}

return {
...queryRes,
data: comments,
commentIds,
isLoading,
isPending,
isFetching,
isSuccess,
status
}
}
9 changes: 5 additions & 4 deletions packages/common/src/context/comments/commentsContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ export function CommentSectionProvider<NavigationProp>(
const currentUserId = useSelector(getUserId)

const {
data: commentIds = [],
data: comments = [],
commentIds,
status,
isFetching,
hasNextPage,
Expand Down Expand Up @@ -170,10 +171,10 @@ export function CommentSectionProvider<NavigationProp>(
make({
eventName: Name.COMMENTS_LOAD_MORE_COMMENTS,
trackId: entityId,
offset: commentIds.length
offset: comments.length
})
)
}, [commentIds.length, entityId, loadMorePages, make, trackEvent])
}, [comments.length, entityId, loadMorePages, make, trackEvent])

const handleResetComments = useCallback(() => {
resetComments()
Expand Down Expand Up @@ -240,7 +241,7 @@ export function CommentSectionProvider<NavigationProp>(
entityType,
commentCount: commentCountData?.currentValue ?? track.comment_count,
isCommentCountLoading,
commentIds: commentIds.map((comment) => comment.id),
commentIds,
commentSectionLoading,
isEntityOwner: currentUserId === owner_id,
isLoadingMorePages,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -503,3 +503,70 @@ def test_get_user_comments_related_field(app):
# Check that comments don't have their own related field
for comment in comments:
assert "related" not in comment


def test_get_user_comments_deleted_tracks(app):
"""Test that comments on deleted tracks are not returned"""
entities = {
"comments": [
{
"comment_id": 1,
"user_id": 1,
"entity_id": 1,
"entity_type": "Track",
"text": "Comment on active track",
"created_at": datetime(2022, 1, 1),
"updated_at": datetime(2022, 1, 1),
},
{
"comment_id": 2,
"user_id": 1,
"entity_id": 2,
"entity_type": "Track",
"text": "Comment on deleted track",
"created_at": datetime(2022, 1, 2),
"updated_at": datetime(2022, 1, 2),
},
],
"tracks": [
{
"track_id": 1,
"owner_id": 10,
"is_current": True,
"is_delete": False,
"title": "Active Track",
},
{
"track_id": 2,
"owner_id": 10,
"is_current": True,
"is_delete": True,
"title": "Deleted Track",
},
],
"users": [
{"user_id": 1, "handle": "user1"},
{"user_id": 10, "handle": "artist"},
],
}

with app.app_context():
db = get_db()
populate_mock_db(db, entities)

args = {
"user_id": 1,
"current_user_id": 10,
}
response = get_user_comments(args)

# Get the comments from the data field
comments = response["data"]

# Only the comment on the active track should be returned
assert len(comments) == 1
assert decode_string_id(comments[0]["id"]) == 1

# Verify the comment on the deleted track is not included
deleted_track_comments = [c for c in comments if decode_string_id(c["id"]) == 2]
assert len(deleted_track_comments) == 0
1 change: 1 addition & 0 deletions packages/discovery-provider/src/queries/comments/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ def build_comments_query(
and_(
Comment.entity_id == Track.track_id,
Track.is_unlisted == False,
Track.is_delete == False, # Filter out comments on deleted tracks
),
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ const CommentItem = ({ comment }: { comment: CommentOrReply }) => {
</Flex>
<CommentText
isEdited={isEdited}
isPreview={true}
commentId={comment.id}
mentions={comment.mentions ?? []}
renderTimestamps={false}
Expand Down Expand Up @@ -171,7 +170,7 @@ const RecentUserCommentsDrawerContent = () => {
const { userId } = useRecentUserCommentsDrawer()
const {
data: comments,
isLoading,
isPending,
hasNextPage,
isFetchingNextPage,
fetchNextPage
Expand All @@ -184,7 +183,7 @@ const RecentUserCommentsDrawerContent = () => {
}, [fetchNextPage, hasNextPage])

// Loading state
if (isLoading) {
if (isPending) {
return (
<Flex pv='m'>
<CommentSkeleton />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,9 @@ const RelatedArtistsTile = ({ userId }: { userId: number }) => {
pageSize: MAX_CARD_PROFILE_PICTURES
})

if (relatedArtists.length === 0) {
return null
}
return (
<ProfileInfoTile
screen='RelatedArtists'
Expand Down Expand Up @@ -397,18 +400,6 @@ export const ProfileInfoTiles = () => {
style={styles.staticTilesContainer}
layout={layoutAnimation}
>
{hasAiAttribution ? (
<ProfileInfoTile
screen='AiGeneratedTracks'
icon={IconRobot}
title={messages.aiGeneratedTracks}
content={
<Text variant='body' size='s' color='subdued'>
{messages.viewAll}
</Text>
}
/>
) : null}
{supporting_count > 0 ? (
<SupportedUsersTile userId={user_id} count={supporting_count} />
) : null}
Expand All @@ -423,6 +414,18 @@ export const ProfileInfoTiles = () => {
) : null}

<RelatedArtistsTile userId={user_id} />
{hasAiAttribution ? (
<ProfileInfoTile
screen='AiGeneratedTracks'
icon={IconRobot}
title={messages.aiGeneratedTracks}
content={
<Text variant='body' size='s' color='subdued'>
{messages.viewAll}
</Text>
}
/>
) : null}
</Animated.View>
</LayoutAnimationConfig>
</ScrollView>
Expand Down
5 changes: 1 addition & 4 deletions packages/web/src/components/comments/CommentThread.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
Flex,
IconCaretDown,
IconCaretUp,
LoadingSpinner,
PlainButton
} from '@audius/harmony'

Expand Down Expand Up @@ -112,11 +111,9 @@ export const CommentThread = ({ commentId }: { commentId: ID }) => {
variant='subdued'
css={{ width: 'max-content' }}
disabled={isFetchingReplies}
isLoading={isFetchingReplies}
>
{messages.showMoreReplies}
{isFetchingReplies ? (
<LoadingSpinner css={{ width: 20, height: 20 }} />
) : null}
</PlainButton>
) : null}
</Flex>
Expand Down
Loading

0 comments on commit b796369

Please sign in to comment.