Update project updated_time (#3810) #3814

jaesuny · 2021-10-20T09:18:18Z

Motivation and context

Resolved #3810

Update db_project.updated_date when a task in the project is created, deleted or moved.

How has this been tested?

Manually tested.

Checklist

I submit my changes into the develop branch
I have added description of my changes into CHANGELOG file
I have updated the documentation accordingly
I have added tests to cover my changes
I have linked related issues (read github docs)
I have increased versions of npm packages if it is necessary (cvat-canvas,
cvat-core, cvat-data and cvat-ui)

License

I submit my code changes under the same MIT License that covers the project.
Feel free to contact the maintainers if that's a concern.
I have updated the license header for each file (see an example below)

# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

Update project.updated_date when included tasks are deleted, created or moved

ActiveChooN

Thanks a lot for your contribution!

ActiveChooN · 2021-10-25T13:38:36Z

@dvkruchinin, hi, can you adjust our project tests to cover these changes?

nmanovic · 2021-10-26T06:58:56Z

@jaesuny , thanks for the great contribution. Could you please add a line about the fix into CHANGELOG?

nmanovic · 2021-10-26T11:44:17Z

cvat/apps/engine/views.py

@@ -564,14 +564,36 @@ def retrieve(self, request, pk=None):
            raise serializers.ValidationError(
                "Unexpected action specified for the request")

+    def update(self, request, *args, **kwargs):


@jaesuny , do we really need to have the method? Can we move all logic into perform_update which is called from update?

nmanovic · 2021-10-26T11:49:22Z

cvat/apps/engine/views.py

+            instance = serializer.save(owner=self.request.user)
+        if instance.project:
+            db_project = instance.project
+            db_project.updated_date = timezone.now()


Need to fix in models.py instead. After that the field will be updated each time when the model is saved.
updated_date = models.DateTimeField(auto_now=True)

https://docs.djangoproject.com/en/3.2/ref/models/fields/#django.db.models.DateField.auto_now

nmanovic · 2021-10-28T11:40:36Z

cvat/apps/engine/views.py

@@ -564,14 +564,25 @@ def retrieve(self, request, pk=None):
            raise serializers.ValidationError(
                "Unexpected action specified for the request")

+    def perform_update(self, serializer):
+        instance = self.get_object()


Serializer should already have the instance. Please use it: serializer.instance.
https://www.django-rest-framework.org/api-guide/serializers/#accessing-the-initial-data-and-instance

It is critical, because self.get_object will call check_object_permissions which ins't good here (it is already called in update method).

Thanks. updated.

nmanovic

@jaesuny , Thanks for your contribution! It is a great job!

jaesuny · 2021-10-29T07:26:21Z

@jaesuny , Thanks for your contribution! It is a great job!

I learned a lot thanks to you.
Thank you.

nmanovic · 2021-10-29T13:35:46Z

@jaesuny , could you please merge develop (to fix most of problems with tests)? Also I see that there are some problems with Django tests. Looks like the PR breaks something.

======================================================================
ERROR: test_move_task (engine.tests.test_rest_api.TaskMoveAPITestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/django/cvat/apps/engine/tests/test_rest_api.py", line 1952, in test_move_task
    self._check_api_v1_tasks(self.task.id, data)
  File "/home/django/cvat/apps/engine/tests/test_rest_api.py", line 1931, in _check_api_v1_tasks
    response = self._run_api_v1_tasks_id(tid, data)
  File "/home/django/cvat/apps/engine/tests/test_rest_api.py", line 1915, in _run_api_v1_tasks_id
    response = self.client.patch('/api/v1/tasks/{}'.format(tid),
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/test.py", line 309, in patch
    response = super().patch(
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/test.py", line 215, in patch
    return self.generic('PATCH', path, data, content_type, **extra)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/test.py", line 231, in generic
    return super().generic(
  File "/opt/venv/lib/python3.8/site-packages/django/test/client.py", line 470, in generic
    return self.request(**r)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/test.py", line 283, in request
    return super().request(**kwargs)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/test.py", line 235, in request
    request = super().request(**kwargs)
  File "/opt/venv/lib/python3.8/site-packages/django/test/client.py", line 716, in request
    self.check_exception(response)
  File "/opt/venv/lib/python3.8/site-packages/django/test/client.py", line 577, in check_exception
    raise exc_value
  File "/opt/venv/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/opt/venv/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/opt/venv/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/viewsets.py", line 114, in view
    return self.dispatch(request, *args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/views.py", line 505, in dispatch
    response = self.handle_exception(exc)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/views.py", line 465, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/views.py", line 476, in raise_uncaught_exception
    raise exc
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/views.py", line 502, in dispatch
    response = handler(request, *args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/django/utils/decorators.py", line 43, in _wrapper
    return bound_method(*args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/mixins.py", line 82, in partial_update
    return self.update(request, *args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/django/utils/decorators.py", line 43, in _wrapper
    return bound_method(*args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/rest_framework/mixins.py", line 68, in update
    self.perform_update(serializer)
  File "/home/django/cvat/apps/engine/views.py", line 572, in perform_update
    Project.objects.get(id=project_id).save()
  File "/opt/venv/lib/python3.8/site-packages/django/db/models/manager.py", line 85, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/cacheops/query.py", line 352, in get
    return qs._no_monkey.get(qs, *args, **kwargs)
  File "/opt/venv/lib/python3.8/site-packages/django/db/models/query.py", line 429, in get
    raise self.model.DoesNotExist(
cvat.apps.engine.models.Project.DoesNotExist: Project matching query does not exist.

----------------------------------------------------------------------
Ran 275 tests in 391.306s

FAILED (errors=1, skipped=30)
Destroying test database for alias 'default'...

nmanovic

@jaesuny , thanks again. One test was failed, but it is an issue with our infra. We will address it soon. I will merge the PR now.

nmanovic · 2021-11-02T13:50:14Z

@dvkruchinin , could you please prepare a test for the issue?

Update project updated_time (#3810)

b36da3e

Update project.updated_date when included tasks are deleted, created or moved

jaesuny requested a review from nmanovic as a code owner October 20, 2021 09:18

ActiveChooN previously approved these changes Oct 25, 2021

View reviewed changes

nmanovic reviewed Oct 26, 2021

View reviewed changes

jaesuny added 2 commits October 27, 2021 07:45

Fix auto_now=True of project.updated_date

1c32049

Update CHANGELOG.md

c64a841

jaesuny dismissed ActiveChooN’s stale review via c64a841 October 27, 2021 07:47

Merge branch 'develop' into project-updated_date

d48517d

nmanovic reviewed Oct 28, 2021

View reviewed changes

Used serializer.instance

ed1d24e

nmanovic previously approved these changes Oct 29, 2021

View reviewed changes

Update project only if exists

fea4ba5

jaesuny dismissed nmanovic’s stale review via fea4ba5 November 2, 2021 10:14

nmanovic approved these changes Nov 2, 2021

View reviewed changes

nmanovic merged commit edb3df0 into cvat-ai:develop Nov 2, 2021

dvkruchinin mentioned this pull request Nov 11, 2021

Cypress. Update the test for check issue 3810. #3900

Merged

8 tasks

dvkruchinin mentioned this pull request Nov 26, 2021

CVAT_server. Test for "Project updated time". #3953

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update project updated_time (#3810) #3814

Update project updated_time (#3810) #3814

jaesuny commented Oct 20, 2021 •

edited

Loading

ActiveChooN left a comment

ActiveChooN commented Oct 25, 2021

nmanovic commented Oct 26, 2021

nmanovic Oct 26, 2021

jaesuny Oct 27, 2021

nmanovic Oct 26, 2021

jaesuny Oct 27, 2021

nmanovic Oct 28, 2021

jaesuny Oct 29, 2021

nmanovic left a comment

jaesuny commented Oct 29, 2021 •

edited

Loading

nmanovic commented Oct 29, 2021

nmanovic left a comment

nmanovic commented Nov 2, 2021

Update project updated_time (#3810) #3814

Update project updated_time (#3810) #3814

Conversation

jaesuny commented Oct 20, 2021 • edited Loading

Motivation and context

How has this been tested?

Checklist

License

ActiveChooN left a comment

Choose a reason for hiding this comment

ActiveChooN commented Oct 25, 2021

nmanovic commented Oct 26, 2021

nmanovic Oct 26, 2021

Choose a reason for hiding this comment

jaesuny Oct 27, 2021

Choose a reason for hiding this comment

nmanovic Oct 26, 2021

Choose a reason for hiding this comment

jaesuny Oct 27, 2021

Choose a reason for hiding this comment

nmanovic Oct 28, 2021

Choose a reason for hiding this comment

jaesuny Oct 29, 2021

Choose a reason for hiding this comment

nmanovic left a comment

Choose a reason for hiding this comment

jaesuny commented Oct 29, 2021 • edited Loading

nmanovic commented Oct 29, 2021

nmanovic left a comment

Choose a reason for hiding this comment

nmanovic commented Nov 2, 2021

jaesuny commented Oct 20, 2021 •

edited

Loading

jaesuny commented Oct 29, 2021 •

edited

Loading