Fix reset of attributes when moving a task to a project (cvat-ai#5764)

- Fixed reset of attributes when moving a task to a project Steps to reproduce the bug: 1. Create a task with a couple of labels and attributes 2. Annotate several frames in the task. Attributes should have non-default value. 3. Create a project and copy the definition of labels with attributes 4. Move the task to the project. 5. Attributes should have default values after the action Co-authored-by: kirill-sizov <[email protected]>
retailnext · Jul 1, 2023 · d8e4698 · d8e4698
1 parent efa16d7
commit d8e4698
Show file tree

Hide file tree

Showing 16 changed files with 1,280 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -78,6 +78,7 @@ Tracks can be exported/imported to/from Datumaro and Sly Pointcloud formats (<ht
 - SiamMask and TransT serverless functions (<https://github.com/opencv/cvat/pull/5658>)
 - Сreating a project or task with the same labels (<https://github.com/opencv/cvat/pull/5700>)
 - \[Server API\] Ability to rename label to an existing name (<https://github.com/opencv/cvat/pull/5662>)
+- Moving a task to a project leads to reset of attributes (<https://github.com/opencv/cvat/pull/5764>)
 - Parsing skeleton sublabels containing spaces results in an error in dataset export (<https://github.com/opencv/cvat/pull/5794>)
 - Missing CVAT_BASE_URL in docker-compose.yml (<https://github.com/opencv/cvat/pull/5792>)
 

diff --git a/cvat-ui/src/components/move-task-modal/label-mapper-item.tsx b/cvat-ui/src/components/move-task-modal/label-mapper-item.tsx
@@ -6,14 +6,12 @@ import React from 'react';
 import { Col, Row } from 'antd/lib/grid';
 import Tag from 'antd/lib/tag';
 import Select from 'antd/lib/select';
-import Checkbox from 'antd/lib/checkbox';
 import { ArrowRightOutlined } from '@ant-design/icons';
 import CVATTooltip from 'components/common/cvat-tooltip';
 
 export interface LabelMapperItemValue {
     labelId: number;
     newLabelName: string | null;
-    clearAttributes: boolean;
 }
 
 export interface LabelMapperItemProps {
@@ -62,18 +60,6 @@ export default function LabelMapperItem(props: LabelMapperItemProps): JSX.Elemen
                         ))}
                 </Select>
             </Col>
-            <Col>
-                <Checkbox
-                    disabled
-                    checked={value.clearAttributes}
-                    onChange={(_value) => onChange({
-                        ...value,
-                        clearAttributes: _value.target.checked,
-                    })}
-                >
-                    Clear attributes
-                </Checkbox>
-            </Col>
         </Row>
     );
 }
diff --git a/cvat-ui/src/components/move-task-modal/move-task-modal.tsx b/cvat-ui/src/components/move-task-modal/move-task-modal.tsx
@@ -49,7 +49,6 @@ function MoveTaskModal({
                 labelValues[labelId] = {
                     labelId,
                     newLabelName: null,
-                    clearAttributes: true,
                 };
             });
         }
@@ -152,7 +151,6 @@ function MoveTaskModal({
                         labelValues[id] = {
                             labelId: label.labelId,
                             newLabelName: autoNewLabel ? autoNewLabel.name : null,
-                            clearAttributes: true,
                         };
                     });
                     setLabelMap(labelValues);

diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py
@@ -905,45 +905,51 @@ def update(self, instance, validated_data):
             project = models.Project.objects.get(id=validated_project_id)
             if project.tasks.count() and project.tasks.first().dimension != instance.dimension:
                 raise serializers.ValidationError(f'Dimension ({instance.dimension}) of the task must be the same as other tasks in project ({project.tasks.first().dimension})')
+
             if instance.project_id is None:
-                for old_label in instance.label_set.all():
-                    try:
-                        if old_label.parent:
-                            new_label = project.label_set.filter(name=old_label.name, parent__name=old_label.parent.name).first()
-                        else:
-                            new_label = project.label_set.filter(name=old_label.name).first()
-                    except ValueError:
-                        raise serializers.ValidationError(f'Target project does not have label with name "{old_label.name}"')
-                    old_label.attributespec_set.all().delete()
-                    for model in (models.LabeledTrack, models.LabeledShape, models.LabeledImage):
-                        model.objects.filter(job__segment__task=instance, label=old_label).update(
-                            label=new_label
-                        )
-                instance.label_set.all().delete()
+                label_set = instance.label_set.all()
             else:
-                for old_label in instance.project.label_set.all():
-                    new_label_for_name = list(filter(lambda x: x.get('id', None) == old_label.id, labels))
-                    if len(new_label_for_name):
-                        old_label.name = new_label_for_name[0].get('name', old_label.name)
-                    try:
-                        if old_label.parent:
-                            new_label = project.label_set.filter(name=old_label.name, parent__name=old_label.parent.name).first()
-                        else:
-                            new_label = project.label_set.filter(name=old_label.name).first()
-                    except ValueError:
-                        raise serializers.ValidationError(f'Target project does not have label with name "{old_label.name}"')
-                    for (model, attr, attr_name) in (
-                        (models.LabeledTrack, models.LabeledTrackAttributeVal, 'track'),
-                        (models.LabeledShape, models.LabeledShapeAttributeVal, 'shape'),
-                        (models.LabeledImage, models.LabeledImageAttributeVal, 'image')
+                label_set = instance.project.label_set.all()
+
+            for old_label in label_set:
+                new_label_for_name = list(filter(lambda x: x.get('id', None) == old_label.id, labels))
+                if len(new_label_for_name):
+                    old_label.name = new_label_for_name[0].get('name', old_label.name)
+                try:
+                    if old_label.parent:
+                        new_label = project.label_set.filter(name=old_label.name, parent__name=old_label.parent.name).first()
+                    else:
+                        new_label = project.label_set.filter(name=old_label.name).first()
+                except ValueError:
+                    raise serializers.ValidationError(f'Target project does not have label with name "{old_label.name}"')
+
+                for old_attr in old_label.attributespec_set.all():
+                    new_attr = new_label.attributespec_set.filter(name=old_attr.name,
+                                                                  values=old_attr.values,
+                                                                  input_type=old_attr.input_type).first()
+                    if new_attr is None:
+                        raise serializers.ValidationError('Target project does not have ' \
+                            f'"{old_label.name}" label with "{old_attr.name}" attribute')
+
+                    for (model, model_name) in (
+                        (models.LabeledTrackAttributeVal, 'track'),
+                        (models.LabeledShapeAttributeVal, 'shape'),
+                        (models.LabeledImageAttributeVal, 'image')
                     ):
-                        attr.objects.filter(**{
-                            f'{attr_name}__job__segment__task': instance,
-                            f'{attr_name}__label': old_label
-                        }).delete()
-                        model.objects.filter(job__segment__task=instance, label=old_label).update(
-                            label=new_label
-                        )
+                        model.objects.filter(**{
+                            f'{model_name}__job__segment__task': instance,
+                            f'{model_name}__label': old_label,
+                            'spec': old_attr
+                        }).update(spec=new_attr)
+
+                for model in (models.LabeledTrack, models.LabeledShape, models.LabeledImage):
+                    model.objects.filter(job__segment__task=instance, label=old_label).update(
+                        label=new_label
+                    )
+
+            if instance.project_id is None:
+                instance.label_set.all().delete()
+
             instance.project = project
 
         # update source and target storages

diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py
@@ -2395,7 +2395,14 @@ def setUpTestData(cls):
             "name": "Project for task move 1",
             "owner": cls.admin,
             "labels": [{
-                "name": "car"
+                "name": "car",
+                "attributes": [{
+                    "name": "color",
+                    "mutable": False,
+                    "input_type": AttributeType.SELECT,
+                    "default_value": "white",
+                    "values": ["white", "yellow", "green", "red"]
+                }]
             }, {
                 "name": "person"
             }]

diff --git a/tests/cypress/e2e/actions_projects_models/case_94_move_task_between_projects.js b/tests/cypress/e2e/actions_projects_models/case_94_move_task_between_projects.js
@@ -18,7 +18,7 @@ context('Move a task between projects.', () => {
         name: `Second project case ${caseID}`,
         label: 'bicycle',
         attrName: 'color',
-        attrVaue: 'yellow',
+        attrVaue: 'red',
         multiAttrParams: false,
     };
 

diff --git a/tests/cypress/e2e/actions_projects_models/case_95_move_task_to_project.js b/tests/cypress/e2e/actions_projects_models/case_95_move_task_to_project.js
@@ -15,9 +15,9 @@ context('Move a task to a project.', { browser: '!firefox' }, () => {
         attrName: 'Kind',
         attrValue: 'Oak',
         nameSecond: `Case ${caseID} second`,
-        labelSecond: 'Car',
-        attrNameSecons: 'Color',
-        attrValueSecond: 'Red',
+        labelSecond: 'Tree',
+        attrNameSecond: 'Kind',
+        attrValueSecond: 'Oak',
         name3d: `Case ${caseID} 3D`,
         label3d: 'Bus',
         attrName3d: 'Type',
@@ -51,7 +51,7 @@ context('Move a task to a project.', { browser: '!firefox' }, () => {
         cy.createZipArchive(directoryToArchive, archivePath);
         cy.goToTaskList();
         cy.createAnnotationTask(
-            task.nameSecond, task.labelSecond, task.attrNameSecons, task.attrValueSecond, archiveName,
+            task.nameSecond, task.labelSecond, task.attrNameSecond, task.attrValueSecond, archiveName,
         );
         cy.createAnnotationTask(task.name3d, task.label3d, task.attrName3d, task.attrValue3d, archiveName3d);
     });

diff --git a/tests/python/rest_api/test_tasks.py b/tests/python/rest_api/test_tasks.py
@@ -976,7 +976,7 @@ def test_can_rename_label(self, tasks, labels, admin_user):
         assert DeepDiff(resulting_labels, task_labels, ignore_order=True) == {}
 
     def test_cannot_rename_label_to_duplicate_name(self, tasks, labels, admin_user):
-        task = [t for t in tasks if t["labels"]["count"] > 1][0]
+        task = [t for t in tasks if t["project_id"] is None and t["labels"]["count"] > 1][0]
         task_labels = deepcopy([l for l in labels if l.get("task_id") == task["id"]])
         task_labels[0].update({"name": task_labels[1]["name"]})
 
@@ -1319,3 +1319,62 @@ def test_can_import_backup(self, fxt_task_with_unequal_jobs: Task):
         for old_job, new_job in zip(old_jobs, new_jobs):
             assert old_job.start_frame == new_job.start_frame
             assert old_job.stop_frame == new_job.stop_frame
+
+
+@pytest.mark.usefixtures("restore_db_per_function")
+class TestPatchTask:
+    @pytest.mark.parametrize("task_id, project_id, user", [(19, 12, "admin1")])
+    def test_move_task_to_project_with_attributes(self, task_id, project_id, user):
+        response = get_method(user, f"tasks/{task_id}/annotations")
+        assert response.status_code == HTTPStatus.OK
+        annotations = response.json()
+
+        response = patch_method(user, f"tasks/{task_id}", {"project_id": project_id})
+        assert response.status_code == HTTPStatus.OK
+
+        response = get_method(user, f"tasks/{task_id}")
+        assert response.status_code == HTTPStatus.OK
+        assert response.json().get("project_id") == project_id
+
+        response = get_method(user, f"tasks/{task_id}/annotations")
+        assert response.status_code == HTTPStatus.OK
+        assert (
+            DeepDiff(
+                annotations,
+                response.json(),
+                ignore_order=True,
+                exclude_regex_paths=[
+                    r"root\['\w+'\]\[\d+\]\['label_id'\]",
+                    r"root\['\w+'\]\[\d+\]\['attributes'\]\[\d+\]\['spec_id'\]",
+                ],
+            )
+            == {}
+        )
+
+    @pytest.mark.parametrize("task_id, project_id, user", [(20, 13, "admin1")])
+    def test_move_task_from_one_project_to_another_with_attributes(self, task_id, project_id, user):
+        response = get_method(user, f"tasks/{task_id}/annotations")
+        assert response.status_code == HTTPStatus.OK
+        annotations = response.json()
+
+        response = patch_method(user, f"tasks/{task_id}", {"project_id": project_id})
+        assert response.status_code == HTTPStatus.OK
+
+        response = get_method(user, f"tasks/{task_id}")
+        assert response.status_code == HTTPStatus.OK
+        assert response.json().get("project_id") == project_id
+
+        response = get_method(user, f"tasks/{task_id}/annotations")
+        assert response.status_code == HTTPStatus.OK
+        assert (
+            DeepDiff(
+                annotations,
+                response.json(),
+                ignore_order=True,
+                exclude_regex_paths=[
+                    r"root\['\w+'\]\[\d+\]\['label_id'\]",
+                    r"root\['\w+'\]\[\d+\]\['attributes'\]\[\d+\]\['spec_id'\]",
+                ],
+            )
+            == {}
+        )