From 6b6ececcad2c3a8bd793055178b3241f07edd2ab Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 19 Oct 2020 15:08:21 +0300 Subject: [PATCH 01/33] tmp --- .../migrations/0032_auto_20201019_1125.py | 74 +++++++++++++++++++ cvat/apps/engine/models.py | 46 ++++++++++++ cvat/apps/engine/serializers.py | 5 +- 3 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 cvat/apps/engine/migrations/0032_auto_20201019_1125.py diff --git a/cvat/apps/engine/migrations/0032_auto_20201019_1125.py b/cvat/apps/engine/migrations/0032_auto_20201019_1125.py new file mode 100644 index 000000000000..864201bbee1f --- /dev/null +++ b/cvat/apps/engine/migrations/0032_auto_20201019_1125.py @@ -0,0 +1,74 @@ +# Generated by Django 3.1.1 on 2020-10-19 11:25 + +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', '0031_auto_20201011_0220'), + ] + + 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(default=0.0)), + ('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()), + ('roi', cvat.apps.engine.models.FloatArrayField()), + ('created_date', models.DateTimeField(auto_now_add=True)), + ('resolved_date', models.DateTimeField(auto_now=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.CharField(default='', max_length=4096)), + ('created_date', models.DateTimeField(auto_now_add=True)), + ('updated_date', models.DateTimeField(auto_now=True)), + ('issue', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='engine.issue')), + ('owner', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.RunPython(create_profile), + ] diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index d99181bcc97f..23105ad18ad6 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -12,6 +12,11 @@ from django.contrib.auth.models import User from django.core.files.storage import FileSystemStorage +def get_user_rating(self): + return self.profile.rating + +User.add_to_class('get_user_rating', get_user_rating) + class SafeCharField(models.CharField): def get_prep_value(self, value): value = super().get_prep_value(value) @@ -248,6 +253,7 @@ class Meta: 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) @@ -331,6 +337,18 @@ def choices(self): 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) @@ -411,3 +429,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(default=0.0) + status = models.CharField(max_length=16, choices=ReviewStatus.choices()) + +class Issue(models.Model): + frame = models.PositiveIntegerField() + roi = 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(auto_now=True, null=True, blank=True) + +class Comment(models.Model): + issue = models.ForeignKey(Issue, on_delete=models.CASCADE) + owner = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL) + message = models.CharField(max_length=4096, 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 71843878313f..76daeed29981 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -360,13 +360,14 @@ class Meta: class UserSerializer(serializers.ModelSerializer): groups = serializers.SlugRelatedField(many=True, slug_field='name', queryset=Group.objects.all()) + rating = serializers.SlugRelatedField(read_only=True, slug_field='get_user_rating') class Meta: model = User fields = ('url', 'id', 'username', 'first_name', 'last_name', 'email', 'groups', 'is_staff', 'is_superuser', 'is_active', 'last_login', - 'date_joined') - read_only_fields = ('last_login', 'date_joined') + 'date_joined', 'rating') + read_only_fields = ('last_login', 'date_joined', 'rating') write_only_fields = ('password', ) ordering = ['-id'] From 8c410cac57c17c1c33930c69f856c74f9f407499 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 19 Oct 2020 18:04:31 +0300 Subject: [PATCH 02/33] Removed migration --- .../migrations/0032_auto_20201019_1125.py | 74 ------------------- 1 file changed, 74 deletions(-) delete mode 100644 cvat/apps/engine/migrations/0032_auto_20201019_1125.py diff --git a/cvat/apps/engine/migrations/0032_auto_20201019_1125.py b/cvat/apps/engine/migrations/0032_auto_20201019_1125.py deleted file mode 100644 index 864201bbee1f..000000000000 --- a/cvat/apps/engine/migrations/0032_auto_20201019_1125.py +++ /dev/null @@ -1,74 +0,0 @@ -# Generated by Django 3.1.1 on 2020-10-19 11:25 - -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', '0031_auto_20201011_0220'), - ] - - 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(default=0.0)), - ('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()), - ('roi', cvat.apps.engine.models.FloatArrayField()), - ('created_date', models.DateTimeField(auto_now_add=True)), - ('resolved_date', models.DateTimeField(auto_now=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.CharField(default='', max_length=4096)), - ('created_date', models.DateTimeField(auto_now_add=True)), - ('updated_date', models.DateTimeField(auto_now=True)), - ('issue', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='engine.issue')), - ('owner', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)), - ], - ), - migrations.RunPython(create_profile), - ] From 5c48c3566347048dbc80c8b6f8ffe89ed9d24acd Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 19 Oct 2020 18:08:11 +0300 Subject: [PATCH 03/33] Rebased --- .../migrations/0033_auto_20201019_1507.py | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 cvat/apps/engine/migrations/0033_auto_20201019_1507.py diff --git a/cvat/apps/engine/migrations/0033_auto_20201019_1507.py b/cvat/apps/engine/migrations/0033_auto_20201019_1507.py new file mode 100644 index 000000000000..5eb51c9ce2a1 --- /dev/null +++ b/cvat/apps/engine/migrations/0033_auto_20201019_1507.py @@ -0,0 +1,74 @@ +# Generated by Django 3.1.1 on 2020-10-19 15:07 + +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', '0032_remove_task_z_order'), + ] + + 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(default=0.0)), + ('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()), + ('roi', cvat.apps.engine.models.FloatArrayField()), + ('created_date', models.DateTimeField(auto_now_add=True)), + ('resolved_date', models.DateTimeField(auto_now=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.CharField(default='', max_length=4096)), + ('created_date', models.DateTimeField(auto_now_add=True)), + ('updated_date', models.DateTimeField(auto_now=True)), + ('issue', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='engine.issue')), + ('owner', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)), + ], + ), + migrations.RunPython(create_profile), + ] From e5a546a9428f27d999e1aa32ecc574fbf4e201be Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 19 Oct 2020 18:43:39 +0300 Subject: [PATCH 04/33] Added signals & rating --- cvat/apps/engine/models.py | 5 ----- cvat/apps/engine/serializers.py | 2 +- cvat/apps/engine/signals.py | 9 +++++++++ 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index 23105ad18ad6..fcaf6b61f123 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -12,11 +12,6 @@ from django.contrib.auth.models import User from django.core.files.storage import FileSystemStorage -def get_user_rating(self): - return self.profile.rating - -User.add_to_class('get_user_rating', get_user_rating) - class SafeCharField(models.CharField): def get_prep_value(self, value): value = super().get_prep_value(value) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 76daeed29981..cfa573995b95 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -360,7 +360,7 @@ class Meta: class UserSerializer(serializers.ModelSerializer): groups = serializers.SlugRelatedField(many=True, slug_field='name', queryset=Group.objects.all()) - rating = serializers.SlugRelatedField(read_only=True, slug_field='get_user_rating') + rating = serializers.FloatField(read_only=True, source='profile.rating') class Meta: model = User diff --git a/cvat/apps/engine/signals.py b/cvat/apps/engine/signals.py index 7850596e8b78..e12178a52358 100644 --- a/cvat/apps/engine/signals.py +++ b/cvat/apps/engine/signals.py @@ -5,12 +5,14 @@ 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,13 @@ def update_task_status(instance, **kwargs): db_task.status = status db_task.save() +@receiver(post_save, sender=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): From 23614c987ec9f34f8ae2bfc0de323188d3ba5ff2 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 19 Oct 2020 22:50:45 +0300 Subject: [PATCH 05/33] Updated API views --- cvat/apps/engine/models.py | 2 +- cvat/apps/engine/serializers.py | 52 +++++++++++ cvat/apps/engine/urls.py | 3 + cvat/apps/engine/views.py | 147 +++++++++++++++++++++++++++++++- 4 files changed, 201 insertions(+), 3 deletions(-) diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index fcaf6b61f123..8f9d9e6499c6 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -433,7 +433,7 @@ 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(default=0.0) + estimated_quality = models.FloatField() status = models.CharField(max_length=16, choices=ReviewStatus.choices()) class Issue(models.Model): diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index cfa573995b95..1d9ae08b9b0b 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -497,3 +497,55 @@ class LogEventSerializer(serializers.Serializer): class AnnotationFileSerializer(serializers.Serializer): annotation_file = serializers.FileField() + +class ReviewSerializer(serializers.ModelSerializer): + class Meta: + model = models.Review + fields = '__all__' + read_only_fields = ('id', ) + write_once_fields = ('job', 'reviewer', 'assignee', 'estimated_quality', 'status', ) + ordering = ['-id'] + +class ReviewSummarySerializer(serializers.Serializer): + reviews = serializers.IntegerField(read_only=True) + average_estimated_quality = serializers.FloatField(read_only=True) + comments_unresolved = serializers.IntegerField(read_only=True) + comments_resolved = serializers.IntegerField(read_only=True) + annotators = serializers.ListField(allow_empty=True, read_only=True) + reviewers = serializers.ListField(allow_empty=True, read_only=True) + + def to_representation(self, instance): + db_reviews = instance.review_set.all() + qualities = list(map(lambda db_review: db_review.estimated_quality, db_reviews)) + reviewers = list(map(lambda db_review: db_review.reviewer, db_reviews)) + annotators = list(map(lambda db_review: db_review.annotator, db_reviews)) + + return { + 'reviews': len(db_reviews), + 'average_estimated_quality': sum(qualities) / len(qualities) if qualities else 0, + 'comments_unresolved': 0, + 'comments_resolved': 0, + 'annotators': list(set(annotators)), + 'reviewers': list(set(reviewers)), + } + +class IssueSerializer(serializers.ModelSerializer): + class Meta: + model = models.Issue + fields = '__all__' + read_only_fields = ('created_date', 'id',) + write_once_fields = ('frame', 'roi', 'job', 'owner', 'review', ) + ordering = ['-id'] + +class IssueListSerializer(serializers.ListSerializer): + child = IssueSerializer() + +class CommentSerializer(serializers.ModelSerializer): + class Meta: + model = models.Comment + fields = '__all__' + read_only_fields = ('created_date', 'id',) + write_once_fields = ('issue', 'owner', ) + +class CommentListSerializer(serializers.ListSerializer): + child = CommentSerializer() diff --git a/cvat/apps/engine/urls.py b/cvat/apps/engine/urls.py index 699ea1db5548..da0c1f2e6bbe 100644 --- a/cvat/apps/engine/urls.py +++ b/cvat/apps/engine/urls.py @@ -49,6 +49,9 @@ def _map_format_to_schema(request, scheme=None): 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 50798c732c5a..51d28c0383c4 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -36,13 +36,18 @@ 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, StorageMethodChoice +from cvat.apps.engine.models import ( + Job, StatusChoice, Task, Review, Issue, + Comment, StorageMethodChoice +) from cvat.apps.engine.serializers import ( AboutSerializer, AnnotationFileSerializer, BasicUserSerializer, DataMetaSerializer, DataSerializer, ExceptionSerializer, FileInfoSerializer, JobSerializer, LabeledDataSerializer, LogEventSerializer, ProjectSerializer, RqStatusSerializer, - TaskSerializer, UserSerializer, PluginsSerializer, + TaskSerializer, UserSerializer, PluginsSerializer, ReviewSerializer, + ReviewSummarySerializer, IssueSerializer, IssueListSerializer, + CommentSerializer, CommentListSerializer ) from cvat.apps.engine.utils import av_scan_paths @@ -704,6 +709,144 @@ def annotations(self, request, pk): 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, many=True) + return Response(serializer.data) + + @swagger_auto_schema(method='get', operation_summary='Get a brief summary about done reviews') + @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer ) + def reviews_summary(self, request, pk): + db_job = self.get_object() + serialize = ReviewSummarySerializer(db_job) + return Response(serialize.data) + + @swagger_auto_schema(method='get', operation_summary='Method returns list of issues for the job', + responses={'200': IssueSerializer(many=True)} + ) + @action(detail=True, methods=['GET'], serializer_class=IssueSerializer) + def issues(self, request, pk): + db_job = self.get_object() + queryset = db_job.issue_set + serializer = IssueSerializer(queryset, many=True) + return Response(serializer.data) + +@method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method submits 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.CreateModelMixin, + mixins.DestroyModelMixin): + queryset = Review.objects.all().order_by('id') + serializer_class = ReviewSerializer + + def get_permissions(self): + http_method = self.request.method + permissions = [IsAuthenticated] + + # TODO: implement + if http_method in SAFE_METHODS: + permissions.append(auth.JobAccessPermission) + elif http_method in ['POST']: + permissions.append(auth.JobChangePermission) + else: + permissions.append(auth.AdminRolePermission) + + return [perm() for perm in permissions] + +@method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method adds list of issues to a job')) +@method_decorator(name='destroy', decorator=swagger_auto_schema(operation_summary='Method removes an issue from a job')) +class IssueViewSet(viewsets.GenericViewSet, mixins.CreateModelMixin, + mixins.DestroyModelMixin): + queryset = Issue.objects.all().order_by('id') + + def get_serializer_class(self): + http_method = self.request.method + if http_method == 'POST': + return IssueListSerializer + return IssueSerializer + + def get_permissions(self): + http_method = self.request.method + permissions = [IsAuthenticated] + + # TODO: implement + if http_method in SAFE_METHODS: + permissions.append(auth.JobAccessPermission) + elif http_method in ['POST']: + permissions.append(auth.JobChangePermission) + else: + permissions.append(auth.AdminRolePermission) + + return [perm() for perm in permissions] + + @swagger_auto_schema(method='patch', operation_summary='The action resolves a specific issue', + responses={'200': IssueSerializer()} + ) + @action(detail=True, methods=['PATCH'], serializer_class=IssueSerializer) + def resolve(self, request, pk): + db_issue = self.get_object() + db_issue.resolved = True + db_issue.resolver = request.user + db_issue.resolved_date = datetime.now() + db_issue.save() + serializer = self.get_serializer_class()(db_issue) + return Response(serializer.data) + + @swagger_auto_schema(method='patch', operation_summary='The action unresolves a specific issue', + responses={'200': IssueSerializer()} + ) + @action(detail=True, methods=['PATCH'], serializer_class=IssueSerializer) + def unresolve(self, request, pk): + db_issue = self.get_object() + db_issue.resolved = False + db_issue.resolver = None + db_issue.resolved_date = None + db_issue.save() + serializer = self.get_serializer_class()(db_issue) + return Response(serializer.data) + + @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, many=True) + return Response(serializer.data) + +@method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method adds list of comments to an issue')) +@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')) +@method_decorator(name='update', decorator=swagger_auto_schema(operation_summary='Method updates comment in an issue')) +class CommentViewSet(viewsets.GenericViewSet, mixins.CreateModelMixin, + mixins.DestroyModelMixin, mixins.UpdateModelMixin): + queryset = Comment.objects.all().order_by('id') + + def get_serializer_class(self): + http_method = self.request.method + if http_method == 'POST': + return CommentListSerializer + return CommentSerializer + + def get_permissions(self): + http_method = self.request.method + permissions = [IsAuthenticated] + + # TODO: implement + if http_method in SAFE_METHODS: + permissions.append(auth.JobAccessPermission) + elif http_method in ['POST']: + permissions.append(auth.JobChangePermission) + else: + permissions.append(auth.AdminRolePermission) + + return [perm() for perm in permissions] + @method_decorator(name='list', decorator=swagger_auto_schema( operation_summary='Method provides a paginated list of users registered on the server')) @method_decorator(name='retrieve', decorator=swagger_auto_schema( From 792afe2364e7f1739d6319bb58a7fcc6bcd29913 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 19 Oct 2020 22:52:10 +0300 Subject: [PATCH 06/33] Added reviewer serializer --- cvat/apps/engine/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 1d9ae08b9b0b..565388cf3963 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -56,7 +56,7 @@ class JobSerializer(serializers.ModelSerializer): class Meta: model = models.Job - fields = ('url', 'id', 'assignee', 'status', 'start_frame', + fields = ('url', 'id', 'assignee', 'reviewer', 'status', 'start_frame', 'stop_frame', 'task_id') class SimpleJobSerializer(serializers.ModelSerializer): From fbeaf00efbb6cd279c1a8f763b44cb1b41a602ef Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 21 Oct 2020 09:22:18 +0300 Subject: [PATCH 07/33] Added permissions --- cvat/apps/authentication/auth.py | 89 ++++++++++++++++++++++++++------ cvat/apps/engine/views.py | 42 ++++++++------- 2 files changed, 96 insertions(+), 35 deletions(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 349df39b06b2..7c7c2500f0ea 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -117,10 +117,27 @@ def is_job_annotator(db_user, db_job): # 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) + 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) and (db_job.status == 'validation') + 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_owner(db_user, db_comment): + has_rights = (db_comment.owner == 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) @@ -145,56 +162,62 @@ def is_job_annotator(db_user, db_job): is_job_owner | is_job_annotator) rules.add_perm('engine.job.change', has_admin_role | is_job_owner | is_job_annotator) +rules.add_perm('engine.job.review', has_admin_role | is_job_reviewer) + +rules.add_perm('engine.issue.destroy', has_admin_role | is_issue_owner) + +rules.add_perm('engine.comment.change', has_admin_role | is_comment_owner) + 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): @@ -234,19 +257,55 @@ def get_queryset(self): 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 ReviewCreatePermission(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.review', db_job) + +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.review', db_job) and + request.user.has_perm('engine.job.change', db_job) + +class CommentCreatePermission(BasePermission): + # pylint: disable=no-self-use + def has_object_permission(self, request, view, obj): + db_job = obj.issue.job + return request.user.has_perm('engine.job.access', db_job) + +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/engine/views.py b/cvat/apps/engine/views.py index 51d28c0383c4..dce097226ca8 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -743,41 +743,44 @@ class ReviewViewSet(viewsets.GenericViewSet, mixins.CreateModelMixin, queryset = Review.objects.all().order_by('id') serializer_class = ReviewSerializer + def ger_serializer_class(self): + # TODO get_serializer_class to push combined results + pass + def get_permissions(self): http_method = self.request.method permissions = [IsAuthenticated] - # TODO: implement - if http_method in SAFE_METHODS: - permissions.append(auth.JobAccessPermission) - elif http_method in ['POST']: - permissions.append(auth.JobChangePermission) + if http_method in ['POST']: + permissions.append(auth.ReviewCreatePermission) else: permissions.append(auth.AdminRolePermission) return [perm() for perm in permissions] -@method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method adds list of issues to a job')) +# @method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method adds list of issues to a job')) @method_decorator(name='destroy', decorator=swagger_auto_schema(operation_summary='Method removes an issue from a job')) class IssueViewSet(viewsets.GenericViewSet, mixins.CreateModelMixin, mixins.DestroyModelMixin): queryset = Issue.objects.all().order_by('id') + serializer_class = IssueSerializer - def get_serializer_class(self): - http_method = self.request.method - if http_method == 'POST': - return IssueListSerializer - return IssueSerializer + # def get_serializer_class(self): + # http_method = self.request.method + # if http_method == 'POST': + # return IssueListSerializer + # return IssueSerializer def get_permissions(self): http_method = self.request.method permissions = [IsAuthenticated] - # TODO: implement if http_method in SAFE_METHODS: - permissions.append(auth.JobAccessPermission) - elif http_method in ['POST']: - permissions.append(auth.JobChangePermission) + permissions.append(auth.IssueAccessPermission) + elif http_method in ['DELETE']: + permissions.append(auth.IssueDestroyPermission) + elif http_method in ['PATCH', 'PUT']: + permissions.append(auth.IssueChangePermission) else: permissions.append(auth.AdminRolePermission) @@ -837,11 +840,10 @@ def get_permissions(self): http_method = self.request.method permissions = [IsAuthenticated] - # TODO: implement - if http_method in SAFE_METHODS: - permissions.append(auth.JobAccessPermission) - elif http_method in ['POST']: - permissions.append(auth.JobChangePermission) + if http_method in ['POST']: + permissions.append(auth.CommentCreatePermission) + else if http_method in ['PATCH', 'PUT', 'DELETE']: + permissions.append(auth.CommentChangePermission) else: permissions.append(auth.AdminRolePermission) From 6fae70a75c312dd5eba928b11bed8cc0fae13c7e Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 21 Oct 2020 09:50:05 +0300 Subject: [PATCH 08/33] Fixed some code issues --- cvat/apps/authentication/auth.py | 4 ++-- cvat/apps/engine/serializers.py | 8 +++++++- cvat/apps/engine/views.py | 34 ++++++++++++++------------------ 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 7c7c2500f0ea..346e729656ad 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -295,8 +295,8 @@ 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.review', db_job) and - request.user.has_perm('engine.job.change', db_job) + return (request.user.has_perm('engine.job.review', db_job) + and request.user.has_perm('engine.job.change', db_job)) class CommentCreatePermission(BasePermission): # pylint: disable=no-self-use diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 565388cf3963..f775604cd4d6 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -544,8 +544,14 @@ class CommentSerializer(serializers.ModelSerializer): class Meta: model = models.Comment fields = '__all__' - read_only_fields = ('created_date', 'id',) + read_only_fields = ('created_date', 'updated_date', 'id',) write_once_fields = ('issue', 'owner', ) class CommentListSerializer(serializers.ListSerializer): child = CommentSerializer() + +class CombinedIssueSerializer(IssueSerializer): + comments = CommentListSerializer() + +class CombinedReviewSerializer(ReviewSerializer): + issues = CombinedIssueSerializer() diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index dce097226ca8..b907cb8e918d 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -46,7 +46,7 @@ FileInfoSerializer, JobSerializer, LabeledDataSerializer, LogEventSerializer, ProjectSerializer, RqStatusSerializer, TaskSerializer, UserSerializer, PluginsSerializer, ReviewSerializer, - ReviewSummarySerializer, IssueSerializer, IssueListSerializer, + ReviewSummarySerializer, CombinedReviewSerializer, IssueSerializer, CommentSerializer, CommentListSerializer ) from cvat.apps.engine.utils import av_scan_paths @@ -741,11 +741,13 @@ def issues(self, request, pk): class ReviewViewSet(viewsets.GenericViewSet, mixins.CreateModelMixin, mixins.DestroyModelMixin): queryset = Review.objects.all().order_by('id') - serializer_class = ReviewSerializer - def ger_serializer_class(self): - # TODO get_serializer_class to push combined results - pass + def get_serializer_class(self): + http_method = self.request.method + if http_method == 'POST': + return CombinedReviewSerializer + else: + return ReviewSerializer def get_permissions(self): http_method = self.request.method @@ -758,18 +760,12 @@ def get_permissions(self): return [perm() for perm in permissions] -# @method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method adds list of issues to a job')) @method_decorator(name='destroy', decorator=swagger_auto_schema(operation_summary='Method removes an issue from a job')) -class IssueViewSet(viewsets.GenericViewSet, mixins.CreateModelMixin, - mixins.DestroyModelMixin): +class IssueViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin): queryset = Issue.objects.all().order_by('id') - serializer_class = IssueSerializer - # def get_serializer_class(self): - # http_method = self.request.method - # if http_method == 'POST': - # return IssueListSerializer - # return IssueSerializer + def get_serializer_class(self): + return IssueSerializer def get_permissions(self): http_method = self.request.method @@ -789,27 +785,27 @@ def get_permissions(self): @swagger_auto_schema(method='patch', operation_summary='The action resolves a specific issue', responses={'200': IssueSerializer()} ) - @action(detail=True, methods=['PATCH'], serializer_class=IssueSerializer) + @action(detail=True, methods=['PATCH'], serializer_class=None) def resolve(self, request, pk): db_issue = self.get_object() db_issue.resolved = True db_issue.resolver = request.user db_issue.resolved_date = datetime.now() db_issue.save() - serializer = self.get_serializer_class()(db_issue) + serializer = IssueSerializer(db_issue) return Response(serializer.data) @swagger_auto_schema(method='patch', operation_summary='The action unresolves a specific issue', responses={'200': IssueSerializer()} ) - @action(detail=True, methods=['PATCH'], serializer_class=IssueSerializer) + @action(detail=True, methods=['PATCH'], serializer_class=None) def unresolve(self, request, pk): db_issue = self.get_object() db_issue.resolved = False db_issue.resolver = None db_issue.resolved_date = None db_issue.save() - serializer = self.get_serializer_class()(db_issue) + serializer = IssueSerializer(db_issue) return Response(serializer.data) @swagger_auto_schema(method='get', operation_summary='The action returns all comments of a specific issue', @@ -842,7 +838,7 @@ def get_permissions(self): if http_method in ['POST']: permissions.append(auth.CommentCreatePermission) - else if http_method in ['PATCH', 'PUT', 'DELETE']: + elif http_method in ['PATCH', 'PUT', 'DELETE']: permissions.append(auth.CommentChangePermission) else: permissions.append(auth.AdminRolePermission) From 4b220959cde8f22ae13a47110942de6a9b218792 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 21 Oct 2020 10:20:29 +0300 Subject: [PATCH 09/33] Fixed swagger docs --- cvat/apps/engine/serializers.py | 6 +++--- cvat/apps/engine/views.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index f775604cd4d6..dc555b16a958 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -537,9 +537,6 @@ class Meta: write_once_fields = ('frame', 'roi', 'job', 'owner', 'review', ) ordering = ['-id'] -class IssueListSerializer(serializers.ListSerializer): - child = IssueSerializer() - class CommentSerializer(serializers.ModelSerializer): class Meta: model = models.Comment @@ -547,6 +544,9 @@ class Meta: read_only_fields = ('created_date', 'updated_date', 'id',) write_once_fields = ('issue', 'owner', ) +class IssueListSerializer(serializers.ListSerializer): + child = IssueSerializer() + class CommentListSerializer(serializers.ListSerializer): child = CommentSerializer() diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index b907cb8e918d..413513d87e0c 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -47,7 +47,7 @@ LogEventSerializer, ProjectSerializer, RqStatusSerializer, TaskSerializer, UserSerializer, PluginsSerializer, ReviewSerializer, ReviewSummarySerializer, CombinedReviewSerializer, IssueSerializer, - CommentSerializer, CommentListSerializer + CombinedIssueSerializer, CommentSerializer, CommentListSerializer ) from cvat.apps.engine.utils import av_scan_paths @@ -727,13 +727,13 @@ def reviews_summary(self, request, pk): return Response(serialize.data) @swagger_auto_schema(method='get', operation_summary='Method returns list of issues for the job', - responses={'200': IssueSerializer(many=True)} + responses={'200': CombinedIssueSerializer(many=True)} ) - @action(detail=True, methods=['GET'], serializer_class=IssueSerializer) + @action(detail=True, methods=['GET'], serializer_class=CombinedIssueSerializer) def issues(self, request, pk): db_job = self.get_object() queryset = db_job.issue_set - serializer = IssueSerializer(queryset, many=True) + serializer = CombinedIssueSerializer(queryset, many=True) return Response(serializer.data) @method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method submits a review for a job')) From a7c2e6a566dfddfb34129e9ccdcb971d1f89e093 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 21 Oct 2020 11:54:05 +0300 Subject: [PATCH 10/33] Some fixes --- cvat/apps/authentication/auth.py | 2 +- cvat/apps/engine/serializers.py | 5 +++++ cvat/apps/engine/views.py | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 346e729656ad..60bb2f18d8b2 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -267,7 +267,7 @@ def has_object_permission(self, request, view, 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 diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index dc555b16a958..aa824f5c40f6 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -530,6 +530,11 @@ def to_representation(self, instance): } class IssueSerializer(serializers.ModelSerializer): + roi = serializers.ListField( + child=serializers.FloatField(), + allow_empty=False, + ) + class Meta: model = models.Issue fields = '__all__' diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 413513d87e0c..f88b32a6cb83 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -760,6 +760,7 @@ def get_permissions(self): return [perm() for perm in permissions] + @method_decorator(name='destroy', decorator=swagger_auto_schema(operation_summary='Method removes an issue from a job')) class IssueViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin): queryset = Issue.objects.all().order_by('id') From 289921e6c40dcdc1e1ace162210a237120a5dec1 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 21 Oct 2020 22:01:24 +0300 Subject: [PATCH 11/33] Updated api method to create review --- cvat/apps/authentication/auth.py | 5 ++- cvat/apps/engine/serializers.py | 27 +++++++++++----- cvat/apps/engine/views.py | 54 +++++++++++++++++++------------- 3 files changed, 54 insertions(+), 32 deletions(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 60bb2f18d8b2..d4e7869e3b2f 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -274,11 +274,10 @@ class JobChangePermission(BasePermission): def has_object_permission(self, request, view, obj): return request.user.has_perm('engine.job.change', obj) -class ReviewCreatePermission(BasePermission): +class JobReviewPermission(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.review', db_job) + return request.user.has_perm('engine.job.review', obj) class IssueAccessPermission(BasePermission): # pylint: disable=no-self-use diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index aa824f5c40f6..e67022ed06e6 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -549,14 +549,25 @@ class Meta: read_only_fields = ('created_date', 'updated_date', 'id',) write_once_fields = ('issue', 'owner', ) -class IssueListSerializer(serializers.ListSerializer): - child = IssueSerializer() - -class CommentListSerializer(serializers.ListSerializer): - child = CommentSerializer() - class CombinedIssueSerializer(IssueSerializer): - comments = CommentListSerializer() + comment_set = CommentSerializer(many=True) class CombinedReviewSerializer(ReviewSerializer): - issues = CombinedIssueSerializer() + 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 + +class CommentListSerializer(serializers.ListSerializer): + child = CommentSerializer() diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index f88b32a6cb83..0d913db29235 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -650,12 +650,15 @@ class JobViewSet(viewsets.GenericViewSet, def get_permissions(self): http_method = self.request.method + http_path = self.request.path permissions = [IsAuthenticated] if http_method in SAFE_METHODS: permissions.append(auth.JobAccessPermission) elif http_method in ["PATCH", "PUT", "DELETE"]: permissions.append(auth.JobChangePermission) + elif http_method == 'POST' and http_path.endswith('reviews/create'): + permissions.append(auth.JobReviewPermission) else: permissions.append(auth.AdminRolePermission) @@ -719,12 +722,36 @@ def reviews(self, request, pk): serializer = ReviewSerializer(queryset, many=True) return Response(serializer.data) + @swagger_auto_schema(method='post', operation_summary='Submit a review for the job') + @action(detail=True, methods=['POST'], url_path='reviews/create', serializer_class=CombinedReviewSerializer) + def create_review(self, request, pk): + db_job = self.get_object() + request.data.update({ + 'job': db_job.id, + 'reviewer': request.user.id, + 'assignee': db_job.assignee, + }) + + issue_set = request.data['issue_set'] + for issue in issue_set: + issue['job'] = db_job.id + issue['owner'] = request.user.id + comment_set = issue['comment_set'] + for comment in comment_set: + comment['owner'] = request.user.id + + serializer = CombinedReviewSerializer(data=request.data, partial=True) + if serializer.is_valid(raise_exception=True): + instance = serializer.save() + return Response(CombinedReviewSerializer(instance).data) + + @swagger_auto_schema(method='get', operation_summary='Get a brief summary about done reviews') - @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer ) + @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer) def reviews_summary(self, request, pk): db_job = self.get_object() - serialize = ReviewSummarySerializer(db_job) - return Response(serialize.data) + serializer = ReviewSummarySerializer(db_job) + return Response(serializer.data) @swagger_auto_schema(method='get', operation_summary='Method returns list of issues for the job', responses={'200': CombinedIssueSerializer(many=True)} @@ -736,28 +763,13 @@ def issues(self, request, pk): serializer = CombinedIssueSerializer(queryset, many=True) return Response(serializer.data) -@method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method submits 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.CreateModelMixin, - mixins.DestroyModelMixin): +class ReviewViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin): queryset = Review.objects.all().order_by('id') - - def get_serializer_class(self): - http_method = self.request.method - if http_method == 'POST': - return CombinedReviewSerializer - else: - return ReviewSerializer + serializer_class = ReviewSerializer def get_permissions(self): - http_method = self.request.method - permissions = [IsAuthenticated] - - if http_method in ['POST']: - permissions.append(auth.ReviewCreatePermission) - else: - permissions.append(auth.AdminRolePermission) - + permissions = [IsAuthenticated, auth.AdminRolePermission] return [perm() for perm in permissions] From b29c7753cc20a484a3fb91feea8a585a8da7ffc9 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Thu, 22 Oct 2020 00:00:10 +0300 Subject: [PATCH 12/33] Added some API tests & some fixes --- cvat/apps/authentication/auth.py | 18 +++- cvat/apps/engine/models.py | 2 +- cvat/apps/engine/serializers.py | 22 ++-- cvat/apps/engine/tests/test_rest_api.py | 132 ++++++++++++++++++++++++ cvat/apps/engine/views.py | 13 ++- 5 files changed, 168 insertions(+), 19 deletions(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index d4e7869e3b2f..cf5e6c9b98c9 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -113,6 +113,16 @@ def is_job_owner(db_user, db_job): @rules.predicate def is_job_annotator(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 + + 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) @@ -125,7 +135,7 @@ def is_job_annotator(db_user, db_job): @rules.predicate def is_job_reviewer(db_user, db_job): - has_rights = (db_job.reviewer == db_user) and (db_job.status == 'validation') + has_rights = db_job.reviewer == db_user return has_rights @rules.predicate @@ -159,10 +169,10 @@ def is_comment_owner(db_user, db_comment): 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) + is_job_owner | is_job_annotator | is_job_reviewer) rules.add_perm('engine.job.change', has_admin_role | is_job_owner | - is_job_annotator) -rules.add_perm('engine.job.review', has_admin_role | is_job_reviewer) + has_change_permissions) +rules.add_perm('engine.job.review', has_admin_role | (is_job_reviewer & has_change_permissions)) rules.add_perm('engine.issue.destroy', has_admin_role | is_issue_owner) diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index 8f9d9e6499c6..332d56192598 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -444,7 +444,7 @@ class Issue(models.Model): 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(auto_now=True, null=True, blank=True) + resolved_date = models.DateTimeField(null=True, blank=True) class Comment(models.Model): issue = models.ForeignKey(Issue, on_delete=models.CASCADE) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index e67022ed06e6..16980eb6e83c 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -6,6 +6,7 @@ import re import shutil +from functools import reduce from rest_framework import serializers from django.contrib.auth.models import User, Group @@ -509,24 +510,25 @@ class Meta: class ReviewSummarySerializer(serializers.Serializer): reviews = serializers.IntegerField(read_only=True) average_estimated_quality = serializers.FloatField(read_only=True) - comments_unresolved = serializers.IntegerField(read_only=True) - comments_resolved = serializers.IntegerField(read_only=True) - annotators = serializers.ListField(allow_empty=True, read_only=True) + issues_unresolved = serializers.IntegerField(read_only=True) + issues_resolved = serializers.IntegerField(read_only=True) + assignees = serializers.ListField(allow_empty=True, read_only=True) reviewers = serializers.ListField(allow_empty=True, read_only=True) def to_representation(self, instance): - db_reviews = instance.review_set.all() + db_reviews = instance.review_set.prefetch_related('issue_set').all() qualities = list(map(lambda db_review: db_review.estimated_quality, db_reviews)) - reviewers = list(map(lambda db_review: db_review.reviewer, db_reviews)) - annotators = list(map(lambda db_review: db_review.annotator, db_reviews)) + reviewers = list(map(lambda db_review: db_review.reviewer.username if db_review.reviewer else None, db_reviews)) + assignees = list(map(lambda db_review: db_review.assignee.username if db_review.assignee else None, db_reviews)) + db_issues = list(reduce(lambda acc, db_review: acc + list(db_review.issue_set.all()), db_reviews, [])) return { 'reviews': len(db_reviews), 'average_estimated_quality': sum(qualities) / len(qualities) if qualities else 0, - 'comments_unresolved': 0, - 'comments_resolved': 0, - 'annotators': list(set(annotators)), - 'reviewers': list(set(reviewers)), + 'issues_unresolved': len(list(filter(lambda db_issue: db_issue.resolved_date is None, db_issues))), + 'issues_resolved': len(list(filter(lambda db_issue: db_issue.resolved_date is not None, db_issues))), + 'assignees': list(set(filter(lambda username: username is not None, assignees))), + 'reviewers': list(set(filter(lambda username: username is not None, reviewers))), } class IssueSerializer(serializers.ModelSerializer): diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index b7448fff7749..b725380e8201 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -360,6 +360,138 @@ def test_api_v1_jobs_id_admin_partial(self): 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 = { + "issue_set": [ + { + "roi": [ + 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 = { + "issue_set": [], + "estimated_quality": 5, + "status": "accepted" + } + + cls.review_further_data = { + "issue_set": [], + "estimated_quality": 4, + "status": "review_further" + } + + def _post_request(self, jid, user, data, path=''): + with ForceLogin(user, self.client): + response = self.client.post('/api/v1/jobs/{}{}'.format(jid, path), data=data, format='json')\ + + return response + + def _patch_request(self, jid, user, data, path=''): + with ForceLogin(user, self.client): + response = self.client.patch('/api/v1/jobs/{}{}'.format(jid, path), data=data, format='json') + + return response + + def _get_request(self, jid, user, path=''): + with ForceLogin(user, self.client): + response = self.client.get('/api/v1/jobs/{}{}'.format(jid, 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(self.job.id, self.admin, {'status': 'annotation'}) + + def _set_validation_status(self): + self._patch_request(self.job.id, self.admin, {'status': 'validation'}) + + def test_api_v1_job_annotation_review(self): + self._set_annotation_status() + response = self._post_request(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create') + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self._post_request(self.job.id, self.assignee, self.accept_review_data, path='/reviews/create') + 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(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create') + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self._fetch_job_from_db() + self.assertEqual(self.job.status, 'completed') + response = self._post_request(self.job.id, self.assignee, self.accept_review_data, path='/reviews/create') + 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(self.job.id, self.reviewer, self.reject_review_data, path='/reviews/create') + 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(self.job.id, self.reviewer, self.review_further_data, path='/reviews/create') + 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_job_review_summary(self): + self._set_validation_status() + response = self._post_request(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create') + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self._set_validation_status() + response = self._post_request(self.job.id, self.reviewer, self.review_further_data, path='/reviews/create') + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + response = self._post_request(self.job.id, self.reviewer, self.reject_review_data, path='/reviews/create') + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + response = self._get_request(self.job.id, self.reviewer, path='/reviews/summary') + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data.get('reviews', None), 3) + self.assertEqual(response.data.get('average_estimated_quality', None), 4) + self.assertEqual(response.data.get('issues_unresolved', None), 1) + self.assertEqual(response.data.get('issues_resolved', None), 0) + self.assertSequenceEqual(response.data.get('assignees', None), ['user2']) + self.assertSequenceEqual(response.data.get('reviewers', None), ['user3']) + + # todo: left comment + # todo: edit comment (+ with wrong permissions) + # todo: remove comment (+ with wrong permissions) + # todo: resolve issue (+ with wrong permissions) + # todo: unresolve issue (+ with wrong permissions) + class ServerAboutAPITestCase(APITestCase): def setUp(self): self.client = APIClient() diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 0d913db29235..8fc476879a0a 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -38,7 +38,7 @@ from cvat.apps.engine.frame_provider import FrameProvider from cvat.apps.engine.models import ( Job, StatusChoice, Task, Review, Issue, - Comment, StorageMethodChoice + Comment, StorageMethodChoice, ReviewStatus ) from cvat.apps.engine.serializers import ( AboutSerializer, AnnotationFileSerializer, BasicUserSerializer, @@ -729,7 +729,7 @@ def create_review(self, request, pk): request.data.update({ 'job': db_job.id, 'reviewer': request.user.id, - 'assignee': db_job.assignee, + 'assignee': db_job.assignee.id, }) issue_set = request.data['issue_set'] @@ -743,8 +743,13 @@ def create_review(self, request, pk): serializer = CombinedReviewSerializer(data=request.data, partial=True) if serializer.is_valid(raise_exception=True): instance = serializer.save() - return Response(CombinedReviewSerializer(instance).data) - + if instance.status == ReviewStatus.ACCEPTED: + db_job.status = StatusChoice.COMPLETED + db_job.save() + elif instance.status == ReviewStatus.REJECTED: + db_job.status = StatusChoice.ANNOTATION + db_job.save() + return Response(CombinedReviewSerializer(instance).data, status=status.HTTP_201_CREATED) @swagger_auto_schema(method='get', operation_summary='Get a brief summary about done reviews') @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer) From 4e453cbeebaffaa30e578a5602d3b9259b7733e9 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 26 Oct 2020 11:14:30 +0300 Subject: [PATCH 13/33] Added some tests --- cvat/apps/authentication/auth.py | 12 +- cvat/apps/engine/tests/test_rest_api.py | 167 ++++++++++++++++++++---- cvat/apps/engine/views.py | 37 ++++-- 3 files changed, 175 insertions(+), 41 deletions(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index cf5e6c9b98c9..21787c7e06e2 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -170,10 +170,10 @@ def is_comment_owner(db_user, db_comment): rules.add_perm('engine.job.access', has_admin_role | has_observer_role | 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.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_owner) @@ -304,13 +304,13 @@ 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.review', db_job) - and request.user.has_perm('engine.job.change', db_job)) + return (request.user.has_perm('engine.job.change', db_job) + or request.user.has_perm('engine.issue.change', obj)) -class CommentCreatePermission(BasePermission): +class IssueCommentPermission(BasePermission): # pylint: disable=no-self-use def has_object_permission(self, request, view, obj): - db_job = obj.issue.job + db_job = obj.job return request.user.has_perm('engine.job.access', db_job) class CommentChangePermission(BasePermission): diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index b725380e8201..c9bacb2fb94e 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -405,21 +405,35 @@ def setUpTestData(cls): "status": "review_further" } - def _post_request(self, jid, user, data, path=''): + 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.post('/api/v1/jobs/{}{}'.format(jid, path), data=data, format='json')\ + response = self.client.patch(path, data=data, format='json') return response - def _patch_request(self, jid, user, data, path=''): + def _get_request(self, path, user): with ForceLogin(user, self.client): - response = self.client.patch('/api/v1/jobs/{}{}'.format(jid, path), data=data, format='json') + response = self.client.get(path) return response - def _get_request(self, jid, user, path=''): + def _delete_request(self, path, user): with ForceLogin(user, self.client): - response = self.client.get('/api/v1/jobs/{}{}'.format(jid, path)) + response = self.client.delete(path) return response @@ -430,31 +444,45 @@ def _fetch_job_from_db(self): 'review_set__issue_set__comment_set').filter(segment__task_id=self.task.id).first() def _set_annotation_status(self): - self._patch_request(self.job.id, self.admin, {'status': 'annotation'}) + self._patch_request('/api/v1/jobs/{}'.format(self.job.id), self.admin, {'status': 'annotation'}) def _set_validation_status(self): - self._patch_request(self.job.id, self.admin, {'status': 'validation'}) + 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(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create') + response = self._post_request( + '/api/v1/jobs/{}/reviews/create'.format(self.job.id), + self.reviewer, self.accept_review_data + ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - response = self._post_request(self.job.id, self.assignee, self.accept_review_data, path='/reviews/create') + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + 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(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create') + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + 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(self.job.id, self.assignee, self.accept_review_data, path='/reviews/create') + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + 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(self.job.id, self.reviewer, self.reject_review_data, path='/reviews/create') + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + 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') @@ -462,7 +490,10 @@ def test_api_v1_job_reject_review(self): def test_api_v1_job_review_further(self): self._set_validation_status() - response = self._post_request(self.job.id, self.reviewer, self.review_further_data, path='/reviews/create') + response = self._post_request( + '/api/v1/jobs/{}/reviews/create'.format(self.job.id), + 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') @@ -470,14 +501,19 @@ def test_api_v1_job_review_further(self): def test_api_v1_job_review_summary(self): self._set_validation_status() - response = self._post_request(self.job.id, self.reviewer, self.accept_review_data, path='/reviews/create') + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + self.reviewer, self.accept_review_data + ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) self._set_validation_status() - response = self._post_request(self.job.id, self.reviewer, self.review_further_data, path='/reviews/create') + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + self.reviewer, self.review_further_data + ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - response = self._post_request(self.job.id, self.reviewer, self.reject_review_data, path='/reviews/create') + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + self.reviewer, self.reject_review_data) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - response = self._get_request(self.job.id, self.reviewer, path='/reviews/summary') + response = self._get_request('/api/v1/jobs/{}/reviews/summary'.format(self.job.id), self.reviewer) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data.get('reviews', None), 3) self.assertEqual(response.data.get('average_estimated_quality', None), 4) @@ -485,12 +521,97 @@ def test_api_v1_job_review_summary(self): self.assertEqual(response.data.get('issues_resolved', None), 0) self.assertSequenceEqual(response.data.get('assignees', None), ['user2']) self.assertSequenceEqual(response.data.get('reviewers', None), ['user3']) + self.job.review_set.all().delete() + + def test_api_v1_create_review_comment(self): + self._set_validation_status() + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + self.reviewer, self.reject_review_data + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + issue_id = response.data['issue_set'][0]['id'] + response = self._post_request('/api/v1/issues/{}/comments/create'.format(issue_id), + self.assignee, self.create_comment_data + ) + 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/jobs/{}/reviews/create'.format(self.job.id), + self.reviewer, self.reject_review_data + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + issue_id = response.data['issue_set'][0]['id'] + response = self._post_request('/api/v1/issues/{}/comments/create'.format(issue_id), + self.assignee, self.create_comment_data + ) + 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' + }) + 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/jobs/{}/reviews/create'.format(self.job.id), + self.reviewer, self.reject_review_data + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + issue_id = response.data['issue_set'][0]['id'] + response = self._post_request('/api/v1/issues/{}/comments/create'.format(issue_id), + self.assignee, self.create_comment_data + ) + 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_unresolve_issue(self): + self._set_validation_status() + response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), + 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'] - # todo: left comment - # todo: edit comment (+ with wrong permissions) - # todo: remove comment (+ with wrong permissions) - # todo: resolve issue (+ with wrong permissions) - # todo: unresolve issue (+ with wrong permissions) + response = self._patch_request('/api/v1/issues/{}/resolve'.format(issue_id), self.assignee, {}) + 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'], self.assignee.id) + + response = self._patch_request('/api/v1/issues/{}/unresolve'.format(issue_id), self.reviewer, {}) + 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/{}/resolve'.format(issue_id), self.reviewer, {}) + 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'], self.reviewer.id) class ServerAboutAPITestCase(APITestCase): def setUp(self): diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 8fc476879a0a..e5c5586b7036 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -47,7 +47,7 @@ LogEventSerializer, ProjectSerializer, RqStatusSerializer, TaskSerializer, UserSerializer, PluginsSerializer, ReviewSerializer, ReviewSummarySerializer, CombinedReviewSerializer, IssueSerializer, - CombinedIssueSerializer, CommentSerializer, CommentListSerializer + CombinedIssueSerializer, CommentSerializer ) from cvat.apps.engine.utils import av_scan_paths @@ -795,6 +795,8 @@ def get_permissions(self): permissions.append(auth.IssueDestroyPermission) elif http_method in ['PATCH', 'PUT']: permissions.append(auth.IssueChangePermission) + elif http_method in ['POST']: + permissions.append(auth.IssueCommentPermission) else: permissions.append(auth.AdminRolePermission) @@ -836,27 +838,38 @@ def comments(self, request, pk): serializer = CommentSerializer(queryset, many=True) return Response(serializer.data) -@method_decorator(name='create', decorator=swagger_auto_schema(operation_summary='Method adds list of comments to an issue')) + @swagger_auto_schema(method='post', operation_summary='The action adds comments to an issue', + responses={'201': CommentSerializer(many=True)} + ) + @action(detail=True, methods=['POST'], url_path='comments/create', serializer_class=CommentSerializer) + def create_comments(self, request, pk): + self.get_object() # call to force check persmissions + for comment in request.data: + comment.update({ + 'owner': request.user.id, + 'issue': pk + }) + + serializer = CommentSerializer(data=request.data, many=True) + if serializer.is_valid(): + serializer.save() + db_issue = Issue.objects.prefetch_related('comment_set').get(pk=pk) + updated_serializer = CommentSerializer(db_issue.comment_set, many=True) + return Response(updated_serializer.data, status=status.HTTP_201_CREATED) + @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')) @method_decorator(name='update', decorator=swagger_auto_schema(operation_summary='Method updates comment in an issue')) -class CommentViewSet(viewsets.GenericViewSet, mixins.CreateModelMixin, +class CommentViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin, mixins.UpdateModelMixin): queryset = Comment.objects.all().order_by('id') - - def get_serializer_class(self): - http_method = self.request.method - if http_method == 'POST': - return CommentListSerializer - return CommentSerializer + serializer_class = CommentSerializer def get_permissions(self): http_method = self.request.method permissions = [IsAuthenticated] - if http_method in ['POST']: - permissions.append(auth.CommentCreatePermission) - elif http_method in ['PATCH', 'PUT', 'DELETE']: + if http_method in ['PATCH', 'PUT', 'DELETE']: permissions.append(auth.CommentChangePermission) else: permissions.append(auth.AdminRolePermission) From d5b3b9425812aa5944443633299598fcd7e54e0d Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 26 Oct 2020 11:17:31 +0300 Subject: [PATCH 14/33] Removed extra code --- cvat/apps/engine/serializers.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 16980eb6e83c..49982f818797 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -570,6 +570,3 @@ def create(self, validated_data): models.Comment.objects.create(**comment) return db_review - -class CommentListSerializer(serializers.ListSerializer): - child = CommentSerializer() From 39aac44126589dcf80d5db06b5ef348ea5bf98d0 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 28 Oct 2020 21:33:46 +0300 Subject: [PATCH 15/33] Some fixes --- cvat/apps/engine/serializers.py | 6 +++--- cvat/apps/engine/tests/test_rest_api.py | 6 +++--- cvat/apps/engine/views.py | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 49982f818797..09b9ea4dd8cf 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -63,7 +63,7 @@ class Meta: class SimpleJobSerializer(serializers.ModelSerializer): class Meta: model = models.Job - fields = ('url', 'id', 'assignee', 'status') + fields = ('url', 'id', 'assignee', 'reviewer', 'status') class SegmentSerializer(serializers.ModelSerializer): jobs = SimpleJobSerializer(many=True, source='job_set') @@ -510,7 +510,7 @@ class Meta: class ReviewSummarySerializer(serializers.Serializer): reviews = serializers.IntegerField(read_only=True) average_estimated_quality = serializers.FloatField(read_only=True) - issues_unresolved = serializers.IntegerField(read_only=True) + issues_unsolved = serializers.IntegerField(read_only=True) issues_resolved = serializers.IntegerField(read_only=True) assignees = serializers.ListField(allow_empty=True, read_only=True) reviewers = serializers.ListField(allow_empty=True, read_only=True) @@ -525,7 +525,7 @@ def to_representation(self, instance): return { 'reviews': len(db_reviews), 'average_estimated_quality': sum(qualities) / len(qualities) if qualities else 0, - 'issues_unresolved': len(list(filter(lambda db_issue: db_issue.resolved_date is None, db_issues))), + 'issues_unsolved': len(list(filter(lambda db_issue: db_issue.resolved_date is None, db_issues))), 'issues_resolved': len(list(filter(lambda db_issue: db_issue.resolved_date is not None, db_issues))), 'assignees': list(set(filter(lambda username: username is not None, assignees))), 'reviewers': list(set(filter(lambda username: username is not None, reviewers))), diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index c9bacb2fb94e..77ca78692514 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -517,7 +517,7 @@ def test_api_v1_job_review_summary(self): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data.get('reviews', None), 3) self.assertEqual(response.data.get('average_estimated_quality', None), 4) - self.assertEqual(response.data.get('issues_unresolved', None), 1) + self.assertEqual(response.data.get('issues_unsolved', None), 1) self.assertEqual(response.data.get('issues_resolved', None), 0) self.assertSequenceEqual(response.data.get('assignees', None), ['user2']) self.assertSequenceEqual(response.data.get('reviewers', None), ['user3']) @@ -590,7 +590,7 @@ def test_api_v1_remove_comment(self): self.job.review_set.all().delete() self.job.issue_set.all().delete() - def test_api_v1_resolve_unresolve_issue(self): + def test_api_v1_resolve_reopen_issue(self): self._set_validation_status() response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), self.reviewer, self.reject_review_data @@ -603,7 +603,7 @@ def test_api_v1_resolve_unresolve_issue(self): response = self._get_request('/api/v1/jobs/{}/issues'.format(self.job.id), self.assignee) self.assertEqual(response.data[0]['resolver'], self.assignee.id) - response = self._patch_request('/api/v1/issues/{}/unresolve'.format(issue_id), self.reviewer, {}) + response = self._patch_request('/api/v1/issues/{}/reopen'.format(issue_id), self.reviewer, {}) 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) diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index e5c5586b7036..f25729bc6ced 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -815,11 +815,11 @@ def resolve(self, request, pk): serializer = IssueSerializer(db_issue) return Response(serializer.data) - @swagger_auto_schema(method='patch', operation_summary='The action unresolves a specific issue', + @swagger_auto_schema(method='patch', operation_summary='The action reopens a specific issue', responses={'200': IssueSerializer()} ) @action(detail=True, methods=['PATCH'], serializer_class=None) - def unresolve(self, request, pk): + def reopen(self, request, pk): db_issue = self.get_object() db_issue.resolved = False db_issue.resolver = None From 7e27d0d1982b59ad4a48dc2e8b98971593cf70df Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Sun, 1 Nov 2020 23:53:41 +0300 Subject: [PATCH 16/33] Applied changes from client part --- cvat/apps/documentation/AWS-Deployment-Guide.md | 3 ++- cvat/apps/engine/tests/test_rest_api.py | 3 ++- cvat/apps/engine/views.py | 7 +++++++ cvat/requirements/base.txt | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/cvat/apps/documentation/AWS-Deployment-Guide.md b/cvat/apps/documentation/AWS-Deployment-Guide.md index 7e354f79c61f..6e0a03d32905 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/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index 77ca78692514..c596a80761d9 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -402,7 +402,8 @@ def setUpTestData(cls): cls.review_further_data = { "issue_set": [], "estimated_quality": 4, - "status": "review_further" + "status": "review_further", + "reviewer": cls.reviewer.id } cls.create_comment_data = [{ diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index f25729bc6ced..fc25336f6e8c 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -726,6 +726,13 @@ def reviews(self, request, pk): @action(detail=True, methods=['POST'], url_path='reviews/create', serializer_class=CombinedReviewSerializer) def create_review(self, request, pk): db_job = self.get_object() + + if request.data['status'] == ReviewStatus.REVIEW_FURTHER: + if 'reviewer' not in request.data: + return Response('Must provide a new reviewer', status=status.HTTP_400_BAD_REQUEST) + db_job.reviewer = User.objects.get(pk=request.data['reviewer']) + db_job.save() + request.data.update({ 'job': db_job.id, 'reviewer': request.user.id, diff --git a/cvat/requirements/base.txt b/cvat/requirements/base.txt index 60613b4356ca..4d9dabf1e336 100644 --- a/cvat/requirements/base.txt +++ b/cvat/requirements/base.txt @@ -44,4 +44,4 @@ tensorflow==2.2.1 # Optional requirement of Datumaro # archives. Don't use as a python module because it has GPL license. patool==1.12 diskcache==5.0.2 -git+https://github.com/openvinotoolkit/datumaro@v0.1.2 \ No newline at end of file +git+https://github.com/openvinotoolkit/datumaro@v0.1.3 \ No newline at end of file From be435af5267378bad7dfb0aff3c790897e8d591a Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 2 Nov 2020 17:19:05 +0300 Subject: [PATCH 17/33] Fixed comments --- cvat/apps/authentication/auth.py | 8 ++++---- ...o_20201019_1507.py => 0033_auto_20201102_1406.py} | 12 ++++++------ cvat/apps/engine/models.py | 6 +++--- cvat/apps/engine/serializers.py | 6 +++--- cvat/apps/engine/tests/test_rest_api.py | 2 +- cvat/apps/engine/views.py | 4 ++-- 6 files changed, 19 insertions(+), 19 deletions(-) rename cvat/apps/engine/migrations/{0033_auto_20201019_1507.py => 0033_auto_20201102_1406.py} (87%) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 21787c7e06e2..68b9cdec2bf9 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -117,7 +117,7 @@ def is_job_annotator(db_user, db_job): # 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 + has_rights |= (db_user == db_job.assignee) return has_rights @@ -144,8 +144,8 @@ def is_issue_owner(db_user, db_issue): return has_rights @rules.predicate -def is_comment_owner(db_user, db_comment): - has_rights = (db_comment.owner == db_user) +def is_comment_author(db_user, db_comment): + has_rights = (db_comment.author == db_user) return has_rights # AUTH PERMISSIONS RULES @@ -176,7 +176,7 @@ def is_comment_owner(db_user, db_comment): 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_owner) +rules.add_perm('engine.comment.change', has_admin_role | is_comment_author) class AdminRolePermission(BasePermission): diff --git a/cvat/apps/engine/migrations/0033_auto_20201019_1507.py b/cvat/apps/engine/migrations/0033_auto_20201102_1406.py similarity index 87% rename from cvat/apps/engine/migrations/0033_auto_20201019_1507.py rename to cvat/apps/engine/migrations/0033_auto_20201102_1406.py index 5eb51c9ce2a1..e418ff9d1ca3 100644 --- a/cvat/apps/engine/migrations/0033_auto_20201019_1507.py +++ b/cvat/apps/engine/migrations/0033_auto_20201102_1406.py @@ -1,4 +1,4 @@ -# Generated by Django 3.1.1 on 2020-10-19 15:07 +# Generated by Django 3.1.1 on 2020-11-02 14:06 import cvat.apps.engine.models from django.conf import settings @@ -30,7 +30,7 @@ class Migration(migrations.Migration): name='Review', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('estimated_quality', models.FloatField(default=0.0)), + ('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')), @@ -50,9 +50,9 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('frame', models.PositiveIntegerField()), - ('roi', cvat.apps.engine.models.FloatArrayField()), + ('position', cvat.apps.engine.models.FloatArrayField()), ('created_date', models.DateTimeField(auto_now_add=True)), - ('resolved_date', models.DateTimeField(auto_now=True, null=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)), @@ -63,11 +63,11 @@ class Migration(migrations.Migration): name='Comment', fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('message', models.CharField(default='', max_length=4096)), + ('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')), - ('owner', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL)), ], ), migrations.RunPython(create_profile), diff --git a/cvat/apps/engine/models.py b/cvat/apps/engine/models.py index 332d56192598..3ff033ef9822 100644 --- a/cvat/apps/engine/models.py +++ b/cvat/apps/engine/models.py @@ -438,7 +438,7 @@ class Review(models.Model): class Issue(models.Model): frame = models.PositiveIntegerField() - roi = FloatArrayField() + 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) @@ -448,7 +448,7 @@ class Issue(models.Model): class Comment(models.Model): issue = models.ForeignKey(Issue, on_delete=models.CASCADE) - owner = models.ForeignKey(User, null=True, blank=True, on_delete=models.SET_NULL) - message = models.CharField(max_length=4096, default='') + 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 09b9ea4dd8cf..74b3edb3d884 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -532,7 +532,7 @@ def to_representation(self, instance): } class IssueSerializer(serializers.ModelSerializer): - roi = serializers.ListField( + position = serializers.ListField( child=serializers.FloatField(), allow_empty=False, ) @@ -541,7 +541,7 @@ class Meta: model = models.Issue fields = '__all__' read_only_fields = ('created_date', 'id',) - write_once_fields = ('frame', 'roi', 'job', 'owner', 'review', ) + write_once_fields = ('frame', 'position', 'job', 'owner', 'review', ) ordering = ['-id'] class CommentSerializer(serializers.ModelSerializer): @@ -549,7 +549,7 @@ class Meta: model = models.Comment fields = '__all__' read_only_fields = ('created_date', 'updated_date', 'id',) - write_once_fields = ('issue', 'owner', ) + write_once_fields = ('issue', 'author', ) class CombinedIssueSerializer(IssueSerializer): comment_set = CommentSerializer(many=True) diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index c596a80761d9..838c3e4f0b47 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -376,7 +376,7 @@ def setUpTestData(cls): cls.reject_review_data = { "issue_set": [ { - "roi": [ + "position": [ 50, 50, 100, 100 ], "comment_set": [ diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index fc25336f6e8c..602ed8d1db56 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -745,7 +745,7 @@ def create_review(self, request, pk): issue['owner'] = request.user.id comment_set = issue['comment_set'] for comment in comment_set: - comment['owner'] = request.user.id + comment['author'] = request.user.id serializer = CombinedReviewSerializer(data=request.data, partial=True) if serializer.is_valid(raise_exception=True): @@ -853,7 +853,7 @@ def create_comments(self, request, pk): self.get_object() # call to force check persmissions for comment in request.data: comment.update({ - 'owner': request.user.id, + 'author': request.user.id, 'issue': pk }) From 36459a48633c0339f52daa678ab245b803ea57a0 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Mon, 2 Nov 2020 17:22:12 +0300 Subject: [PATCH 18/33] Removed extra change --- cvat/apps/documentation/AWS-Deployment-Guide.md | 1 - 1 file changed, 1 deletion(-) diff --git a/cvat/apps/documentation/AWS-Deployment-Guide.md b/cvat/apps/documentation/AWS-Deployment-Guide.md index 6e0a03d32905..26b78a0ad701 100644 --- a/cvat/apps/documentation/AWS-Deployment-Guide.md +++ b/cvat/apps/documentation/AWS-Deployment-Guide.md @@ -18,5 +18,4 @@ 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. From 322e60db2e90740529ab57d0fa8632159e301177 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Thu, 12 Nov 2020 17:21:04 +0300 Subject: [PATCH 19/33] Merged develop --- cvat/__init__.py | 2 +- .../email_confirmation_signup_message.html | 4 +- .../authentication/password_reset_email.html | 4 +- cvat/apps/dataset_manager/formats/README.md | 33 +++++ cvat/apps/dataset_manager/formats/imagenet.py | 41 ++++++ cvat/apps/dataset_manager/formats/registry.py | 3 +- .../dataset_manager/tests/test_formats.py | 7 +- .../documentation/AWS-Deployment-Guide.md | 1 + .../templates/documentation/base_page.html | 2 +- .../templates/documentation/user_guide.html | 2 +- .../templates/documentation/xml_format.html | 2 +- cvat/apps/engine/media_extractors.py | 21 ++- cvat/apps/engine/prepare.py | 28 +++- cvat/apps/engine/serializers.py | 116 +++++++++------ cvat/apps/engine/task.py | 1 + cvat/apps/engine/tests/test_rest_api.py | 133 +++++++++++++----- cvat/apps/engine/utils.py | 14 ++ cvat/apps/engine/views.py | 42 ++++-- 18 files changed, 347 insertions(+), 109 deletions(-) create mode 100644 cvat/apps/dataset_manager/formats/imagenet.py diff --git a/cvat/__init__.py b/cvat/__init__.py index f96a17247d20..f4cd65b43f53 100644 --- a/cvat/__init__.py +++ b/cvat/__init__.py @@ -4,6 +4,6 @@ from cvat.utils.version import get_version -VERSION = (1, 2, 0, 'alpha', 0) +VERSION = (1, 2, 0, 'beta', 0) __version__ = get_version(VERSION) diff --git a/cvat/apps/authentication/templates/account/email/email_confirmation_signup_message.html b/cvat/apps/authentication/templates/account/email/email_confirmation_signup_message.html index c121b810c453..b63ce574b020 100644 --- a/cvat/apps/authentication/templates/account/email/email_confirmation_signup_message.html +++ b/cvat/apps/authentication/templates/account/email/email_confirmation_signup_message.html @@ -1,5 +1,5 @@ -{% load account %}{% user_display user as user_display %}{% load i18n %}{% autoescape off %}{% blocktrans with -site_name=current_site.name site_domain=current_site.domain %}Hello from {{ site_name }}! +{% load account %}{% user_display user as user_display %}{% load i18n %}{% autoescape off %} {% blocktrans with +site_name=current_site.name site_domain=current_site.domain %} Hello from {{ site_name }}!

You're receiving this e-mail because user {{ user_display }} has given yours as an e-mail address diff --git a/cvat/apps/authentication/templates/authentication/password_reset_email.html b/cvat/apps/authentication/templates/authentication/password_reset_email.html index e2c68db10772..25321ffdf1d6 100644 --- a/cvat/apps/authentication/templates/authentication/password_reset_email.html +++ b/cvat/apps/authentication/templates/authentication/password_reset_email.html @@ -1,5 +1,5 @@ -{% load i18n %}{% autoescape off %} {% blocktrans %}You're receiving this email because you requested a password reset -for your user account at {{ site_name }}.{% endblocktrans %} {% trans "Please go to the following page and choose a new +{% load i18n %}{% autoescape off %} {% blocktrans %} You're receiving this email because you requested a password reset +for your user account at {{ site_name }}. {% endblocktrans %} {% trans "Please go to the following page and choose a new password:" %} {% block reset_link %} {{ protocol }}://{{ domain }}/auth/password/reset/confirm?uid={{ uid }}&token={{ token }} {% endblock %} {% trans "Your username, in case you've forgotten:" %} {{ user.get_username }} {% trans "Thanks for using our site!" %} {% blocktrans %}The {{ site_name }} team{% endblocktrans %} {% endautoescape %} diff --git a/cvat/apps/dataset_manager/formats/README.md b/cvat/apps/dataset_manager/formats/README.md index c3a9b6d0e1db..b47287013eaf 100644 --- a/cvat/apps/dataset_manager/formats/README.md +++ b/cvat/apps/dataset_manager/formats/README.md @@ -18,6 +18,7 @@ - [PASCAL VOC and mask](#voc) - [YOLO](#yolo) - [TF detection API](#tfrecord) + - [ImageNet](#imagenet) ## How to add a new annotation format support @@ -802,3 +803,35 @@ taskname.zip/ ``` - supported annotations: Rectangles, Polygons, Masks (as polygons) + +### [ImageNet](http://www.image-net.org) + +#### ImageNet Dumper + +Downloaded file: a zip archive of the following structure: + +```bash +# if we save images: +taskname.zip/ +└── label1/ + ├── label1_image1.jpg + └── label1_image2.jpg +└── label2/ + ├── label2_image1.jpg + ├── label2_image3.jpg + └── label2_image4.jpg + +# if we keep only annotation: +taskname.zip/ +└── .txt +└── synsets.txt + +``` + +- supported annotations: Labels + +#### ImageNet Loader + +Uploaded file: a zip archive of the structure above + +- supported annotations: Labels diff --git a/cvat/apps/dataset_manager/formats/imagenet.py b/cvat/apps/dataset_manager/formats/imagenet.py new file mode 100644 index 000000000000..d9847549f9e1 --- /dev/null +++ b/cvat/apps/dataset_manager/formats/imagenet.py @@ -0,0 +1,41 @@ +# Copyright (C) 2020 Intel Corporation +# +# SPDX-License-Identifier: MIT + +import os.path as osp +from glob import glob + +import zipfile +from tempfile import TemporaryDirectory + +from datumaro.components.project import Dataset +from cvat.apps.dataset_manager.bindings import CvatTaskDataExtractor, \ + import_dm_annotations +from cvat.apps.dataset_manager.util import make_zip_archive + +from .registry import dm_env, exporter, importer + + +@exporter(name='ImageNet', ext='ZIP', version='1.0') +def _export(dst_file, task_data, save_images=False): + extractor = CvatTaskDataExtractor(task_data, include_images=save_images) + extractor = Dataset.from_extractors(extractor) # apply lazy transform + with TemporaryDirectory() as temp_dir: + if save_images: + dm_env.converters.get('imagenet').convert(extractor, + save_dir=temp_dir, save_images=save_images) + else: + dm_env.converters.get('imagenet_txt').convert(extractor, + save_dir=temp_dir, save_images=save_images) + + make_zip_archive(temp_dir, dst_file) + +@importer(name='ImageNet', ext='ZIP', version='1.0') +def _import(src_file, task_data): + with TemporaryDirectory() as tmp_dir: + zipfile.ZipFile(src_file).extractall(tmp_dir) + if glob(osp.join(tmp_dir, '*.txt')): + dataset = dm_env.make_importer('imagenet_txt')(tmp_dir).make_dataset() + else: + dataset = dm_env.make_importer('imagenet')(tmp_dir).make_dataset() + import_dm_annotations(dataset, task_data) \ No newline at end of file diff --git a/cvat/apps/dataset_manager/formats/registry.py b/cvat/apps/dataset_manager/formats/registry.py index c84d60fc603e..c175a42b7728 100644 --- a/cvat/apps/dataset_manager/formats/registry.py +++ b/cvat/apps/dataset_manager/formats/registry.py @@ -90,4 +90,5 @@ def make_exporter(name): import cvat.apps.dataset_manager.formats.mots import cvat.apps.dataset_manager.formats.pascal_voc import cvat.apps.dataset_manager.formats.tfrecord -import cvat.apps.dataset_manager.formats.yolo \ No newline at end of file +import cvat.apps.dataset_manager.formats.yolo +import cvat.apps.dataset_manager.formats.imagenet \ No newline at end of file diff --git a/cvat/apps/dataset_manager/tests/test_formats.py b/cvat/apps/dataset_manager/tests/test_formats.py index 1eb3e2b56da9..d41e253c5821 100644 --- a/cvat/apps/dataset_manager/tests/test_formats.py +++ b/cvat/apps/dataset_manager/tests/test_formats.py @@ -218,8 +218,6 @@ def _generate_task_images(self, count): # pylint: disable=no-self-use def _generate_task(self, images): task = { "name": "my task #1", - "owner": '', - "assignee": '', "overlap": 0, "segment_size": 100, "labels": [ @@ -271,6 +269,7 @@ def test_export_formats_query(self): 'Segmentation mask 1.1', 'TFRecord 1.0', 'YOLO 1.1', + 'ImageNet 1.0', }) def test_import_formats_query(self): @@ -287,6 +286,7 @@ def test_import_formats_query(self): 'Segmentation mask 1.1', 'TFRecord 1.0', 'YOLO 1.1', + 'ImageNet 1.0', }) def test_exports(self): @@ -322,6 +322,7 @@ def test_empty_images_are_exported(self): ('Segmentation mask 1.1', 'voc'), ('TFRecord 1.0', 'tf_detection_api'), ('YOLO 1.1', 'yolo'), + ('ImageNet 1.0', 'imagenet_txt'), ]: with self.subTest(format=format_name): if not dm.formats.registry.EXPORT_FORMATS[format_name].ENABLED: @@ -435,8 +436,6 @@ def _generate_task_images(self, paths): # pylint: disable=no-self-use def _generate_task(self, images): task = { "name": "my task #1", - "owner": '', - "assignee": '', "overlap": 0, "segment_size": 100, "labels": [ diff --git a/cvat/apps/documentation/AWS-Deployment-Guide.md b/cvat/apps/documentation/AWS-Deployment-Guide.md index 26b78a0ad701..6e0a03d32905 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. diff --git a/cvat/apps/documentation/templates/documentation/base_page.html b/cvat/apps/documentation/templates/documentation/base_page.html index be1271114901..e6117ae3af60 100644 --- a/cvat/apps/documentation/templates/documentation/base_page.html +++ b/cvat/apps/documentation/templates/documentation/base_page.html @@ -1,5 +1,5 @@ diff --git a/cvat/apps/documentation/templates/documentation/user_guide.html b/cvat/apps/documentation/templates/documentation/user_guide.html index 7468b38da29b..3565dd96b727 100644 --- a/cvat/apps/documentation/templates/documentation/user_guide.html +++ b/cvat/apps/documentation/templates/documentation/user_guide.html @@ -1,5 +1,5 @@ diff --git a/cvat/apps/documentation/templates/documentation/xml_format.html b/cvat/apps/documentation/templates/documentation/xml_format.html index de89b1f2531d..496411cce4e2 100644 --- a/cvat/apps/documentation/templates/documentation/xml_format.html +++ b/cvat/apps/documentation/templates/documentation/xml_format.html @@ -1,5 +1,5 @@ diff --git a/cvat/apps/engine/media_extractors.py b/cvat/apps/engine/media_extractors.py index 21430838661d..b58bf98c4cfd 100644 --- a/cvat/apps/engine/media_extractors.py +++ b/cvat/apps/engine/media_extractors.py @@ -14,6 +14,7 @@ import numpy as np from pyunpack import Archive from PIL import Image, ImageFile +from cvat.apps.engine.utils import rotate_image # fixes: "OSError:broken data stream" when executing line 72 while loading images downloaded from the web # see: https://stackoverflow.com/questions/42462431/oserror-broken-data-stream-when-reading-image-file @@ -228,6 +229,16 @@ def _decode(self, container): for image in packet.decode(): frame_num += 1 if self._has_frame(frame_num - 1): + if packet.stream.metadata.get('rotate'): + old_image = image + image = av.VideoFrame().from_ndarray( + rotate_image( + image.to_ndarray(format='bgr24'), + 360 - int(container.streams.video[0].metadata.get('rotate')) + ), + format ='bgr24' + ) + image.pts = old_image.pts yield (image, self._source_path[0], image.pts) def __iter__(self): @@ -252,7 +263,15 @@ def get_preview(self): container = self._get_av_container() stream = container.streams.video[0] preview = next(container.decode(stream)) - return self._get_preview(preview.to_image()) + return self._get_preview(preview.to_image() if not stream.metadata.get('rotate') \ + else av.VideoFrame().from_ndarray( + rotate_image( + preview.to_ndarray(format='bgr24'), + 360 - int(container.streams.video[0].metadata.get('rotate')) + ), + format ='bgr24' + ).to_image() + ) def get_image_size(self, i): image = (next(iter(self)))[0] diff --git a/cvat/apps/engine/prepare.py b/cvat/apps/engine/prepare.py index 9465b680f3a0..9ee546309142 100644 --- a/cvat/apps/engine/prepare.py +++ b/cvat/apps/engine/prepare.py @@ -6,6 +6,7 @@ from collections import OrderedDict import hashlib import os +from cvat.apps.engine.utils import rotate_image class WorkWithVideo: def __init__(self, **kwargs): @@ -24,7 +25,6 @@ def _get_video_stream(self, container): video_stream.thread_type = 'AUTO' return video_stream - class AnalyzeVideo(WorkWithVideo): def check_type_first_frame(self): container = self._open_video_container(self.source_path, mode='r') @@ -76,7 +76,17 @@ def get_task_size(self): @property def frame_sizes(self): + container = self._open_video_container(self.source_path, 'r') frame = next(iter(self.key_frames.values())) + if container.streams.video[0].metadata.get('rotate'): + frame = av.VideoFrame().from_ndarray( + rotate_image( + frame.to_ndarray(format='bgr24'), + 360 - int(container.streams.video[0].metadata.get('rotate')) + ), + format ='bgr24' + ) + self._close_video_container(container) return (frame.width, frame.height) def check_key_frame(self, container, video_stream, key_frame): @@ -150,6 +160,14 @@ def decode_needed_frames(self, chunk_number, db_data): if frame_number < start_chunk_frame_number: continue elif frame_number < end_chunk_frame_number and not ((frame_number - start_chunk_frame_number) % step): + if video_stream.metadata.get('rotate'): + frame = av.VideoFrame().from_ndarray( + rotate_image( + frame.to_ndarray(format='bgr24'), + 360 - int(container.streams.video[0].metadata.get('rotate')) + ), + format ='bgr24' + ) yield frame elif (frame_number - start_chunk_frame_number) % step: continue @@ -177,6 +195,14 @@ def frame_sizes(self): container.seek(offset=next(iter(self.key_frames.values())), stream=video_stream) for packet in container.demux(video_stream): for frame in packet.decode(): + if video_stream.metadata.get('rotate'): + frame = av.VideoFrame().from_ndarray( + rotate_image( + frame.to_ndarray(format='bgr24'), + 360 - int(container.streams.video[0].metadata.get('rotate')) + ), + format ='bgr24' + ) self._close_video_container(container) return (frame.width, frame.height) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 74b3edb3d884..346ad839c82d 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -14,6 +14,36 @@ from cvat.apps.engine.log import slogger from cvat.apps.dataset_manager.formats.utils import get_label_color +class BasicUserSerializer(serializers.ModelSerializer): + def validate(self, data): + if hasattr(self, 'initial_data'): + unknown_keys = set(self.initial_data.keys()) - set(self.fields.keys()) + if unknown_keys: + if set(['is_staff', 'is_superuser', 'groups']) & unknown_keys: + message = 'You do not have permissions to access some of' + \ + ' these fields: {}'.format(unknown_keys) + else: + message = 'Got unknown fields: {}'.format(unknown_keys) + raise serializers.ValidationError(message) + return data + + class Meta: + model = User + fields = ('url', 'id', 'username', 'first_name', 'last_name') + ordering = ['-id'] + +class UserSerializer(serializers.ModelSerializer): + groups = serializers.SlugRelatedField(many=True, + slug_field='name', queryset=Group.objects.all()) + + class Meta: + model = User + fields = ('url', 'id', 'username', 'first_name', 'last_name', 'email', + 'groups', 'is_staff', 'is_superuser', 'is_active', 'last_login', + 'date_joined') + read_only_fields = ('last_login', 'date_joined') + write_only_fields = ('password', ) + ordering = ['-id'] class AttributeSerializer(serializers.ModelSerializer): class Meta: @@ -54,16 +84,27 @@ class JobSerializer(serializers.ModelSerializer): task_id = serializers.ReadOnlyField(source="segment.task.id") start_frame = serializers.ReadOnlyField(source="segment.start_frame") 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', 'reviewer', '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', 'reviewer', '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') @@ -240,14 +281,18 @@ class TaskSerializer(WriteOnceMixin, serializers.ModelSerializer): size = serializers.ReadOnlyField(source='data.size') image_quality = serializers.ReadOnlyField(source='data.image_quality') data = serializers.ReadOnlyField(source='data.id') + owner = BasicUserSerializer(required=False) + owner_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) + assignee = BasicUserSerializer(allow_null=True, required=False) + assignee_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) class Meta: model = models.Task - fields = ('url', 'id', 'name', 'mode', 'owner', 'assignee', + fields = ('url', 'id', 'name', 'mode', 'owner', 'assignee', 'owner_id', 'assignee_id', 'bug_tracker', 'created_date', 'updated_date', 'overlap', 'segment_size', 'status', 'labels', 'segments', 'project', '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', + 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') ordering = ['-id'] @@ -279,8 +324,8 @@ def create(self, validated_data): # pylint: disable=no-self-use def update(self, instance, validated_data): instance.name = validated_data.get('name', instance.name) - instance.owner = validated_data.get('owner', instance.owner) - instance.assignee = validated_data.get('assignee', instance.assignee) + instance.owner_id = validated_data.get('owner_id', instance.owner_id) + instance.assignee_id = validated_data.get('assignee_id', instance.assignee_id) instance.bug_tracker = validated_data.get('bug_tracker', instance.bug_tracker) instance.project = validated_data.get('project', instance.project) @@ -340,38 +385,6 @@ class Meta: read_only_fields = ('created_date', 'updated_date', 'status') ordering = ['-id'] -class BasicUserSerializer(serializers.ModelSerializer): - def validate(self, data): - if hasattr(self, 'initial_data'): - unknown_keys = set(self.initial_data.keys()) - set(self.fields.keys()) - if unknown_keys: - if set(['is_staff', 'is_superuser', 'groups']) & unknown_keys: - message = 'You do not have permissions to access some of' + \ - ' these fields: {}'.format(unknown_keys) - else: - message = 'Got unknown fields: {}'.format(unknown_keys) - raise serializers.ValidationError(message) - return data - - class Meta: - model = User - fields = ('url', 'id', 'username', 'first_name', 'last_name') - ordering = ['-id'] - -class UserSerializer(serializers.ModelSerializer): - groups = serializers.SlugRelatedField(many=True, - slug_field='name', queryset=Group.objects.all()) - rating = serializers.FloatField(read_only=True, source='profile.rating') - - class Meta: - model = User - fields = ('url', 'id', 'username', 'first_name', 'last_name', 'email', - 'groups', 'is_staff', 'is_superuser', 'is_active', 'last_login', - 'date_joined', 'rating') - read_only_fields = ('last_login', 'date_joined', 'rating') - write_only_fields = ('password', ) - ordering = ['-id'] - class ExceptionSerializer(serializers.Serializer): system = serializers.CharField(max_length=255) client = serializers.CharField(max_length=255) @@ -500,11 +513,16 @@ 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', ) - write_once_fields = ('job', 'reviewer', 'assignee', 'estimated_quality', 'status', ) + read_only_fields = ('id', 'assignee', 'reviewer', ) + write_once_fields = ('job', 'reviewer_id', 'assignee_id', 'estimated_quality', 'status', ) ordering = ['-id'] class ReviewSummarySerializer(serializers.Serializer): @@ -532,6 +550,11 @@ def to_representation(self, instance): } 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, @@ -540,16 +563,19 @@ class IssueSerializer(serializers.ModelSerializer): class Meta: model = models.Issue fields = '__all__' - read_only_fields = ('created_date', 'id',) - write_once_fields = ('frame', 'position', 'job', 'owner', 'review', ) + 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',) - write_once_fields = ('issue', 'author', ) + read_only_fields = ('created_date', 'updated_date', 'id', 'author', ) + write_once_fields = ('issue', 'author_id', ) class CombinedIssueSerializer(IssueSerializer): comment_set = CommentSerializer(many=True) diff --git a/cvat/apps/engine/task.py b/cvat/apps/engine/task.py index fad3654fc385..e724d2425d39 100644 --- a/cvat/apps/engine/task.py +++ b/cvat/apps/engine/task.py @@ -294,6 +294,7 @@ def update_progress(progress): if settings.USE_CACHE and db_data.storage_method == StorageMethodChoice.CACHE: for media_type, media_files in media.items(): + if not media_files: continue diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index 838c3e4f0b47..cc76af65eaed 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -297,47 +297,47 @@ def _check_request(self, response, data): self.assertEqual(response.data["id"], self.job.id) self.assertEqual(response.data["status"], data.get('status', self.job.status)) assignee = self.job.assignee.id if self.job.assignee else None - self.assertEqual(response.data["assignee"], data.get('assignee', assignee)) + self.assertEqual(response.data["assignee"]["id"], data.get('assignee_id', assignee)) self.assertEqual(response.data["start_frame"], self.job.segment.start_frame) self.assertEqual(response.data["stop_frame"], self.job.segment.stop_frame) def test_api_v1_jobs_id_admin(self): - data = {"status": StatusChoice.COMPLETED, "assignee": self.owner.id} + data = {"status": StatusChoice.COMPLETED, "assignee_id": self.owner.id} response = self._run_api_v1_jobs_id(self.job.id, self.admin, data) self._check_request(response, data) response = self._run_api_v1_jobs_id(self.job.id + 10, self.admin, data) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_api_v1_jobs_id_owner(self): - data = {"status": StatusChoice.VALIDATION, "assignee": self.annotator.id} + data = {"status": StatusChoice.VALIDATION, "assignee_id": self.annotator.id} response = self._run_api_v1_jobs_id(self.job.id, self.owner, data) self._check_request(response, data) response = self._run_api_v1_jobs_id(self.job.id + 10, self.owner, data) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_api_v1_jobs_id_annotator(self): - data = {"status": StatusChoice.ANNOTATION, "assignee": self.user.id} + data = {"status": StatusChoice.ANNOTATION, "assignee_id": self.user.id} response = self._run_api_v1_jobs_id(self.job.id, self.annotator, data) self._check_request(response, data) response = self._run_api_v1_jobs_id(self.job.id + 10, self.annotator, data) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_api_v1_jobs_id_observer(self): - data = {"status": StatusChoice.ANNOTATION, "assignee": self.admin.id} + data = {"status": StatusChoice.ANNOTATION, "assignee_id": self.admin.id} response = self._run_api_v1_jobs_id(self.job.id, self.observer, data) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) response = self._run_api_v1_jobs_id(self.job.id + 10, self.observer, data) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_api_v1_jobs_id_user(self): - data = {"status": StatusChoice.ANNOTATION, "assignee": self.user.id} + data = {"status": StatusChoice.ANNOTATION, "assignee_id": self.user.id} response = self._run_api_v1_jobs_id(self.job.id, self.user, data) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) response = self._run_api_v1_jobs_id(self.job.id + 10, self.user, data) self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_api_v1_jobs_id_no_auth(self): - data = {"status": StatusChoice.ANNOTATION, "assignee": self.user.id} + data = {"status": StatusChoice.ANNOTATION, "assignee_id": self.user.id} response = self._run_api_v1_jobs_id(self.job.id, None, data) self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) response = self._run_api_v1_jobs_id(self.job.id + 10, None, data) @@ -356,7 +356,7 @@ def test_api_v1_jobs_id_annotator_partial(self): self._check_request(response, data) def test_api_v1_jobs_id_admin_partial(self): - data = {"assignee": self.user.id} + data = {"assignee_id": self.user.id} response = self._run_api_v1_jobs_id(self.job.id, self.owner, data) self._check_request(response, data) @@ -403,7 +403,7 @@ def setUpTestData(cls): "issue_set": [], "estimated_quality": 4, "status": "review_further", - "reviewer": cls.reviewer.id + "reviewer_id": cls.reviewer.id } cls.create_comment_data = [{ @@ -557,6 +557,10 @@ def test_api_v1_edit_review_comment(self): 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) @@ -602,17 +606,17 @@ def test_api_v1_resolve_reopen_issue(self): response = self._patch_request('/api/v1/issues/{}/resolve'.format(issue_id), self.assignee, {}) 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'], self.assignee.id) + self.assertEqual(response.data[0]['resolver_id'], self.assignee.id) response = self._patch_request('/api/v1/issues/{}/reopen'.format(issue_id), self.reviewer, {}) 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) + self.assertEqual(response.data[0]['resolver_id'], None) response = self._patch_request('/api/v1/issues/{}/resolve'.format(issue_id), self.reviewer, {}) 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'], self.reviewer.id) + self.assertEqual(response.data[0]['resolver_id'], self.reviewer.id) class ServerAboutAPITestCase(APITestCase): def setUp(self): @@ -1327,9 +1331,11 @@ def _check_response(self, response, db_task): self.assertEqual(response.data["size"], db_task.data.size) self.assertEqual(response.data["mode"], db_task.mode) owner = db_task.owner.id if db_task.owner else None - self.assertEqual(response.data["owner"], owner) + response_owner = response.data["owner"]["id"] if response.data["owner"] else None + self.assertEqual(response_owner, owner) assignee = db_task.assignee.id if db_task.assignee else None - self.assertEqual(response.data["assignee"], assignee) + response_assignee = response.data["assignee"]["id"] if response.data["assignee"] else None + self.assertEqual(response_assignee, assignee) self.assertEqual(response.data["overlap"], db_task.overlap) self.assertEqual(response.data["segment_size"], db_task.segment_size) self.assertEqual(response.data["image_quality"], db_task.data.image_quality) @@ -1433,11 +1439,13 @@ def _check_response(self, response, db_task, data): mode = data.get("mode", db_task.mode) self.assertEqual(response.data["mode"], mode) owner = db_task.owner.id if db_task.owner else None - owner = data.get("owner", owner) - self.assertEqual(response.data["owner"], owner) + owner = data.get("owner_id", owner) + response_owner = response.data["owner"]["id"] if response.data["owner"] else None + self.assertEqual(response_owner, owner) assignee = db_task.assignee.id if db_task.assignee else None - assignee = data.get("assignee", assignee) - self.assertEqual(response.data["assignee"], assignee) + assignee = data.get("assignee_id", assignee) + response_assignee = response.data["assignee"]["id"] if response.data["assignee"] else None + self.assertEqual(response_assignee, assignee) self.assertEqual(response.data["overlap"], db_task.overlap) self.assertEqual(response.data["segment_size"], db_task.segment_size) image_quality = data.get("image_quality", db_task.data.image_quality) @@ -1467,7 +1475,7 @@ def _check_api_v1_tasks_id(self, user, data): def test_api_v1_tasks_id_admin(self): data = { "name": "new name for the task", - "owner": self.owner.id, + "owner_id": self.owner.id, "labels": [{ "name": "non-vehicle", "attributes": [{ @@ -1483,7 +1491,7 @@ def test_api_v1_tasks_id_admin(self): def test_api_v1_tasks_id_user(self): data = { "name": "new name for the task", - "owner": self.assignee.id, + "owner_id": self.assignee.id, "labels": [{ "name": "car", "attributes": [{ @@ -1531,7 +1539,7 @@ def test_api_v1_tasks_id_admin_partial(self): data = { "name": "new name for the task", - "owner": self.owner.id + "owner_id": self.owner.id } self._check_api_v1_tasks_id(self.admin, data) # Now owner is updated, but self.db_tasks are obsolete @@ -1554,8 +1562,8 @@ def test_api_v1_tasks_id_user_partial(self): self._check_api_v1_tasks_id(self.user, data) data = { - "owner": self.observer.id, - "assignee": self.annotator.id + "owner_id": self.observer.id, + "assignee_id": self.annotator.id } self._check_api_v1_tasks_id(self.user, data) @@ -1593,8 +1601,9 @@ def _check_response(self, response, user, data): self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(response.data["name"], data["name"]) self.assertEqual(response.data["mode"], "") - self.assertEqual(response.data["owner"], data.get("owner", user.id)) - self.assertEqual(response.data["assignee"], data.get("assignee")) + self.assertEqual(response.data["owner"]["id"], data.get("owner_id", user.id)) + assignee = response.data["assignee"]["id"] if response.data["assignee"] else None + self.assertEqual(assignee, data.get("assignee_id", None)) self.assertEqual(response.data["bug_tracker"], data.get("bug_tracker", "")) self.assertEqual(response.data["overlap"], data.get("overlap", None)) self.assertEqual(response.data["segment_size"], data.get("segment_size", 0)) @@ -1631,7 +1640,7 @@ def test_api_v1_tasks_admin(self): def test_api_v1_tasks_user(self): data = { "name": "new name for the task", - "owner": self.assignee.id, + "owner_id": self.assignee.id, "labels": [{ "name": "car", "attributes": [{ @@ -1802,6 +1811,16 @@ def setUpClass(cls): video.write(data.read()) cls._image_sizes[filename] = img_sizes + filename = "test_rotated_90_video.mp4" + path = os.path.join(os.path.dirname(__file__), 'assets', 'test_rotated_90_video.mp4') + container = av.open(path, 'r') + for frame in container.decode(video=0): + # pyav ignores rotation record in metadata when decoding frames + img_sizes = [(frame.height, frame.width)] * container.streams.video[0].frames + break + container.close() + cls._image_sizes[filename] = img_sizes + filename = os.path.join("videos", "test_video_1.mp4") path = os.path.join(settings.SHARE_ROOT, filename) os.makedirs(os.path.dirname(path)) @@ -1907,8 +1926,8 @@ def _test_api_v1_tasks_id_data_spec(self, user, spec, data, expected_compressed_ response = self._get_task(user, task_id) expected_status_code = status.HTTP_200_OK - if user == self.user and "owner" in spec and spec["owner"] != user.id and \ - "assignee" in spec and spec["assignee"] != user.id: + if user == self.user and "owner_id" in spec and spec["owner_id"] != user.id and \ + "assignee_id" in spec and spec["assignee_id"] != user.id: expected_status_code = status.HTTP_403_FORBIDDEN self.assertEqual(response.status_code, expected_status_code) @@ -1990,8 +2009,8 @@ def _test_api_v1_tasks_id_data_spec(self, user, spec, data, expected_compressed_ def _test_api_v1_tasks_id_data(self, user): task_spec = { "name": "my task #1", - "owner": self.owner.id, - "assignee": self.assignee.id, + "owner_id": self.owner.id, + "assignee_id": self.assignee.id, "overlap": 0, "segment_size": 100, "labels": [ @@ -2257,7 +2276,7 @@ def _test_api_v1_tasks_id_data(self, user): os.path.join(settings.SHARE_ROOT, "videos") ) task_spec = { - "name": "my video with meta info task #11", + "name": "my video with meta info task #13", "overlap": 0, "segment_size": 0, "labels": [ @@ -2276,6 +2295,47 @@ def _test_api_v1_tasks_id_data(self, user): self._test_api_v1_tasks_id_data_spec(user, task_spec, task_data, self.ChunkType.VIDEO, self.ChunkType.VIDEO, image_sizes, StorageMethodChoice.CACHE) + task_spec = { + "name": "my cached video task #14", + "overlap": 0, + "segment_size": 0, + "labels": [ + {"name": "car"}, + {"name": "person"}, + ] + } + + task_data = { + "client_files[0]": open(os.path.join(os.path.dirname(__file__), 'assets', 'test_rotated_90_video.mp4'), 'rb'), + "image_quality": 70, + "use_zip_chunks": True + } + + image_sizes = self._image_sizes['test_rotated_90_video.mp4'] + self._test_api_v1_tasks_id_data_spec(user, task_spec, task_data, self.ChunkType.IMAGESET, + self.ChunkType.VIDEO, image_sizes, StorageMethodChoice.FILE_SYSTEM) + + task_spec = { + "name": "my video task #15", + "overlap": 0, + "segment_size": 0, + "labels": [ + {"name": "car"}, + {"name": "person"}, + ] + } + + task_data = { + "client_files[0]": open(os.path.join(os.path.dirname(__file__), 'assets', 'test_rotated_90_video.mp4'), 'rb'), + "image_quality": 70, + "use_cache": True, + "use_zip_chunks": True + } + + image_sizes = self._image_sizes['test_rotated_90_video.mp4'] + self._test_api_v1_tasks_id_data_spec(user, task_spec, task_data, self.ChunkType.IMAGESET, + self.ChunkType.VIDEO, image_sizes, StorageMethodChoice.CACHE) + def test_api_v1_tasks_id_data_admin(self): self._test_api_v1_tasks_id_data(self.admin) @@ -2288,8 +2348,8 @@ def test_api_v1_tasks_id_data_user(self): def test_api_v1_tasks_id_data_no_auth(self): data = { "name": "my task #3", - "owner": self.owner.id, - "assignee": self.assignee.id, + "owner_id": self.owner.id, + "assignee_id": self.assignee.id, "overlap": 0, "segment_size": 100, "labels": [ @@ -2334,8 +2394,8 @@ def setUpTestData(cls): def _create_task(self, owner, assignee): data = { "name": "my task #1", - "owner": owner.id, - "assignee": assignee.id, + "owner_id": owner.id, + "assignee_id": assignee.id, "overlap": 0, "segment_size": 100, "labels": [ @@ -3609,6 +3669,9 @@ def _get_initial_annotation(annotation_format): + polygon_shapes_with_attrs annotations["tags"] = tags_with_attrs + tags_wo_attrs + elif annotation_format == "ImageNet 1.0": + annotations["tags"] = tags_wo_attrs + else: raise Exception("Unknown format {}".format(annotation_format)) diff --git a/cvat/apps/engine/utils.py b/cvat/apps/engine/utils.py index e1ad9ef8381a..854393cfa75f 100644 --- a/cvat/apps/engine/utils.py +++ b/cvat/apps/engine/utils.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: MIT import ast +import cv2 as cv from collections import namedtuple import importlib import sys @@ -74,3 +75,16 @@ def av_scan_paths(*paths): res = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) if res.returncode: raise ValidationError(res.stdout) + +def rotate_image(image, angle): + height, width = image.shape[:2] + image_center = (width/2, height/2) + matrix = cv.getRotationMatrix2D(image_center, angle, 1.) + abs_cos = abs(matrix[0,0]) + abs_sin = abs(matrix[0,1]) + bound_w = int(height * abs_sin + width * abs_cos) + bound_h = int(height * abs_cos + width * abs_sin) + matrix[0, 2] += bound_w/2 - image_center[0] + matrix[1, 2] += bound_h/2 - image_center[1] + matrix = cv.warpAffine(image, matrix, (bound_w, bound_h)) + return matrix diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 602ed8d1db56..d7932323d834 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -719,7 +719,7 @@ def annotations(self, request, pk): def reviews(self, request, pk): db_job = self.get_object() queryset = db_job.review_set - serializer = ReviewSerializer(queryset, many=True) + serializer = ReviewSerializer(queryset, context={'request': request}, many=True) return Response(serializer.data) @swagger_auto_schema(method='post', operation_summary='Submit a review for the job') @@ -728,24 +728,28 @@ def create_review(self, request, pk): db_job = self.get_object() if request.data['status'] == ReviewStatus.REVIEW_FURTHER: - if 'reviewer' not in request.data: + if 'reviewer_id' not in request.data: return Response('Must provide a new reviewer', status=status.HTTP_400_BAD_REQUEST) - db_job.reviewer = User.objects.get(pk=request.data['reviewer']) + db_job.reviewer = User.objects.get(pk=request.data['reviewer_id']) db_job.save() request.data.update({ 'job': db_job.id, - 'reviewer': request.user.id, - 'assignee': db_job.assignee.id, + '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'] = request.user.id + issue['owner_id'] = request.user.id comment_set = issue['comment_set'] for comment in comment_set: - comment['author'] = request.user.id + comment['author_id'] = request.user.id serializer = CombinedReviewSerializer(data=request.data, partial=True) if serializer.is_valid(raise_exception=True): @@ -756,7 +760,7 @@ def create_review(self, request, pk): elif instance.status == ReviewStatus.REJECTED: db_job.status = StatusChoice.ANNOTATION db_job.save() - return Response(CombinedReviewSerializer(instance).data, status=status.HTTP_201_CREATED) + return Response(CombinedReviewSerializer(instance, context={'request': request}).data, status=status.HTTP_201_CREATED) @swagger_auto_schema(method='get', operation_summary='Get a brief summary about done reviews') @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer) @@ -772,7 +776,7 @@ def reviews_summary(self, request, pk): def issues(self, request, pk): db_job = self.get_object() queryset = db_job.issue_set - serializer = CombinedIssueSerializer(queryset, many=True) + serializer = CombinedIssueSerializer(queryset, context={'request': request}, many=True) return Response(serializer.data) @method_decorator(name='destroy', decorator=swagger_auto_schema(operation_summary='Method removes a review from a job')) @@ -819,7 +823,7 @@ def resolve(self, request, pk): db_issue.resolver = request.user db_issue.resolved_date = datetime.now() db_issue.save() - serializer = IssueSerializer(db_issue) + serializer = IssueSerializer(db_issue, context={'request': request}) return Response(serializer.data) @swagger_auto_schema(method='patch', operation_summary='The action reopens a specific issue', @@ -832,7 +836,7 @@ def reopen(self, request, pk): db_issue.resolver = None db_issue.resolved_date = None db_issue.save() - serializer = IssueSerializer(db_issue) + serializer = IssueSerializer(db_issue, context={'request': request}) return Response(serializer.data) @swagger_auto_schema(method='get', operation_summary='The action returns all comments of a specific issue', @@ -842,7 +846,7 @@ def reopen(self, request, pk): def comments(self, request, pk): db_issue = self.get_object() queryset = db_issue.comment_set - serializer = CommentSerializer(queryset, many=True) + serializer = CommentSerializer(queryset, context={'request': request}, many=True) return Response(serializer.data) @swagger_auto_schema(method='post', operation_summary='The action adds comments to an issue', @@ -853,7 +857,7 @@ def create_comments(self, request, pk): self.get_object() # call to force check persmissions for comment in request.data: comment.update({ - 'author': request.user.id, + 'author_id': request.user.id, 'issue': pk }) @@ -861,7 +865,7 @@ def create_comments(self, request, pk): if serializer.is_valid(): serializer.save() db_issue = Issue.objects.prefetch_related('comment_set').get(pk=pk) - updated_serializer = CommentSerializer(db_issue.comment_set, many=True) + updated_serializer = CommentSerializer(db_issue.comment_set, context={'request': request}, many=True) return Response(updated_serializer.data, status=status.HTTP_201_CREATED) @method_decorator(name='partial_update', decorator=swagger_auto_schema(operation_summary='Method updates comment in an issue')) @@ -883,7 +887,15 @@ def get_permissions(self): 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), + ], operation_summary='Method provides a paginated list of users registered on the server')) @method_decorator(name='retrieve', decorator=swagger_auto_schema( operation_summary='Method provides information of a specific user')) @@ -895,6 +907,8 @@ class UserViewSet(viewsets.GenericViewSet, mixins.ListModelMixin, mixins.RetrieveModelMixin, mixins.UpdateModelMixin, mixins.DestroyModelMixin): queryset = User.objects.prefetch_related('groups').all().order_by('id') http_method_names = ['get', 'post', 'head', 'patch', 'delete'] + search_fields = ('username', 'first_name', 'last_name') + filterset_class = UserFilter def get_serializer_class(self): user = self.request.user From cdfdf4d62de838872e0aaf5ad498f5f1ddfa0b1b Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Thu, 12 Nov 2020 17:32:09 +0300 Subject: [PATCH 20/33] Removed extra changes --- .../case_16_z_order_features.js | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/tests/cypress/integration/actions_tasks_objects/case_16_z_order_features.js b/tests/cypress/integration/actions_tasks_objects/case_16_z_order_features.js index 53ae82c3574d..4b64e4e1a5df 100644 --- a/tests/cypress/integration/actions_tasks_objects/case_16_z_order_features.js +++ b/tests/cypress/integration/actions_tasks_objects/case_16_z_order_features.js @@ -38,17 +38,16 @@ context('Actions on polygon', () => { }); describe(`Testing case "${caseId}"`, () => { + it('Create a first polygon shape', () => { cy.createPolygon(createPolygonShapeFirst); }); it('Increase z-layer with a special switcher', () => { cy.get('.cvat-canvas-z-axis-wrapper').within(() => { - cy.get('[role="slider"]') - .should('have.attr', 'aria-valuenow') - .then(($zLayer) => { - zLayer = Number($zLayer); - }); + cy.get('[role="slider"]').should('have.attr', 'aria-valuenow').then($zLayer=> { + zLayer = Number($zLayer); + }); cy.get('i[aria-label="icon: plus-circle"]').click(); cy.get('[role="slider"]').should('have.attr', 'aria-valuenow', zLayer + 1); }); @@ -62,25 +61,22 @@ context('Actions on polygon', () => { cy.get('.cvat-canvas-container').click(); }); - it('Second shape is over the first shape', () => { + it('Second shape is over the first shape', () => { // The larger the index of an element in the array the closer it is to us - cy.get('.cvat_canvas_shape').then(($canvasShape) => { - expect(Number($canvasShape[1].id.match(/\d+$/))).to.be.equal(2); + cy.get('.cvat_canvas_shape').then($canvasShape => { +                expect(Number($canvasShape[1].id.match(/\d+$/))).to.be.equal(2); }); }); - it('Activate first shape', () => { + it('Activate first shape', () => { cy.get('#cvat_canvas_shape_1').trigger('mousemove').trigger('mouseover'); }); - it('First shape is over the second shape', () => { + it('First shape is over the second shape', () => { // The larger the index of an element in the array the closer it is to us - cy.get('.cvat_canvas_shape').then(($canvasShape) => { - expect(Number($canvasShape[1].id.match(/\d+$/))).to.be.equal(1); - assert.isAbove( - Number($canvasShape.eq(-1).attr('fill-opacity')), - Number($canvasShape.eq(0).attr('fill-opacity')), - ); +            cy.get('.cvat_canvas_shape').then($canvasShape => { +                expect(Number($canvasShape[1].id.match(/\d+$/))).to.be.equal(1); + assert.isAbove(Number($canvasShape.eq(-1).attr('fill-opacity')), Number($canvasShape.eq(0).attr('fill-opacity'))); }); }); @@ -101,11 +97,9 @@ context('Actions on polygon', () => { it('Increase z-layer with a special switcher', () => { cy.get('.cvat-canvas-z-axis-wrapper').within(() => { - cy.get('[role="slider"]') - .should('have.attr', 'aria-valuenow') - .then(($zLayer) => { - zLayer = Number($zLayer); - }); + cy.get('[role="slider"]').should('have.attr', 'aria-valuenow').then($zLayer=> { + zLayer = Number($zLayer); + }); cy.get('i[aria-label="icon: plus-circle"]').click(); cy.get('[role="slider"]').should('have.attr', 'aria-valuenow', zLayer + 2); }); From e1da6098f27f819c2dbc04847ed8ee3cd06701d6 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Thu, 12 Nov 2020 17:33:06 +0300 Subject: [PATCH 21/33] Removed extra changes --- .../case_15_group_features.js | 88 ++++++++----------- 1 file changed, 35 insertions(+), 53 deletions(-) diff --git a/tests/cypress/integration/actions_tasks_objects/case_15_group_features.js b/tests/cypress/integration/actions_tasks_objects/case_15_group_features.js index 8eb00f690ce8..5b0bd1d04cde 100644 --- a/tests/cypress/integration/actions_tasks_objects/case_15_group_features.js +++ b/tests/cypress/integration/actions_tasks_objects/case_15_group_features.js @@ -62,23 +62,19 @@ context('Group features', () => { }); it('Set option "Color by" to "Group".', () => { cy.changeAppearance('Group'); - cy.get('.cvat_canvas_shape').then(($listCanvasShapes) => { - for (let i = 0; i < $listCanvasShapes.length; i++) { - cy.get($listCanvasShapes[i]) - .should('have.css', 'fill') - .then(($fill) => { - defaultGroupColor = $fill; - }); + cy.get('.cvat_canvas_shape').then($listCanvasShapes => { + for (let i=0; i<$listCanvasShapes.length; i++) { + cy.get($listCanvasShapes[i]).should('have.css', 'fill').then($fill => { + defaultGroupColor = $fill; + }); } }); - cy.get('.cvat-objects-sidebar-state-item').then(($listObjectsSidebarStateItem) => { - for (let i = 0; i < $listObjectsSidebarStateItem.length; i++) { - cy.get($listObjectsSidebarStateItem[i]) - .should('have.css', 'background-color') - .then(($bColorObjectsSidebarStateItem) => { - // expected rgba(224, 224, 224, 0.533) to include [ 224, 224, 224, index: 4, input: 'rgb(224, 224, 224)', groups: undefined ] - expect($bColorObjectsSidebarStateItem).contain(defaultGroupColor.match(/\d+, \d+, \d+/)); - }); + cy.get('.cvat-objects-sidebar-state-item').then($listObjectsSidebarStateItem => { + for (let i=0; i<$listObjectsSidebarStateItem.length; i++) { + cy.get($listObjectsSidebarStateItem[i]).should('have.css', 'background-color').then($bColorObjectsSidebarStateItem => { + // expected rgba(224, 224, 224, 0.533) to include [ 224, 224, 224, index: 4, input: 'rgb(224, 224, 224)', groups: undefined ] + expect($bColorObjectsSidebarStateItem).contain(defaultGroupColor.match(/\d+, \d+, \d+/)); + }); } }); }); @@ -89,26 +85,19 @@ context('Group features', () => { } cy.get('.cvat-group-control').click(); for (const groupedShape of ['#cvat_canvas_shape_1', '#cvat_canvas_shape_2']) { - cy.get(groupedShape) - .should('have.css', 'fill') - .then(($shapesGroupColor) => { - // expected rgb(250, 50, 83) to not equal rgb(224, 224, 224) - expect($shapesGroupColor).to.not.equal(defaultGroupColor); - shapesGroupColor = $shapesGroupColor; - }); + cy.get(groupedShape).should('have.css', 'fill').then($shapesGroupColor => { + // expected rgb(250, 50, 83) to not equal rgb(224, 224, 224) + expect($shapesGroupColor).to.not.equal(defaultGroupColor); + shapesGroupColor = $shapesGroupColor; + }); } - for (const objectSideBarShape of [ - '#cvat-objects-sidebar-state-item-1', - '#cvat-objects-sidebar-state-item-2', - ]) { - cy.get(objectSideBarShape) - .should('have.css', 'background-color') - .then(($bColorobjectSideBarShape) => { - // expected rgba(250, 50, 83, 0.533) to not include [ 224, 224, 224, index: 4, input: 'rgb(224, 224, 224)', groups: undefined ] - expect($bColorobjectSideBarShape).to.not.contain(defaultGroupColor.match(/\d+, \d+, \d+/)); - // expected rgba(250, 50, 83, 0.533) to include [ 250, 50, 83, index: 4, input: 'rgb(250, 50, 83)', groups: undefined ] - expect($bColorobjectSideBarShape).to.be.contain(shapesGroupColor.match(/\d+, \d+, \d+/)); - }); + for (const objectSideBarShape of ['#cvat-objects-sidebar-state-item-1', '#cvat-objects-sidebar-state-item-2']) { + cy.get(objectSideBarShape).should('have.css', 'background-color').then($bColorobjectSideBarShape => { + // expected rgba(250, 50, 83, 0.533) to not include [ 224, 224, 224, index: 4, input: 'rgb(224, 224, 224)', groups: undefined ] + expect($bColorobjectSideBarShape).to.not.contain(defaultGroupColor.match(/\d+, \d+, \d+/)); + // expected rgba(250, 50, 83, 0.533) to include [ 250, 50, 83, index: 4, input: 'rgb(250, 50, 83)', groups: undefined ] + expect($bColorobjectSideBarShape).to.be.contain(shapesGroupColor.match(/\d+, \d+, \d+/)); + }); } }); it('With group button unite two track. They have corresponding colors.', () => { @@ -118,26 +107,19 @@ context('Group features', () => { } cy.get('.cvat-group-control').click(); for (const groupedTrack of ['#cvat_canvas_shape_3', '#cvat_canvas_shape_4']) { - cy.get(groupedTrack) - .should('have.css', 'fill') - .then(($tracksGroupColor) => { - // expected rgb(250, 50, 83) to not equal rgb(224, 224, 224) - expect($tracksGroupColor).to.not.equal(defaultGroupColor); - tracksGroupColor = $tracksGroupColor; - }); + cy.get(groupedTrack).should('have.css', 'fill').then($tracksGroupColor => { + // expected rgb(250, 50, 83) to not equal rgb(224, 224, 224) + expect($tracksGroupColor).to.not.equal(defaultGroupColor); + tracksGroupColor = $tracksGroupColor; + }); } - for (const objectSideBarTrack of [ - '#cvat-objects-sidebar-state-item-3', - '#cvat-objects-sidebar-state-item-4', - ]) { - cy.get(objectSideBarTrack) - .should('have.css', 'background-color') - .then(($bColorobjectSideBarTrack) => { - // expected rgba(52, 209, 183, 0.533) to not include [ 224, 224, 224, index: 4, input: 'rgb(224, 224, 224)', groups: undefined ] - expect($bColorobjectSideBarTrack).to.not.contain(defaultGroupColor.match(/\d+, \d+, \d+/)); - // expected rgba(52, 209, 183, 0.533) to include [ 52, 209, 183, index: 4, input: 'rgb(52, 209, 183)', groups: undefined ] - expect($bColorobjectSideBarTrack).to.be.contain(tracksGroupColor.match(/\d+, \d+, \d+/)); - }); + for (const objectSideBarTrack of ['#cvat-objects-sidebar-state-item-3', '#cvat-objects-sidebar-state-item-4']) { + cy.get(objectSideBarTrack).should('have.css', 'background-color').then($bColorobjectSideBarTrack => { + // expected rgba(52, 209, 183, 0.533) to not include [ 224, 224, 224, index: 4, input: 'rgb(224, 224, 224)', groups: undefined ] + expect($bColorobjectSideBarTrack).to.not.contain(defaultGroupColor.match(/\d+, \d+, \d+/)); + // expected rgba(52, 209, 183, 0.533) to include [ 52, 209, 183, index: 4, input: 'rgb(52, 209, 183)', groups: undefined ] + expect($bColorobjectSideBarTrack).to.be.contain(tracksGroupColor.match(/\d+, \d+, \d+/)); + }); } }); }); From b035dc3403b394a0554132f0125ea065c8d7a9e7 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Thu, 12 Nov 2020 20:20:45 +0300 Subject: [PATCH 22/33] Fixed tests --- cvat/apps/engine/tests/test_rest_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index cc76af65eaed..ada3ee459ba2 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -606,17 +606,17 @@ def test_api_v1_resolve_reopen_issue(self): response = self._patch_request('/api/v1/issues/{}/resolve'.format(issue_id), self.assignee, {}) 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) + self.assertEqual(response.data[0]['resolver']['id'], self.assignee.id) response = self._patch_request('/api/v1/issues/{}/reopen'.format(issue_id), self.reviewer, {}) 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'], None) + self.assertEqual(response.data[0]['resolver'], None) response = self._patch_request('/api/v1/issues/{}/resolve'.format(issue_id), self.reviewer, {}) 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) + self.assertEqual(response.data[0]['resolver']['id'], self.reviewer.id) class ServerAboutAPITestCase(APITestCase): def setUp(self): From ec74ce3457feeb9e1b0c3c5713e7ad69e96769bf Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 18 Nov 2020 11:56:42 +0300 Subject: [PATCH 23/33] Removed /comments/ [PUT] --- cvat/apps/engine/views.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index d7932323d834..8d0b529364a9 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -870,17 +870,20 @@ def create_comments(self, request, pk): @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')) -@method_decorator(name='update', decorator=swagger_auto_schema(operation_summary='Method updates comment in an issue')) class CommentViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin, mixins.UpdateModelMixin): queryset = Comment.objects.all().order_by('id') serializer_class = CommentSerializer + http_method_names = ['get', 'post', 'patch', 'delete', 'options'] + + def update(self, *args, **kwargs): + raise MethodNotAllowed('PUT', detail='Use PATCH instead') def get_permissions(self): http_method = self.request.method permissions = [IsAuthenticated] - if http_method in ['PATCH', 'PUT', 'DELETE']: + if http_method in ['PATCH', 'DELETE']: permissions.append(auth.CommentChangePermission) else: permissions.append(auth.AdminRolePermission) From 3fba002476df9a0fc801ffb10dbb347e17298a17 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 18 Nov 2020 13:06:36 +0300 Subject: [PATCH 24/33] /issue/comments/create [POST] -> /comments [POST] --- cvat/apps/authentication/auth.py | 4 +-- cvat/apps/engine/tests/test_rest_api.py | 33 +++++++++++++-------- cvat/apps/engine/views.py | 38 +++++++++---------------- 3 files changed, 37 insertions(+), 38 deletions(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 68b9cdec2bf9..1dfb2e35af0b 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -307,9 +307,9 @@ def has_object_permission(self, request, view, obj): return (request.user.has_perm('engine.job.change', db_job) or request.user.has_perm('engine.issue.change', obj)) -class IssueCommentPermission(BasePermission): +class CommentCreatePermission(BasePermission): # pylint: disable=no-self-use - def has_object_permission(self, request, view, obj): + def has_object_permission(self, request, view, db_issue): db_job = obj.job return request.user.has_perm('engine.job.access', db_job) diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index ada3ee459ba2..cb7e2239042b 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -531,10 +531,13 @@ def test_api_v1_create_review_comment(self): ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) issue_id = response.data['issue_set'][0]['id'] - response = self._post_request('/api/v1/issues/{}/comments/create'.format(issue_id), - self.assignee, self.create_comment_data - ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) + 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) @@ -548,10 +551,13 @@ def test_api_v1_edit_review_comment(self): ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) issue_id = response.data['issue_set'][0]['id'] - response = self._post_request('/api/v1/issues/{}/comments/create'.format(issue_id), - self.assignee, self.create_comment_data - ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) + 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({ @@ -579,10 +585,13 @@ def test_api_v1_remove_comment(self): ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) issue_id = response.data['issue_set'][0]['id'] - response = self._post_request('/api/v1/issues/{}/comments/create'.format(issue_id), - self.assignee, self.create_comment_data - ) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) + 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) diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 8d0b529364a9..7b3807692606 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -806,8 +806,6 @@ def get_permissions(self): permissions.append(auth.IssueDestroyPermission) elif http_method in ['PATCH', 'PUT']: permissions.append(auth.IssueChangePermission) - elif http_method in ['POST']: - permissions.append(auth.IssueCommentPermission) else: permissions.append(auth.AdminRolePermission) @@ -849,35 +847,25 @@ def comments(self, request, pk): serializer = CommentSerializer(queryset, context={'request': request}, many=True) return Response(serializer.data) - @swagger_auto_schema(method='post', operation_summary='The action adds comments to an issue', - responses={'201': CommentSerializer(many=True)} - ) - @action(detail=True, methods=['POST'], url_path='comments/create', serializer_class=CommentSerializer) - def create_comments(self, request, pk): - self.get_object() # call to force check persmissions - for comment in request.data: - comment.update({ - 'author_id': request.user.id, - 'issue': pk - }) - - serializer = CommentSerializer(data=request.data, many=True) - if serializer.is_valid(): - serializer.save() - db_issue = Issue.objects.prefetch_related('comment_set').get(pk=pk) - updated_serializer = CommentSerializer(db_issue.comment_set, context={'request': request}, many=True) - return Response(updated_serializer.data, status=status.HTTP_201_CREATED) - @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.DestroyModelMixin, mixins.UpdateModelMixin, mixins.CreateModelMixin): queryset = Comment.objects.all().order_by('id') serializer_class = CommentSerializer http_method_names = ['get', 'post', 'patch', 'delete', 'options'] - def update(self, *args, **kwargs): - raise MethodNotAllowed('PUT', detail='Use PATCH instead') + def create(self, request, *args, **kwargs): + request.data.update({ + 'author_id': request.user.id, + }) + issue_id = request.data['issue'] + try: + db_issue = Issue.objects.get(pk=1) + except MyModel.DoesNotExist: + raise Http404('Issue with id {} does not exist'.format(issue_id)) + self.check_object_permissions(self.request, db_issue) + return super().create(request, args, kwargs) def get_permissions(self): http_method = self.request.method @@ -885,6 +873,8 @@ def get_permissions(self): if http_method in ['PATCH', 'DELETE']: permissions.append(auth.CommentChangePermission) + elif http_method in ['POST']: + permissions.append(auth.CommentCreatePermission) else: permissions.append(auth.AdminRolePermission) From d89569975be8f517f9bdbda7a82692b49612ae33 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 18 Nov 2020 14:07:59 +0300 Subject: [PATCH 25/33] /job//reviews/create [POST] -> /reviews [POST] --- cvat/apps/authentication/auth.py | 5 +- cvat/apps/engine/signals.py | 3 +- cvat/apps/engine/tests/test_rest_api.py | 59 ++++--------- cvat/apps/engine/views.py | 109 +++++++++++++----------- 4 files changed, 78 insertions(+), 98 deletions(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 1dfb2e35af0b..48ab04aaa1c5 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -309,9 +309,8 @@ def has_object_permission(self, request, view, obj): class CommentCreatePermission(BasePermission): # pylint: disable=no-self-use - def has_object_permission(self, request, view, db_issue): - db_job = obj.job - return request.user.has_perm('engine.job.access', db_job) + 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 diff --git a/cvat/apps/engine/signals.py b/cvat/apps/engine/signals.py index e12178a52358..5ef6e5f3c34e 100644 --- a/cvat/apps/engine/signals.py +++ b/cvat/apps/engine/signals.py @@ -30,14 +30,13 @@ def update_task_status(instance, **kwargs): db_task.status = status db_task.save() -@receiver(post_save, sender=User) +@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): shutil.rmtree(instance.get_task_dirname(), ignore_errors=True) diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index cb7e2239042b..5cd753b5d90c 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -374,6 +374,7 @@ def setUpTestData(cls): cls.job.assignee = cls.assignee cls.job.save() cls.reject_review_data = { + "job": cls.job.id, "issue_set": [ { "position": [ @@ -394,12 +395,14 @@ def setUpTestData(cls): } 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", @@ -452,38 +455,24 @@ def _set_validation_status(self): def test_api_v1_job_annotation_review(self): self._set_annotation_status() - response = self._post_request( - '/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.accept_review_data - ) + 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/jobs/{}/reviews/create'.format(self.job.id), - self.assignee, self.accept_review_data - ) + 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/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, - self.accept_review_data - ) + 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/jobs/{}/reviews/create'.format(self.job.id), - self.assignee, - self.accept_review_data - ) + 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/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, - self.reject_review_data - ) + 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') @@ -491,10 +480,7 @@ def test_api_v1_job_reject_review(self): def test_api_v1_job_review_further(self): self._set_validation_status() - response = self._post_request( - '/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.review_further_data - ) + 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') @@ -502,17 +488,12 @@ def test_api_v1_job_review_further(self): def test_api_v1_job_review_summary(self): self._set_validation_status() - response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.accept_review_data - ) + response = self._post_request('/api/v1/reviews', self.reviewer, self.accept_review_data) self.assertEqual(response.status_code, status.HTTP_201_CREATED) self._set_validation_status() - response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.review_further_data - ) + response = self._post_request('/api/v1/reviews', self.reviewer, self.review_further_data) self.assertEqual(response.status_code, status.HTTP_201_CREATED) - response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.reject_review_data) + response = self._post_request('/api/v1/reviews', self.reviewer, self.reject_review_data) self.assertEqual(response.status_code, status.HTTP_201_CREATED) response = self._get_request('/api/v1/jobs/{}/reviews/summary'.format(self.job.id), self.reviewer) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -526,9 +507,7 @@ def test_api_v1_job_review_summary(self): def test_api_v1_create_review_comment(self): self._set_validation_status() - response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.reject_review_data - ) + 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[:] @@ -546,9 +525,7 @@ def test_api_v1_create_review_comment(self): def test_api_v1_edit_review_comment(self): self._set_validation_status() - response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.reject_review_data - ) + 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[:] @@ -580,9 +557,7 @@ def test_api_v1_edit_review_comment(self): def test_api_v1_remove_comment(self): self._set_validation_status() - response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.reject_review_data - ) + 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[:] @@ -606,9 +581,7 @@ def test_api_v1_remove_comment(self): def test_api_v1_resolve_reopen_issue(self): self._set_validation_status() - response = self._post_request('/api/v1/jobs/{}/reviews/create'.format(self.job.id), - self.reviewer, self.reject_review_data - ) + 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'] diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 7b3807692606..a2eee9399213 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -11,6 +11,7 @@ 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 @@ -650,15 +651,12 @@ class JobViewSet(viewsets.GenericViewSet, def get_permissions(self): http_method = self.request.method - http_path = self.request.path permissions = [IsAuthenticated] if http_method in SAFE_METHODS: permissions.append(auth.JobAccessPermission) elif http_method in ["PATCH", "PUT", "DELETE"]: permissions.append(auth.JobChangePermission) - elif http_method == 'POST' and http_path.endswith('reviews/create'): - permissions.append(auth.JobReviewPermission) else: permissions.append(auth.AdminRolePermission) @@ -722,22 +720,57 @@ def reviews(self, request, pk): serializer = ReviewSerializer(queryset, context={'request': request}, many=True) return Response(serializer.data) - @swagger_auto_schema(method='post', operation_summary='Submit a review for the job') - @action(detail=True, methods=['POST'], url_path='reviews/create', serializer_class=CombinedReviewSerializer) - def create_review(self, request, pk): + @swagger_auto_schema(method='get', operation_summary='Get a brief summary about done reviews') + @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer) + def reviews_summary(self, request, pk): + db_job = self.get_object() + serializer = ReviewSummarySerializer(db_job) + 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) - db_job.reviewer = User.objects.get(pk=request.data['reviewer_id']) - db_job.save() + reviewer_id = request.data['reviewer_id'] + reviewer = get_object_or_404(User, pk=reviewer_id) request.data.update({ - 'job': db_job.id, 'reviewer_id': request.user.id, }) - if db_job.assignee: request.data.update({ 'assignee_id': db_job.assignee.id, @@ -751,43 +784,22 @@ def create_review(self, request, pk): for comment in comment_set: comment['author_id'] = request.user.id - serializer = CombinedReviewSerializer(data=request.data, partial=True) - if serializer.is_valid(raise_exception=True): - instance = serializer.save() - if instance.status == ReviewStatus.ACCEPTED: - db_job.status = StatusChoice.COMPLETED - db_job.save() - elif instance.status == ReviewStatus.REJECTED: - db_job.status = StatusChoice.ANNOTATION - db_job.save() - return Response(CombinedReviewSerializer(instance, context={'request': request}).data, status=status.HTTP_201_CREATED) - - @swagger_auto_schema(method='get', operation_summary='Get a brief summary about done reviews') - @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer) - def reviews_summary(self, request, pk): - db_job = self.get_object() - serializer = ReviewSummarySerializer(db_job) - 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='destroy', decorator=swagger_auto_schema(operation_summary='Method removes a review from a job')) -class ReviewViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin): - queryset = Review.objects.all().order_by('id') - serializer_class = ReviewSerializer + 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) - def get_permissions(self): - permissions = [IsAuthenticated, auth.AdminRolePermission] - return [perm() for perm in permissions] + 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')) class IssueViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin): @@ -860,11 +872,8 @@ def create(self, request, *args, **kwargs): 'author_id': request.user.id, }) issue_id = request.data['issue'] - try: - db_issue = Issue.objects.get(pk=1) - except MyModel.DoesNotExist: - raise Http404('Issue with id {} does not exist'.format(issue_id)) - self.check_object_permissions(self.request, db_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): From 44c0ec2ba57d6f9975772eca38cd18f7d7a5f57f Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 18 Nov 2020 14:30:15 +0300 Subject: [PATCH 26/33] [PATCH] /issue//resolve(reopen) -> [PATCH] /issue/id --- cvat/apps/engine/tests/test_rest_api.py | 6 ++-- cvat/apps/engine/views.py | 47 ++++++++++--------------- 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index 5cd753b5d90c..3161d86089bf 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -585,17 +585,17 @@ def test_api_v1_resolve_reopen_issue(self): 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/{}/resolve'.format(issue_id), self.assignee, {}) + 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/{}/reopen'.format(issue_id), self.reviewer, {}) + 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/{}/resolve'.format(issue_id), self.reviewer, {}) + 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) diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index a2eee9399213..380c21caa0e6 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -802,12 +802,29 @@ def create(self, request, *args, **kwargs): 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')) -class IssueViewSet(viewsets.GenericViewSet, mixins.DestroyModelMixin): +@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 = ['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] @@ -816,39 +833,13 @@ def get_permissions(self): permissions.append(auth.IssueAccessPermission) elif http_method in ['DELETE']: permissions.append(auth.IssueDestroyPermission) - elif http_method in ['PATCH', 'PUT']: + elif http_method in ['PATCH']: permissions.append(auth.IssueChangePermission) else: permissions.append(auth.AdminRolePermission) return [perm() for perm in permissions] - @swagger_auto_schema(method='patch', operation_summary='The action resolves a specific issue', - responses={'200': IssueSerializer()} - ) - @action(detail=True, methods=['PATCH'], serializer_class=None) - def resolve(self, request, pk): - db_issue = self.get_object() - db_issue.resolved = True - db_issue.resolver = request.user - db_issue.resolved_date = datetime.now() - db_issue.save() - serializer = IssueSerializer(db_issue, context={'request': request}) - return Response(serializer.data) - - @swagger_auto_schema(method='patch', operation_summary='The action reopens a specific issue', - responses={'200': IssueSerializer()} - ) - @action(detail=True, methods=['PATCH'], serializer_class=None) - def reopen(self, request, pk): - db_issue = self.get_object() - db_issue.resolved = False - db_issue.resolver = None - db_issue.resolved_date = None - db_issue.save() - serializer = IssueSerializer(db_issue, context={'request': request}) - return Response(serializer.data) - @swagger_auto_schema(method='get', operation_summary='The action returns all comments of a specific issue', responses={'200': CommentSerializer(many=True)} ) From 71e1a69e8f4f57937c2757a048eb759efb2aa1e4 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 18 Nov 2020 14:31:59 +0300 Subject: [PATCH 27/33] Minor fix --- cvat/apps/engine/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 380c21caa0e6..d10ff7694c31 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -805,7 +805,7 @@ def create(self, request, *args, **kwargs): @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 = ['patch', 'delete', 'options'] + http_method_names = ['get', 'patch', 'delete', 'options'] def get_serializer_class(self): return IssueSerializer From 6e09135198cbaea6f0f3e5c142f373c4e9787eef Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 18 Nov 2020 16:43:19 +0300 Subject: [PATCH 28/33] Reviewed review summary --- cvat/apps/engine/serializers.py | 24 ------------------------ cvat/apps/engine/tests/test_rest_api.py | 19 ------------------- cvat/apps/engine/views.py | 16 +++++----------- 3 files changed, 5 insertions(+), 54 deletions(-) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index 346ad839c82d..f22eb80e6e0a 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -525,30 +525,6 @@ class Meta: write_once_fields = ('job', 'reviewer_id', 'assignee_id', 'estimated_quality', 'status', ) ordering = ['-id'] -class ReviewSummarySerializer(serializers.Serializer): - reviews = serializers.IntegerField(read_only=True) - average_estimated_quality = serializers.FloatField(read_only=True) - issues_unsolved = serializers.IntegerField(read_only=True) - issues_resolved = serializers.IntegerField(read_only=True) - assignees = serializers.ListField(allow_empty=True, read_only=True) - reviewers = serializers.ListField(allow_empty=True, read_only=True) - - def to_representation(self, instance): - db_reviews = instance.review_set.prefetch_related('issue_set').all() - qualities = list(map(lambda db_review: db_review.estimated_quality, db_reviews)) - reviewers = list(map(lambda db_review: db_review.reviewer.username if db_review.reviewer else None, db_reviews)) - assignees = list(map(lambda db_review: db_review.assignee.username if db_review.assignee else None, db_reviews)) - db_issues = list(reduce(lambda acc, db_review: acc + list(db_review.issue_set.all()), db_reviews, [])) - - return { - 'reviews': len(db_reviews), - 'average_estimated_quality': sum(qualities) / len(qualities) if qualities else 0, - 'issues_unsolved': len(list(filter(lambda db_issue: db_issue.resolved_date is None, db_issues))), - 'issues_resolved': len(list(filter(lambda db_issue: db_issue.resolved_date is not None, db_issues))), - 'assignees': list(set(filter(lambda username: username is not None, assignees))), - 'reviewers': list(set(filter(lambda username: username is not None, reviewers))), - } - class IssueSerializer(serializers.ModelSerializer): owner = BasicUserSerializer(allow_null=True, required=False) owner_id = serializers.IntegerField(write_only=True, allow_null=True, required=False) diff --git a/cvat/apps/engine/tests/test_rest_api.py b/cvat/apps/engine/tests/test_rest_api.py index 3161d86089bf..fd878f4eb5eb 100644 --- a/cvat/apps/engine/tests/test_rest_api.py +++ b/cvat/apps/engine/tests/test_rest_api.py @@ -486,25 +486,6 @@ def test_api_v1_job_review_further(self): self.assertEqual(self.job.status, 'validation') self.job.review_set.first().delete() - def test_api_v1_job_review_summary(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._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) - response = self._post_request('/api/v1/reviews', self.reviewer, self.reject_review_data) - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - response = self._get_request('/api/v1/jobs/{}/reviews/summary'.format(self.job.id), self.reviewer) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.data.get('reviews', None), 3) - self.assertEqual(response.data.get('average_estimated_quality', None), 4) - self.assertEqual(response.data.get('issues_unsolved', None), 1) - self.assertEqual(response.data.get('issues_resolved', None), 0) - self.assertSequenceEqual(response.data.get('assignees', None), ['user2']) - self.assertSequenceEqual(response.data.get('reviewers', None), ['user3']) - self.job.review_set.all().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) diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index d10ff7694c31..1b86c57a0375 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -47,8 +47,7 @@ FileInfoSerializer, JobSerializer, LabeledDataSerializer, LogEventSerializer, ProjectSerializer, RqStatusSerializer, TaskSerializer, UserSerializer, PluginsSerializer, ReviewSerializer, - ReviewSummarySerializer, CombinedReviewSerializer, IssueSerializer, - CombinedIssueSerializer, CommentSerializer + CombinedReviewSerializer, IssueSerializer, CombinedIssueSerializer, CommentSerializer ) from cvat.apps.engine.utils import av_scan_paths @@ -651,11 +650,13 @@ class JobViewSet(viewsets.GenericViewSet, def get_permissions(self): http_method = self.request.method + http_path = self.request.path permissions = [IsAuthenticated] if http_method in SAFE_METHODS: - permissions.append(auth.JobAccessPermission) - elif http_method in ["PATCH", "PUT", "DELETE"]: + if not http_path.endswith('/reviews'): + permissions.append(auth.JobAccessPermission) + elif http_method in ['PATCH', 'PUT', 'DELETE']: permissions.append(auth.JobChangePermission) else: permissions.append(auth.AdminRolePermission) @@ -720,13 +721,6 @@ def reviews(self, request, pk): serializer = ReviewSerializer(queryset, context={'request': request}, many=True) return Response(serializer.data) - @swagger_auto_schema(method='get', operation_summary='Get a brief summary about done reviews') - @action(detail=True, methods=['GET'], url_path='reviews/summary', serializer_class=ReviewSummarySerializer) - def reviews_summary(self, request, pk): - db_job = self.get_object() - serializer = ReviewSummarySerializer(db_job) - return Response(serializer.data) - @swagger_auto_schema(method='get', operation_summary='Method returns list of issues for the job', responses={'200': CombinedIssueSerializer(many=True)} ) From 43e8d8dc5dd5706ef9b4ee1a8048e651699135f9 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 18 Nov 2020 17:15:42 +0300 Subject: [PATCH 29/33] Removed unused import --- cvat/apps/engine/serializers.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cvat/apps/engine/serializers.py b/cvat/apps/engine/serializers.py index f22eb80e6e0a..3c14601d698d 100644 --- a/cvat/apps/engine/serializers.py +++ b/cvat/apps/engine/serializers.py @@ -6,7 +6,6 @@ import re import shutil -from functools import reduce from rest_framework import serializers from django.contrib.auth.models import User, Group From 2bcbb846f247d6bf6a581224c3fef7e36989af12 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 18 Nov 2020 17:34:56 +0300 Subject: [PATCH 30/33] Checking job permissions --- cvat/apps/engine/views.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cvat/apps/engine/views.py b/cvat/apps/engine/views.py index 1b86c57a0375..17b261e6d0dd 100644 --- a/cvat/apps/engine/views.py +++ b/cvat/apps/engine/views.py @@ -650,12 +650,10 @@ class JobViewSet(viewsets.GenericViewSet, def get_permissions(self): http_method = self.request.method - http_path = self.request.path permissions = [IsAuthenticated] if http_method in SAFE_METHODS: - if not http_path.endswith('/reviews'): - permissions.append(auth.JobAccessPermission) + permissions.append(auth.JobAccessPermission) elif http_method in ['PATCH', 'PUT', 'DELETE']: permissions.append(auth.JobChangePermission) else: From ae82d615305135b10ae817aee65c85bc62680164 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Tue, 24 Nov 2020 11:32:36 +0300 Subject: [PATCH 31/33] Fixed permissions for reviewers --- cvat/apps/authentication/auth.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 48ab04aaa1c5..6b7b0d9dd5eb 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_task): + 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()) @@ -163,7 +174,7 @@ def is_comment_author(db_user, db_comment): 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) @@ -240,7 +251,8 @@ def get_queryset(self): 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 @@ -248,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) From ca52ef0e1a30fef89b2c8f198fee462d9a3d7dc3 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Tue, 24 Nov 2020 11:49:20 +0300 Subject: [PATCH 32/33] Fixed typos --- cvat/apps/authentication/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cvat/apps/authentication/auth.py b/cvat/apps/authentication/auth.py index 6b7b0d9dd5eb..5e19efb7609c 100644 --- a/cvat/apps/authentication/auth.py +++ b/cvat/apps/authentication/auth.py @@ -92,7 +92,7 @@ def is_project_annotator(db_user, db_project): return any([is_task_annotator(db_user, db_task) for db_task in db_tasks]) @rules.predicate -def is_project_reviewer(db_user, db_task): +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]) From 396fca436125347bdb03c74daae767ba8b2982b4 Mon Sep 17 00:00:00 2001 From: Boris Sekachev Date: Wed, 25 Nov 2020 17:27:36 +0300 Subject: [PATCH 33/33] Fixed migration --- ...{0033_auto_20201102_1406.py => 0034_auto_20201125_1426.py} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename cvat/apps/engine/migrations/{0033_auto_20201102_1406.py => 0034_auto_20201125_1426.py} (97%) diff --git a/cvat/apps/engine/migrations/0033_auto_20201102_1406.py b/cvat/apps/engine/migrations/0034_auto_20201125_1426.py similarity index 97% rename from cvat/apps/engine/migrations/0033_auto_20201102_1406.py rename to cvat/apps/engine/migrations/0034_auto_20201125_1426.py index e418ff9d1ca3..457861a3942c 100644 --- a/cvat/apps/engine/migrations/0033_auto_20201102_1406.py +++ b/cvat/apps/engine/migrations/0034_auto_20201125_1426.py @@ -1,4 +1,4 @@ -# Generated by Django 3.1.1 on 2020-11-02 14:06 +# Generated by Django 3.1.1 on 2020-11-25 14:26 import cvat.apps.engine.models from django.conf import settings @@ -17,7 +17,7 @@ class Migration(migrations.Migration): dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ('engine', '0032_remove_task_z_order'), + ('engine', '0033_projects_adjastment'), ] operations = [