Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {
useManageBusEventProps,
useManageChallengeSubmissions,
useManageChallengeSubmissionsProps,
useManageSubmissionReprocess,
useManageSubmissionReprocessProps,
} from '../../lib/hooks'
import {
ActionLoading,
Expand All @@ -26,7 +28,7 @@ import {
TableLoading,
TableNoRecord,
} from '../../lib'
import { checkIsMM } from '../../lib/utils'
import { checkIsMM, getSubmissionReprocessTopic } from '../../lib/utils'

import styles from './ManageSubmissionPage.module.scss'

Expand All @@ -46,6 +48,10 @@ export const ManageSubmissionPage: FC<Props> = (props: Props) => {
challengeInfo,
}: useFetchChallengeProps = useFetchChallenge(challengeId)
const isMM = useMemo(() => checkIsMM(challengeInfo), [challengeInfo])
const submissionReprocessTopic = useMemo(

Choose a reason for hiding this comment

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

[💡 performance]
Using useMemo for submissionReprocessTopic might not be necessary if getSubmissionReprocessTopic is not computationally expensive. Consider removing useMemo if performance is not a concern here.

() => getSubmissionReprocessTopic(challengeInfo),
[challengeInfo],
)

const {
isLoading: isLoadingSubmission,
Expand All @@ -71,6 +77,12 @@ export const ManageSubmissionPage: FC<Props> = (props: Props) => {
isLoadingBool: isDoingAvScanBool,
doPostBusEvent: doPostBusEventAvScan,
}: useManageAVScanProps = useManageAVScan()
const {
isLoading: isReprocessingSubmission,
isLoadingBool: isReprocessingSubmissionBool,
doReprocessSubmission,
}: useManageSubmissionReprocessProps
= useManageSubmissionReprocess(submissionReprocessTopic)

Choose a reason for hiding this comment

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

[⚠️ correctness]
Ensure that submissionReprocessTopic is always a valid topic string before passing it to useManageSubmissionReprocess. If getSubmissionReprocessTopic can return an invalid or empty value, it might cause unexpected behavior.


const isLoading = isLoadingSubmission || isLoadingChallenge

Expand Down Expand Up @@ -111,13 +123,21 @@ export const ManageSubmissionPage: FC<Props> = (props: Props) => {
showSubmissionHistory={showSubmissionHistory}
setShowSubmissionHistory={setShowSubmissionHistory}
isMM={isMM}
isReprocessingSubmission={
isReprocessingSubmission
}
doReprocessSubmission={doReprocessSubmission}
canReprocessSubmission={Boolean(
submissionReprocessTopic,
)}
/>

{(isDoingAvScanBool
|| isDownloadingSubmissionBool
|| isRemovingSubmissionBool
|| isRunningTestBool
|| isRemovingReviewSummationsBool) && (
|| isRemovingReviewSummationsBool
|| isReprocessingSubmissionBool) && (
<ActionLoading />
)}
</div>
Expand Down
25 changes: 25 additions & 0 deletions src/apps/admin/src/config/busEvent.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
RequestBusAPI,
RequestBusAPIAVScan,
RequestBusAPIAVScanPayload,
RequestBusAPIReprocess,
SubmissionReprocessPayload,
} from '../lib/models'

/**
Expand Down Expand Up @@ -47,3 +49,26 @@ export const CREATE_BUS_EVENT_AV_RESCAN = (
.toISOString(),
topic: 'avscan.action.scan',
})

export const SUBMISSION_REPROCESS_TOPICS = {
FIRST2FINISH: 'first2finish.submission.received',
TOPGEAR_TASK: 'topgear.submission.received',
}

/**
* Create data for submission reprocess bus event
* @param topic kafka topic
* @param payload submission data
* @returns data for bus event
*/
export const CREATE_BUS_EVENT_REPROCESS_SUBMISSION = (
topic: string,
payload: SubmissionReprocessPayload,
): RequestBusAPIReprocess => ({
'mime-type': 'application/json',
originator: 'review-api-v6',
payload,
timestamp: new Date()

Choose a reason for hiding this comment

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

[💡 performance]
Consider using Date.now() for generating the timestamp to avoid creating a new Date object unnecessarily. This can slightly improve performance by reducing object creation overhead.

.toISOString(),
topic,
})
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ interface Props {
downloadSubmission: (submissionId: string) => void
isDoingAvScan: IsRemovingType
doPostBusEventAvScan: (submission: Submission) => void
isReprocessingSubmission: IsRemovingType
doReprocessSubmission: (submission: Submission) => void
canReprocessSubmission?: boolean
}

export const SubmissionTable: FC<Props> = (props: Props) => {
Expand Down Expand Up @@ -229,6 +232,15 @@ export const SubmissionTable: FC<Props> = (props: Props) => {
doPostBusEventAvScan={function doPostBusEventAvScan() {
props.doPostBusEventAvScan(data)
}}
canReprocessSubmission={
props.canReprocessSubmission
}
isReprocessingSubmission={
props.isReprocessingSubmission
}
doReprocessSubmission={function doReprocessSubmission() {

Choose a reason for hiding this comment

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

[⚠️ performance]
Consider using an arrow function for doReprocessSubmission to maintain consistency with other function props and avoid unnecessary function creation on each render.

props.doReprocessSubmission(data)
}}
isDownloading={props.isDownloading}
downloadSubmission={function downloadSubmission() {
props.downloadSubmission(data.id)
Expand Down Expand Up @@ -258,6 +270,9 @@ export const SubmissionTable: FC<Props> = (props: Props) => {
props.downloadSubmission,
props.isDoingAvScan,
props.doPostBusEventAvScan,
props.isReprocessingSubmission,
props.doReprocessSubmission,
props.canReprocessSubmission,
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ interface Props {
downloadSubmission: () => void
isDoingAvScan: IsRemovingType
doPostBusEventAvScan: () => void
canReprocessSubmission?: boolean
isReprocessingSubmission: IsRemovingType
doReprocessSubmission: () => void
}

export const SubmissionTableActionsNonMM: FC<Props> = (props: Props) => (
Expand All @@ -39,6 +42,17 @@ export const SubmissionTableActionsNonMM: FC<Props> = (props: Props) => (
AV Rescan
</Button>
)}
{props.canReprocessSubmission && (

Choose a reason for hiding this comment

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

[⚠️ correctness]
The canReprocessSubmission prop is optional, but its usage here assumes it is always defined. Consider providing a default value or handling the case where it might be undefined to avoid potential runtime errors.

<Button
onClick={function onClick() {
props.doReprocessSubmission()
}}
primary
disabled={props.isReprocessingSubmission[props.data.id]}

Choose a reason for hiding this comment

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

[⚠️ correctness]
Ensure that props.data.id is always a valid key in isReprocessingSubmission. If props.data.id can be undefined or not present in isReprocessingSubmission, this could lead to unexpected behavior.

>
Reprocess submission
</Button>
)}
</div>
)

Expand Down
1 change: 1 addition & 0 deletions src/apps/admin/src/lib/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,4 @@ export * from './useAutoScrollTopWhenInit'
export * from './useFetchChallenge'
export * from './useDownloadSubmission'
export * from './useManageAVScan'
export * from './useManageSubmissionReprocess'
78 changes: 78 additions & 0 deletions src/apps/admin/src/lib/hooks/useManageSubmissionReprocess.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/**
* Manage submission reprocess event
*/
import { useCallback, useMemo, useState } from 'react'
import { toast } from 'react-toastify'
import _ from 'lodash'

import {
CREATE_BUS_EVENT_REPROCESS_SUBMISSION,
} from '../../config/busEvent.config'
import { createSubmissionReprocessPayload, reqToBusAPI } from '../services'
import { handleError } from '../utils'
import { IsRemovingType, Submission } from '../models'

export interface useManageSubmissionReprocessProps {
isLoading: IsRemovingType
isLoadingBool: boolean
doReprocessSubmission: (submission: Submission) => Promise<void>
}

/**
* Manage submission reprocess
*/
export function useManageSubmissionReprocess(
topic?: string,
): useManageSubmissionReprocessProps {
const [isLoading, setIsLoading] = useState<IsRemovingType>({})
const isLoadingBool = useMemo(
() => _.some(isLoading, value => value === true),
[isLoading],
)

const doReprocessSubmission = useCallback(
async (submission: Submission) => {
if (!topic) {

Choose a reason for hiding this comment

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

[⚠️ design]
The topic parameter is used to determine if the reprocess action should proceed. Consider validating topic earlier or at the point of function call to prevent unnecessary function execution.

toast.error(
'Submission reprocess is only available for specific challenge types.',
{
toastId: 'Reprocess submission',

Choose a reason for hiding this comment

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

[💡 maintainability]
The toastId is hardcoded as 'Reprocess submission'. If multiple toasts are triggered simultaneously, consider using a unique identifier to prevent potential conflicts.

},
)
return
}

setIsLoading(previous => ({
...previous,
[submission.id]: true,
}))

try {
const payload
= await createSubmissionReprocessPayload(submission)

Choose a reason for hiding this comment

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

[❗❗ correctness]
The createSubmissionReprocessPayload function is awaited, which implies it returns a promise. Ensure this function handles errors internally or consider wrapping it in a try-catch block to handle potential rejections.

const data = CREATE_BUS_EVENT_REPROCESS_SUBMISSION(
topic,
payload,
)
await reqToBusAPI(data)
toast.success('Reprocess submission request sent', {
toastId: 'Reprocess submission',
})
} catch (error) {
handleError(error as Error)
} finally {
setIsLoading(previous => ({
...previous,
[submission.id]: false,
}))
}
},
[topic],
)

return {
doReprocessSubmission,
isLoading,
isLoadingBool,
}
}
5 changes: 4 additions & 1 deletion src/apps/admin/src/lib/models/CommonRequestBusAPI.type.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { RequestBusAPI } from './RequestBusAPI.model'
import { RequestBusAPIAVScan } from './RequestBusAPIAVScan.model'
import { RequestBusAPIReprocess } from './RequestBusAPIReprocess.model'

/**
* Common type for bus api request
*/
export type CommonRequestBusAPI = RequestBusAPI | RequestBusAPIAVScan
export type CommonRequestBusAPI = RequestBusAPI
| RequestBusAPIAVScan
| RequestBusAPIReprocess
19 changes: 19 additions & 0 deletions src/apps/admin/src/lib/models/RequestBusAPIReprocess.model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Request to reprocess submission bus api
*/
export interface SubmissionReprocessPayload {
submissionId: string
challengeId: string
submissionUrl: string
memberHandle: string
memberId: string
submittedDate: string

Choose a reason for hiding this comment

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

[⚠️ correctness]
The submittedDate field is typed as string. Consider using a more specific type like Date or a validated date string format to ensure proper handling of date values.

}

export interface RequestBusAPIReprocess {
topic: string
originator: string
timestamp: string

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider using a more specific type for timestamp such as Date or a validated date string format to ensure consistency and prevent potential parsing errors.

'mime-type': string

Choose a reason for hiding this comment

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

[💡 style]
The property name 'mime-type' is using a string literal which could lead to potential issues with property access. Consider using camelCase mimeType for consistency with TypeScript conventions.

payload: SubmissionReprocessPayload
}
1 change: 1 addition & 0 deletions src/apps/admin/src/lib/models/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export * from './RoleMemberInfo.model'
export * from './Submission.model'
export * from './RequestBusAPI.model'
export * from './RequestBusAPIAVScan.model'
export * from './RequestBusAPIReprocess.model'
export * from './MemberSubmission.model'
export * from './SubmissionReviewSummation.model'
export * from './SSOUserLogin.model'
Expand Down
60 changes: 60 additions & 0 deletions src/apps/admin/src/lib/services/submissions.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
MemberSubmission,
RequestBusAPIAVScanPayload,
Submission,
SubmissionReprocessPayload,
ValidateS3URIResult,
} from '../models'
import { validateS3URI } from '../utils'
Expand Down Expand Up @@ -125,3 +126,62 @@ export const createAvScanSubmissionPayload = async (
url,
}
}

/**
* Create submission reprocess payload
* @param submissionInfo submission info
* @returns resolves to the reprocess payload
*/
export const createSubmissionReprocessPayload = async (
submissionInfo: Submission,
): Promise<SubmissionReprocessPayload> => {
const submissionId = submissionInfo.id
const challengeId = submissionInfo.challengeId
const submissionUrl = submissionInfo.url
const memberId = submissionInfo.memberId
const memberHandle = submissionInfo.submitterHandle ?? submissionInfo.createdBy

Choose a reason for hiding this comment

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

[⚠️ correctness]
The use of submissionInfo.submitterHandle ?? submissionInfo.createdBy assumes that createdBy is a valid fallback for submitterHandle. Ensure that createdBy is always a valid and appropriate substitute, or consider handling cases where both might be undefined.

const submittedDateValue = submissionInfo.submittedDate

if (!submissionId) {
throw new Error('Submission id is not valid')
}

const normalizedChallengeId = challengeId
? challengeId.toString()
: ''
if (!normalizedChallengeId) {
throw new Error('Challenge id is not valid')
}

if (!submissionUrl) {
throw new Error('Submission url is not valid')
}

const normalizedMemberId = memberId ? memberId.toString() : ''
if (!normalizedMemberId) {
throw new Error('Member id is not valid')
}

if (!memberHandle) {
throw new Error('Member handle is not valid')
}

const submittedDate = submittedDateValue

Choose a reason for hiding this comment

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

[⚠️ correctness]
The conversion of submittedDateValue to a Date object and subsequent validation could fail silently if submittedDateValue is an invalid date string. Consider adding explicit error handling for invalid date formats to prevent unexpected behavior.

? new Date(submittedDateValue)
: undefined
const submittedDateIso = submittedDate && !Number.isNaN(submittedDate.valueOf())
? submittedDate.toISOString()
: ''
if (!submittedDateIso) {
throw new Error('Submitted date is not valid')
}

return {
challengeId: normalizedChallengeId,
memberHandle,
memberId: normalizedMemberId,
submissionId,
submissionUrl,
submittedDate: submittedDateIso,
}
}
Loading
Loading