From 1766eeec3202329583a1ea337f0cf1bd3bb9f420 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 9 Dec 2020 11:05:08 +0300 Subject: [PATCH 1/4] Fixed some not critical issues --- cvat-ui/src/actions/projects-actions.ts | 4 ++- cvat-ui/src/actions/review-actions.ts | 20 ++---------- cvat-ui/src/actions/tasks-actions.ts | 8 ++--- .../annotation-page/annotation-page.tsx | 6 ++-- .../src/components/task-page/task-page.tsx | 14 +++++---- cvat-ui/src/reducers/annotation-reducer.ts | 2 +- cvat-ui/src/reducers/review-reducer.ts | 9 ------ cvat-ui/src/reducers/tasks-reducer.ts | 31 +++++++++++++------ 8 files changed, 44 insertions(+), 50 deletions(-) diff --git a/cvat-ui/src/actions/projects-actions.ts b/cvat-ui/src/actions/projects-actions.ts index 2aa385db7093..d2794db3d028 100644 --- a/cvat-ui/src/actions/projects-actions.ts +++ b/cvat-ui/src/actions/projects-actions.ts @@ -141,9 +141,11 @@ export function updateProjectAsync(projectInstance: any): ThunkAction { dispatch(projectActions.updateProject()); await projectInstance.save(); const [project] = await cvat.projects.get({ id: projectInstance.id }); + // TODO: Check case when a project is not available anymore after update + // (assignee changes assignee and project is not public) dispatch(projectActions.updateProjectSuccess(project)); project.tasks.forEach((task: any) => { - dispatch(updateTaskSuccess(task)); + dispatch(updateTaskSuccess(task, task.id)); }); } catch (error) { let project = null; diff --git a/cvat-ui/src/actions/review-actions.ts b/cvat-ui/src/actions/review-actions.ts index 6cca02ecf59a..b433d96872fa 100644 --- a/cvat-ui/src/actions/review-actions.ts +++ b/cvat-ui/src/actions/review-actions.ts @@ -52,13 +52,7 @@ export const reviewActions = { reopenIssueSuccess: () => createAction(ReviewActionTypes.REOPEN_ISSUE_SUCCESS), reopenIssueFailed: (error: any) => createAction(ReviewActionTypes.REOPEN_ISSUE_FAILED, { error }), submitReview: (reviewId: number) => createAction(ReviewActionTypes.SUBMIT_REVIEW, { reviewId }), - submitReviewSuccess: (activeReview: any, reviews: any[], issues: any[], frame: number) => - createAction(ReviewActionTypes.SUBMIT_REVIEW_SUCCESS, { - activeReview, - reviews, - issues, - frame, - }), + submitReviewSuccess: () => createAction(ReviewActionTypes.SUBMIT_REVIEW_SUCCESS), submitReviewFailed: (error: any) => createAction(ReviewActionTypes.SUBMIT_REVIEW_FAILED, { error }), switchIssuesHiddenFlag: (hidden: boolean) => createAction(ReviewActionTypes.SWITCH_ISSUES_HIDDEN_FLAG, { hidden }), }; @@ -193,9 +187,6 @@ export const submitReviewAsync = (review: any): ThunkAction => async (dispatch, const { annotation: { job: { instance: jobInstance }, - player: { - frame: { number: frame }, - }, }, } = state; @@ -204,13 +195,8 @@ export const submitReviewAsync = (review: any): ThunkAction => async (dispatch, await review.submit(jobInstance.id); const [task] = await cvat.tasks.get({ id: jobInstance.task.id }); - dispatch(updateTaskSuccess(task)); - - const reviews = await jobInstance.reviews(); - const issues = await jobInstance.issues(); - const reviewInstance = new cvat.classes.Review({ job: jobInstance.id }); - - dispatch(reviewActions.submitReviewSuccess(reviewInstance, reviews, issues, frame)); + dispatch(updateTaskSuccess(task, jobInstance.task.id)); + dispatch(reviewActions.submitReviewSuccess()); } catch (error) { dispatch(reviewActions.submitReviewFailed(error)); } diff --git a/cvat-ui/src/actions/tasks-actions.ts b/cvat-ui/src/actions/tasks-actions.ts index 2be26d82d5bb..bd0fd45066b4 100644 --- a/cvat-ui/src/actions/tasks-actions.ts +++ b/cvat-ui/src/actions/tasks-actions.ts @@ -437,10 +437,10 @@ function updateTask(): AnyAction { return action; } -export function updateTaskSuccess(task: any): AnyAction { +export function updateTaskSuccess(task: any, taskID: number): AnyAction { const action = { type: TasksActionTypes.UPDATE_TASK_SUCCESS, - payload: { task }, + payload: { task, taskID }, }; return action; @@ -465,7 +465,7 @@ export function updateTaskAsync(taskInstance: any): ThunkAction, C const userFetching = getState().auth.fetching; if (!userFetching && nextUser && currentUser.username === nextUser.username) { const [task] = await cvat.tasks.get({ id: taskInstance.id }); - dispatch(updateTaskSuccess(task)); + dispatch(updateTaskSuccess(task, taskInstance.id)); } } catch (error) { // try abort all changes @@ -490,7 +490,7 @@ export function updateJobAsync(jobInstance: any): ThunkAction, {}, dispatch(updateTask()); await jobInstance.save(); const [task] = await cvat.tasks.get({ id: jobInstance.task.id }); - dispatch(updateTaskSuccess(task)); + dispatch(updateTaskSuccess(task, jobInstance.task.id)); } catch (error) { // try abort all changes let task = null; diff --git a/cvat-ui/src/components/annotation-page/annotation-page.tsx b/cvat-ui/src/components/annotation-page/annotation-page.tsx index 2ef039fa2fa1..062f83b6cc82 100644 --- a/cvat-ui/src/components/annotation-page/annotation-page.tsx +++ b/cvat-ui/src/components/annotation-page/annotation-page.tsx @@ -53,11 +53,13 @@ export default function AnnotationPageComponent(props: Props): JSX.Element { }; }, []); - if (job === null) { - if (!fetching) { + useEffect(() => { + if (job === null && !fetching) { getJob(); } + }, [job, fetching]); + if (job === null) { return ; } diff --git a/cvat-ui/src/components/task-page/task-page.tsx b/cvat-ui/src/components/task-page/task-page.tsx index 7249911cefb1..ac440600db5e 100644 --- a/cvat-ui/src/components/task-page/task-page.tsx +++ b/cvat-ui/src/components/task-page/task-page.tsx @@ -29,7 +29,13 @@ type Props = TaskPageComponentProps & RouteComponentProps<{ id: string }>; class TaskPageComponent extends React.PureComponent { public componentDidUpdate(): void { - const { deleteActivity, history } = this.props; + const { + deleteActivity, history, task, fetching, getTask, + } = this.props; + + if (task === null && !fetching) { + getTask(); + } if (deleteActivity) { history.replace('/tasks'); @@ -37,13 +43,9 @@ class TaskPageComponent extends React.PureComponent { } public render(): JSX.Element { - const { task, fetching, updating, getTask } = this.props; + const { task, updating } = this.props; if (task === null || updating) { - if (task === null && !fetching) { - getTask(); - } - return ; } diff --git a/cvat-ui/src/reducers/annotation-reducer.ts b/cvat-ui/src/reducers/annotation-reducer.ts index 7780eb9d3cc1..5b5b89cb60ed 100644 --- a/cvat-ui/src/reducers/annotation-reducer.ts +++ b/cvat-ui/src/reducers/annotation-reducer.ts @@ -202,7 +202,7 @@ export default (state = defaultState, action: AnyAction): AnnotationState => { fetching: false, }, }, - } + }; } case AnnotationActionTypes.CHANGE_FRAME: { return { diff --git a/cvat-ui/src/reducers/review-reducer.ts b/cvat-ui/src/reducers/review-reducer.ts index a8c6d32e0d94..919ea7bad54c 100644 --- a/cvat-ui/src/reducers/review-reducer.ts +++ b/cvat-ui/src/reducers/review-reducer.ts @@ -61,17 +61,8 @@ export default function (state: ReviewState = defaultState, action: any): Review }; } case ReviewActionTypes.SUBMIT_REVIEW_SUCCESS: { - const { - activeReview, reviews, issues, frame, - } = action.payload; - const frameIssues = computeFrameIssues(issues, activeReview, frame); - return { ...state, - activeReview, - reviews, - issues, - frameIssues, fetching: { ...state.fetching, reviewId: null, diff --git a/cvat-ui/src/reducers/tasks-reducer.ts b/cvat-ui/src/reducers/tasks-reducer.ts index 148d0a0eadf9..c68a5f793d24 100644 --- a/cvat-ui/src/reducers/tasks-reducer.ts +++ b/cvat-ui/src/reducers/tasks-reducer.ts @@ -84,9 +84,9 @@ export default (state: TasksState = defaultState, action: AnyAction): TasksState const { dumps } = state.activities; dumps[task.id] = - task.id in dumps && !dumps[task.id].includes(dumper.name) - ? [...dumps[task.id], dumper.name] - : dumps[task.id] || [dumper.name]; + task.id in dumps && !dumps[task.id].includes(dumper.name) ? + [...dumps[task.id], dumper.name] : + dumps[task.id] || [dumper.name]; return { ...state, @@ -122,9 +122,9 @@ export default (state: TasksState = defaultState, action: AnyAction): TasksState const { exports: activeExports } = state.activities; activeExports[task.id] = - task.id in activeExports && !activeExports[task.id].includes(exporter.name) - ? [...activeExports[task.id], exporter.name] - : activeExports[task.id] || [exporter.name]; + task.id in activeExports && !activeExports[task.id].includes(exporter.name) ? + [...activeExports[task.id], exporter.name] : + activeExports[task.id] || [exporter.name]; return { ...state, @@ -299,15 +299,26 @@ export default (state: TasksState = defaultState, action: AnyAction): TasksState }; } case TasksActionTypes.UPDATE_TASK_SUCCESS: { + // a task will be undefined after updating when a user doesn't have access to the task anymore + const { task, taskID } = action.payload; + + if (typeof task === 'undefined') { + return { + ...state, + updating: false, + current: state.current.filter((_task: Task): boolean => _task.instance.id !== taskID), + }; + } + return { ...state, updating: false, current: state.current.map( - (task): Task => { - if (task.instance.id === action.payload.task.id) { + (_task): Task => { + if (_task.instance.id === task.id) { return { - ...task, - instance: action.payload.task, + ..._task, + instance: task, }; } From b89ea5841fe5c674862e1ca90231014692054c41 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 9 Dec 2020 11:10:10 +0300 Subject: [PATCH 2/4] Updated version & changelog --- CHANGELOG.md | 1 + cvat-ui/package-lock.json | 2 +- cvat-ui/package.json | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 205b76579992..f47497a61f7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - TypeError: Cannot read property 'toString' of undefined () - Extra shapes are drawn after Esc, or G pressed while drawing a region in grouping () - Reset state (reviews, issues) after logout or changing a job () +- TypeError: Cannot read property 'id' of undefined when updating a task () ### Security diff --git a/cvat-ui/package-lock.json b/cvat-ui/package-lock.json index 866cc47fad00..f139b848ae3c 100644 --- a/cvat-ui/package-lock.json +++ b/cvat-ui/package-lock.json @@ -1,6 +1,6 @@ { "name": "cvat-ui", - "version": "1.12.0", + "version": "1.12.1", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/cvat-ui/package.json b/cvat-ui/package.json index b4e9637f56dc..e5bd2c6d4b93 100644 --- a/cvat-ui/package.json +++ b/cvat-ui/package.json @@ -1,6 +1,6 @@ { "name": "cvat-ui", - "version": "1.12.0", + "version": "1.12.1", "description": "CVAT single-page application", "main": "src/index.tsx", "scripts": { From cf99cd6086c852b851ae23c4b0227099416d63cd Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 9 Dec 2020 11:21:28 +0300 Subject: [PATCH 3/4] using DidMount instead of DidUpdate --- cvat-ui/src/components/projects-page/project-list.tsx | 2 +- cvat-ui/src/components/task-page/task-page.tsx | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/cvat-ui/src/components/projects-page/project-list.tsx b/cvat-ui/src/components/projects-page/project-list.tsx index 134286be3b0c..431f5da70f1c 100644 --- a/cvat-ui/src/components/projects-page/project-list.tsx +++ b/cvat-ui/src/components/projects-page/project-list.tsx @@ -42,7 +42,7 @@ export default function ProjectListComponent(): JSX.Element { {projectInstances.map( (row: any[]): JSX.Element => ( - + {row.map((instance: any) => ( diff --git a/cvat-ui/src/components/task-page/task-page.tsx b/cvat-ui/src/components/task-page/task-page.tsx index ac440600db5e..7537ce5a1460 100644 --- a/cvat-ui/src/components/task-page/task-page.tsx +++ b/cvat-ui/src/components/task-page/task-page.tsx @@ -28,14 +28,16 @@ interface TaskPageComponentProps { type Props = TaskPageComponentProps & RouteComponentProps<{ id: string }>; class TaskPageComponent extends React.PureComponent { - public componentDidUpdate(): void { - const { - deleteActivity, history, task, fetching, getTask, - } = this.props; + public componentDidMount(): void { + const { task, fetching, getTask } = this.props; if (task === null && !fetching) { getTask(); } + } + + public componentDidUpdate(): void { + const { deleteActivity, history } = this.props; if (deleteActivity) { history.replace('/tasks'); From 355afcba7bab24d517dca6df6f18327594c9790b Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 9 Dec 2020 11:30:45 +0300 Subject: [PATCH 4/4] Fixed reducer --- cvat-ui/src/reducers/tasks-reducer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cvat-ui/src/reducers/tasks-reducer.ts b/cvat-ui/src/reducers/tasks-reducer.ts index c68a5f793d24..2cc8db82e97d 100644 --- a/cvat-ui/src/reducers/tasks-reducer.ts +++ b/cvat-ui/src/reducers/tasks-reducer.ts @@ -322,7 +322,7 @@ export default (state: TasksState = defaultState, action: AnyAction): TasksState }; } - return task; + return _task; }, ), };