Merge pull request #2544 from openvinotoolkit/bs/fixes

Fixed: TypeError: Cannot read property 'id' of undefined

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 (<https://github.com/openvinotoolkit/cvat/pull/2517>)
 - Extra shapes are drawn after Esc, or G pressed while drawing a region in grouping (<https://github.com/openvinotoolkit/cvat/pull/2507>)
 - Reset state (reviews, issues) after logout or changing a job (<https://github.com/openvinotoolkit/cvat/pull/2525>)
+- TypeError: Cannot read property 'id' of undefined when updating a task (<https://github.com/openvinotoolkit/cvat/pull/2544>)
 
 ### Security
 

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": {

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": {

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;

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));
     }

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<Promise<void>, 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<Promise<void>, {},
             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;

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 <Spin size='large' className='cvat-spinner' />;
     }
 

cvat-ui/src/components/projects-page/project-list.tsx

@@ -42,7 +42,7 @@ export default function ProjectListComponent(): JSX.Element {
                 <Col className='cvat-projects-list' md={22} lg={18} xl={16} xxl={14}>
                     {projectInstances.map(
                         (row: any[]): JSX.Element => (
-                            <Row gutter={[8, 8]}>
+                            <Row key={row[0].id} gutter={[8, 8]}>
                                 {row.map((instance: any) => (
                                     <Col span={6} key={instance.id}>
                                         <ProjectItem projectInstance={instance} />

cvat-ui/src/components/task-page/task-page.tsx

@@ -28,6 +28,14 @@ interface TaskPageComponentProps {
 type Props = TaskPageComponentProps & RouteComponentProps<{ id: string }>;
 
 class TaskPageComponent extends React.PureComponent<Props> {
+    public componentDidMount(): void {
+        const { task, fetching, getTask } = this.props;
+
+        if (task === null && !fetching) {
+            getTask();
+        }
+    }
+
     public componentDidUpdate(): void {
         const { deleteActivity, history } = this.props;
 
@@ -37,13 +45,9 @@ class TaskPageComponent extends React.PureComponent<Props> {
     }
 
     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 <Spin size='large' className='cvat-spinner' />;
         }
 

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 {

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,

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,19 +299,30 @@ 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,
                             };
                         }
 
-                        return task;
+                        return _task;
                     },
                 ),
             };