Allow admins the ability to reprocess F2F and TG Task submissions #1316
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| isLoadingBool: isReprocessingSubmissionBool, | ||
| doReprocessSubmission, | ||
| }: useManageSubmissionReprocessProps | ||
| = useManageSubmissionReprocess(submissionReprocessTopic) |
There was a problem hiding this comment.
[correctness]
The useManageSubmissionReprocess hook is called with submissionReprocessTopic, which is derived from challengeInfo. Ensure that challengeInfo is always in a valid state before using it to derive submissionReprocessTopic to prevent potential runtime errors.
| isReprocessingSubmission | ||
| } | ||
| doReprocessSubmission={doReprocessSubmission} | ||
| canReprocessSubmission={Boolean( |
There was a problem hiding this comment.
[correctness]
The canReprocessSubmission prop is determined by converting submissionReprocessTopic to a Boolean. Ensure that submissionReprocessTopic is correctly initialized and not undefined or null to avoid unexpected behavior.
| topic: 'avscan.action.scan', | ||
| }) | ||
|
|
||
| export const SUBMISSION_REPROCESS_TOPICS = { |
There was a problem hiding this comment.
[💡 maintainability]
Consider using a more descriptive name for SUBMISSION_REPROCESS_TOPICS to clarify its purpose and context, such as SUBMISSION_REPROCESS_KAFKA_TOPICS. This can improve maintainability by making the code's intent clearer to future developers.
| doPostBusEventAvScan: (submission: Submission) => void | ||
| isReprocessingSubmission: IsRemovingType | ||
| doReprocessSubmission: (submission: Submission) => void | ||
| canReprocessSubmission?: boolean |
There was a problem hiding this comment.
[correctness]
The canReprocessSubmission prop is marked as optional, but it's used directly without a default value or null check. Consider providing a default value or handling the case where it might be undefined to prevent potential runtime errors.
| AV Rescan | ||
| </Button> | ||
| )} | ||
| {props.canReprocessSubmission && ( |
There was a problem hiding this comment.
[❗❗ security]
The condition props.canReprocessSubmission is used to render the 'Reprocess submission' button. Ensure that canReprocessSubmission is correctly set to true only when the user has the necessary permissions to reprocess a submission. This is important for maintaining proper access control.
| props.doReprocessSubmission() | ||
| }} | ||
| primary | ||
| disabled={props.isReprocessingSubmission[props.data.id]} |
There was a problem hiding this comment.
[correctness]
The disabled attribute is set using props.isReprocessingSubmission[props.data.id]. Ensure that isReprocessingSubmission is correctly initialized and updated to prevent potential runtime errors if props.data.id is not present in the object.
|
|
||
| const doReprocessSubmission = useCallback( | ||
| async (submission: Submission) => { | ||
| if (!topic) { |
There was a problem hiding this comment.
[correctness]
The topic parameter is checked for truthiness, but there is no validation on its content. Consider validating topic to ensure it is a valid and expected value before proceeding with the reprocess logic.
|
|
||
| try { | ||
| const payload | ||
| = await createSubmissionReprocessPayload(submission) |
There was a problem hiding this comment.
[correctness]
The createSubmissionReprocessPayload function is awaited, which implies it returns a promise. Ensure that this function handles errors internally or consider wrapping it in a try-catch block to handle any potential errors that might occur during payload creation.
| submissionUrl: string | ||
| memberHandle: string | ||
| memberId: string | ||
| submittedDate: string |
There was a problem hiding this comment.
[correctness]
Consider using a more specific date type, such as Date, instead of string for submittedDate to ensure proper date handling and validation.
| export interface RequestBusAPIReprocess { | ||
| topic: string | ||
| originator: string | ||
| timestamp: string |
There was a problem hiding this comment.
[correctness]
Consider using a more specific date type, such as Date, instead of string for timestamp to ensure proper date handling and validation.
| topic: string | ||
| originator: string | ||
| timestamp: string | ||
| 'mime-type': string |
There was a problem hiding this comment.
[💡 style]
Consider using camelCase for mime-type to maintain consistency with JavaScript naming conventions.
| const memberHandle = submissionInfo.submitterHandle ?? submissionInfo.createdBy | ||
| const submittedDateValue = submissionInfo.submittedDate | ||
|
|
||
| if (!submissionId) { |
There was a problem hiding this comment.
[💡 readability]
Consider using a more descriptive error message, such as 'Submission ID is missing or invalid', to provide clearer context for debugging.
| ? challengeId.toString() | ||
| : '' | ||
| if (!normalizedChallengeId) { | ||
| throw new Error('Challenge id is not valid') |
There was a problem hiding this comment.
[💡 readability]
Consider using a more descriptive error message, such as 'Challenge ID is missing or invalid', to provide clearer context for debugging.
| } | ||
|
|
||
| if (!submissionUrl) { | ||
| throw new Error('Submission url is not valid') |
There was a problem hiding this comment.
[💡 readability]
Consider using a more descriptive error message, such as 'Submission URL is missing or invalid', to provide clearer context for debugging.
|
|
||
| const normalizedMemberId = memberId ? memberId.toString() : '' | ||
| if (!normalizedMemberId) { | ||
| throw new Error('Member id is not valid') |
There was a problem hiding this comment.
[💡 readability]
Consider using a more descriptive error message, such as 'Member ID is missing or invalid', to provide clearer context for debugging.
| } | ||
|
|
||
| if (!memberHandle) { | ||
| throw new Error('Member handle is not valid') |
There was a problem hiding this comment.
[💡 readability]
Consider using a more descriptive error message, such as 'Member handle is missing or invalid', to provide clearer context for debugging.
| ? submittedDate.toISOString() | ||
| : '' | ||
| if (!submittedDateIso) { | ||
| throw new Error('Submitted date is not valid') |
There was a problem hiding this comment.
[💡 readability]
Consider using a more descriptive error message, such as 'Submitted date is missing or invalid', to provide clearer context for debugging.
| } | ||
|
|
||
| return filter | ||
| return qs.stringify(params, { arrayFormat: 'brackets', skipNulls: true }) |
There was a problem hiding this comment.
[correctness]
Consider specifying the encode option in qs.stringify to ensure consistent encoding behavior across different environments. This can prevent potential issues with special characters in query parameters.
| export function getSubmissionReprocessTopic( | ||
| challenge?: Challenge, | ||
| ): string | undefined { | ||
| const normalizedType = challenge?.type?.name |
There was a problem hiding this comment.
[correctness]
The use of replace(/\s+/g, '') to normalize the challenge type name may lead to unexpected results if the challenge type contains multiple words separated by spaces. Consider using a more robust normalization approach if the challenge types are expected to have spaces.
Related to PS-448, PS-450