Skip to content

Conversation

@jmgasper
Copy link
Collaborator

…show own submissions (PM-3255)
https://topcoder.atlassian.net/browse/PM-3255

@jmgasper jmgasper requested a review from kkartunov as a code owner December 17, 2025 04:28
@jmgasper jmgasper merged commit 19ecbcf into dev Dec 17, 2025
4 of 6 checks passed
const { challengeId = '' }: { challengeId?: string } = useParams<{
challengeId: string
}>()
const { loginUserInfo }: ReviewAppContextModel = useContext(ReviewAppContext)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The destructuring of loginUserInfo from ReviewAppContext assumes that useContext(ReviewAppContext) will always return a valid object. Consider adding a default value or null check to prevent potential runtime errors if the context is not properly initialized.

}: useFetchChallengeSubmissionsProps = useFetchChallengeSubmissions(challengeId)
}: useFetchChallengeSubmissionsProps = useFetchChallengeSubmissions(
challengeId,
submissionViewer,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ performance]
The submissionViewer object is passed as a dependency to useFetchChallengeSubmissions. Ensure that useFetchChallengeSubmissions is memoized or stable, as changes in submissionViewer will trigger re-fetching, which could lead to performance issues if not handled correctly.

})

const normalizedRoles = useMemo<string[]>(
() => (viewer?.roles ?? [])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The normalization of roles involves converting each role to a string and trimming it. Consider handling cases where roles might not be strings initially to avoid potential runtime errors.

)
const normalizedTokenRoles = useMemo<string[]>(
() => (viewer?.tokenRoles ?? [])
.map(role => (typeof role === 'string' ? role.toLowerCase()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The normalization of token roles assumes roles are strings. Ensure that all roles are validated as strings before processing to prevent unexpected behavior.

[normalizedRoles, normalizedTokenRoles],
)
const canViewAllSubmissions = useMemo<boolean>(
() => (viewer ? (

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ security]
The logic for canViewAllSubmissions assumes that if viewer is not provided, all submissions can be viewed. Verify that this behavior aligns with the intended access control policy.

})

const visibleSubmissions = shouldRestrictToCurrentMember
? activeSubmissions.filter(submission => (viewerMemberId

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The filtering logic for visibleSubmissions uses string comparison for memberId. Ensure that memberId is consistently a string across all relevant data sources to avoid mismatches.

myResources,
myRoles,
}: ChallengeDetailContextModel = useContext(ChallengeDetailContext)
const submissionViewer = useMemo(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[💡 performance]
The useMemo hook is used to memoize the submissionViewer object. Ensure that the dependencies [loginUserInfo?.roles, loginUserInfo?.userId, myRoles] are sufficient and cover all properties accessed within the memoized function. If any of these properties change frequently, consider whether memoization provides a significant performance benefit.

deletedSubmissionIds,
isLoading,
}: useFetchChallengeSubmissionsProps = useFetchChallengeSubmissions(challengeId)
}: useFetchChallengeSubmissionsProps = useFetchChallengeSubmissions(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The useFetchChallengeSubmissions function now takes submissionViewer as an additional argument. Ensure that this change is reflected in the implementation of useFetchChallengeSubmissions and that it handles the submissionViewer object correctly. Verify that this does not introduce any unintended side effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants