Track last assignee update #8119
Conversation
zhiltsov-max
requested review from
azhavoro,
SpecLad and
Marishka17
as code owners
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe introduced changes enable the server to track and report the last assignee update time for jobs, tasks, and projects. A new Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
@coderabbitai review |
Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (19)
- changelog.d/20240704_201201_mzhiltso_track_last_assignee_update.md (1 hunks)
- cvat/apps/engine/migrations/0079_job_assignee_updated_date_and_more.py (1 hunks)
- cvat/apps/engine/models.py (5 hunks)
- cvat/apps/engine/serializers.py (13 hunks)
- cvat/apps/engine/tests/test_rest_api.py (1 hunks)
- cvat/apps/quality_control/migrations/0002_qualityreport_assignee.py (1 hunks)
- cvat/apps/quality_control/models.py (3 hunks)
- cvat/apps/quality_control/quality_reports.py (9 hunks)
- cvat/apps/quality_control/serializers.py (3 hunks)
- cvat/apps/quality_control/views.py (2 hunks)
- cvat/schema.yml (4 hunks)
- tests/python/rest_api/test_jobs.py (3 hunks)
- tests/python/rest_api/test_projects.py (3 hunks)
- tests/python/rest_api/test_quality_control.py (3 hunks)
- tests/python/rest_api/test_tasks.py (3 hunks)
- tests/python/shared/assets/jobs.json (25 hunks)
- tests/python/shared/assets/projects.json (12 hunks)
- tests/python/shared/assets/quality_reports.json (12 hunks)
- tests/python/shared/assets/tasks.json (18 hunks)
Files skipped from review due to trivial changes (5)
- cvat/apps/engine/migrations/0079_job_assignee_updated_date_and_more.py
- cvat/apps/engine/tests/test_rest_api.py
- tests/python/shared/assets/jobs.json
- tests/python/shared/assets/quality_reports.json
- tests/python/shared/assets/tasks.json
Additional comments not posted (43)
changelog.d/20240704_201201_mzhiltso_track_last_assignee_update.md (1)
1-4: Changelog entry looks good.The changelog entry accurately describes the changes made in the pull request.
cvat/apps/quality_control/migrations/0002_qualityreport_assignee.py (1)
1-27: Migration script looks good.The migration script correctly adds the
assigneefield to theQualityReportmodel.cvat/apps/quality_control/serializers.py (1)
Line range hint
47-62:
Serializer changes look good.The
assigneefield is correctly added to theQualityReportSerializerand serialized usingengine_serializers.BasicUserSerializer.cvat/apps/quality_control/models.py (1)
89-91: Model changes look good.The
assigneefield is correctly added to theQualityReportmodel as a foreign key to theUsermodel withSET_NULLon delete.tests/python/shared/assets/projects.json (7)
8-8: Field addition looks good.The
assignee_updated_datefield is correctly added to the project JSON object.
47-47: Field addition looks good.The
assignee_updated_datefield is correctly added to the project JSON object.
86-86: Field addition looks good.The
assignee_updated_datefield is correctly added to the project JSON object.
131-131: Field addition looks good.The
assignee_updated_datefield is correctly added to the project JSON object.
170-170: Field addition looks good.The
assignee_updated_datefield is correctly added to the project JSON object.
215-215: Field addition looks good.The
assignee_updated_datefield is correctly added to the project JSON object.
260-260: Field addition looks good.The
assignee_updated_datefield is correctly added to the project JSON object.cvat/apps/quality_control/views.py (1)
314-316: Context parameter addition looks good.The
contextparameter is correctly added to theQualityReportSerializerinstantiation.cvat/apps/engine/models.py (3)
334-335: Field addition looks good.The
assignee_updated_datefield is correctly added to theProjectmodel.
407-407: Field addition looks good.The
assignee_updated_datefield is correctly added to theTaskmodel.
668-670: Field addition looks good.The
assignee_updated_datefield is correctly added to theJobmodel.tests/python/rest_api/test_projects.py (2)
498-515: LGTM! But verify the function coverage in the codebase.The code changes are approved.
However, ensure that all scenarios for project creation with assignees are covered.
Verification successful
Verified: The function
test_can_create_with_assigneeis adequately covered in the codebase.The test function is found in multiple files, ensuring that various scenarios for project creation with assignees are tested.
tests/python/rest_api/test_jobs.pytests/python/rest_api/test_tasks.pytests/python/rest_api/test_projects.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all test functions for project creation with assignees. # Test: Search for the test function usage. Expect: Only occurances of the new test function. rg --type python $'test_can_create_with_assignee'Length of output: 79
Script:
#!/bin/bash # Description: Verify all test functions for project creation with assignees. # Test: Search for the test function usage. Expect: Only occurrences of the new test function. rg 'def test_can_create_with_assignee' --glob '*.py'Length of output: 423
1170-1198: LGTM! But verify the function coverage in the codebase.The code changes are approved.
However, ensure that all scenarios for updating assignees are covered.
Verification successful
Let's correct the
rgcommand and re-run the verification script to ensure the function is properly covered in the codebase.
To proceed, we need to verify the content of the
test_can_update_assignee_updated_date_on_assignee_updatesfunction in each of the identified files to ensure they are appropriate and consistent.
Verified: The function
test_can_update_assignee_updated_date_on_assignee_updatesis consistently implemented across multiple test files and covers the necessary scenarios for updating assignees.
tests/python/rest_api/test_tasks.pytests/python/rest_api/test_projects.pytests/python/rest_api/test_jobs.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all test functions for updating assignees. # Test: Search for the test function usage. Expect: Only occurances of the new test function. rg --type python $'test_can_update_assignee_updated_date_on_assignee_updates'Length of output: 107
Script:
#!/bin/bash # Description: Verify all test functions for updating assignees. # Test: Search for the test function usage. Expect: Only occurrences of the new test function. rg --glob '*.py' 'test_can_update_assignee_updated_date_on_assignee_updates'Length of output: 387
Script:
#!/bin/bash # Description: Display the content of the test_can_update_assignee_updated_date_on_assignee_updates function in the identified files. # Extract the function content from the identified files. for file in tests/python/rest_api/test_tasks.py tests/python/rest_api/test_projects.py tests/python/rest_api/test_jobs.py; do echo "File: $file" rg -A 20 -B 2 'def test_can_update_assignee_updated_date_on_assignee_updates' $file doneLength of output: 4222
tests/python/rest_api/test_quality_control.py (2)
431-483: LGTM!The test method
test_can_get_report_data_with_job_assigneesis well-structured and includes necessary assertions to verify the presence of assignee data.
500-525: LGTM!The test method
test_can_create_report_with_job_assigneesis well-structured and includes necessary assertions to verify the creation of reports with assignee data.tests/python/rest_api/test_jobs.py (1)
314-340: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
test_can_create_with_assigneematch the new signature.Verification successful
Verified: No issues found with the function
test_can_create_with_assignee.The function
test_can_create_with_assigneeintest_jobs.py,test_projects.py, andtest_tasks.pyare context-specific and correctly implemented with different signatures and purposes.
tests/python/rest_api/test_jobs.pytests/python/rest_api/test_projects.pytests/python/rest_api/test_tasks.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `test_can_create_with_assignee` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python $'test_can_create_with_assignee'Length of output: 79
Script:
#!/bin/bash # Description: Verify all function calls to `test_can_create_with_assignee` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type py 'test_can_create_with_assignee'Length of output: 413
Script:
#!/bin/bash # Description: Extract and compare the function signatures and implementations of `test_can_create_with_assignee` in `test_projects.py` and `test_tasks.py`. # Extract the function definition and its body for `test_can_create_with_assignee` in `test_projects.py` echo "Function in test_projects.py:" rg --pcre2 --context 10 'def test_can_create_with_assignee' tests/python/rest_api/test_projects.py # Extract the function definition and its body for `test_can_create_with_assignee` in `test_tasks.py` echo -e "\nFunction in test_tasks.py:" rg --pcre2 --context 10 'def test_can_create_with_assignee' tests/python/rest_api/test_tasks.pyLength of output: 2252
cvat/apps/quality_control/quality_reports.py (5)
37-37: Import change approved.The import of
engine_serializersis necessary for the newassigneefield in theQualityReportSerializer.
2304-2304: Addition ofassignee_idfield approved.The
assignee_idfield is added to the job report dictionary to track the last assignee update time.
2316-2316: Addition ofassignee_idfield approved.The
assignee_idfield is added to the task report dictionary to track the last assignee update time.
2422-2422: Addition ofassignee_idfield approved.The
assignee_idfield is added to the task report dictionary to store the assignee information in the database.
2434-2434: Addition ofassignee_idfield approved.The
assignee_idfield is added to the job report dictionary to store the assignee information in the database.cvat/apps/engine/serializers.py (13)
595-595: Change Approved: Addedassignee_updated_datefield.The
assignee_updated_datefield has been correctly added as aDateTimeField.
614-614: Change Approved: Updatedfieldsandread_only_fields.The
assignee_updated_datefield has been correctly added to thefieldsandread_only_fieldsattributes.
752-752: Change Approved: Addedassignee_updated_datetocreatemethod.The logic to update
assignee_updated_datein thecreatemethod is appropriate.
759-762: Change Approved: Updatedassignee_updated_dateincreatemethod.The logic to update
assignee_updated_datewhen the assignee is set during creation is correct.
781-785: Change Approved: Addedassignee_updated_datetoupdatemethod.The logic to update
assignee_updated_datein theupdatemethod is appropriate.
1114-1114: Change Approved: Addedassignee_updated_datefield.The
assignee_updated_datefield has been correctly added as aDateTimeField.
1130-1130: Change Approved: Updatedfieldsandread_only_fields.The
assignee_updated_datefield has been correctly added to thefieldsandread_only_fieldsattributes.
1198-1200: Change Approved: Addedassignee_updated_datetocreatemethod.The logic to update
assignee_updated_datein thecreatemethod is appropriate.
1213-1219: Change Approved: Addedassignee_updated_datetoupdatemethod.The logic to update
assignee_updated_datein theupdatemethod is appropriate.
1345-1345: Change Approved: Addedassignee_updated_datefield.The
assignee_updated_datefield has been correctly added as aDateTimeField.
1359-1359: Change Approved: Updatedfieldsandread_only_fields.The
assignee_updated_datefield has been correctly added to thefieldsandread_only_fieldsattributes.
1413-1416: Change Approved: Addedassignee_updated_datetocreatemethod.The logic to update
assignee_updated_datein thecreatemethod is appropriate.
1426-1432: Change Approved: Addedassignee_updated_datetoupdatemethod.The logic to update
assignee_updated_datein theupdatemethod is appropriate.tests/python/rest_api/test_tasks.py (2)
415-432: LGTM! Ensure the function usage is consistent.The code changes are approved.
However, ensure that all function calls to
test_can_create_with_assigneeare consistent with the expected parameters.Verification successful
Function
test_can_create_with_assigneeis correctly defined with appropriate parameters in each context.
tests/python/rest_api/test_projects.py: Parameters areself, admin_user, users_by_name, assignee.tests/python/rest_api/test_jobs.py: Parameters areself, admin_user, tasks, jobs, users_by_name, assignee.tests/python/rest_api/test_tasks.py: Parameters areself, admin_user, users_by_name, assignee.The function definitions are consistent within their respective contexts.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `test_can_create_with_assignee` match the expected parameters. # Test: Search for the function usage. Expect: Only occurrences with correct parameters. rg --type python $'test_can_create_with_assignee'Length of output: 79
Script:
#!/bin/bash # Description: Verify all function calls to `test_can_create_with_assignee` match the expected parameters. # Test: Search for the function usage. Expect: Only occurrences with correct parameters. rg --type py 'test_can_create_with_assignee'Length of output: 413
Script:
#!/bin/bash # Description: Extract the function definitions and their surrounding context for `test_can_create_with_assignee` to verify the parameters. # Extract the function definitions with surrounding lines for context. rg --type py -A 5 -B 5 'def test_can_create_with_assignee' tests/python/rest_api/Length of output: 2818
2677-2706: LGTM! Ensure the function usage is consistent.The code changes are approved.
However, ensure that all function calls to
test_can_update_assignee_updated_date_on_assignee_updatesare consistent with the expected parameters.Verification successful
The function
test_can_update_assignee_updated_date_on_assignee_updatesis consistently used across the codebase.
tests/python/rest_api/test_jobs.pytests/python/rest_api/test_projects.pytests/python/rest_api/test_tasks.pyThe parameters and logic are consistent in all occurrences.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `test_can_update_assignee_updated_date_on_assignee_updates` match the expected parameters. # Test: Search for the function usage. Expect: Only occurrences with correct parameters. rg --type python $'test_can_update_assignee_updated_date_on_assignee_updates'Length of output: 107
Script:
#!/bin/bash # Description: Verify all function calls to `test_can_update_assignee_updated_date_on_assignee_updates` match the expected parameters. # Test: Search for the function usage. Expect: Only occurrences with correct parameters. rg 'test_can_update_assignee_updated_date_on_assignee_updates'Length of output: 373
Script:
#!/bin/bash # Description: Extract function definitions for `test_can_update_assignee_updated_date_on_assignee_updates` from the identified files. # Extract the function definitions from the specified files. rg -A 20 'def test_can_update_assignee_updated_date_on_assignee_updates' tests/python/rest_api/test_projects.py tests/python/rest_api/test_tasks.py tests/python/rest_api/test_jobs.pyLength of output: 5304
cvat/schema.yml (3)
8095-8098: Addition ofassignee_updated_dateto Job schema looks good.The field is correctly defined as a nullable date-time string.
9577-9580: Addition ofassignee_updated_dateto Project schema looks good.The field is correctly defined as a nullable date-time string.
9678-9682: Addition ofassigneeto QualityReport schema looks good.The field is correctly defined as a nullable BasicUser reference.
| @pytest.mark.parametrize("has_old_assignee", [False, True]) | ||
| @pytest.mark.parametrize("new_assignee", [None, "same", "different"]) | ||
| def test_can_update_assignee_updated_date_on_assignee_updates( | ||
| self, admin_user, jobs, users, has_old_assignee, new_assignee | ||
| ): | ||
| job = next(j for j in jobs if bool(j.get("assignee")) == has_old_assignee) | ||
|
|
||
| old_assignee_id = (job.get("assignee") or {}).get("id") | ||
|
|
||
| new_assignee_id = None | ||
| if new_assignee == "same": | ||
| new_assignee_id = old_assignee_id | ||
| elif new_assignee == "different": | ||
| new_assignee_id = next(u for u in users if u["id"] != old_assignee_id)["id"] | ||
|
|
||
| with make_api_client(admin_user) as api_client: | ||
| (updated_job, _) = api_client.jobs_api.partial_update( | ||
| job["id"], patched_job_write_request={"assignee": new_assignee_id} | ||
| ) | ||
|
|
||
| if new_assignee_id == old_assignee_id: | ||
| assert updated_job.assignee_updated_date == job["assignee_updated_date"] | ||
| else: | ||
| assert updated_job.assignee_updated_date != job["assignee_updated_date"] | ||
|
|
||
| if new_assignee_id: | ||
| assert updated_job.assignee.id == new_assignee_id | ||
| else: | ||
| assert updated_job.assignee is None | ||
|
|
There was a problem hiding this comment.
Tip
Codebase Verification
Inconsistent Function Signatures Detected
The function test_can_update_assignee_updated_date_on_assignee_updates has different signatures and slight variations in implementation across multiple files:
-
tests/python/rest_api/test_projects.py
- Parameters:
self, admin_user, projects, users, has_old_assignee, new_assignee - Uses
projects_apiandpatched_project_write_request
- Parameters:
-
tests/python/rest_api/test_jobs.py
- Parameters:
self, admin_user, jobs, users, has_old_assignee, new_assignee - Uses
jobs_apiandpatched_job_write_request - This matches the new signature provided in the review comment.
- Parameters:
-
tests/python/rest_api/test_tasks.py
- Parameters:
self, admin_user, tasks, users, has_old_assignee, new_assignee - Uses
tasks_apiandpatched_task_write_request
- Parameters:
Please ensure that the function signatures and implementations are consistent across all test files if the intention is to have a uniform test function.
- tests/python/rest_api/test_projects.py
- tests/python/rest_api/test_tasks.py
Analysis chain
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to test_can_update_assignee_updated_date_on_assignee_updates match the new signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all function calls to `test_can_update_assignee_updated_date_on_assignee_updates` match the new signature.
# Test: Search for the function usage. Expect: Only occurances of the new signature.
rg --type python $'test_can_update_assignee_updated_date_on_assignee_updates'
Length of output: 107
Script:
#!/bin/bash
# Description: Verify all function calls to `test_can_update_assignee_updated_date_on_assignee_updates` match the new signature.
# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg 'test_can_update_assignee_updated_date_on_assignee_updates'
Length of output: 373
Script:
#!/bin/bash
# Description: Extract and display the function definitions of `test_can_update_assignee_updated_date_on_assignee_updates` in the identified files.
# Extract function definition in test_projects.py
echo "Function definition in test_projects.py:"
rg -A 20 'def test_can_update_assignee_updated_date_on_assignee_updates' tests/python/rest_api/test_projects.py
# Extract function definition in test_jobs.py
echo -e "\nFunction definition in test_jobs.py:"
rg -A 20 'def test_can_update_assignee_updated_date_on_assignee_updates' tests/python/rest_api/test_jobs.py
# Extract function definition in test_tasks.py
echo -e "\nFunction definition in test_tasks.py:"
rg -A 20 'def test_can_update_assignee_updated_date_on_assignee_updates' tests/python/rest_api/test_tasks.py
Length of output: 3409
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8119 +/- ##
===========================================
+ Coverage 83.39% 83.46% +0.07%
===========================================
Files 388 388
Lines 41272 41298 +26
Branches 3852 3852
===========================================
+ Hits 34417 34469 +52
+ Misses 6855 6829 -26
|
|
<!-- Raise an issue to propose your change (https://github.com/cvat-ai/cvat/issues). It helps to avoid duplication of efforts from multiple independent contributors. Discuss your ideas with maintainers to be sure that changes will be approved and merged. Read the [Contribution guide](https://docs.cvat.ai/docs/contributing/). --> <!-- Provide a general summary of your changes in the Title above --> Depends on #8119 ### Motivation and context <!-- Why is this change required? What problem does it solve? If it fixes an open issue, please link to the issue here. Describe your changes in detail, add screenshots. --> - Changed source for job assignees on the Quality analytics page from the current job data to the report data ### How has this been tested? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> ### Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. If an item isn't applicable for some reason, then ~~explicitly strikethrough~~ the whole line. If you don't do that, GitHub will show incorrect progress for the pull request. If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] I submit my changes into the `develop` branch - [ ] I have created a changelog fragment <!-- see top comment in CHANGELOG.md --> - [ ] I have updated the documentation accordingly - [ ] I have added tests to cover my changes - [ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)) - [ ] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning)) ### License - [ ] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced tracking and reporting of the last assignee update time. - Added an `assignee` field to Quality Reports for better user assignment management. - **Enhancements** - Updated Project, Task, and Job models to include an `assignee_updated_date` field, improving assignment tracking. - **Bug Fixes** - Adjusted job and task serializers to handle new assignee-related fields correctly. - **Tests** - Added test cases for job, project, task, and quality report creation and assignee updates. - **Documentation** - Updated schema to reflect new `assignee_updated_date` field. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Kirill Lakhov <[email protected]>
Depends on #8162
Motivation and context
How has this been tested?
Unit tests
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
assignee_updated_datefield.