Skip to content

hotfix(PM-3208): add manager comment button showing to submitters and normal reviewer #1380

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
kkartunov merged 2 commits into from
Dec 12, 2025
Merged

hotfix(PM-3208): add manager comment button showing to submitters and normal reviewer #1380

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
4b24324
fix: add manager comment button showing to submitters and normal revi…
hentrymartin Dec 12, 2025
667ea4e
fix: add manager comment button showing to submitters and normal revi…
hentrymartin Dec 12, 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 .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ workflows:
branches:
only:
- dev
- pm-3141_3
- pm-3141_fix

- deployQa:
context: org-global
Expand Down
3 changes: 2 additions & 1 deletion ...cardViewer/ScorecardQuestion/ReviewResponse/ReviewManagerComment/ReviewManagerComment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ interface ReviewManagerCommentProps {
const ReviewManagerComment: FC<ReviewManagerCommentProps> = props => {
const {
isManagerEdit,
canAddManagerComment,
isSavingManagerComment,
addManagerComment,
}: ScorecardViewerContextValue = useScorecardViewerContext()
Expand Down Expand Up @@ -131,7 +132,7 @@ const ReviewManagerComment: FC<ReviewManagerCommentProps> = props => {
</div>
)}

{!showCommentForm && !comment && isManagerEdit && (
{!showCommentForm && !comment && isManagerEdit && canAddManagerComment && (
<button
type='button'
onClick={handleShowCommentForm}
Expand Down
4 changes: 4 additions & 0 deletions src/apps/review/src/lib/components/Scorecard/ScorecardViewer/ScorecardViewer.context.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface ScorecardViewerContextProps {
children: ReactNode;
scorecard: Scorecard | ScorecardInfo
aiFeedbackItems?: AiFeedbackItem[]
canAddManagerComment?: boolean
reviewInfo?: ReviewInfo
isEdit?: boolean
isManagerEdit?: boolean
Expand Down Expand Up @@ -66,6 +67,7 @@ export interface ScorecardViewerContextProps {

export type ScorecardViewerContextValue = {
aiFeedbackItems?: AiFeedbackItem[]
canAddManagerComment?: boolean
toggledItems: {[key: string]: boolean}
toggleItem: (id: string, toggle?: boolean) => void
reviewInfo?: ReviewInfo
Expand Down Expand Up @@ -142,6 +144,7 @@ export const ScorecardViewerContextProvider: FC<ScorecardViewerContextProps> = p
addAppealResponse: props.addAppealResponse,
addManagerComment: props.addManagerComment,
aiFeedbackItems: props.aiFeedbackItems,
canAddManagerComment: props.canAddManagerComment,
doDeleteAppeal: props.doDeleteAppeal,
form: props.isEdit ? reviewFormCtx.form : undefined,
formErrors: props.isEdit ? reviewFormCtx.form.formState.errors : undefined,
Expand All @@ -161,6 +164,7 @@ export const ScorecardViewerContextProvider: FC<ScorecardViewerContextProps> = p
touchedAllFields: reviewFormCtx.touchedAllFields,
}), [
props.aiFeedbackItems,
props.canAddManagerComment,
props.reviewInfo,
props.isEdit,
props.isManagerEdit,
Expand Down
2 changes: 2 additions & 0 deletions src/apps/review/src/lib/components/Scorecard/ScorecardViewer/ScorecardViewer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ interface ScorecardViewerProps {
isSavingAppeal?: boolean
isSavingAppealResponse?: boolean
isSavingManagerComment?: boolean
canAddManagerComment?: boolean
setReviewStatus?: (status: ReviewCtxStatus) => void
setActionButtons?: (buttons?: ReactNode) => void
saveReviewInfo?: (
Expand Down Expand Up @@ -335,6 +336,7 @@ const ScorecardViewer: FC<ScorecardViewerProps> = props => (
<ScorecardViewerContextProvider
scorecard={props.scorecard}
aiFeedbackItems={props.aiFeedback}
canAddManagerComment={props.canAddManagerComment}
reviewInfo={props.reviewInfo}
isEdit={props.isEdit}
isManagerEdit={props.isManagerEdit}
Expand Down
1 change: 1 addition & 0 deletions src/apps/review/src/lib/components/TableAppeals/TableAppeals.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ export const TableAppeals: FC<TableAppealsProps> = (props: TableAppealsProps) =>
? ownedMemberIds.has(submissionOwnerId)
: false
},
canRespondToAppeals: isAdmin || hasReviewerRole,
Copy link

@github-actions github-actions bot Dec 12, 2025

Choose a reason for hiding this comment

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

[⚠️ security]
The condition isAdmin || hasReviewerRole is used to determine canRespondToAppeals. Ensure that this logic aligns with the intended access control policy, as it might inadvertently allow more users than intended to respond to appeals.

canViewScorecard: canAccessScorecards,
isAppealsTab: true,
}
Expand Down
5 changes: 3 additions & 2 deletions src/apps/review/src/lib/components/TableAppealsResponse/TableAppealsResponse.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,10 +319,11 @@ export const TableAppealsResponse: FC<TableAppealsResponseProps> = (props: Table
const scoreVisibilityConfig = useMemo<ScoreVisibilityConfig>(
() => ({
canDisplayScores: () => true,
canRespondToAppeals,
canViewScorecard: true,
isAppealsTab: false,
}),
[],
[canRespondToAppeals],
Copy link

@github-actions github-actions bot Dec 12, 2025

Choose a reason for hiding this comment

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

[⚠️ performance]
The useMemo dependency array now includes canRespondToAppeals, which is correct for ensuring the memoized value updates when canRespondToAppeals changes. However, ensure that canRespondToAppeals is a stable value (e.g., derived from props or state) to prevent unnecessary recalculations.

)

const columns = useMemo<TableColumn<SubmissionRow>[]>(() => {
Expand Down Expand Up @@ -467,7 +468,7 @@ export const TableAppealsResponse: FC<TableAppealsResponseProps> = (props: Table
>
<Link
className={styles.respondButton}
to={getReviewRoute(submission.id, reviewId)}
to={getReviewRoute(submission.id, reviewId, canRespondToAppeals)}
Copy link

@github-actions github-actions bot Dec 12, 2025

Choose a reason for hiding this comment

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

[❗❗ correctness]
The getReviewRoute function now receives an additional canRespondToAppeals argument. Ensure that this function is designed to handle this new parameter correctly and that it does not introduce any unintended side effects.

>
Respond to Appeals
</Link>
Expand Down
10 changes: 8 additions & 2 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 @@ -522,6 +522,7 @@ export function renderScoreCell(
canViewScorecard,
viewOwnScorecardTooltip = VIEW_OWN_SCORECARD_TOOLTIP,
getReviewUrl,
canRespondToAppeals,
}: ScoreVisibilityConfig = configWithDefaults

const reviewDetail = submission.aggregated?.reviews?.[reviewIndex]
Expand Down Expand Up @@ -585,7 +586,9 @@ export function renderScoreCell(
)
}

const reviewUrl = getReviewUrl ? getReviewUrl(reviewId) : getReviewRoute(submission.id, reviewId)
const reviewUrl = getReviewUrl
? getReviewUrl(reviewId)
: getReviewRoute(submission.id, reviewId, canRespondToAppeals)
Copy link

@github-actions github-actions bot Dec 12, 2025

Choose a reason for hiding this comment

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

[❗❗ correctness]
The getReviewRoute function now takes an additional canRespondToAppeals parameter. Ensure that this function can handle the new parameter correctly and that it doesn't introduce any unintended side effects or errors.


return (
<div className={styles.scoreReopenBlock}>
Expand Down Expand Up @@ -615,6 +618,7 @@ export function renderAppealsCell(
canViewScorecard,
viewOwnScorecardTooltip = VIEW_OWN_SCORECARD_TOOLTIP,
getReviewUrl,
canRespondToAppeals,
}: ScoreVisibilityConfig = configWithDefaults

const reviewDetail = submission.aggregated?.reviews?.[reviewIndex]
Expand Down Expand Up @@ -668,7 +672,9 @@ export function renderAppealsCell(
)
}

const reviewUrl = getReviewUrl ? getReviewUrl(reviewId) : getReviewRoute(submission.id, reviewId)
const reviewUrl = getReviewUrl
? getReviewUrl(reviewId)
: getReviewRoute(submission.id, reviewId, canRespondToAppeals)
Copy link

@github-actions github-actions bot Dec 12, 2025

Choose a reason for hiding this comment

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

[❗❗ correctness]
The getReviewRoute function now takes an additional canRespondToAppeals parameter. Ensure that this function can handle the new parameter correctly and that it doesn't introduce any unintended side effects or errors.


return (
<Link
Expand Down
2 changes: 2 additions & 0 deletions src/apps/review/src/lib/components/common/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export interface ScoreVisibilityConfig {
isAppealsTab?: boolean
/** Optional function to build review detail URLs by ID. */
getReviewUrl?: (reviewId: string) => string
/** Indicates whether the viewer can appeal to respond. */
canRespondToAppeals?: boolean
}

export type { AggregatedReviewDetail, AggregatedSubmissionReviews }
5 changes: 4 additions & 1 deletion src/apps/review/src/lib/utils/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const CHALLENGE_DETAILS_SEGMENT = /(active-challenges|past-challenges)\/([^/]+)\
export function getReviewRoute(
submissionId: string,
reviewId: string,
addRespondToAppeals?: boolean,
currentPathname?: string,
): string {
const encodedReviewId = encodeURIComponent(reviewId)
Expand All @@ -26,7 +27,9 @@ export function getReviewRoute(
? `${prefix}/${match[1]}/${match[2]}`
: `/${match[1]}/${match[2]}`

return `${basePath}/reviews/${submissionId}?reviewId=${encodedReviewId}&respondToAppeals=true`
const respondToAppeals = addRespondToAppeals ? '&respondToAppeals=true' : ''

return `${basePath}/reviews/${submissionId}?reviewId=${encodedReviewId}${respondToAppeals}`
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/apps/review/src/pages/reviews/components/ReviewViewer/ReviewViewer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,12 @@ const ReviewViewer: FC = () => {
isSavingAppeal={isSavingAppeal}
isSavingAppealResponse={isSavingAppealResponse}
isSavingManagerComment={isSavingManagerComment}
canAddManagerComment={
hasChallengeAdminRole
|| hasTopcoderAdminRole
|| hasChallengeManagerRole
|| hasChallengeCopilotRole
}
saveReviewInfo={saveReviewInfo}
addAppeal={addAppeal}
addAppealResponse={addAppealResponse}
Expand Down
Loading