Skip to content

Fix 2446 #1268

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 23, 2025
Merged

Fix 2446 #1268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4cdc040
Fix reopen review view
himaniraghav3 Oct 21, 2025
49c1347
PM-2057 Fix line breaks in markdown
himaniraghav3 Oct 21, 2025
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
2 changes: 1 addition & 1 deletion src/apps/review/src/lib/components/ScorecardQuestionView/ScorecardQuestionView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ function renderResponseCommentRow({
</span>

<MarkdownReview
value={commentItem.content}
value={commentItem.content.replace(/\n\n/g, '<br><br>')}
Copy link

@github-actions github-actions bot Oct 21, 2025

Choose a reason for hiding this comment

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

[❗❗ security]
Using replace to convert newlines to <br> tags could lead to unexpected behavior if commentItem.content contains HTML or special characters. Consider using a library like DOMPurify to sanitize the content before rendering it as HTML to prevent XSS vulnerabilities.

className={styles.mardownReview}
/>
</div>
Expand Down
99 changes: 15 additions & 84 deletions src/apps/review/src/lib/components/TableReview/TableReview.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import {
aggregateSubmissionReviews,
challengeHasSubmissionLimit,
isReviewPhase,
isReviewPhaseCurrentlyOpen,
refreshChallengeReviewData,
REOPEN_MESSAGE_OTHER,
REOPEN_MESSAGE_SELF,
Expand Down Expand Up @@ -85,69 +84,6 @@ type RestrictionResult = {
message?: string
}

function createReopenActionButtons(
challengeInfo: ChallengeDetailContextModel['challengeInfo'],
submission: SubmissionRow,
aggregatedReviews: AggregatedReviewDetail[] | undefined,
{
canManageCompletedReviews,
isReopening,
openReopenDialog,
pendingReopen,
}: {
canManageCompletedReviews: boolean
isReopening: boolean
openReopenDialog: (submission: SubmissionRow, review: AggregatedReviewDetail) => void
pendingReopen: PendingReopenState | undefined
},
): JSX.Element[] {
if (!canManageCompletedReviews) {
return []
}

const buttons: JSX.Element[] = []
const reviews = aggregatedReviews ?? []

reviews.forEach(review => {
const reviewInfo = review.reviewInfo
if (!reviewInfo?.id) {
return
}

if ((reviewInfo.status ?? '').toUpperCase() !== 'COMPLETED') {
return
}

if (!isReviewPhaseCurrentlyOpen(challengeInfo, reviewInfo.phaseId)) {
return
}

const isTargetReview = pendingReopen?.review?.reviewInfo?.id === reviewInfo.id

function handleReopenClick(): void {
openReopenDialog(submission, {
...review,
reviewInfo,
} as AggregatedReviewDetail)
}

buttons.push(
<button
key={`reopen-${reviewInfo.id}`}
type='button'
className={classNames(styles.actionButton, styles.textBlue)}
onClick={handleReopenClick}
disabled={isReopening && isTargetReview}
>
<i className='icon-reopen' />
Reopen review
</button>,
)
})

return buttons
}

export const TableReview: FC<TableReviewProps> = (props: TableReviewProps) => {
const className: string | undefined = props.className
const datas: SubmissionInfo[] = props.datas
Expand Down Expand Up @@ -493,20 +429,6 @@ export const TableReview: FC<TableReviewProps> = (props: TableReviewProps) => {
}

appendAction(buildPrimaryAction(), 'primary')

const reopenButtons = createReopenActionButtons(
challengeInfo,
submission,
reviews,
{
canManageCompletedReviews,
isReopening,
openReopenDialog,
pendingReopen,
},
)

reopenButtons.forEach(button => appendAction(button, 'reopen'))
appendAction(buildHistoryAction(), 'history')

if (!actionEntries.length) {
Expand Down Expand Up @@ -536,16 +458,11 @@ export const TableReview: FC<TableReviewProps> = (props: TableReviewProps) => {
</span>
)
}, [
canManageCompletedReviews,
canViewHistory,
challengeInfo,
handleHistoryButtonClick,
hasReviewRole,
historyByMember,
isReopening,
myReviewerResourceIds,
openReopenDialog,
pendingReopen,
shouldShowHistoryActions,
])

Expand Down Expand Up @@ -600,7 +517,16 @@ export const TableReview: FC<TableReviewProps> = (props: TableReviewProps) => {
{
columnId: `score-${index}`,
label: `Score ${index + 1}`,
renderer: (submission: SubmissionRow) => renderScoreCell(submission, index, scoreVisibilityConfig),
renderer: (submission: SubmissionRow) => renderScoreCell(
Copy link

@github-actions github-actions bot Oct 21, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The renderScoreCell function now takes additional parameters: challengeInfo, pendingReopen, canManageCompletedReviews, isReopening, and openReopenDialog. Ensure that renderScoreCell is updated to handle these new parameters appropriately, and verify that they are necessary for its logic. If any of these parameters are not used within renderScoreCell, consider removing them to maintain clarity and simplicity.

submission,
index,
scoreVisibilityConfig,
challengeInfo,
pendingReopen,
canManageCompletedReviews,
isReopening,
openReopenDialog,
),
type: 'element',
},
)
Expand All @@ -624,6 +550,11 @@ export const TableReview: FC<TableReviewProps> = (props: TableReviewProps) => {
renderActionsCell,
scoreVisibilityConfig,
shouldShowAggregatedActions,
canManageCompletedReviews,
isReopening,
openReopenDialog,
challengeInfo,
pendingReopen,
])

const columnsMobile = useMemo<MobileTableColumn<SubmissionRow>[][]>(
Expand Down
6 changes: 6 additions & 0 deletions src/apps/review/src/lib/components/common/TableColumnRenderers.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
}
}

.scoreReopenBlock {
display: flex;
gap: 4px;
align-items: center;
}

.tooltipTrigger {
display: inline-block;
}
Expand Down
109 changes: 103 additions & 6 deletions src/apps/review/src/lib/components/common/TableColumnRenderers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,16 @@ import {
VIRUS_SCAN_FAILED_MESSAGE,
} from '../../utils/constants'
import { getReviewRoute } from '../../utils/routes'
import { ChallengeDetailContextModel, ChallengeInfo } from '../../models'
import { isReviewPhaseCurrentlyOpen } from '../../utils'

import {
formatScoreDisplay,
getHandleColor,
getProfileUrl,
} from './columnUtils'
import type {
AggregatedReviewDetail,
DownloadButtonConfig,
ScoreVisibilityConfig,
SubmissionRow,
Expand Down Expand Up @@ -301,13 +304,88 @@ export function renderReviewerCell(
)
}

interface PendingReopenState {
review?: AggregatedReviewDetail
submission?: SubmissionRow
isOwnReview?: boolean
}

function createReopenActionButtons(
challengeInfo: ChallengeDetailContextModel['challengeInfo'],
submission: SubmissionRow,
aggregatedReviews: AggregatedReviewDetail[] | undefined,
{
canManageCompletedReviews,
isReopening,
openReopenDialog,
pendingReopen,
}: {
canManageCompletedReviews: boolean
isReopening: boolean
openReopenDialog: (submission: SubmissionRow, review: AggregatedReviewDetail) => void
pendingReopen: PendingReopenState | undefined
},
): JSX.Element[] {
if (!canManageCompletedReviews) {
return []
}

const buttons: JSX.Element[] = []
const reviews = aggregatedReviews ?? []

reviews.forEach(review => {
const reviewInfo = review.reviewInfo
if (!reviewInfo?.id) {
return
}

if ((reviewInfo.status ?? '').toUpperCase() !== 'COMPLETED') {
return
}

if (!isReviewPhaseCurrentlyOpen(challengeInfo, reviewInfo.phaseId)) {
Copy link

@github-actions github-actions bot Oct 21, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
The function isReviewPhaseCurrentlyOpen is used to check if the review phase is open. Ensure that this function handles all edge cases, such as time zone differences or unexpected phase IDs, to prevent incorrect button rendering.

return
}

const isTargetReview = pendingReopen?.review?.reviewInfo?.id === reviewInfo.id

function handleReopenClick(): void {
openReopenDialog(submission, {
...review,
reviewInfo,
} as AggregatedReviewDetail)
}

buttons.push(
// eslint-disable-next-line jsx-a11y/control-has-associated-label
<button
key={`reopen-${reviewInfo.id}`}
Copy link

@github-actions github-actions bot Oct 21, 2025

Choose a reason for hiding this comment

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

[⚠️ accessibility]
Consider adding an accessible label to the button to improve accessibility for screen readers. This will help users who rely on assistive technologies to understand the purpose of the button.

type='button'
className={classNames(styles.actionButton, styles.textBlue)}
onClick={handleReopenClick}
disabled={isReopening && isTargetReview}
title='Reopen review'
>
<i className='icon-reopen' />
</button>,
)
})

return buttons
}

/**
* Renders an individual review score, linking to the review detail when allowed.
*/
export function renderScoreCell(
submission: SubmissionRow,
reviewIndex: number,
config: ScoreVisibilityConfig,
challengeInfo?: ChallengeInfo | undefined,
pendingReopen?: PendingReopenState | undefined,
canManageCompletedReviews?: boolean,
isReopening?: boolean,
openReopenDialog?: (submission: SubmissionRow, review: AggregatedReviewDetail) => void,
): JSX.Element {
const configWithDefaults: ScoreVisibilityConfig = config
const {
Expand Down Expand Up @@ -349,6 +427,20 @@ export function renderScoreCell(
const formattedScore = formatScoreDisplay(reviewDetail.finalScore)
const scoreLabel = formattedScore ?? '--'

const reopenButtons = createReopenActionButtons(
challengeInfo,
submission,
[reviewDetail], // pass single review in an array
{
canManageCompletedReviews: !!canManageCompletedReviews,
isReopening: !!isReopening,
openReopenDialog: openReopenDialog as (submission: SubmissionRow, review: AggregatedReviewDetail) => void,
pendingReopen,
Copy link

@github-actions github-actions bot Oct 21, 2025

Choose a reason for hiding this comment

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

[❗❗ correctness]
The openReopenDialog function is cast to a specific type. Ensure that the function passed in matches this type signature to avoid runtime errors.

},
)

const reopenButton = reopenButtons.length ? reopenButtons[0] : undefined

if (!canViewScorecard) {
return (
<Tooltip
Expand All @@ -367,13 +459,18 @@ export function renderScoreCell(
const reviewUrl = getReviewUrl ? getReviewUrl(reviewId) : getReviewRoute(reviewId)

return (
<Link
to={reviewUrl}
className={styles.textBlue}
>
{scoreLabel}
</Link>
<div className={styles.scoreReopenBlock}>
<Link
to={reviewUrl}
className={styles.textBlue}
>
{scoreLabel}

</Link>
<span>{reopenButton}</span>
</div>
)

}

/**
Expand Down