From f2fb053bb65a73b9377db322c52fab87f8ab2517 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Tue, 1 Dec 2020 14:47:49 +0300 Subject: [PATCH] Simple Review Pipeline (Server) (#2338) --- cvat/apps/authentication/auth.py | 119 ++++++++-- .../documentation/AWS-Deployment-Guide.md | 3 +- .../migrations/0034_auto_20201125_1426.py | 74 ++++++ cvat/apps/engine/models.py | 41 ++++ cvat/apps/engine/serializers.py | 75 +++++- cvat/apps/engine/signals.py | 8 + cvat/apps/engine/tests/test_rest_api.py | 221 ++++++++++++++++++ cvat/apps/engine/urls.py | 3 + cvat/apps/engine/views.py | 178 +++++++++++++- 9 files changed, 691 insertions(+), 31 deletions(-) create mode 100644 cvat/apps/engine/migrations/0034_auto_20201125_1426.py diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 349df39b..5e19efb7 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -91,6 +91,11 @@ def is_project_annotator(db_user, db_project): db_tasks = list(db_project.tasks.prefetch_related('segment_set').all()) return any([is_task_annotator(db_user, db_task) for db_task in db_tasks]) +@rules.predicate +def is_project_reviewer(db_user, db_project): + db_tasks = list(db_project.tasks.prefetch_related('segment_set').all()) + return any([is_task_reviewer(db_user, db_task) for db_task in db_tasks]) + @rules.predicate def is_task_owner(db_user, db_task): # If owner is None (null) the task can be accessed/changed/deleted @@ -101,6 +106,12 @@ def is_task_owner(db_user, db_task): def is_task_assignee(db_user, db_task): return db_task.assignee == db_user or is_project_assignee(db_user, db_task.project) +@rules.predicate +def is_task_reviewer(db_user, db_task): + db_segments = list(db_task.segment_set.prefetch_related('job_set__assignee').all()) + return any([is_job_reviewer(db_user, db_job) + for db_segment in db_segments for db_job in db_segment.job_set.all()]) + @rules.predicate def is_task_annotator(db_user, db_task): db_segments = list(db_task.segment_set.prefetch_related('job_set__assignee').all()) @@ -121,6 +132,33 @@ def is_job_annotator(db_user, db_job): return has_rights +@rules.predicate +def has_change_permissions(db_user, db_job): + db_task = db_job.segment.task + # A job can be annotated by any user if the task's assignee is None. + has_rights = (db_task.assignee is None and not settings.RESTRICTIONS['reduce_task_visibility']) or is_task_assignee(db_user, db_task) + if db_job.assignee is not None: + has_rights |= (db_user == db_job.assignee) and (db_job.status == 'annotation') + if db_job.reviewer is not None: + has_rights |= (db_user == db_job.reviewer) and (db_job.status == 'validation') + + return has_rights + +@rules.predicate +def is_job_reviewer(db_user, db_job): + has_rights = db_job.reviewer == db_user + return has_rights + +@rules.predicate +def is_issue_owner(db_user, db_issue): + has_rights = db_issue.owner == db_user + return has_rights + +@rules.predicate +def is_comment_author(db_user, db_comment): + has_rights = (db_comment.author == db_user) + return has_rights + # AUTH PERMISSIONS RULES rules.add_perm('engine.role.user', has_user_role) rules.add_perm('engine.role.admin', has_admin_role) @@ -136,65 +174,71 @@ rules.add_perm('engine.project.delete', has_admin_role | is_project_owner) rules.add_perm('engine.task.create', has_admin_role | has_user_role) rules.add_perm('engine.task.access', has_admin_role | has_observer_role | - is_task_owner | is_task_annotator) + is_task_owner | is_task_annotator | is_task_reviewer) rules.add_perm('engine.task.change', has_admin_role | is_task_owner | is_task_assignee) rules.add_perm('engine.task.delete', has_admin_role | is_task_owner) rules.add_perm('engine.job.access', has_admin_role | has_observer_role | - is_job_owner | is_job_annotator) -rules.add_perm('engine.job.change', has_admin_role | is_job_owner | - is_job_annotator) + is_job_owner | is_job_annotator | is_job_reviewer) +rules.add_perm('engine.job.change', has_admin_role | is_job_owner | has_change_permissions) +rules.add_perm('engine.job.review', has_admin_role | (is_job_reviewer & has_change_permissions)) + +rules.add_perm('engine.issue.change', has_admin_role | is_issue_owner) +rules.add_perm('engine.issue.destroy', has_admin_role | is_issue_owner) + +rules.add_perm('engine.comment.change', has_admin_role | is_comment_author) + class AdminRolePermission(BasePermission): # pylint: disable=no-self-use def has_permission(self, request, view): - return request.user.has_perm("engine.role.admin") + return request.user.has_perm('engine.role.admin') class UserRolePermission(BasePermission): # pylint: disable=no-self-use def has_permission(self, request, view): - return request.user.has_perm("engine.role.user") + return request.user.has_perm('engine.role.user') class AnnotatorRolePermission(BasePermission): # pylint: disable=no-self-use def has_permission(self, request, view): - return request.user.has_perm("engine.role.annotator") + return request.user.has_perm('engine.role.annotator') class ObserverRolePermission(BasePermission): # pylint: disable=no-self-use def has_permission(self, request, view): - return request.user.has_perm("engine.role.observer") + return request.user.has_perm('engine.role.observer') class ProjectCreatePermission(BasePermission): # pylint: disable=no-self-use def has_permission(self, request, view): - return request.user.has_perm("engine.project.create") + return request.user.has_perm('engine.project.create') class ProjectAccessPermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - return request.user.has_perm("engine.project.access", obj) + return request.user.has_perm('engine.project.access', obj) class ProjectChangePermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - return request.user.has_perm("engine.project.change", obj) + return request.user.has_perm('engine.project.change', obj) class ProjectDeletePermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - return request.user.has_perm("engine.project.delete", obj) + return request.user.has_perm('engine.project.delete', obj) class TaskCreatePermission(BasePermission): # pylint: disable=no-self-use def has_permission(self, request, view): - return request.user.has_perm("engine.task.create") + return request.user.has_perm('engine.task.create') class TaskAccessPermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - return request.user.has_perm("engine.task.access", obj) + return request.user.has_perm('engine.task.access', obj) class ProjectGetQuerySetMixin(object): @@ -207,7 +251,8 @@ class ProjectGetQuerySetMixin(object): else: return queryset.filter(Q(owner=user) | Q(assignee=user) | Q(task__owner=user) | Q(task__assignee=user) | - Q(task__segment__job__assignee=user)).distinct() + Q(task__segment__job__assignee=user) | + Q(task__segment__job__reviewer=user)).distinct() def filter_task_queryset(queryset, user): # Don't filter queryset for admin, observer @@ -215,7 +260,7 @@ def filter_task_queryset(queryset, user): return queryset query_filter = Q(owner=user) | Q(assignee=user) | \ - Q(segment__job__assignee=user) + Q(segment__job__assignee=user) | Q(segment__job__reviewer=user) if not settings.RESTRICTIONS['reduce_task_visibility']: query_filter |= Q(assignee=None) @@ -234,19 +279,53 @@ class TaskGetQuerySetMixin(object): class TaskChangePermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - return request.user.has_perm("engine.task.change", obj) + return request.user.has_perm('engine.task.change', obj) class TaskDeletePermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - return request.user.has_perm("engine.task.delete", obj) + return request.user.has_perm('engine.task.delete', obj) class JobAccessPermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - return request.user.has_perm("engine.job.access", obj) + return request.user.has_perm('engine.job.access', obj) class JobChangePermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - return request.user.has_perm("engine.job.change", obj) + return request.user.has_perm('engine.job.change', obj) + +class JobReviewPermission(BasePermission): + # pylint: disable=no-self-use + def has_object_permission(self, request, view, obj): + return request.user.has_perm('engine.job.review', obj) + +class IssueAccessPermission(BasePermission): + # pylint: disable=no-self-use + def has_object_permission(self, request, view, obj): + db_job = obj.job + return request.user.has_perm('engine.job.access', db_job) + +class IssueDestroyPermission(BasePermission): + # pylint: disable=no-self-use + def has_object_permission(self, request, view, obj): + return request.user.has_perm('engine.issue.destroy', obj) + +class IssueChangePermission(BasePermission): + # pylint: disable=no-self-use + def has_object_permission(self, request, view, obj): + db_job = obj.job + return (request.user.has_perm('engine.job.change', db_job) + or request.user.has_perm('engine.issue.change', obj)) + +class CommentCreatePermission(BasePermission): + # pylint: disable=no-self-use + def has_object_permission(self, request, view, obj): # obj is db_job + return request.user.has_perm('engine.job.access', obj) + +class CommentChangePermission(BasePermission): + # pylint: disable=no-self-use + def has_object_permission(self, request, view, obj): + return request.user.has_perm('engine.comment.change', obj) + diff --git a/cvat/apps/documentation/AWS-Deployment-Guide.md b/cvat/apps/documentation/AWS-Deployment-Guide.md index 7e354f79..6e0a03d3 100644 --- a/cvat/apps/documentation/AWS-Deployment-Guide.md +++ b/cvat/apps/documentation/AWS-Deployment-Guide.md @@ -18,4 +18,5 @@ services: environment: CVAT_HOST: your-instance.amazonaws.com ``` -In case of problems with using hostname, you can also use the public IPV4 instead of hostname. For AWS or any cloud based machines where the instances need to be terminated or stopped, the public IPV4 and hostname changes with every stop and reboot. To address this efficiently, avoid using spot instances that cannot be stopped, since copying the EBS to an AMI and restarting it throws problems. On the other hand, when a regular instance is stopped and restarted, the new hostname/IPV4 can be used in the `CVAT_HOST` variable in the `docker-compose.override.yml` and the build can happen instantly with CVAT tasks being available through the new IPV4. + +In case of problems with using hostname, you can also use the public IPV4 instead of hostname. For AWS or any cloud based machines where the instances need to be terminated or stopped, the public IPV4 and hostname changes with every stop and reboot. To address this efficiently, avoid using spot instances that cannot be stopped, since copying the EBS to an AMI and restarting it throws problems. On the other hand, when a regular instance is stopped and restarted, the new hostname/IPV4 can be used in the `CVAT_HOST` variable in the `docker-compose.override.yml` and the build can happen instantly with CVAT tasks being available through the new IPV4. diff --git a/cvat/apps/engine/migrations/0034_auto_20201125_1426.py b/cvat/apps/engine/migrations/0034_auto_20201125_1426.py new file mode 100644 index 00000000..457861a3 --- /dev/null +++ b/cvat/apps/engine/migrations/0034_auto_20201125_1426.py @@ -0,0 +1,74 @@ +# Generated by Django 3.1.1 on 2020-11-25 14:26 + +import cvat.apps.engine.models +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + +def create_profile(apps, schema_editor): + User = apps.get_model('auth', 'User') + Profile = apps.get_model('engine', 'Profile') + for user in User.objects.all(): + profile = Profile() + profile.user = user + profile.save() + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('engine', '0033_projects_adjastment'), + ] + + operations = [ + migrations.AddField( + model_name='job', + name='reviewer', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='review_job_set', to=settings.AUTH_USER_MODEL), + ), + migrations.CreateModel( + name='Review', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('estimated_quality', models.FloatField()), + ('status', models.CharField(choices=[('accepted', 'ACCEPTED'), ('rejected', 'REJECTED'), ('review_further', 'REVIEW_FURTHER')], max_length=16)), + ('assignee', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='reviewed', to=settings.AUTH_USER_MODEL)), + ('job', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='engine.job')), + ('reviewer', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='reviews', to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.CreateModel( + name='Profile', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('rating', models.FloatField(default=0.0)), + ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.CreateModel( + name='Issue', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('frame', models.PositiveIntegerField()), + ('position', cvat.apps.engine.models.FloatArrayField()), + ('created_date', models.DateTimeField(auto_now_add=True)), + ('resolved_date', models.DateTimeField(blank=True, null=True)), + ('job', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='engine.job')), + ('owner', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='issues', to=settings.AUTH_USER_MODEL)), + ('resolver', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='resolved_issues', to=settings.AUTH_USER_MODEL)), + ('review', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='engine.review')), + ], + ), + migrations.CreateModel( + name='Comment', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('message', models.TextField(default='')), + ('created_date', models.DateTimeField(auto_now_add=True)), + ('updated_date', models.DateTimeField(auto_now=True)), + ('author', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)), + ('issue', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='engine.issue')), + ], + ), + migrations.RunPython(create_profile), + ] diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index 623b9d2e..09037191 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -263,6 +263,7 @@ class Segment(models.Model): class Job(models.Model): segment = models.ForeignKey(Segment, on_delete=models.CASCADE) assignee = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL) + reviewer = models.ForeignKey(User, null=True, blank=True, related_name='review_job_set', on_delete=models.SET_NULL) status = models.CharField(max_length=32, choices=StatusChoice.choices(), default=StatusChoice.ANNOTATION) @@ -347,6 +348,18 @@ class SourceType(str, Enum): def __str__(self): return self.value +class ReviewStatus(str, Enum): + ACCEPTED = 'accepted' + REJECTED = 'rejected' + REVIEW_FURTHER = 'review_further' + + @classmethod + def choices(self): + return tuple((x.value, x.name) for x in self) + + def __str__(self): + return self.value + class Annotation(models.Model): id = models.BigAutoField(primary_key=True) job = models.ForeignKey(Job, on_delete=models.CASCADE) @@ -427,3 +440,31 @@ class TrackedShape(Shape): class TrackedShapeAttributeVal(AttributeVal): shape = models.ForeignKey(TrackedShape, on_delete=models.CASCADE) + +class Profile(models.Model): + user = models.OneToOneField(User, on_delete=models.CASCADE) + rating = models.FloatField(default=0.0) + +class Review(models.Model): + job = models.ForeignKey(Job, on_delete=models.CASCADE) + reviewer = models.ForeignKey(User, null=True, blank=True, related_name='reviews', on_delete=models.SET_NULL) + assignee = models.ForeignKey(User, null=True, blank=True, related_name='reviewed', on_delete=models.SET_NULL) + estimated_quality = models.FloatField() + status = models.CharField(max_length=16, choices=ReviewStatus.choices()) + +class Issue(models.Model): + frame = models.PositiveIntegerField() + position = FloatArrayField() + job = models.ForeignKey(Job, on_delete=models.CASCADE) + review = models.ForeignKey(Review, null=True, blank=True, on_delete=models.SET_NULL) + owner = models.ForeignKey(User, null=True, blank=True, related_name='issues', on_delete=models.SET_NULL) + resolver = models.ForeignKey(User, null=True, blank=True, related_name='resolved_issues', on_delete=models.SET_NULL) + created_date = models.DateTimeField(auto_now_add=True) + resolved_date = models.DateTimeField(null=True, blank=True) + +class Comment(models.Model): + issue = models.ForeignKey(Issue, on_delete=models.CASCADE) + author = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL) + message = models.TextField(default='') + created_date = models.DateTimeField(auto_now_add=True) + updated_date = models.DateTimeField(auto_now=True) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 7fe7c853..1ec73871 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -126,19 +126,25 @@ class JobSerializer(serializers.ModelSerializer): stop_frame = serializers.ReadOnlyField(source="segment.stop_frame") assignee = BasicUserSerializer(allow_null=True, required=False) assignee_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) + reviewer = BasicUserSerializer(allow_null=True, required=False) + reviewer_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) class Meta: model = models.Job - fields = ('url', 'id', 'assignee', 'assignee_id', 'status', 'start_frame', - 'stop_frame', 'task_id') + fields = ('url', 'id', 'assignee', 'assignee_id', 'reviewer', + 'reviewer_id', 'status', 'start_frame', 'stop_frame', 'task_id') + read_only_fields = ('assignee', 'reviewer') class SimpleJobSerializer(serializers.ModelSerializer): assignee = BasicUserSerializer(allow_null=True) assignee_id = serializers.IntegerField(write_only=True, allow_null=True) + reviewer = BasicUserSerializer(allow_null=True, required=False) + reviewer_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) class Meta: model = models.Job - fields = ('url', 'id', 'assignee', 'assignee_id', 'status') + fields = ('url', 'id', 'assignee', 'assignee_id', 'reviewer', 'reviewer_id', 'status') + read_only_fields = ('assignee', 'reviewer') class SegmentSerializer(serializers.ModelSerializer): jobs = SimpleJobSerializer(many=True, source='job_set') @@ -328,7 +334,7 @@ class TaskSerializer(WriteOnceMixin, serializers.ModelSerializer): 'bug_tracker', 'created_date', 'updated_date', 'overlap', 'segment_size', 'status', 'labels', 'segments', 'data_chunk_size', 'data_compressed_chunk_type', 'data_original_chunk_type', 'size', 'image_quality', 'data') - read_only_fields = ('mode', 'created_date', 'updated_date', 'status', 'data_chunk_size', 'owner', 'asignee', + read_only_fields = ('mode', 'created_date', 'updated_date', 'status', 'data_chunk_size', 'owner', 'assignee', 'data_compressed_chunk_type', 'data_original_chunk_type', 'size', 'image_quality', 'data') write_once_fields = ('overlap', 'segment_size', 'project_id') ordering = ['-id'] @@ -579,3 +585,64 @@ class LogEventSerializer(serializers.Serializer): class AnnotationFileSerializer(serializers.Serializer): annotation_file = serializers.FileField() + +class ReviewSerializer(serializers.ModelSerializer): + assignee = BasicUserSerializer(allow_null=True, required=False) + assignee_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) + reviewer = BasicUserSerializer(allow_null=True, required=False) + reviewer_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) + + class Meta: + model = models.Review + fields = '__all__' + read_only_fields = ('id', 'assignee', 'reviewer', ) + write_once_fields = ('job', 'reviewer_id', 'assignee_id', 'estimated_quality', 'status', ) + ordering = ['-id'] + +class IssueSerializer(serializers.ModelSerializer): + owner = BasicUserSerializer(allow_null=True, required=False) + owner_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) + resolver = BasicUserSerializer(allow_null=True, required=False) + resolver_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) + + position = serializers.ListField( + child=serializers.FloatField(), + allow_empty=False, + ) + + class Meta: + model = models.Issue + fields = '__all__' + read_only_fields = ('created_date', 'id', 'owner', 'resolver', ) + write_once_fields = ('frame', 'position', 'job', 'owner_id', 'review', ) + ordering = ['-id'] + +class CommentSerializer(serializers.ModelSerializer): + author = BasicUserSerializer(allow_null=True, required=False) + author_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) + + class Meta: + model = models.Comment + fields = '__all__' + read_only_fields = ('created_date', 'updated_date', 'id', 'author', ) + write_once_fields = ('issue', 'author_id', ) + +class CombinedIssueSerializer(IssueSerializer): + comment_set = CommentSerializer(many=True) + +class CombinedReviewSerializer(ReviewSerializer): + issue_set = CombinedIssueSerializer(many=True) + + def create(self, validated_data): + issues_validated_data = validated_data.pop('issue_set') + db_review = models.Review.objects.create(**validated_data) + for issue in issues_validated_data: + issue['review'] = db_review + + comments_validated_data = issue.pop('comment_set') + db_issue = models.Issue.objects.create(**issue) + for comment in comments_validated_data: + comment['issue'] = db_issue + models.Comment.objects.create(**comment) + + return db_review diff --git a/cvat/apps/engine/signals.py b/cvat/apps/engine/signals.py index 7850596e..5ef6e5f3 100644 --- a/cvat/apps/engine/signals.py +++ b/cvat/apps/engine/signals.py @@ -5,12 +5,14 @@ import shutil from django.db.models.signals import post_delete, post_save from django.dispatch import receiver +from django.contrib.auth.models import User from .models import ( Data, Job, StatusChoice, Task, + Profile, ) @@ -28,6 +30,12 @@ def update_task_status(instance, **kwargs): db_task.status = status db_task.save() +@receiver(post_save, sender=User, dispatch_uid="create_a_profile_on_create_a_user") +def create_profile(instance, **kwargs): + if not hasattr(instance, 'profile'): + profile = Profile() + profile.user = instance + profile.save() @receiver(post_delete, sender=Task, dispatch_uid="delete_task_files_on_delete_task") def delete_task_files_on_delete_task(instance, **kwargs): diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index b3b4058e..15c7c4ee 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -360,6 +360,227 @@ class JobPartialUpdateAPITestCase(JobUpdateAPITestCase): response = self._run_api_v1_jobs_id(self.job.id, self.owner, data) self._check_request(response, data) +class JobReview(APITestCase): + def setUp(self): + self.client = APIClient() + + @classmethod + def setUpTestData(cls): + create_db_users(cls) + cls.task = create_dummy_db_tasks(cls)[0] + cls.job = Job.objects.filter(segment__task_id=cls.task.id).first() + cls.reviewer = cls.annotator + cls.job.reviewer = cls.reviewer + cls.job.assignee = cls.assignee + cls.job.save() + cls.reject_review_data = { + "job": cls.job.id, + "issue_set": [ + { + "position": [ + 50, 50, 100, 100 + ], + "comment_set": [ + { + "message": "This is wrong!" + }, { + "message": "This is wrong 2!" + } + ], + "frame": 0 + } + ], + "estimated_quality": 3, + "status": "rejected" + } + + cls.accept_review_data = { + "job": cls.job.id, + "issue_set": [], + "estimated_quality": 5, + "status": "accepted" + } + + cls.review_further_data = { + "job": cls.job.id, + "issue_set": [], + "estimated_quality": 4, + "status": "review_further", + "reviewer_id": cls.reviewer.id + } + + cls.create_comment_data = [{ + "message": "This is testing message" + }, { + "message": "This is testing message 2" + }, { + "message": "This is testing message 3" + }] + + def _post_request(self, path, user, data): + with ForceLogin(user, self.client): + response = self.client.post(path, data=data, format='json') + + return response + + def _patch_request(self, path, user, data): + with ForceLogin(user, self.client): + response = self.client.patch(path, data=data, format='json') + + return response + + def _get_request(self, path, user): + with ForceLogin(user, self.client): + response = self.client.get(path) + + return response + + def _delete_request(self, path, user): + with ForceLogin(user, self.client): + response = self.client.delete(path) + + return response + + def _fetch_job_from_db(self): + self.job = Job.objects.prefetch_related( + 'review_set', + 'review_set__issue_set', + 'review_set__issue_set__comment_set').filter(segment__task_id=self.task.id).first() + + def _set_annotation_status(self): + self._patch_request('/api/v1/jobs/{}'.format(self.job.id), self.admin, {'status': 'annotation'}) + + def _set_validation_status(self): + self._patch_request('/api/v1/jobs/{}'.format(self.job.id), self.admin, {'status': 'validation'}) + + def test_api_v1_job_annotation_review(self): + self._set_annotation_status() + response = self._post_request('/api/v1/reviews', self.reviewer, self.accept_review_data) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self._post_request('/api/v1/reviews', self.assignee, self.accept_review_data) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_api_v1_job_validation_review_create(self): + self._set_validation_status() + response = self._post_request('/api/v1/reviews', self.reviewer, self.accept_review_data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self._fetch_job_from_db() + self.assertEqual(self.job.status, 'completed') + response = self._post_request('/api/v1/reviews', self.assignee, self.accept_review_data) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.job.review_set.first().delete() + + def test_api_v1_job_reject_review(self): + self._set_validation_status() + response = self._post_request('/api/v1/reviews', self.reviewer, self.reject_review_data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self._fetch_job_from_db() + self.assertEqual(self.job.status, 'annotation') + self.job.review_set.first().delete() + + def test_api_v1_job_review_further(self): + self._set_validation_status() + response = self._post_request('/api/v1/reviews', self.reviewer, self.review_further_data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self._fetch_job_from_db() + self.assertEqual(self.job.status, 'validation') + self.job.review_set.first().delete() + + def test_api_v1_create_review_comment(self): + self._set_validation_status() + response = self._post_request('/api/v1/reviews', self.reviewer, self.reject_review_data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + issue_id = response.data['issue_set'][0]['id'] + comments = self.create_comment_data[:] + for comment in comments: + comment.update({ + 'issue': issue_id + }) + response = self._post_request('/api/v1/comments', self.assignee, comment) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + response = self._get_request('/api/v1/issues/{}/comments'.format(issue_id), self.reviewer) + self.assertIsInstance(response.data, cls = list) + self.assertEqual(len(response.data), 5) + self.job.review_set.all().delete() + self.job.issue_set.all().delete() + + def test_api_v1_edit_review_comment(self): + self._set_validation_status() + response = self._post_request('/api/v1/reviews', self.reviewer, self.reject_review_data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + issue_id = response.data['issue_set'][0]['id'] + comments = self.create_comment_data[:] + for comment in comments: + comment.update({ + 'issue': issue_id + }) + response = self._post_request('/api/v1/comments', self.assignee, comment) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + response = self._get_request('/api/v1/issues/{}/comments'.format(issue_id), self.reviewer) + last_comment = max(response.data, key=lambda comment: comment['id']) + last_comment.update({ + 'message': 'fixed message 3' + }) + last_comment.update({ + 'author_id': last_comment['author']['id'], + 'author': None + }) + response = self._patch_request('/api/v1/comments/{}'.format(last_comment['id']), self.reviewer, last_comment) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self._patch_request('/api/v1/comments/{}'.format(last_comment['id']), self.assignee, last_comment) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data['message'], last_comment['message']) + response = self._get_request('/api/v1/issues/{}/comments'.format(issue_id), self.reviewer) + updated_last_comment = max(response.data, key=lambda comment: comment['id']) + self.assertEqual(updated_last_comment['message'], last_comment['message']) + self.job.review_set.all().delete() + self.job.issue_set.all().delete() + + def test_api_v1_remove_comment(self): + self._set_validation_status() + response = self._post_request('/api/v1/reviews', self.reviewer, self.reject_review_data) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + issue_id = response.data['issue_set'][0]['id'] + comments = self.create_comment_data[:] + for comment in comments: + comment.update({ + 'issue': issue_id + }) + response = self._post_request('/api/v1/comments', self.assignee, comment) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + response = self._get_request('/api/v1/issues/{}/comments'.format(issue_id), self.reviewer) + last_comment = max(response.data, key=lambda comment: comment['id']) + response = self._delete_request('/api/v1/comments/{}'.format(last_comment['id']), self.reviewer) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self._delete_request('/api/v1/comments/{}'.format(last_comment['id']), self.assignee) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self._fetch_job_from_db() + ids = list(map(lambda comment: comment.id, self.job.issue_set.first().comment_set.all())) + self.assertNotIn(last_comment['id'], ids) + self.job.review_set.all().delete() + self.job.issue_set.all().delete() + + def test_api_v1_resolve_reopen_issue(self): + self._set_validation_status() + response = self._post_request('/api/v1/reviews', self.reviewer, self.reject_review_data) + response = self._get_request('/api/v1/jobs/{}/issues'.format(self.job.id), self.assignee) + issue_id = response.data[0]['id'] + + response = self._patch_request('/api/v1/issues/{}'.format(issue_id), self.assignee, {'resolver_id': self.assignee.id}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self._get_request('/api/v1/jobs/{}/issues'.format(self.job.id), self.assignee) + self.assertEqual(response.data[0]['resolver']['id'], self.assignee.id) + + response = self._patch_request('/api/v1/issues/{}'.format(issue_id), self.reviewer, {'resolver_id': None}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self._get_request('/api/v1/jobs/{}/issues'.format(self.job.id), self.assignee) + self.assertEqual(response.data[0]['resolver'], None) + + response = self._patch_request('/api/v1/issues/{}'.format(issue_id), self.reviewer, {'resolver_id': self.reviewer.id}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self._get_request('/api/v1/jobs/{}/issues'.format(self.job.id), self.reviewer) + self.assertEqual(response.data[0]['resolver']['id'], self.reviewer.id) + class ServerAboutAPITestCase(APITestCase): def setUp(self): self.client = APIClient() diff --git a/cvat/apps/engine/urls.py b/cvat/apps/engine/urls.py index 699ea1db..da0c1f2e 100644 --- a/cvat/apps/engine/urls.py +++ b/cvat/apps/engine/urls.py @@ -49,6 +49,9 @@ router.register('tasks', views.TaskViewSet) router.register('jobs', views.JobViewSet) router.register('users', views.UserViewSet) router.register('server', views.ServerViewSet, basename='server') +router.register('reviews', views.ReviewViewSet) +router.register('issues', views.IssueViewSet) +router.register('comments', views.CommentViewSet) router.register('restrictions', RestrictionsViewSet, basename='restrictions') urlpatterns = [ diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index e6d1745e..bd877aad 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -11,6 +11,7 @@ from distutils.util import strtobool from tempfile import mkstemp import django_rq +from django.shortcuts import get_object_or_404 from django.apps import apps from django.conf import settings from django.contrib.auth.models import User @@ -36,14 +37,17 @@ import cvat.apps.dataset_manager.views # pylint: disable=unused-import from cvat.apps.authentication import auth from cvat.apps.dataset_manager.serializers import DatasetFormatsSerializer from cvat.apps.engine.frame_provider import FrameProvider -from cvat.apps.engine.models import Job, StatusChoice, Task, Project, StorageMethodChoice +from cvat.apps.engine.models import ( + Job, StatusChoice, Task, Project, Review, Issue, + Comment, StorageMethodChoice, ReviewStatus +) from cvat.apps.engine.serializers import ( AboutSerializer, AnnotationFileSerializer, BasicUserSerializer, DataMetaSerializer, DataSerializer, ExceptionSerializer, FileInfoSerializer, JobSerializer, LabeledDataSerializer, - LogEventSerializer, ProjectSerializer, ProjectSearchSerializer, - RqStatusSerializer, TaskSerializer, UserSerializer, - PluginsSerializer, + LogEventSerializer, ProjectSerializer, ProjectSearchSerializer, RqStatusSerializer, + TaskSerializer, UserSerializer, PluginsSerializer, ReviewSerializer, + CombinedReviewSerializer, IssueSerializer, CombinedIssueSerializer, CommentSerializer ) from cvat.apps.engine.utils import av_scan_paths @@ -665,7 +669,7 @@ class JobViewSet(viewsets.GenericViewSet, if http_method in SAFE_METHODS: permissions.append(auth.JobAccessPermission) - elif http_method in ["PATCH", "PUT", "DELETE"]: + elif http_method in ['PATCH', 'PUT', 'DELETE']: permissions.append(auth.JobChangePermission) else: permissions.append(auth.AdminRolePermission) @@ -720,12 +724,174 @@ class JobViewSet(viewsets.GenericViewSet, return Response(data=str(e), status=status.HTTP_400_BAD_REQUEST) return Response(data) + @swagger_auto_schema(method='get', operation_summary='Method returns list of reviews for the job', + responses={'200': ReviewSerializer(many=True)} + ) + @action(detail=True, methods=['GET'], serializer_class=ReviewSerializer) + def reviews(self, request, pk): + db_job = self.get_object() + queryset = db_job.review_set + serializer = ReviewSerializer(queryset, context={'request': request}, many=True) + return Response(serializer.data) + + @swagger_auto_schema(method='get', operation_summary='Method returns list of issues for the job', + responses={'200': CombinedIssueSerializer(many=True)} + ) + @action(detail=True, methods=['GET'], serializer_class=CombinedIssueSerializer) + def issues(self, request, pk): + db_job = self.get_object() + queryset = db_job.issue_set + serializer = CombinedIssueSerializer(queryset, context={'request': request}, many=True) + return Response(serializer.data) + +@method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Submit a review for a job')) +@method_decorator(name='destroy', decorator=swagger_auto_schema(operation_summary='Method removes a review from a job')) +class ReviewViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin, mixins.CreateModelMixin): + queryset = Review.objects.all().order_by('id') + + def get_serializer_class(self): + if self.request.method == 'POST': + return CombinedReviewSerializer + else: + return ReviewSerializer + + def get_permissions(self): + permissions = [IsAuthenticated] + if self.request.method == 'POST': + permissions.append(auth.JobReviewPermission) + else: + permissions.append(auth.AdminRolePermission) + + return [perm() for perm in permissions] + + def create(self, request, *args, **kwargs): + job_id = request.data['job'] + db_job = get_object_or_404(Job, pk=job_id) + self.check_object_permissions(self.request, db_job) + + if request.data['status'] == ReviewStatus.REVIEW_FURTHER: + if 'reviewer_id' not in request.data: + return Response('Must provide a new reviewer', status=status.HTTP_400_BAD_REQUEST) + reviewer_id = request.data['reviewer_id'] + reviewer = get_object_or_404(User, pk=reviewer_id) + + request.data.update({ + 'reviewer_id': request.user.id, + }) + if db_job.assignee: + request.data.update({ + 'assignee_id': db_job.assignee.id, + }) + + issue_set = request.data['issue_set'] + for issue in issue_set: + issue['job'] = db_job.id + issue['owner_id'] = request.user.id + comment_set = issue['comment_set'] + for comment in comment_set: + comment['author_id'] = request.user.id + + serializer = self.get_serializer(data=request.data, partial=True) + serializer.is_valid(raise_exception=True) + self.perform_create(serializer) + headers = self.get_success_headers(serializer.data) + + if serializer.data['status'] == ReviewStatus.ACCEPTED: + db_job.status = StatusChoice.COMPLETED + db_job.save() + elif serializer.data['status'] == ReviewStatus.REJECTED: + db_job.status = StatusChoice.ANNOTATION + db_job.save() + else: + db_job.reviewer = reviewer + db_job.save() + + return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) + +@method_decorator(name='destroy', decorator=swagger_auto_schema(operation_summary='Method removes an issue from a job')) +@method_decorator(name='partial_update', decorator=swagger_auto_schema(operation_summary='Method updates an issue. It is used to resolve/reopen an issue')) +class IssueViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin, mixins.UpdateModelMixin): + queryset = Issue.objects.all().order_by('id') + http_method_names = ['get', 'patch', 'delete', 'options'] + + def get_serializer_class(self): + return IssueSerializer + + def partial_update(self, request, *args, **kwargs): + db_issue = self.get_object() + if 'resolver_id' in request.data and request.data['resolver_id'] and db_issue.resolver is None: + # resolve + db_issue.resolver = request.user + db_issue.resolved_date = datetime.now() + db_issue.save(update_fields=['resolver', 'resolved_date']) + elif 'resolver_id' in request.data and not request.data['resolver_id'] and db_issue.resolver is not None: + # reopen + db_issue.resolver = None + db_issue.resolved_date = None + db_issue.save(update_fields=['resolver', 'resolved_date']) + serializer = self.get_serializer(db_issue) + return Response(serializer.data) + + def get_permissions(self): + http_method = self.request.method + permissions = [IsAuthenticated] + + if http_method in SAFE_METHODS: + permissions.append(auth.IssueAccessPermission) + elif http_method in ['DELETE']: + permissions.append(auth.IssueDestroyPermission) + elif http_method in ['PATCH']: + permissions.append(auth.IssueChangePermission) + else: + permissions.append(auth.AdminRolePermission) + + return [perm() for perm in permissions] + + @swagger_auto_schema(method='get', operation_summary='The action returns all comments of a specific issue', + responses={'200': CommentSerializer(many=True)} + ) + @action(detail=True, methods=['GET'], serializer_class=CommentSerializer) + def comments(self, request, pk): + db_issue = self.get_object() + queryset = db_issue.comment_set + serializer = CommentSerializer(queryset, context={'request': request}, many=True) + return Response(serializer.data) + +@method_decorator(name='partial_update', decorator=swagger_auto_schema(operation_summary='Method updates comment in an issue')) +@method_decorator(name='destroy', decorator=swagger_auto_schema(operation_summary='Method removes a comment from an issue')) +class CommentViewSet(viewsets.GenericViewSet, + mixins.DestroyModelMixin, mixins.UpdateModelMixin, mixins.CreateModelMixin): + queryset = Comment.objects.all().order_by('id') + serializer_class = CommentSerializer + http_method_names = ['get', 'post', 'patch', 'delete', 'options'] + + def create(self, request, *args, **kwargs): + request.data.update({ + 'author_id': request.user.id, + }) + issue_id = request.data['issue'] + db_issue = get_object_or_404(Issue, pk=issue_id) + self.check_object_permissions(self.request, db_issue.job) + return super().create(request, args, kwargs) + + def get_permissions(self): + http_method = self.request.method + permissions = [IsAuthenticated] + + if http_method in ['PATCH', 'DELETE']: + permissions.append(auth.CommentChangePermission) + elif http_method in ['POST']: + permissions.append(auth.CommentCreatePermission) + else: + permissions.append(auth.AdminRolePermission) + + return [perm() for perm in permissions] + class UserFilter(filters.FilterSet): class Meta: model = User fields = ("id",) - @method_decorator(name='list', decorator=swagger_auto_schema( manual_parameters=[ openapi.Parameter('id',openapi.IN_QUERY,description="A unique number value identifying this user",type=openapi.TYPE_NUMBER),