feat: Add a new published job stage
#8018
Conversation
- All organization members can view the published jobs, but cannot make revision. - All organization members can open issue on the published jobs.
likelikeslike
requested review from
bsekachev,
mdacoca,
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 recent updates introduce 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
site/content/en/docs/manual/basics/task-details.md (1)
Line range hint
47-47: Consider adding a comma before 'and' for better readability.- Other frames will be loaded and you will be able to annotate first images. + Other frames will be loaded, and you will be able to annotate first images.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (17)
- cvat-core/src/enums.ts (1 hunks)
- cvat-ui/src/components/analytics-page/task-quality/job-list.tsx (1 hunks)
- cvat-ui/src/components/annotation-page/top-bar/annotation-menu.tsx (3 hunks)
- cvat-ui/src/components/job-item/job-actions-menu.tsx (2 hunks)
- cvat-ui/src/components/job-item/job-item.tsx (1 hunks)
- cvat-ui/src/components/jobs-page/jobs-filter-configuration.ts (1 hunks)
- cvat-ui/src/components/task-page/jobs-filter-configuration.ts (1 hunks)
- cvat/apps/engine/models.py (2 hunks)
- cvat/apps/engine/permissions.py (2 hunks)
- cvat/apps/engine/rules/issues.rego (6 hunks)
- cvat/apps/engine/rules/jobs.rego (3 hunks)
- cvat/apps/engine/serializers.py (1 hunks)
- cvat/schema.yml (2 hunks)
- site/content/en/docs/manual/advanced/iam_user_roles.md (1 hunks)
- site/content/en/docs/manual/basics/CVAT-annotation-Interface/navbar.md (1 hunks)
- site/content/en/docs/manual/basics/task-details.md (1 hunks)
- site/content/en/docs/manual/basics/vocabulary.md (1 hunks)
Files skipped from review due to trivial changes (4)
- cvat-core/src/enums.ts
- cvat-ui/src/components/analytics-page/task-quality/job-list.tsx
- cvat-ui/src/components/job-item/job-item.tsx
- cvat-ui/src/components/task-page/jobs-filter-configuration.ts
Additional context used
LanguageTool
site/content/en/docs/manual/basics/task-details.md
[uncategorized] ~45-~45: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ...u can have several links. It depends on size of your task andOverlap Sizeand `Se...
[uncategorized] ~47-~47: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE)
Context: ...t chunk of several frames will be loaded and you will be able to annotate first imag...
[uncategorized] ~48-~48: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ... images. Other frames will be loaded in background.site/content/en/docs/manual/basics/vocabulary.md
[duplication] ~7-~7: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...rtaining to annotation in CVAT.' --- ## Label Label is a type of an annotated object (e.g. ...
[grammar] ~8-~8: If ‘type’ is a classification term, ‘an’ is not necessary. Use “type of”. (The phrases ‘kind of’ and ‘sort of’ are informal if they mean ‘to some extent’.) (KIND_OF_A)
Context: ...ation in CVAT.' --- ## Label Label is a type of an annotated object (e.g. person, car, veh...
[duplication] ~14-~14: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...](/images/image032_detrac.jpg) --- ## Attribute Attribute is a property of an annotated object (e...
[duplication] ~18-~18: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...There are two types of attributes: ### Unique Unique immutable and can't be changed from fra...
[duplication] ~23-~23: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...etc.)### Temporary Temporary mutable and can be changed on any frame...
[duplication] ~30-~30: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...)--- ## Track Track is a set of shapes on different frames ...
[duplication] ~38-~38: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...--- ## Annotation Annotation is a set of shapes and tracks. There ar...
[duplication] ~47-~47: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...y without a person in the loop --- ## Approximation Approximation allows you to reduce the number of poin...
[style] ~48-~48: To form a complete sentence, be sure to include a subject. (MISSING_IT_THERE)
Context: ...ce the number of points in the polygon. Can be used to reduce the annotation file a...
[duplication] ~55-~55: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...es/approximation_accuracy.gif) --- ## Trackable Trackable object will be tracked automatically if...
[grammar] ~56-~56: Use “the” before the superlative. (THE_SUPERLATIVE)
Context: ...automatically if the previous frame was a latest keyframe for the object. More de...
[grammar] ~82-~82: It appears that a pronoun is missing. (SENT_START_DEPENDS)
Context: ...s" "Shape mode" >}} --- ## Dimension Depends on the task data type that is defined w...
[grammar] ~88-~88: Did you mean “2D”(= two-dimensional) or “2nd” (= second)? (THREE_D)
Context: ...eated" >}}. ### 2D The data format of 2d tasks are images and videos. Related se...
[grammar] ~94-~94: Did you mean “3D”(= three-dimensional) or “3rd” (= third)? (THREE_D)
Context: ...n task" >}} ### 3D The data format of 3d tasks is a cloud of points. {{< ilink "...
[duplication] ~104-~104: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...on" "3D Object annotation" >}} --- ## State State of the job. The state can be changed by...
[duplication] ~111-~111: Possible typo: you repeated a word (ENGLISH_WORD_REPEAT_RULE)
Context: ...ess,rejected,completed`. --- ## Stage Stage of the job. The stage is specified with...
[typographical] ~114-~114: Two consecutive dots (DOUBLE_PUNCTUATION)
Context: ...validation,acceptanceorpublished`.. This value affects the task progress ba...site/content/en/docs/manual/advanced/iam_user_roles.md
[uncategorized] ~16-~16: A comma might be missing here. (AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
Context: ...determining the visibility of different resources for example, tasks or jobs. Limits...
[style] ~29-~29: Consider shortening or rephrasing this to strengthen your wording. (MAKE_CHANGES)
Context: ...policy-language/). > Note: Once you've made changes to the.regofiles, you must > rebuild a...
[uncategorized] ~73-~73: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2)
Context: ...on and assign roles to the invited users so the team can collaborate. | | **Maintai...
[uncategorized] ~74-~74: You might be missing the article “the” here. (AI_EN_LECTOR_MISSING_DETERMINER_THE)
Context: ... see all tasks within the organization. Maintainer has complete access to Cloud Storages, ...site/content/en/docs/manual/basics/CVAT-annotation-Interface/navbar.md
[style] ~35-~35: Consider an alternative to strengthen your wording. (CHANGES_ADJUSTMENTS)
Context: ...en finished, reviewed, and approved. No further changes are necessary.| | **Finish the jo...
[typographical] ~84-~84: Unpaired symbol: ‘"’ seems to be missing (EN_UNPAIRED_QUOTES)
Context: .... For more information, see Manual QA- Start frame - the number o...
Additional comments not posted (23)
site/content/en/docs/manual/basics/task-details.md (1)
30-30: The addition of 'published' to the job stages is clear and integrates well with the existing content.cvat-ui/src/components/jobs-page/jobs-filter-configuration.ts (1)
35-35: The addition of 'published' to the job stage filter options is correctly implemented.cvat-ui/src/components/job-item/job-actions-menu.tsx (1)
72-75: The addition of the 'publish_job' action is correctly implemented and integrates well with the existing menu structure.Also applies to: 89-89
cvat/apps/engine/rules/issues.rego (6)
57-59: Theis_job_publishedfunction correctly checks if a job's stage ispublished. Ensure this function is utilized appropriately in permission checks.
39-39: Ensure the newpublishedstage is consistently handled across all relevant policies.Verification successful
The search results indicate that the
publishedstage is consistently handled across various files, including policy files, schema definitions, and UI components. This suggests that the newpublishedstage is integrated properly throughout the codebase.
cvat/apps/engine/rules/jobs.rego: Multiple references to thepublishedstage.cvat/apps/engine/rules/issues.rego: Multiple references to thepublishedstage.cvat/schema.yml: Definitions including thepublishedstage.cvat/apps/engine/models.py: Definition of thePUBLISHEDconstant.- Various UI components and documentation files also reference the
publishedstage.This comprehensive handling confirms that the
publishedstage is consistently integrated.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the 'published' stage is handled in all relevant policy files. # Test: Search for handling of 'published' stage in other policy files. Expect: Consistent handling across files. rg --type rego 'published'Length of output: 55
Script:
#!/bin/bash # Description: Verify that the 'published' stage is handled in all relevant policy files. # Test: Search for handling of 'published' stage in all files. Expect: Consistent handling across files. rg 'published'Length of output: 2383
283-289: This rule allows deleting issues if the job is published. Confirm that deletion policies are adhered to and that no critical data is lost.
261-266: This rule allows updating issues if the job is published. Ensure that only authorized modifications are permitted.
239-244: This rule allows viewing issues if the job is published. Verify that this does not inadvertently expose sensitive data.Verification successful
The
is_job_publishedfunction checks if the job's stage is "published". Theallow ifrules incvat/apps/engine/rules/issues.regoinclude multiple conditions for different scopes, ensuring that only authorized users can view, create, update, or delete issues.The specific rule in question allows viewing issues if the job is published and the user is a member of the organization. This rule does not seem to expose sensitive data inadvertently, as it still requires the user to be a member of the organization.
is_job_publishedfunction:is_job_published if { input.resource.job.stage == "published" }Relevant
allow ifrule:allow if { input.scope == utils.VIEW input.auth.organization.id == input.resource.organization.id organizations.is_member is_job_published }Based on this, the rule appears to be correctly scoped and does not expose sensitive data.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential data exposure in viewing issues of published jobs. # Test: Review logs and access records for unauthorized data access attempts. Expect: No unauthorized access. cat logs/access.log | grep 'published'Length of output: 86
Script:
#!/bin/bash # Search for the implementation of the `is_job_published` function in the codebase. ast-grep --lang rego --pattern 'is_job_published { $$$ }' # Search for other rules related to viewing issues in the codebase. ast-grep --lang rego --pattern 'allow if { $$$ }'Length of output: 329
Script:
#!/bin/bash # Search for the implementation of the `is_job_published` function in the codebase. rg 'is_job_published' --glob '*.rego' # Search for other rules related to viewing issues in the codebase. rg 'allow if' --glob '*.rego'Length of output: 10409
Script:
#!/bin/bash # Extract the implementation of the `is_job_published` function from cvat/apps/engine/rules/issues.rego. sed -n '/is_job_published if {/,/}/p' cvat/apps/engine/rules/issues.rego # Extract the `allow if` rules from cvat/apps/engine/rules/issues.rego. sed -n '/allow if {/,/}/p' cvat/apps/engine/rules/issues.regoLength of output: 2709
155-160: This rule allows issue creation if the job is published. Confirm that this aligns with organizational policies on issue visibility and creation.cvat/apps/engine/rules/jobs.rego (3)
91-93: Theis_job_publishedfunction correctly checks if a job's stage ispublished. Ensure this function is utilized appropriately in permission checks.
197-204: This rule allows viewing job-related data if the job is published. Confirm that this aligns with organizational policies on data visibility.
39-39: Ensure the newpublishedstage is consistently handled across all relevant policies.site/content/en/docs/manual/advanced/iam_user_roles.md (1)
101-101: The documentation correctly introduces thePublishedstage, making it accessible to all organization members. Ensure this description aligns with the actual implementation in the code.cvat-ui/src/components/annotation-page/top-bar/annotation-menu.tsx (3)
47-47: The addition ofPUBLISH_JOBto theActionsenum is consistent with the new functionality. Ensure this action is integrated properly in the UI.
118-127: ThepublishJobfunction correctly updates the job stage toPUBLISHEDand the state toCOMPLETED. Ensure that this change triggers appropriate updates in the UI and backend.
332-345: The UI integration of thepublishJobaction is well implemented, providing clear user feedback and navigation. Confirm that the modal's behavior aligns with user expectations.site/content/en/docs/manual/basics/CVAT-annotation-Interface/navbar.md (1)
37-37: Ensure the new "Publish the job" section is clear and consistent with existing documentation.cvat/apps/engine/models.py (2)
93-93: The addition of the "PUBLISHED" stage to theStageChoiceenum is correctly implemented.
383-389: The update to thewith_job_summarymethod inTaskQuerySetcorrectly includes the "PUBLISHED" stage in job summaries.cvat/apps/engine/permissions.py (1)
946-947: Validate job stage updates correctly.The
get_resourcemethod in theIssuePermissionclass includes the job's stage in the returned data. Given the newpublishedstage introduced in the system, ensure that all necessary validations and checks are updated to handle this new stage appropriately.cvat/apps/engine/serializers.py (1)
759-760: The logic to set the job status toCOMPLETEDwhen the stage isPUBLISHEDaligns with the PR's objectives. Ensure that this change does not unintentionally affect other job stages or transitions.Verification successful
The logic to set the job status to
COMPLETEDwhen the stage isPUBLISHEDaligns with the PR's objectives and does not unintentionally affect other job stages or transitions.
cvat/apps/engine/serializers.py: TheCOMPLETEDstatus is correctly set for thePUBLISHEDstage and for theACCEPTANCEstage when the state isCOMPLETED.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that no other job stages incorrectly transition to COMPLETED. # Test: Search for job stage transitions in the application logic. rg --type python "stage == models.StageChoice" -C 5Length of output: 84
Script:
#!/bin/bash # Description: Verify that no other job stages incorrectly transition to COMPLETED. # Test: Search for job stage transitions in the application logic. rg --type py "stage == models.StageChoice" -C 5Length of output: 1379
cvat/schema.yml (2)
1866-1866: Addition of 'published' stage to the schema is consistent with the new feature.
7553-7559: Addition of 'published' stage to the schema is consistent with the new feature.Verification successful
The 'published' stage appears three times in the schema file, indicating consistent usage. However, to ensure thorough verification, let's review the context around each instance to confirm they are correctly placed and aligned with the intended changes.
The 'published' stage is correctly added to the 'enum' lists and descriptions for job stages, aligning with the intended changes. The integration appears consistent and correct.
- Lines around the first instance show 'published' added to the 'enum' list for job stages.
- Lines around the second instance show 'published' added to the 'enum' list and description for job stages.
These changes are consistent with the schema's structure and the intended feature addition.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that 'published' stage is consistently added across all relevant parts of the schema. # Test: Search for 'published' in the schema. Expect: Consistent usage across the file. rg --type yaml 'published' cvat/schema.ymlLength of output: 116
Script:
#!/bin/bash # Description: Display lines around each instance of 'published' to verify context and placement. # Display 5 lines before and after each match rg --type yaml -C 5 'published' cvat/schema.ymlLength of output: 690
|
|
Show the published jobs count in the
|
|
Hi @bsekachev, could you please take a moment to review this PR and let me know if it can be merged? Thank you! |
We added a new stage for
Jobin CVAT calledpublished, which allows users to make oneJobaccessible to all members of their organization.Motivation and context
First, thank you for the amazing project that has significantly helped us with our annotation program.
In our work, we encountered a problem: we want to provide annotation examples for new annotators to help them understand the annotation standards easily. However, with the current CVAT design, the only way to do this is by duplicating several
Tasksand assigning each one to an annotator. While this method achieves our goal, it is not that convenient, and there is a risk that the annotators might accidentally change the pre-annotated examples.Our ideal solution is that each annotator can share a single
Jobwhere they can only add comments by opening anIssue.To achieve this, we added a new stage for
Job, calledpublished. If we change aJobto thepublishedstage, any annotator with the link to thisJobcan view it and add comments.How has this been tested?
Yes, we followed the Development environment guidelines and tested it on Ubuntu 22.04. The results were as we expected.
However, due to our limited knowledge of this project, we made the changes as minor as possible. We may also need your help to make this PR more useful in the future.
Checklist
developbranch- [ ] I have linked related issues (see GitHub docs)- [ ] I have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation