Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reset of attributes when moving a task to a project #5764

Merged
merged 37 commits into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
4bd18a3
Fix TaskWriteSeralizer
yasakova-anastasia Feb 23, 2023
9b5be45
Merge branch 'develop' into ay/fix-task-transfer
yasakova-anastasia Feb 23, 2023
450f9b6
Fix error message
yasakova-anastasia Feb 23, 2023
3c82e87
Add test
yasakova-anastasia Feb 23, 2023
5407e23
Remove unused import
yasakova-anastasia Feb 24, 2023
0d3842d
Merge branch 'develop' into ay/fix-task-transfer
yasakova-anastasia Feb 24, 2023
0ab7a13
Update Changelog
yasakova-anastasia Feb 24, 2023
e5acd3a
Fix TaskWriteSerializer
yasakova-anastasia Feb 24, 2023
68cbc07
Add a test
yasakova-anastasia Feb 24, 2023
01b6e8a
Merge branch 'develop' into ay/fix-task-transfer
yasakova-anastasia Feb 24, 2023
feaa0c0
Small fix
yasakova-anastasia Feb 24, 2023
9e64b62
Fix tests
yasakova-anastasia Feb 27, 2023
bf3dcf3
Fix linters
yasakova-anastasia Feb 27, 2023
f38a70f
Merge branch 'develop' into ay/fix-task-transfer
yasakova-anastasia Feb 27, 2023
eb5c5b4
Fix e2e test
yasakova-anastasia Feb 27, 2023
549f6c0
Merge branch 'develop' into ay/fix-task-transfer
yasakova-anastasia Feb 27, 2023
d322c91
Fix e2e test
yasakova-anastasia Feb 27, 2023
ba8dca3
Merge branch 'develop' into ay/fix-task-transfer
yasakova-anastasia Feb 28, 2023
5605a53
Remove useless changes
yasakova-anastasia Feb 28, 2023
73f36f2
Add attribute checking for values and type
yasakova-anastasia Mar 1, 2023
8bf459b
Fix e2e test
yasakova-anastasia Mar 1, 2023
72c8d75
Fix assets
yasakova-anastasia Mar 1, 2023
a4b9544
Resolve conflicts
yasakova-anastasia Mar 6, 2023
12d5cbc
Remove 'Clear attributes' checkbox
yasakova-anastasia Mar 6, 2023
8af97c5
Remove code duplication
yasakova-anastasia Mar 6, 2023
26252b0
Merge remote-tracking branch 'remotes/origin/ay/fix-task-transfer' in…
yasakova-anastasia Mar 6, 2023
aab707c
debug helm tests
sizov-kirill Mar 7, 2023
fb2b332
rest api tests: wait for services first then init test db
sizov-kirill Mar 7, 2023
c387f18
helm tests: try to run tests before mounting files
sizov-kirill Mar 8, 2023
5fde235
fix cvat_data.tar.bz2
sizov-kirill Mar 8, 2023
19c9017
revert changes for cvat_data.tar.bz2
sizov-kirill Mar 9, 2023
1794cf9
remove cache from cvat_data.tar.bz2
sizov-kirill Mar 9, 2023
1ad0332
remove unecessary data from cvat_data.tar.bz2
sizov-kirill Mar 9, 2023
a2b3d4a
Resolve conflicts
yasakova-anastasia Mar 10, 2023
7aed610
remove cache from cvat_data.tar.bz2
sizov-kirill Mar 10, 2023
3b676ab
Merge branch 'develop' into ay/fix-task-transfer
yasakova-anastasia Mar 13, 2023
cd203ad
Remove useless changes
yasakova-anastasia Mar 13, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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>)

Expand Down
14 changes: 0 additions & 14 deletions cvat-ui/src/components/move-task-modal/label-mapper-item.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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>
);
}
2 changes: 0 additions & 2 deletions cvat-ui/src/components/move-task-modal/move-task-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ function MoveTaskModal({
labelValues[labelId] = {
labelId,
newLabelName: null,
clearAttributes: true,
};
});
}
Expand Down Expand Up @@ -152,7 +151,6 @@ function MoveTaskModal({
labelValues[id] = {
labelId: label.labelId,
newLabelName: autoNewLabel ? autoNewLabel.name : null,
clearAttributes: true,
};
});
setLabelMap(labelValues);
Expand Down
78 changes: 42 additions & 36 deletions cvat/apps/engine/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion cvat/apps/engine/tests/test_rest_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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);
});
Expand Down
61 changes: 60 additions & 1 deletion tests/python/rest_api/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]})

Expand Down Expand Up @@ -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'\]",
],
)
== {}
)
Loading