-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
removing business group #8607
removing business group #8607
Conversation
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 changes in this pull request primarily focus on the removal of the "business" privilege from various user roles across multiple files in the CVAT project. This includes modifications to TypeScript interfaces, Rego authorization rules, and Python test cases. The updates streamline the privilege model, affecting how user roles are defined and evaluated in both the application logic and the associated tests. Additionally, user data in JSON files has been updated to reflect these changes, standardizing names and roles. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (8)
cvat/apps/analytics_report/rules/analytics_reports.rego (1)
Line range hint 1-42
: Consider updating the input structure documentation.
The input structure documentation in the comments should be marked as a breaking change in the changelog, as it modifies the API contract.
Add a comment indicating when this change was made:
# input: {
# "scope": <"list"|"create"> or null,
# "auth": {
# "user": {
# "id": <num>,
+# # Note: "business" privilege was removed in PR #8607
# "privilege": <"admin"|"user"|"worker"> or null
# },
site/content/en/docs/administration/basics/admin-account.md (1)
14-14
: Consider adding a transition note for existing users.
To improve documentation clarity, consider adding a note explaining that the "business" role has been removed and providing guidance for any existing users who might have been using this role.
Here's a suggested addition:
<br>Available roles are: user (default), admin, worker.
+
+> Note: The previously available "business" role has been removed as it is no longer supported by the system.
+> Existing users with the "business" role should contact their system administrator for role reassignment.
cvat/apps/engine/rules/cloudstorages.rego (1)
Line range hint 1-141
: Consider adding documentation for the authorization model.
While the code is well-structured, it would benefit from documentation that explains:
- The privilege hierarchy and relationships between user privileges and organization roles
- The implications of each scope (create, list, view, etc.)
- The difference between sandbox and organization contexts
Would you like me to help draft the documentation?
cvat/apps/quality_control/rules/conflicts.rego (1)
Line range hint 1-13
: Update documentation to reflect privilege changes.
Since we're removing the "business" privilege option from the input structure, the documentation at the top of the file should be updated to reflect this change.
Apply this diff to update the documentation:
# input: {
# "scope": <"list"> or null,
# "auth": {
# "user": {
# "id": <num>,
-# "privilege": <"admin"|"user"|"worker"> or null
+# "privilege": <"admin"|"user"|"worker"> or null # Note: "business" privilege was removed in PR #8607
cvat/apps/engine/rules/tests/generators/server_test.gen.rego.py (1)
44-44
: Consider documenting the privilege hierarchy.
The GROUPS list defines an implicit privilege hierarchy used in eval_rule()
through index-based comparisons. Consider adding a comment to document this ordering.
-GROUPS = ["admin", "user", "worker", "none"]
+# Privilege hierarchy (highest to lowest)
+GROUPS = ["admin", "user", "worker", "none"]
cvat/apps/events/rules/tests/generators/events_test.gen.rego.py (1)
Line range hint 82-91
: Consider documenting the privilege hierarchy.
The eval_rule
function uses GROUPS.index()
for privilege level comparison. While the implementation is correct, adding a comment explaining the privilege hierarchy would improve maintainability, especially after removing the "business" group.
def eval_rule(scope, context, ownership, privilege, membership, data):
+ # Privilege hierarchy (highest to lowest): admin > user > worker > none
+ # Lower index in GROUPS list indicates higher privilege level
if privilege == "admin":
return True
cvat/apps/engine/rules/tests/generators/annotationguides_test.gen.rego.py (1)
49-49
: Consider documenting the privilege hierarchy.
The GROUPS list defines an implicit privilege hierarchy through index-based comparisons in eval_rule()
. Consider adding a comment to document this hierarchy for better maintainability.
-GROUPS = ["admin", "user", "worker"]
+# Privilege hierarchy from highest to lowest access level
+GROUPS = ["admin", "user", "worker"]
tests/python/shared/assets/tasks.json (1)
Line range hint 1-1200
: Consider splitting unrelated changes into separate PRs.
While the user assignment changes align with removing the business group feature, this PR also includes unrelated changes:
- Storage configuration updates (local to cloud storage)
- Bug tracker URL additions
Consider splitting these changes into separate PRs to maintain better change isolation and make the changes easier to review and maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
cvat/apps/log_viewer/rules/tests/configs/analytics.csv
is excluded by!**/*.csv
📒 Files selected for processing (67)
- cvat-core/src/server-response-types.ts (1 hunks)
- cvat-core/src/user.ts (1 hunks)
- cvat/apps/analytics_report/rules/analytics_reports.rego (1 hunks)
- cvat/apps/dataset_manager/tests/test_rest_api_formats.py (1 hunks)
- cvat/apps/engine/rules/annotationguides.rego (1 hunks)
- cvat/apps/engine/rules/cloudstorages.rego (1 hunks)
- cvat/apps/engine/rules/comments.rego (1 hunks)
- cvat/apps/engine/rules/issues.rego (1 hunks)
- cvat/apps/engine/rules/jobs.rego (1 hunks)
- cvat/apps/engine/rules/labels.rego (1 hunks)
- cvat/apps/engine/rules/projects.rego (1 hunks)
- cvat/apps/engine/rules/server.rego (1 hunks)
- cvat/apps/engine/rules/tasks.rego (1 hunks)
- cvat/apps/engine/rules/tests/generators/annotationguides_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/tests/generators/cloudstorages_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/tests/generators/comments_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/tests/generators/issues_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/tests/generators/jobs_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/tests/generators/projects_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/tests/generators/server_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/tests/generators/tasks_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/tests/generators/users_test.gen.rego.py (1 hunks)
- cvat/apps/engine/rules/users.rego (1 hunks)
- cvat/apps/engine/tests/test_rest_api.py (1 hunks)
- cvat/apps/events/rules/events.rego (1 hunks)
- cvat/apps/events/rules/tests/generators/events_test.gen.rego.py (1 hunks)
- cvat/apps/iam/rules/utils.rego (0 hunks)
- cvat/apps/lambda_manager/rules/lambda.rego (1 hunks)
- cvat/apps/lambda_manager/rules/tests/generators/lambda_test.gen.rego.py (1 hunks)
- cvat/apps/lambda_manager/tests/test_lambda.py (1 hunks)
- cvat/apps/log_viewer/permissions.py (0 hunks)
- cvat/apps/log_viewer/rules/analytics.rego (1 hunks)
- cvat/apps/log_viewer/rules/tests/generators/analytics_test.gen.rego.py (1 hunks)
- cvat/apps/organizations/rules/invitations.rego (1 hunks)
- cvat/apps/organizations/rules/memberships.rego (1 hunks)
- cvat/apps/organizations/rules/organizations.rego (1 hunks)
- cvat/apps/organizations/rules/tests/generators/invitations_test.gen.rego.py (1 hunks)
- cvat/apps/organizations/rules/tests/generators/memberships_test.gen.rego.py (1 hunks)
- cvat/apps/organizations/rules/tests/generators/organizations_test.gen.rego.py (1 hunks)
- cvat/apps/quality_control/rules/conflicts.rego (1 hunks)
- cvat/apps/quality_control/rules/quality_reports.rego (1 hunks)
- cvat/apps/quality_control/rules/quality_settings.rego (1 hunks)
- cvat/apps/webhooks/rules/tests/generators/webhooks_test.gen.rego.py (1 hunks)
- cvat/apps/webhooks/rules/webhooks.rego (1 hunks)
- cvat/settings/base.py (1 hunks)
- site/content/en/docs/administration/advanced/ldap.md (0 hunks)
- site/content/en/docs/administration/basics/admin-account.md (1 hunks)
- tests/python/rest_api/test_analytics.py (0 hunks)
- tests/python/rest_api/test_cloud_storages.py (0 hunks)
- tests/python/rest_api/test_issues.py (0 hunks)
- tests/python/rest_api/test_jobs.py (6 hunks)
- tests/python/rest_api/test_memberships.py (1 hunks)
- tests/python/rest_api/test_organizations.py (0 hunks)
- tests/python/rest_api/test_projects.py (1 hunks)
- tests/python/rest_api/test_tasks.py (0 hunks)
- tests/python/rest_api/test_webhooks.py (12 hunks)
- tests/python/shared/assets/cloudstorages.json (1 hunks)
- tests/python/shared/assets/comments.json (2 hunks)
- tests/python/shared/assets/cvat_db/data.json (46 hunks)
- tests/python/shared/assets/invitations.json (8 hunks)
- tests/python/shared/assets/issues.json (2 hunks)
- tests/python/shared/assets/memberships.json (4 hunks)
- tests/python/shared/assets/organizations.json (1 hunks)
- tests/python/shared/assets/projects.json (5 hunks)
- tests/python/shared/assets/tasks.json (4 hunks)
- tests/python/shared/assets/users.json (2 hunks)
- tests/python/shared/assets/webhooks.json (1 hunks)
🔥 Files not summarized due to errors (1)
- tests/python/shared/assets/cvat_db/data.json: Error: Server error: no LLM provider could handle the message
💤 Files with no reviewable changes (8)
- cvat/apps/iam/rules/utils.rego
- cvat/apps/log_viewer/permissions.py
- site/content/en/docs/administration/advanced/ldap.md
- tests/python/rest_api/test_analytics.py
- tests/python/rest_api/test_cloud_storages.py
- tests/python/rest_api/test_issues.py
- tests/python/rest_api/test_organizations.py
- tests/python/rest_api/test_tasks.py
🔇 Additional comments (103)
cvat/apps/log_viewer/rules/analytics.rego (2)
12-12
: LGTM! Privilege enum update aligns with business group removal.
The removal of the "business" privilege from the input structure is consistent with the PR objective of removing the unused business group feature.
Line range hint 1-35
: Verify analytics access rules in related files.
The simplified allow rule based solely on has_analytics_access
appears correct. However, let's verify that this change is consistent with other analytics-related policy files.
✅ Verification successful
Analytics access rules are correctly implemented and consistent
The verification shows that there are two separate analytics-related policy files with distinct purposes:
log_viewer/rules/analytics.rego
: Controls general analytics access usinghas_analytics_access
analytics_report/rules/analytics_reports.rego
: Handles specific analytics reports operations with role-based permissions
The simplified rule based on has_analytics_access
is appropriate for the log viewer analytics, as it's a separate concern from analytics reports. The two implementations are consistent and handle different use cases correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to business privilege in analytics-related Rego files
# and verify consistency of analytics access rules
# Search for any remaining "business" references in analytics-related Rego files
echo "Checking for remaining business privilege references in analytics policies..."
rg -l "business.*privilege" "cvat/apps/**/analytics*.rego"
# Look for other analytics access rules to ensure consistency
echo "Checking other analytics access rules..."
rg -A 5 "has_analytics_access" "cvat/apps/**/*.rego"
Length of output: 498
Script:
#!/bin/bash
# Let's try a different approach using fd to find Rego files first
echo "Finding all Rego files in the repository..."
fd -e rego
echo "Checking for analytics access rules in all Rego files..."
rg -A 5 "has_analytics_access" $(fd -e rego)
echo "Checking for business privilege references in all Rego files..."
rg "business.*privilege" $(fd -e rego)
Length of output: 3894
Script:
#!/bin/bash
# Let's check the analytics_reports.rego file since it's related to analytics
echo "Checking analytics_reports.rego content..."
cat cvat/apps/analytics_report/rules/analytics_reports.rego
# Also check for any other files that might contain analytics-related rules
echo "Checking for other analytics-related rules..."
rg -A 10 "package analytics" $(fd -e rego)
Length of output: 3450
cvat/apps/engine/rules/server.rego (2)
12-12
: LGTM! Documentation accurately reflects the removal of "business" privilege.
The input structure documentation has been properly updated to remove the unused "business" privilege option while maintaining the correct format and remaining privileges.
12-12
: Verify no dependencies on "business" privilege exist in other Rego rules.
Let's ensure no other authorization rules depend on the removed "business" privilege.
✅ Verification successful
No dependencies on "business" privilege found in Rego rules
The verification shows no occurrences of "business" privilege in the Rego rules under cvat/apps/engine/rules/
. This confirms that the removal of "business" privilege from the comment line is safe as there are no remaining dependencies on it.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to "business" privilege in Rego rules
# Expected: No matches should be found in rule definitions
# Search for "business" string in all Rego files
rg -t rego '"business"' cvat/apps/
Length of output: 65
Script:
#!/bin/bash
# Search for "business" string in all files under cvat/apps/engine/rules/
# Using ripgrep without file type filter since .rego extension is not recognized
rg '"business"' cvat/apps/engine/rules/
Length of output: 41
tests/python/shared/assets/organizations.json (1)
15-19
: LGTM! Changes align with business group removal.
The updates to owner details (changing from business-related to user-based naming) are consistent with the PR objective of removing the unused business group feature.
Let's verify consistency across other test assets:
✅ Verification successful
✓ Changes are consistent with test data naming patterns
After reviewing both organizations.json and users.json, I can confirm that:
- No "business" related references remain in the test assets
- The owner details in organizations.json follow the consistent user naming pattern seen throughout users.json
- The change from "business1" to "user7" correctly maps to an existing user (ID: 10) in users.json with matching details
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining "business" references in test assets
# Expected: No occurrences of business-related naming in user data
# Search for any remaining "business" references in JSON test assets
rg -i "business" "tests/python/shared/assets/*.json"
Length of output: 126
Script:
#!/bin/bash
# Let's first find the correct path to test assets
fd -t f -e json . tests/
# Then search for any remaining "business" references
rg -i "business" $(fd -t f -e json . tests/)
Length of output: 1569
Script:
#!/bin/bash
# Let's check the content of users.json and organizations.json specifically
# as they are most likely to contain user-related data
echo "=== Content of organizations.json ==="
cat tests/python/shared/assets/organizations.json
echo -e "\n=== Content of users.json ==="
cat tests/python/shared/assets/users.json
Length of output: 10378
cvat/apps/engine/rules/users.rego (1)
13-13
: LGTM! Verify complete removal of "business" privilege.
The removal of the "business" privilege option is consistent with the PR objective and maintains the authorization logic integrity.
Let's verify that all references to the "business" privilege have been removed from the codebase:
✅ Verification successful
✅ "Business" privilege successfully removed from the codebase
The comprehensive search across all relevant file types (JavaScript, TypeScript, Python, JSON, and Rego policy files) shows no remaining references to the "business" privilege, confirming its complete removal from the codebase. The change in the users.rego file accurately reflects the current state of the system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to "business" privilege/group
# across the codebase that might need attention.
echo "Searching for 'business' privilege references in code..."
rg -i --type-add 'policy:*.rego' -g '!{*.md,*.txt,CHANGELOG,LICENSE}' \
-g '!tests/assets/*' \
'\b(privilege|group|role).*["\']business["\']|\b["\']business["\'].*\b(privilege|group|role)\b'
echo "Searching for 'business' in test fixtures..."
rg -i --type json '"privilege":\s*"business"|"group":\s*"business"' tests/
Length of output: 223
Script:
#!/bin/bash
# Let's try a different approach with separate commands and simpler patterns
# Search for "business" in policy files
echo "=== Searching in policy files ==="
rg -i "business" "cvat/apps/engine/rules/"
# Search for "business" in Python files
echo "=== Searching in Python files ==="
rg -i "privilege.*business|business.*privilege" "cvat/apps/"
# Search for "business" in test files
echo "=== Searching in test files ==="
rg -i "business" "tests/" --type json --type python
# Search for "business" in frontend code
echo "=== Searching in frontend code ==="
rg -i "business" "cvat-core/" "cvat-ui/"
Length of output: 525
Script:
#!/bin/bash
# Let's try one more time with more specific patterns and correct file types
# Search in all relevant files using ripgrep
echo "=== Searching for 'business' references ==="
rg -i "business" \
--type js \
--type ts \
--type py \
--type json \
--type-add 'rego:*.rego' \
-g '!node_modules' \
-g '!dist' \
-g '!build' \
-B 2 -A 2
# Also specifically check the policy files to be thorough
echo "=== Checking policy files specifically ==="
find . -name "*.rego" -type f -exec grep -i "business" {} \;
Length of output: 397
cvat/apps/events/rules/events.rego (1)
13-13
: LGTM! The privilege enum update looks safe.
The removal of the "business" privilege from the input schema is consistent with the PR objective and doesn't impact the existing authorization rules since none of them specifically referenced this privilege.
Let's verify that no other Rego files still reference the "business" privilege:
✅ Verification successful
After analyzing all the Rego policy files in the codebase, I can now generate the final response:
✓ The privilege enum update is safe and consistent
The removal of "business" privilege is safe as there are no references to this privilege value in any of the Rego policy files. All authorization rules consistently use only "admin", "user", and "worker" privileges across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to "business" privilege in Rego files
# Expect: No matches as all references should be removed
# Test: Search for "business" string in all Rego files
rg -t rego '"business"' cvat/apps/
Length of output: 65
Script:
#!/bin/bash
# Let's try a different approach to find Rego files and search for "business" references
# First find all .rego files, then search within them
# Find all .rego files and search for "business" string
fd '\.rego$' cvat/apps/ --exec grep -l "business" {} \;
# Also let's see the content of all .rego files to ensure we're not missing anything
fd '\.rego$' cvat/apps/ --exec cat {}
Length of output: 75829
site/content/en/docs/administration/basics/admin-account.md (1)
14-14
: LGTM! The updated role list accurately reflects the system changes.
The removal of the "business" role from the available roles list is consistent with the broader changes across the codebase.
cvat/apps/organizations/rules/organizations.rego (1)
12-12
: LGTM! The removal of "business" privilege is safe.
The change aligns with the PR objective of removing the unused "business group" feature. The modification only affects the input structure documentation, and the actual authorization rules don't reference the "business" privilege, making this a safe change.
Let's verify the consistency of this change across the codebase:
✅ Verification successful
The removal of "business" privilege is safe and consistent
The verification shows that:
- No Rego policy files contain references to "business" privilege in their rules
- The only mentions of "business" in documentation are unrelated to privileges (they refer to business days, business names, and general business context)
- The change is isolated to the input structure documentation in organizations.rego
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that "business" privilege is not referenced in other Rego files
# or test cases that might depend on it.
echo "Checking for any remaining references to 'business' privilege in Rego files..."
rg -l '"business"' '**/*.rego'
echo "Checking for any test cases that might still use 'business' privilege..."
rg -l '"business"' '**/test_*.py' '**/test_*.ts'
echo "Checking for any documentation that might need updating..."
rg -l '"business"' '**/*.md' '**/*.rst'
Length of output: 839
Script:
#!/bin/bash
# Let's try with proper path patterns using ripgrep
echo "Checking for any remaining references to 'business' privilege in Rego files..."
rg -l "business" --type rego
echo "Checking for any test cases that might still use 'business' privilege..."
rg -l "business" -g "test_*.py" -g "test_*.ts"
echo "Checking for any documentation that might need updating..."
rg -l "business" -g "*.md" -g "*.rst"
# Let's also verify if there are any other Rego policy files that might need similar updates
echo "Listing all Rego policy files..."
fd -e rego
Length of output: 1760
Script:
#!/bin/bash
# Let's check the content of the found documentation files and other Rego files
# for any business privilege references that might need updating
# Check documentation files content
echo "=== Content of overview.md where 'business' is mentioned ==="
rg -C 3 business "site/content/en/docs/getting_started/overview.md"
echo -e "\n=== Content of subscription-management.md where 'business' is mentioned ==="
rg -C 3 business "site/content/en/docs/enterprise/subscription-management.md"
# Let's check all Rego files content for any business privilege references
echo -e "\n=== Checking all Rego files for 'business' privilege references ==="
for rego_file in $(fd -e rego); do
echo "Checking $rego_file..."
rg -C 3 '"privilege".*"business"' "$rego_file"
rg -C 3 '"business".*"privilege"' "$rego_file"
done
Length of output: 10695
cvat/apps/lambda_manager/rules/lambda.rego (2)
13-13
: LGTM! Change aligns with business group removal.
The removal of the "business" privilege option from the input structure is consistent with the PR objective of removing the unused business group feature.
13-13
: Verify migration path for existing users.
Please ensure there's a migration strategy for any existing users who might currently have the "business" privilege. We should verify:
- How existing users with "business" privilege will be handled
- What privilege level they will be mapped to
- If there are any database migrations needed
Let's check for any existing users with business privilege:
tests/python/shared/assets/cloudstorages.json (1)
39-43
: LGTM! Owner details updated consistently with business group removal.
The changes align with the PR objective of removing the business group feature, replacing business-specific user references with standard user naming.
Let's verify the consistency of user references across test assets:
✅ Verification successful
✓ User references are consistent across test assets
The verification confirms:
- No remaining "business" references found in test assets
- "user8" references are consistently used across all test assets including users, tasks, invitations, comments, cloud storages, and database fixtures
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining "business" references in test assets
# Expected: No occurrences of "business" in usernames or names
echo "Checking for remaining 'business' references in test assets..."
rg -i "business" "tests/python/shared/assets/" --type json
echo "Verifying user8 references are consistent..."
rg -i "user8" "tests/python/shared/assets/" --type json
Length of output: 2219
tests/python/shared/assets/comments.json (2)
40-44
: LGTM! Changes align with business group removal
The modifications to owner details are consistent with the PR objective of removing the business group feature. The changes maintain the existing data structure while standardizing the user naming convention used throughout the test assets.
Also applies to: 54-58
40-44
: Verify user ID consistency across test assets
The owner changes from "Business Second" (business2) to "User Eighth" (user8) maintain the same user ID (11). Let's verify this ID is consistent across other test assets.
Also applies to: 54-58
✅ Verification successful
User ID 11 consistently maps to "User Eighth" (user8) across test assets
The verification shows that all user references with ID 11 consistently map to the same user details:
- First Name: "User"
- Last Name: "Eighth"
- Username: "user8"
- URL: "http://localhost:8080/api/users/11"
The changes in the comments.json file maintain this consistency, indicating proper alignment across test assets.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any inconsistencies in user ID 11 across test assets
# Expected: All references to user ID 11 should map to the same user details
# Search for user ID 11 in all JSON files under tests
fd -e json -x jq -r 'recurse | select(type == "object") | select(.id == 11)' {} \; 2>/dev/null
Length of output: 12639
cvat/apps/engine/rules/annotationguides.rego (1)
13-13
: LGTM! Verify consistency across other Rego files.
The removal of the "business" privilege from the input structure documentation is consistent with the PR objective.
Let's verify that this change is consistent across all Rego files:
✅ Verification successful
The removal of "business" privilege is consistent across all Rego files
The verification shows that:
- No occurrences of "business" privilege were found in any Rego files
- All privilege enums consistently use only "admin", "user", and "worker" privileges
- The change in
annotationguides.rego
aligns with the privilege structure used throughout the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" privilege in Rego files
# Expected: No matches, as all Rego files should have been updated
echo "Checking for 'business' privilege references in Rego files..."
rg -t rego '"business"' cvat/apps/
Length of output: 204
Script:
#!/bin/bash
# Let's try a different approach using regular grep to search for "business" in all files under cvat/apps/engine/rules/
echo "Searching for 'business' in Rego files..."
find cvat/apps/engine/rules/ -type f -name "*.rego" -exec grep -l "business" {} \;
echo -e "\nChecking content of found files..."
find cvat/apps/engine/rules/ -type f -name "*.rego" -exec grep -B2 -A2 "business" {} \; 2>/dev/null
echo -e "\nListing all privilege enums in Rego files..."
find cvat/apps/engine/rules/ -type f -name "*.rego" -exec grep -l "privilege.*admin\|worker\|user" {} \;
find cvat/apps/engine/rules/ -type f -name "*.rego" -exec grep "privilege.*admin\|worker\|user" {} \; 2>/dev/null
Length of output: 7998
cvat/apps/engine/rules/labels.rego (1)
13-13
: LGTM! Verify related utility functions.
The removal of the "business" privilege from the input schema is consistent with the PR objective of removing the unused business group feature. The policy rules in this file don't explicitly depend on the business privilege.
Let's verify there are no remaining business privilege references in related utility functions:
✅ Verification successful
Removal of "business" privilege is safe
The verification shows that:
- No other Rego policy files in the rules directory contain any references to "business" privilege
- All policy files use the same imports (utils, organizations) and there are no direct dependencies on business-specific functions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to business privilege in related Rego files
# Search for "business" references in all Rego files
echo "Checking for 'business' references in Rego files..."
rg -l "business" --type rego
# Specifically check the utils package that this file depends on
echo "Checking utils.rego for business-specific functions..."
rg "business" "cvat/apps/engine/rules/utils.rego"
Length of output: 417
Script:
#!/bin/bash
# Let's try a different approach using basic file search and grep
# First, find all Rego policy files
echo "Finding all Rego policy files..."
fd ".rego$" cvat/apps/engine/rules/
# Then search for "business" in these files
echo -e "\nSearching for 'business' references..."
fd ".rego$" cvat/apps/engine/rules/ -x grep -l "business" {} \;
# Get content of utils.rego to verify dependencies
echo -e "\nChecking utils.rego content..."
fd "utils.rego" cvat/apps/engine/rules/ -x cat {} \;
Length of output: 778
Script:
#!/bin/bash
# Let's check the content of each Rego file to see any business privilege references
# Using grep with context to better understand the usage
echo "Checking content of all Rego files for 'business' references with context..."
for file in cvat/apps/engine/rules/*.rego; do
echo -e "\n=== Checking $file ==="
grep -A 5 -B 5 "business" "$file" || true
done
# Also check if there are any utility functions that these policies depend on
echo -e "\nChecking for utility imports and package declarations..."
grep -E "^(import|package)" cvat/apps/engine/rules/*.rego
Length of output: 5219
cvat/apps/quality_control/rules/quality_settings.rego (1)
13-13
: LGTM! Verify complete removal of "business" privilege.
The removal of the "business" privilege from the input structure is consistent with the PR objective and doesn't impact the existing authorization logic in this file.
Let's verify that all references to the "business" privilege have been removed from the codebase:
✅ Verification successful
Verified: "business" privilege has been completely removed
The search results confirm that:
- All Rego policy files consistently define privilege as
<"admin"|"user"|"worker">
with no references to "business" - Python code only uses "admin", "user", and "worker" privileges in tests and implementations
- No TypeScript definitions contain "business" privilege references
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to "business" privilege
# Expected: No matches indicating "business" privilege in authorization contexts
# Search for "business" in Rego files
echo "Checking Rego files for 'business' privilege..."
rg -g "*.rego" '"business"'
# Search for "business" in TypeScript type definitions
echo "Checking TypeScript files for 'business' privilege..."
rg -g "*.ts" -g "*.tsx" '"business".*privilege'
# Search for "business" in Python files
echo "Checking Python files for 'business' privilege..."
rg -g "*.py" '"business".*privilege'
Length of output: 467
Script:
#!/bin/bash
# Let's try a broader search with more context and variations
# Search for "business" in all files, showing context
echo "Searching for 'business' with context..."
rg -C 2 "business" --type-not binary --type-not lock
# Search specifically for privilege enums/types
echo "Searching for privilege type definitions..."
ast-grep --pattern 'type $_ = "admin" | "user" | $_'
# Search for privilege-related constants or enums
echo "Searching for privilege-related constants..."
rg -C 2 'privilege.*=.*(\[|\{|enum)' --type-not binary
Length of output: 519
Script:
#!/bin/bash
# Let's try simpler searches without file type exclusions
# Search for "business" string in context
echo "Searching for 'business' references..."
rg -C 2 '"business"'
# Search for privilege definitions in TypeScript
echo "Searching for privilege types in TypeScript..."
rg -t ts -t tsx 'type.*Privilege.*=.*|interface.*Privilege.*\{|enum.*Privilege'
# Search for privilege in Python
echo "Searching for privilege in Python..."
rg -t py 'privilege.*=.*|PRIVILEGE.*=.*'
# Search for privilege in Rego
echo "Searching for privilege in Rego..."
rg -g "*.rego" '"privilege"'
Length of output: 14314
cvat/apps/engine/rules/cloudstorages.rego (1)
13-13
: LGTM! Clean removal of the business privilege.
The removal of the "business" privilege from the user roles is clean and doesn't affect the existing authorization logic since no rules specifically depended on this privilege.
Let's verify that the business privilege has been consistently removed across all Rego rules:
✅ Verification successful
Verified: Business privilege has been completely removed from all Rego rules
The search results confirm that:
- No references to "business" privilege exist in any Rego rules
- All privilege definitions consistently use only "admin", "user", or "worker" across all rule files
- The change in cloudstorages.rego aligns with the privilege definitions in other rules
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" privilege in Rego rules
# Expected: No matches, confirming complete removal
echo "Checking for 'business' references in Rego rules..."
rg -g '*.rego' '"business"' cvat/apps/engine/rules/
echo "Checking for privilege definitions in Rego rules..."
rg -g '*.rego' 'privilege.*admin|user|worker' cvat/apps/engine/rules/
Length of output: 12280
cvat/apps/organizations/rules/memberships.rego (2)
13-13
: LGTM! The privilege enum update is correct and safe.
The removal of the "business" privilege from the authorization rules is clean and maintains the security model. The remaining privileges ("admin"|"user"|"worker") provide sufficient granularity for access control.
13-13
: Verify complete removal of business privilege references.
Let's ensure all references to the "business" privilege have been removed from the codebase.
✅ Verification successful
All "business" privilege references have been successfully removed
Based on the comprehensive search results and review of the specific file, all occurrences of "business" found in the codebase are only in license headers (referring to "business interruption") and comments about emoji. There are no remaining references to "business" privilege in the authentication/authorization logic or membership rules.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to "business" privilege or group
# that might have been missed during the removal.
echo "Searching for 'business' references in policy files..."
rg -g '*.rego' '"business"'
echo "Searching for 'business' references in TypeScript/JavaScript files..."
rg -g '*.{ts,js}' '"business"'
echo "Searching for 'business' references in Python files..."
rg -g '*.py' '"business"'
echo "Searching for 'business' in JSON schema or configuration files..."
rg -g '*.{json,yaml,yml}' '"business"'
Length of output: 683
Script:
#!/bin/bash
# Let's try a more thorough search without quotes and case-sensitive
# to catch any variations of "business" in privilege/role related contexts
echo "Searching for business-related terms in all files..."
rg -i "business.*(?:privilege|role|group|permission)" --type-add 'config:*.{json,yaml,yml,rego}' --type config --type python --type typescript --type javascript
echo "Searching specifically for business privilege assignments..."
rg -i "privilege.*business|business.*privilege" --type-add 'config:*.{json,yaml,yml,rego}' --type config --type python --type typescript --type javascript
# Let's also check for any enum or constant definitions that might contain business
echo "Checking for business in enums or constants..."
ast-grep --pattern 'enum $_ {
$$$
business
$$$
}'
ast-grep --pattern 'const $_ = {
$$$
business
$$$
}'
Length of output: 849
Script:
#!/bin/bash
# Let's try a simpler approach with separate commands for each file type
# and without complex regex patterns
echo "Searching in .rego files..."
rg -i "business" --type rego
echo "Searching in .ts and .js files..."
rg -i "business" -g "*.{ts,js}"
echo "Searching in .py files..."
rg -i "business" -g "*.py"
echo "Searching in config files..."
rg -i "business" -g "*.{json,yaml,yml}"
Length of output: 426
Script:
#!/bin/bash
# Let's try one final approach using find and grep
# to ensure we're not missing anything due to ripgrep configuration
echo "Searching for 'business' in all relevant files..."
find . -type f \( -name "*.rego" -o -name "*.ts" -o -name "*.js" -o -name "*.py" -o -name "*.json" -o -name "*.yaml" -o -name "*.yml" \) -exec grep -i "business" {} \;
# Also let's specifically check the file being modified
echo "Checking the specific file under review..."
cat cvat/apps/organizations/rules/memberships.rego
Length of output: 6465
cvat-core/src/user.ts (2)
14-14
: Verify SerializedUser interface consistency
The groups property type change should be reflected in the SerializedUser interface.
Let's verify the interface consistency:
✅ Verification successful
Let me try a different search pattern to locate the SerializedUser interface:
SerializedUser interface is already consistent
The SerializedUser interface in cvat-core/src/server-response-types.ts
already has the correct type definition for the groups
property as ('user' | 'admin')[]
, which matches the type in the User class. No changes are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if SerializedUser interface is updated consistently
# Look for SerializedUser interface definition
echo "Checking SerializedUser interface..."
ast-grep --pattern 'interface SerializedUser {
$$$
groups: $_;
$$$
}'
Length of output: 160
Script:
#!/bin/bash
# Search for SerializedUser interface with ripgrep for better context
rg "interface SerializedUser" -A 10 -B 2
Length of output: 1566
14-14
: Verify handling of existing 'business' group data
While removing the 'business' group type is aligned with the PR objective, we need to ensure proper handling of existing data.
Let's verify the impact:
✅ Verification successful
Removal of 'business' group type is safe to proceed
Based on the comprehensive verification:
- The
SerializedUser
interface in server-response-types.ts already defines groups as('user' | 'admin')[]
- No references to 'business' group were found in:
- TypeScript/JavaScript files
- Database migrations
- Backend group handling code
- The User class implementation properly handles the restricted group types
The change is consistent with the existing codebase and there are no data migration concerns as the 'business' group type is not used anywhere.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to 'business' group in the codebase
# and potential data migration needs
# Check for 'business' string in migration files
echo "Checking for related migrations..."
fd -e py -e sql . cvat/apps/engine/migrations | xargs rg -l "business"
# Check for other references to 'business' group
echo "Checking for other references..."
rg -g '!*.md' -g '!package*.json' -g '!yarn.lock' "'business'"
Length of output: 295
Script:
#!/bin/bash
# Let's try a more comprehensive search strategy
# 1. Search for business group in TypeScript/JavaScript files
echo "Checking TypeScript/JavaScript files for 'business' group references..."
rg -t ts -t js "(?i)(\"|')business(\"|')"
# 2. Look for group-related type definitions and interfaces
echo "Checking for group-related type definitions..."
ast-grep --pattern 'type $_ = $$$"business"$$$'
ast-grep --pattern 'interface $_ { $$$group$$$: $_ }'
# 3. Check for any database migration files
echo "Checking migration files..."
fd migration cvat/apps/engine
# 4. Check the specific user-related files
echo "Checking user-related files..."
fd -e ts -e js user
Length of output: 1553
Script:
#!/bin/bash
# Let's check the specific user-related implementation and tests
# 1. Check the user class implementation for any business group handling
echo "Checking user.ts implementation..."
cat cvat-core/src/user.ts
# 2. Check for any test files that might verify group handling
echo "Checking test files for group-related tests..."
rg -t ts -t js "groups.*(\"|')(?:user|admin|business)(\"|')" tests/
# 3. Check for any group-related API calls or handlers
echo "Checking for group-related API implementations..."
rg -t ts -t js "groups?: *\[.*\]" cvat-core/
Length of output: 3864
Script:
#!/bin/bash
# Let's check the server-response-types for the complete interface definition
# and any migration-related files
# 1. Check the SerializedUser interface definition
echo "Checking SerializedUser interface..."
cat cvat-core/src/server-response-types.ts
# 2. Check for any group-related migrations or database changes
echo "Checking for database migrations..."
fd -e py -e sql . cvat/apps/engine/migrations | xargs grep -l "group"
# 3. Check for any group validation or processing in the backend
echo "Checking backend group handling..."
fd -e py . cvat/apps/engine | xargs rg "groups.*=.*\[.*\]"
Length of output: 14141
tests/python/shared/assets/webhooks.json (1)
98-102
: LGTM! Owner details updated consistently with business group removal.
The changes align with the PR objective of removing business group references, maintaining data integrity while standardizing the user role structure.
Let's verify the consistency of user references across test assets:
✅ Verification successful
✓ Owner details successfully updated with no remaining business references
After examining the entire webhooks.json file, I can confirm that:
- All owner details have been properly updated to use standard user roles
- No instances of "business" or "Business" remain in the file
- The changes maintain consistency with other webhook configurations
- The JSON structure and data integrity are preserved
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining "business" references in test assets
# Expected: No occurrences of "business" in user-related fields
# Search for any remaining "business" references in JSON test assets
rg -i "business[0-9]?" "tests/python/shared/assets/*.json"
# Search for "Business" in name fields
rg -i '"first_name":\s*"Business"' "tests/python/shared/assets/*.json"
Length of output: 281
Script:
#!/bin/bash
# Let's first find the correct path to the test assets
fd -t f "webhooks.json"
# Once we find the file, let's check for any remaining business references
fd -t f "webhooks.json" --exec rg -i "business[0-9]?" {}
# And check for "Business" in name fields
fd -t f "webhooks.json" --exec rg -i '"first_name":\s*"Business"' {}
# Let's also check the entire test directory for any business references
fd -t f -e json . --exec rg -i "business[0-9]?" {}
Length of output: 249
Script:
#!/bin/bash
# Let's examine the content of webhooks.json to verify all business references are properly replaced
cat tests/python/shared/assets/webhooks.json
# Also search for any variations of business/Business case-insensitively
rg -i "business" tests/python/shared/assets/webhooks.json
Length of output: 3719
tests/python/shared/assets/issues.json (2)
75-79
: LGTM! Owner details updated consistently for issue #3.
The changes align with the PR objective of removing business group references while maintaining data consistency.
101-105
: LGTM! Owner details updated consistently for issue #2.
The changes align with the PR objective of removing business group references while maintaining data consistency.
Let's verify that all business group references have been removed from related test assets:
✅ Verification successful
No business group references found in test assets
Based on the verification results:
- No occurrences of "business" or "business group" terms were found in the test assets
- The user details in issues.json follow the expected structure with basic user fields (first_name, last_name, id, url)
- The changes in issue Support Pascal VOC Format #2's owner details maintain consistency with the standard user information format
The verification confirms that business group references have been successfully removed while maintaining proper data structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining "business" references in test assets
# Expected: No occurrences of "business" in user-related fields
echo "Checking for remaining 'business' references in test assets..."
rg -i '"business' tests/python/shared/assets/*.json
Length of output: 845
Script:
#!/bin/bash
# Let's do a more thorough check for any business-related terms in the test assets
# using different patterns and checking the content around user fields
echo "Checking test assets for business-related terms..."
rg -i 'business.*group|group.*business' tests/python/shared/assets/*.json
echo -e "\nChecking specifically the users.json and issues.json for any business references..."
cat tests/python/shared/assets/users.json tests/python/shared/assets/issues.json | grep -i 'business'
echo -e "\nListing all user-related fields in issues.json for verification..."
cat tests/python/shared/assets/issues.json | jq '.[] | select(.id == 2) | {owner_id, assignee, assignee_id, first_name, last_name, username}'
Length of output: 1480
Script:
#!/bin/bash
# Let's check the actual content of issues.json to understand its structure
echo "Checking content of issues.json..."
cat tests/python/shared/assets/issues.json | head -n 20
echo -e "\nChecking for any business-related terms with surrounding context..."
rg -i 'business' tests/python/shared/assets/issues.json -C 5
Length of output: 808
cvat/apps/quality_control/rules/conflicts.rego (1)
13-13
: Verify API compatibility after removing "business" privilege.
The removal of "business" from the privilege enum is correct and aligns with the PR objective. However, this is a breaking change in the API contract.
Let's verify if any API clients depend on this privilege:
cvat/apps/webhooks/rules/webhooks.rego (1)
14-14
: LGTM! Input structure updated correctly.
The removal of the "business" privilege option from the input structure aligns with the PR objective of removing the unused business group feature. The change is safe as the authorization rules in this file don't contain any business-specific logic.
Let's verify that this change is consistent across other authorization files:
✅ Verification successful
Removal of "business" privilege is consistent with codebase
The verification confirms that:
- The utils.rego file defines only three privilege levels: "admin", "user", and "worker"
- All privilege definitions across Rego files consistently use these three levels
- No references to "business" privilege were found in any authorization rules
The removal of "business" from the input structure in webhooks.rego aligns perfectly with the existing privilege system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" privilege in other Rego files
# Expected: No occurrences of "business" privilege in authorization rules
echo "Checking for 'business' privilege references in Rego files..."
rg -l "business.*privilege" "cvat/apps/*/rules/*.rego"
echo "Checking for other potential business-related rules..."
rg -l "business" "cvat/apps/*/rules/*.rego"
Length of output: 488
Script:
#!/bin/bash
# Let's first find the correct path to Rego files
echo "Finding Rego files in the repository..."
fd -e rego
# Then check for business-related content in those files
echo -e "\nChecking for 'business' privilege references in Rego files..."
fd -e rego -x rg -l "business.*privilege" {}
echo -e "\nChecking for other potential business-related rules..."
fd -e rego -x rg -l "business" {}
Length of output: 1417
Script:
#!/bin/bash
# Let's check the content of utils.rego since it might define privilege constants
echo "Checking utils.rego for privilege definitions..."
cat cvat/apps/iam/rules/utils.rego
# Let's also check for any privilege-related content in all Rego files
echo -e "\nChecking all Rego files for privilege definitions and usage..."
fd -e rego -x rg -A 2 -B 2 '"privilege"'
Length of output: 5768
cvat/apps/quality_control/rules/quality_reports.rego (2)
14-14
: LGTM! Privilege enum correctly updated.
The removal of the "business" privilege from the input structure documentation aligns with the PR objective of removing the unused business group feature. The change is safe as none of the policy rules depend on this privilege.
14-14
: Verify consistent privilege removal across all Rego files.
Let's ensure this change is consistently applied across all authorization policy files.
✅ Verification successful
Privilege enum changes are consistently applied across the codebase
The verification confirms that:
- No references to "business" privilege exist in any Rego files
- All privilege enums consistently use the pattern
<"admin"|"user"|"worker">
across all authorization policy files - The changes have been uniformly applied throughout the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" privilege in Rego files
# Expected: No matches, confirming complete removal
echo "Checking for 'business' privilege in Rego files..."
rg -l "business" "cvat/apps/*/rules/*.rego"
echo "Checking privilege enums in Rego files..."
rg -A 1 "privilege.*:" "cvat/apps/*/rules/*.rego"
Length of output: 435
Script:
#!/bin/bash
# Let's try a different approach using fd to find Rego files first
echo "Finding all Rego files in the repository..."
fd -e rego
echo -e "\nChecking for 'business' privilege in found Rego files..."
fd -e rego -x rg -l "business" {}
echo -e "\nChecking privilege enums in found Rego files..."
fd -e rego -x rg -A 1 "privilege.*:" {}
Length of output: 3031
cvat/apps/organizations/rules/invitations.rego (1)
13-13
: LGTM! The removal of "business" privilege is safe for this policy.
The authorization rules in this file don't have any special cases for the "business" privilege, making this change safe from a policy perspective.
Let's verify the impact on existing users and dependencies:
✅ Verification successful
Let me gather more information about the privilege constants and their usage:
Removal of "business" privilege is safe and properly handled
The verification confirms that:
- No remaining references to "business" privilege exist in the Rego rules
- The privilege system is cleanly implemented with only "admin", "user", and "worker" privileges in the utils package
- The privilege priority system and permission checks (
has_perm
,is_admin
, etc.) are properly defined without any dependency on "business" privilege - All privilege comparisons in rules only check for ADMIN, USER, and WORKER constants
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" privilege
# and potential migration needs
# Test 1: Check for any remaining "business" references in Rego files
echo "Checking Rego files for 'business' references..."
rg -t rego '"business"'
# Test 2: Check for potential user migration code
echo "Checking for user migration handling..."
rg -g '*.py' 'business.*group|group.*business'
# Test 3: Check utils package for privilege handling
echo "Checking utils package privilege handling..."
fd utils.rego -t f --exec cat {} | rg -A 5 'privilege'
Length of output: 898
Script:
#!/bin/bash
# Check for privilege constant definitions and their usage
echo "Checking for privilege constant definitions..."
ast-grep --pattern 'ADMIN := $_'
ast-grep --pattern 'USER := $_'
ast-grep --pattern 'WORKER := $_'
ast-grep --pattern 'BUSINESS := $_'
echo "Checking for privilege usage in rules..."
rg -g '*.rego' -A 3 'privilege =='
Length of output: 1171
cvat/apps/engine/rules/tests/generators/server_test.gen.rego.py (1)
44-44
: LGTM! Verify test coverage impact.
The removal of "business" from GROUPS aligns with the PR objective. However, this change affects the number of generated test cases and privilege hierarchy evaluation.
Let's verify the impact on test coverage:
✅ Verification successful
LGTM! Change is consistent across test generators
The removal of "business" from GROUPS is consistent with the codebase state. The search results show that:
- All test generators use the same GROUPS definition: ["admin", "user", "worker", "none"]
- No remaining references to "business" group were found
- The privilege hierarchy in eval_rule() is consistently implemented across all test generators
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare the number of generated test cases before and after the change
# Test 1: Count the number of test cases in the generated Rego file
echo "Counting test cases in the generated Rego file..."
rg -c "test.*if \{" cvat/apps/engine/rules/tests/**/server_test.gen.rego
# Test 2: Check if there are any remaining references to "business" privilege
echo "Checking for any remaining 'business' privilege references..."
rg -i "business.*privilege" cvat/apps/engine/rules/
Length of output: 471
Script:
#!/bin/bash
# Let's try a different approach to verify the impact
# First, check if the file exists and its content
echo "Checking the test generator file..."
cat cvat/apps/engine/rules/tests/generators/server_test.gen.rego.py
# Check for any other files that might reference business groups
echo -e "\nChecking for any business group references..."
rg -l "business.*group" cvat/apps/engine/rules/
# Look for other group-related constants or configurations
echo -e "\nChecking for other group-related definitions..."
rg "GROUPS.*=.*\[.*\]" cvat/apps/engine/rules/
Length of output: 8111
cvat/apps/engine/rules/projects.rego (2)
15-15
: Verify impact of removing 'business' privilege from API contract.
The removal of the 'business' privilege from the input structure is a breaking change that could affect API consumers.
Let's check for any API documentation or OpenAPI specs that need updating:
Line range hint 1-250
: LGTM! Authorization rules maintain proper security controls.
The removal of business privilege has been done cleanly while maintaining proper security controls:
- Admin privileges are properly checked
- Organization membership verification is intact
- Resource ownership validation is preserved
- Role-based access control is maintained for remaining privileges
Let's verify no business privilege checks remain in the codebase:
✅ Verification successful
Business privilege has been completely removed from the codebase
After thorough verification of the codebase:
- No references to business privilege found in any Rego files
- The utils.rego defines only three privilege levels: ADMIN, USER, and WORKER
- The authorization rules in projects.rego correctly use these three privilege levels
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining business privilege checks in Rego files
ast-grep --pattern 'utils.has_perm(utils.BUSINESS)'
Length of output: 53
Script:
#!/bin/bash
# Let's try a broader search for any mentions of BUSINESS privilege
rg -i "business" --type rego
# Also search for any references to business in utils.rego which might define the constant
rg -i "business" cvat/apps/engine/rules/utils.rego
Length of output: 182
Script:
#!/bin/bash
# First, let's find all Rego files in the repository
fd "\.rego$"
# Then search for any mentions of "business" in all files
rg -i "business"
Length of output: 2162
Script:
#!/bin/bash
# Let's check the content of utils.rego in the IAM rules since it's likely to define the privileges
cat cvat/apps/iam/rules/utils.rego
# Also check for any business-related constants or permissions in all rego files
rg -i "business|privilege" --type-list
rg -i "business|privilege" cvat/apps/iam/rules/utils.rego
Length of output: 3039
cvat/apps/organizations/rules/tests/generators/organizations_test.gen.rego.py (1)
44-44
: LGTM! Verify test coverage after business group removal.
The removal of "business" from GROUPS aligns with the PR objective of removing the unused business group feature. The change is straightforward and maintains the existing privilege hierarchy.
Let's verify the impact on test coverage:
✅ Verification successful
LGTM! Business group removal is complete and consistent.
The verification confirms that the removal of "business" from GROUPS is safe and complete:
- No references to "business" group/privilege in any Rego policy files
- No references in CSV config files that define the rules
- The remaining groups ["admin", "user", "worker", "none"] are consistently used across:
- Group privilege comparisons in eval_rule() functions
- Role hierarchies in test generators
- Authorization rules in CSV configs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any remaining references to "business" group/privilege
# in the test files and authorization rules
# Check for any remaining "business" references in Rego files
echo "Checking Rego files for 'business' references..."
rg -l 'business' '**/*.rego'
# Check for any remaining "business" references in test files
echo "Checking test files for 'business' references..."
rg -l 'business' '**/*test*.py'
# Check the organizations.csv file that defines the rules
echo "Checking organizations.csv for 'business' privilege..."
if [ -f "cvat/apps/organizations/rules/organizations.csv" ]; then
rg 'business' 'cvat/apps/organizations/rules/organizations.csv'
fi
Length of output: 575
Script:
#!/bin/bash
# Let's try with correct paths and use fd to find files first
# Find and check Rego files
echo "Finding and checking Rego files..."
fd -e rego -x rg -l "business" {}
# Find and check Python test files
echo "Finding and checking Python test files..."
fd -e py -g "*test*" -x rg -l "business" {}
# Find and check organizations.csv
echo "Finding organizations.csv..."
fd -g "organizations.csv" -x rg "business" {}
# List all files in the organizations rules directory to understand structure
echo "Listing organizations rules directory..."
fd . "cvat/apps/organizations/rules"
Length of output: 1179
Script:
#!/bin/bash
# Let's check the content of relevant files to verify complete removal of business group
# Check Rego policy files
echo "Checking Rego policy files content..."
for file in cvat/apps/organizations/rules/*.rego; do
echo "=== $file ==="
cat "$file"
done
# Check CSV config files
echo -e "\nChecking CSV config files..."
for file in cvat/apps/organizations/rules/tests/configs/*.csv; do
echo "=== $file ==="
cat "$file"
done
# Check test generator files
echo -e "\nChecking test generator files..."
for file in cvat/apps/organizations/rules/tests/generators/*_test.gen.rego.py; do
echo "=== $file ==="
cat "$file"
done
Length of output: 34886
cvat/apps/lambda_manager/rules/tests/generators/lambda_test.gen.rego.py (1)
44-44
: LGTM! The removal of "business" group aligns with PR objectives.
The change correctly removes the unused "business" privilege level while maintaining the proper hierarchy of remaining groups (admin > user > worker > none).
Let's verify that the test cases are regenerated correctly:
✅ Verification successful
The removal of "business" group is correctly implemented and no references remain
The verification shows:
- No occurrences of "business" string in the test files
- The privilege hierarchy in the code correctly handles the remaining groups (admin, user, worker, none)
- The test generator functions (eval_rule, get_data, etc.) continue to work with the updated GROUPS list
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the generated test cases reflect the updated privilege hierarchy
# Expected: No references to "business" privilege in generated tests
# Check if any generated test cases still reference the business group
rg -i "business" "cvat/apps/lambda_manager/rules/tests/"
# Verify the privilege hierarchy in generated test files
rg -A 5 "privilege.*:" "cvat/apps/lambda_manager/rules/tests/"
Length of output: 4018
cvat/apps/engine/rules/tests/generators/users_test.gen.rego.py (1)
Line range hint 89-107
: Verify generated test coverage after business group removal.
The eval_rule() function's privilege comparison logic remains valid, but we should verify that the generated test cases maintain adequate coverage for the remaining groups.
Let's analyze the generated test coverage:
✅ Verification successful
Based on the analysis of the generator code and the rules file (users.csv), I can now provide a conclusive response about the test coverage verification:
Test coverage is maintained after business group removal
The test coverage remains comprehensive because:
- The generator uses product() of all parameters including GROUPS = ["admin", "user", "worker", "none"]
- The rules in users.csv only require "Admin" and "None" privilege levels
- The eval_rule() function uses index comparison which remains valid with the reduced GROUPS list
- All test scenarios in users.csv will still be covered as they don't reference the "business" group
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare the number of test cases before and after the change
# Get the previous version of the generated test file
git checkout HEAD^ cvat/apps/engine/rules/tests/users_test.gen.rego
echo "Previous test cases count:"
grep -c "^test" cvat/apps/engine/rules/tests/users_test.gen.rego
# Get the current version
git checkout - cvat/apps/engine/rules/tests/users_test.gen.rego
echo "Current test cases count:"
grep -c "^test" cvat/apps/engine/rules/tests/users_test.gen.rego
Length of output: 771
Script:
#!/bin/bash
# Let's first check if the file exists and its location
fd users_test.gen.rego
# Let's also check for Python generator files
fd users_test.gen.rego.py
# Let's examine the content of the generator file to understand the test generation logic
if [ -f "cvat/apps/engine/rules/tests/generators/users_test.gen.rego.py" ]; then
echo "=== Generator file content ==="
cat cvat/apps/engine/rules/tests/generators/users_test.gen.rego.py
fi
# Let's check if there are any generated test files in the rules/tests directory
if [ -d "cvat/apps/engine/rules/tests" ]; then
echo "=== Generated test files ==="
ls -la cvat/apps/engine/rules/tests/
fi
Length of output: 6590
Script:
#!/bin/bash
# Let's check the content of the users.csv file to understand the rules
if [ -f "cvat/apps/engine/rules/tests/configs/users.csv" ]; then
echo "=== Rules file content ==="
cat cvat/apps/engine/rules/tests/configs/users.csv
fi
# Let's generate the test file and check its content
cd cvat/apps/engine/rules/tests/generators/
echo "=== Generating tests ==="
python3 users_test.gen.rego.py ../configs/
echo "=== Generated test content ==="
cat users_test.gen.rego
Length of output: 1108
cvat/apps/engine/rules/issues.rego (2)
Line range hint 1-277
: Authorization rules remain intact after business privilege removal.
I've reviewed all the authorization rules and can confirm that:
- The allow rules use only admin, worker, or user privileges
- The filtering logic is independent of the business privilege
- The role-based checks (owner, assignee, staff) are unaffected
No additional changes are needed in this file.
13-13
: LGTM! The privilege enum update aligns with the PR objective.
The removal of the "business" privilege from the user authentication structure is consistent with the goal of removing the unused business group feature.
Let's verify that there are no remaining references to the "business" privilege in other Rego files:
✅ Verification successful
Privilege enum update is consistent across all Rego files
The comprehensive search confirms that:
- No references to "business" privilege remain in any Rego files
- All privilege enums across the codebase consistently use only
<"admin"|"user"|"worker">
or null - The change in
issues.rego
aligns with the privilege definitions in all other authorization rule files
The removal of the "business" privilege has been thoroughly and consistently implemented across all authorization rules in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" privilege in Rego files
# Expect: No occurrences of "business" privilege in authorization rules
echo "Checking for 'business' privilege references in Rego files..."
rg -g '*.rego' '"business"' cvat/apps/
Length of output: 179
Script:
#!/bin/bash
# Let's try a more comprehensive search to verify the removal of "business" privilege
# 1. Search for any remaining "business" string in Rego files with context
# 2. Search for privilege-related patterns to ensure consistency
# 3. Check for any potential comments or documentation mentioning "business"
echo "1. Searching for any 'business' string in Rego files with context..."
rg -g '*.rego' -i 'business' cvat/apps/ -C 2
echo -e "\n2. Checking privilege patterns in Rego files..."
rg -g '*.rego' 'privilege.*("admin"|"user"|"worker"|"business")' cvat/apps/ -C 2
echo -e "\n3. Listing all privilege-related lines in Rego files..."
rg -g '*.rego' '"privilege"' cvat/apps/ -C 2
Length of output: 15785
cvat/apps/events/rules/tests/generators/events_test.gen.rego.py (1)
45-45
: Verify privilege level comparisons after removing "business" group.
The removal of "business" from GROUPS affects the relative ordering of privileges in eval_rule()
. The current order ["admin", "user", "worker", "none"]
maintains the correct hierarchy, but we should verify that all privilege comparisons in the generated tests remain valid.
✅ Verification successful
After analyzing the test generator implementation and search results, I can now provide a final response:
The removal of "business" from GROUPS is safe and maintains correct privilege hierarchy
The code analysis reveals:
- The GROUPS list
["admin", "user", "worker", "none"]
maintains a clear hierarchical order - The
eval_rule()
function usesGROUPS.index()
for privilege comparison, which remains valid with the current ordering - No remaining references to "business" privilege in the authorization rules codebase
- The only "business" references found are in documentation files, not in functional code
The privilege comparison in eval_rule()
will continue to work correctly because:
- Admin remains the highest privilege (index 0)
- User and worker maintain their relative positions
- None remains the lowest privilege (index 3)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" privilege in Rego files
# and verify the generated test cases maintain proper privilege hierarchy
# Test 1: Search for any remaining "business" references in Rego files
echo "Checking for remaining 'business' references in Rego files..."
rg -l 'business' --type rego
# Test 2: Check the generated test file for proper privilege ordering
echo "Verifying privilege comparisons in generated tests..."
python3 cvat/apps/events/rules/tests/generators/events_test.gen.rego.py . && \
rg -A 2 -B 2 '"privilege":' events_test.gen.rego
Length of output: 465
Script:
#!/bin/bash
# Let's try a different approach to verify the privilege hierarchy
# and any remaining business references
# Search for any remaining "business" references in all files
echo "Checking for remaining 'business' references..."
rg -l "business"
# Check the content of the test generator file to understand privilege usage
echo "Checking test generator implementation..."
cat cvat/apps/events/rules/tests/generators/events_test.gen.rego.py
# Look for other files in the same directory that might help understand the test structure
echo "Checking related files in the directory..."
ls -la cvat/apps/events/rules/tests/generators/
Length of output: 6630
Script:
#!/bin/bash
# Let's examine the events.csv file that defines the rules and check the test generation logic
# Check the events.csv file content
echo "Checking events.csv content..."
cat cvat/apps/events/rules/tests/events.csv
# Check if there are any generated test files to see the actual output
echo "Looking for generated test files..."
find cvat/apps/events/rules/tests/ -name "*_test.gen.rego"
Length of output: 330
cvat/apps/log_viewer/rules/tests/generators/analytics_test.gen.rego.py (2)
44-44
: LGTM: Group removal aligns with PR objectives.
The removal of "business" from GROUPS is consistent with the PR's goal of removing the unused business group feature.
Let's verify that this change is consistent across related test files:
✅ Verification successful
LGTM: Group removal is safe and consistent with codebase
From analyzing the test generator code:
- The GROUPS list is used to define privilege levels in a hierarchical order (admin > user > worker > none)
- The removal of "business" from GROUPS doesn't break the test logic as:
- The privilege comparison still works correctly with remaining groups
- The test generator properly handles all group combinations
- No other code depends on the specific "business" group value
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to business groups in test files
rg -i "business.*group" "cvat/apps/**/test*.py"
# Search for business privilege in Rego test files
rg -i "business.*privilege" "cvat/apps/**/*.rego"
Length of output: 223
Script:
#!/bin/bash
# Let's try with fd to find the correct paths first
fd -t f "test.*\.py$" cvat/apps/log_viewer/
fd -t f "\.rego$" cvat/apps/log_viewer/
# Then search for business-related terms in the found files
fd -t f "test.*\.py$" cvat/apps/log_viewer/ -x rg -i "business.*group" {}
fd -t f "\.rego$" cvat/apps/log_viewer/ -x rg -i "business.*privilege" {}
# Let's also check the specific test generator file for any other group-related code
cat cvat/apps/log_viewer/rules/tests/generators/analytics_test.gen.rego.py
Length of output: 6441
Script:
#!/bin/bash
# Let's check the analytics.csv file that defines the rules
cat cvat/apps/log_viewer/rules/analytics.csv
# Also check the generated test file to see the test cases
fd -t f "analytics_test\.gen\.rego$" -x cat {}
Length of output: 170
Line range hint 1-150
: Verify test coverage after business group removal.
The test generator needs to maintain comprehensive coverage of authorization scenarios after removing the business group.
Let's verify the test generation is complete:
✅ Verification successful
Let me try a different approach to verify the test coverage by checking the test generator code and its dependencies.
Based on the gathered information, I can now provide a final response:
Test coverage is properly maintained after business group removal
The test generator's configuration is correct and complete:
- The GROUPS array in the code correctly includes ["admin", "user", "worker", "none"] which matches the current authorization model
- The analytics.csv configuration file defines the base rules that are used to generate test cases
- The test generator creates comprehensive test combinations using product() of all possible values for:
- Scopes (from analytics.csv)
- Contexts (sandbox, organization)
- Ownerships
- Groups (admin, user, worker, none)
- Organization roles
- Analytics access flags
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Compare the generated test file before and after changes
git show HEAD~1:cvat/apps/log_viewer/rules/tests/analytics_test.gen.rego > old_tests.rego
git show HEAD:cvat/apps/log_viewer/rules/tests/analytics_test.gen.rego > new_tests.rego
# Count and compare the number of test cases
echo "Old test cases:"
grep -c "test.*if {" old_tests.rego
echo "New test cases:"
grep -c "test.*if {" new_tests.rego
# Cleanup
rm old_tests.rego new_tests.rego
Length of output: 538
Script:
#!/bin/bash
# Check if the analytics.csv file exists and examine its content
fd -e csv analytics
if [ $? -eq 0 ]; then
echo "=== Content of analytics.csv ==="
cat $(fd -e csv analytics)
fi
# Look for other related test files
echo -e "\n=== Related test files ==="
fd -e rego analytics
fd -e py "analytics.*test"
# Check the groups definition in the codebase
echo -e "\n=== Checking group definitions ==="
rg "GROUPS.*=.*\[.*\]" -A 2
Length of output: 12033
cvat/apps/engine/rules/comments.rego (2)
13-13
: LGTM: Privilege removal aligns with project objectives.
The removal of the "business" privilege option is consistent with the PR objective of removing the unused business group feature. The remaining privileges ("admin"|"user"|"worker") maintain the necessary role hierarchy for the authorization system.
13-13
: Verify impact on existing users.
Please ensure that any existing users with "business" privilege are properly migrated to appropriate roles.
tests/python/shared/assets/memberships.json (3)
14-18
: LGTM: User renaming aligns with business group removal.
The changes consistently rename business users to generic users, which aligns with the PR objective of removing the business group feature. The modifications maintain data integrity by:
- Consistently updating all occurrences of each user
- Preserving user IDs and relationships
- Maintaining the JSON structure
Also applies to: 149-153, 164-168, 179-183
164-168
: Verify user ID consistency for user7.
The user with username "user7" appears in two memberships (id: 5 and id: 4) with consistent details:
- first_name: "User"
- last_name: "Seventh"
- id: 10
#!/bin/bash
# Description: Verify all occurrences of user7 have consistent details
rg -U "user7.*?}" --multiline
Also applies to: 179-183
14-18
: Verify user ID consistency for user8.
The user with username "user8" appears in two memberships (id: 15 and id: 6) with consistent details:
- first_name: "User"
- last_name: "Eighth"
- id: 11
Also applies to: 149-153
✅ Verification successful
User8 details are consistent across all occurrences
After verification, user8 (id: 11) appears in two memberships (id: 15 and id: 6) with completely consistent details:
- Membership All checkboxes in temporary attributes are checked when reopening job after saving the job #15: maintainer role in organization 1
- Membership Undo functionality #6: maintainer role in organization 2
- Both entries have identical user details:
{ "first_name": "User", "id": 11, "last_name": "Eighth", "url": "http://localhost:8080/api/users/11", "username": "user8" }
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all occurrences of user8 have consistent details
rg -U "user8.*?}" --multiline
Length of output: 31
Script:
#!/bin/bash
# Let's try a different approach to find user8 entries in the JSON file
cat tests/python/shared/assets/memberships.json | jq -c '.[] | select(.username == "user8")'
Length of output: 165
Script:
#!/bin/bash
# Let's examine the structure of the JSON file first
cat tests/python/shared/assets/memberships.json | jq '.'
Length of output: 6425
cvat/apps/engine/rules/tests/generators/cloudstorages_test.gen.rego.py (2)
44-44
: LGTM: Removal of "business" group aligns with PR objectives.
The removal of "business" from the GROUPS list is consistent with the PR's goal of removing unused business group functionality.
44-44
: Verify impact on existing test cases.
Since GROUPS is used in privilege level comparisons via GROUPS.index()
, removing "business" changes the relative privilege levels. Let's verify there are no existing test cases that depend on the "business" privilege.
✅ Verification successful
No impact from removing "business" group - safe to proceed
The verification shows that:
- The GROUPS list is consistently defined as ["admin", "user", "worker", "none"] across all test generators
- No references to "business" group were found in any test files or rules
- All privilege comparisons in the codebase use the current group hierarchy without "business"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any existing test cases or rules that might reference the "business" group
# Test 1: Look for "business" references in Rego test files
echo "Checking Rego test files for 'business' references..."
rg -l "business" "cvat/apps/engine/rules/tests/"
# Test 2: Check the source CSV rules file for any "business" privilege references
echo "Checking CSV rules file..."
rg "business" "cvat/apps/engine/rules/cloudstorages.csv"
# Test 3: Check for any generated test cases with "business" in their names
echo "Checking generated test case names..."
rg "test.*business" "cvat/apps/engine/rules/tests/"
Length of output: 505
Script:
#!/bin/bash
# Let's verify more thoroughly by:
# 1. Finding all files that might contain group definitions or tests
# 2. Checking the actual content of these files for any group-related logic
# Find all relevant Python and Rego files
echo "=== Finding all test generator and Rego files ==="
fd -e py -e rego . cvat/apps/engine/rules/
# Check content of these files for group-related code
echo -e "\n=== Checking content for group comparisons and indices ==="
rg -A 3 "GROUPS\." cvat/apps/engine/rules/
# Look for any group index comparisons
echo -e "\n=== Checking for group index comparisons ==="
rg -A 3 "index.*group" cvat/apps/engine/rules/
# Check for any other potential group-related logic
echo -e "\n=== Checking for other group-related logic ==="
rg -A 3 "\"admin\"|\"user\"|\"worker\"|\"business\"" cvat/apps/engine/rules/
Length of output: 36973
cvat/apps/organizations/rules/tests/generators/invitations_test.gen.rego.py (1)
44-44
: LGTM! Removal of "business" group aligns with PR objectives.
The modification of the GROUPS constant is consistent with the PR's goal of removing the unused "business group" feature. The remaining privilege hierarchy (admin
> user
> worker
> none
) is maintained.
Let's verify that this change doesn't break existing test cases and that all references to "business" group have been removed:
✅ Verification successful
Based on the analysis of the codebase, I can now generate the final response:
Removal of "business" group from GROUPS is safe and consistent
The verification shows that:
- The invitations.rego policy file doesn't contain any references to "business" group
- The policy only uses "admin", "user", and "worker" privilege levels, which matches the updated GROUPS constant
- The test generator's privilege hierarchy (
admin
>user
>worker
>none
) aligns with the actual policy implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify removal of "business" group references and test integrity
# Test 1: Check for any remaining references to "business" group in Rego test files
echo "Checking for remaining 'business' references in Rego tests..."
rg -l "business" "cvat/apps/*/rules/tests/*.rego"
# Test 2: Verify that the test generator still produces valid output
echo "Verifying test generator output..."
python3 "cvat/apps/organizations/rules/tests/generators/invitations_test.gen.rego.py" 2>&1 || echo "Test generation failed"
Length of output: 555
Script:
#!/bin/bash
# Let's try a different approach to verify the changes
# Check for any remaining references to "business" in the entire codebase under cvat/apps/organizations
echo "Checking for 'business' references in organizations app..."
rg -l "business" "cvat/apps/organizations/"
# Check the actual content of the test generator file
echo "Content of the test generator file:"
cat "cvat/apps/organizations/rules/tests/generators/invitations_test.gen.rego.py"
# Check if there are any existing Rego test files
echo "Looking for Rego test files:"
fd ".+\.rego$" "cvat/apps/organizations/"
Length of output: 7447
Script:
#!/bin/bash
# Let's check the content of the existing Rego files and the CSV file used for test generation
# Check the content of invitations.rego to see if it has any business group references
echo "Content of invitations.rego:"
cat "cvat/apps/organizations/rules/invitations.rego"
# Check the CSV file that's used to generate the tests
echo "Content of invitations.csv:"
cat "cvat/apps/organizations/rules/invitations.csv"
# Check if there are any generated test files
echo "Looking for generated test files:"
fd ".*test\.gen\.rego$" "cvat/apps/organizations/"
Length of output: 4850
cvat/apps/webhooks/rules/tests/generators/webhooks_test.gen.rego.py (2)
Line range hint 109-127
: LGTM: Privilege comparison logic remains valid.
The eval_rule
function's privilege comparison using GROUPS.index()
remains correct after removing "business". The remaining privilege levels maintain a clear hierarchy that aligns with the application's permission model.
43-43
: Verify privilege hierarchy after business group removal.
The removal of "business" from GROUPS maintains a logical privilege hierarchy from highest (admin) to lowest (none). However, let's verify that all generated test cases will still provide adequate coverage of the permission model.
✅ Verification successful
Removal of "business" from GROUPS is safe and maintains correct privilege hierarchy
After analyzing the implementation, I can confirm that:
- The webhook rules use a simple privilege hierarchy where "admin" has the highest privilege (with an explicit check), and other levels are compared using index positions
- The remaining groups ["admin", "user", "worker", "none"] maintain a logical descending order of privileges
- The Rego rules implementation in
webhooks.rego
only references "admin", "user", and "worker" privileges, with no mentions of "business" - The test generator correctly handles privilege comparisons using the GROUPS index, and removing "business" doesn't affect the relative ordering of other privileges
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any remaining references to "business" privilege in webhook rules
# and verify test coverage after its removal
# Test 1: Check for any remaining "business" references in webhook rules
echo "Checking for remaining 'business' references in webhook rules..."
rg -l "business.*privilege" "cvat/apps/webhooks/rules/"
# Test 2: Compare the generated test cases before and after the change
echo "Analyzing impact on test coverage..."
python3 cvat/apps/webhooks/rules/tests/generators/webhooks_test.gen.rego.py
rg "^test.*privilege_(ADMIN|USER|WORKER|NONE)" "webhooks_test.gen.rego" | wc -l
Length of output: 582
Script:
#!/bin/bash
# Let's try a different approach to verify the privilege hierarchy
# Check the webhook rules implementation and tests
echo "Checking webhook rules implementation..."
rg -A 5 "privilege.*level" cvat/apps/webhooks/rules/
# Look for any privilege comparisons or checks
echo "Checking privilege comparisons..."
rg -A 5 "groups.*index" cvat/apps/webhooks/rules/
# Check for any other files that might reference these groups
echo "Checking for group references..."
rg -l "GROUPS.*=.*\[.*admin.*user.*worker" cvat/apps/webhooks/
Length of output: 482
Script:
#!/bin/bash
# Let's examine the test generator implementation to understand how GROUPS is used
echo "Checking the test generator implementation..."
cat cvat/apps/webhooks/rules/tests/generators/webhooks_test.gen.rego.py
echo "Checking for any Rego rule files that might use these groups..."
fd -e rego . cvat/apps/webhooks/rules/
Length of output: 7471
Script:
#!/bin/bash
# Let's check the actual Rego rules implementation to understand how privileges are used
echo "Checking the webhook rules implementation..."
cat cvat/apps/webhooks/rules/webhooks.rego
Length of output: 4335
cvat/apps/engine/rules/tests/generators/projects_test.gen.rego.py (1)
44-44
: LGTM! The removal of "business" from GROUPS aligns with the PR objectives.
The change maintains the correct privilege hierarchy while removing the unused business group. The remaining groups ["admin", "user", "worker", "none"] provide a clear and sufficient privilege structure.
Let's verify that the test coverage remains comprehensive after removing the business group:
✅ Verification successful
The removal of "business" from GROUPS is safe and properly implemented
The verification confirms:
- The test generator file is intact and the GROUPS list is correctly modified to ["admin", "user", "worker", "none"]
- No remaining references to "business" exist in projects-related test files
- The test generation logic in
eval_rule()
continues to work correctly with the modified GROUPS list, as it relies on index-based privilege comparison - The test coverage remains comprehensive as the generator creates test cases for all valid combinations of the remaining groups
The change is safe to proceed with as it properly removes the unused business group while maintaining the test coverage for the remaining privilege levels.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test coverage after business group removal
# Test 1: Check if any remaining references to "business" exist in test files
rg -i "business.*group" "cvat/apps/engine/rules/tests/"
# Test 2: Verify that test generation still produces the expected number of test cases
# Count the number of test cases in the generated Rego file
fd -e rego -x wc -l {} "cvat/apps/engine/rules/tests/"
Length of output: 3649
Script:
#!/bin/bash
# Let's try a more focused approach to verify the test coverage
# Check the content of the test generator file
cat cvat/apps/engine/rules/tests/generators/projects_test.gen.rego.py
# Check for any generated test files in the directory
fd -e rego "projects_test" "cvat/apps/engine/rules/tests/"
# Look for any remaining references to "business" in the projects-related test files
rg -i "business" "cvat/apps/engine/rules/tests/" -g "*projects*"
Length of output: 7304
cvat/apps/organizations/rules/tests/generators/memberships_test.gen.rego.py (1)
44-44
: LGTM! Removal of "business" group aligns with project objectives.
The removal of "business" from GROUPS is consistent with the PR's goal of removing the unused business group feature.
Let's verify the impact on the generated test files:
✅ Verification successful
Removal of "business" group is safe and consistent with the codebase
After analyzing the generator script and its dependencies:
- The GROUPS list is used only for privilege comparison in
eval_rule()
function - The memberships.csv configuration file doesn't reference the "business" group in any rules
- The generator script creates test cases based on the current GROUPS list and CSV rules, so removing "business" won't break any existing test cases
Note: The test generator will automatically adjust its output to the new group structure when run, and there are no dependencies on the "business" group in the rules configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if any remaining references to "business" group exist in generated Rego test files
# and verify the updated test coverage matches the new group structure.
# Look for any remaining "business" references in generated test files
echo "Checking for remaining 'business' references in Rego test files..."
rg -l "business" "cvat/apps/organizations/rules/tests/*.rego"
# Check the structure of generated test files to ensure they reflect the new groups
echo "Verifying test structure in generated files..."
rg -A 5 "package memberships" "cvat/apps/organizations/rules/tests/memberships_test.gen.rego"
Length of output: 592
Script:
#!/bin/bash
# Let's first find the correct path to the Rego test files
echo "Finding Rego test files..."
fd -e rego -p "memberships.*test.*"
# Let's also check the Python generator file to understand its location and usage
echo "Checking the Python generator file..."
fd "memberships_test.gen.rego.py"
# Let's look at the directory structure to understand the layout
echo "Checking directory structure..."
fd -t d "organizations"
Length of output: 417
Script:
#!/bin/bash
# Let's check if the generator script is meant to create files in a different location
echo "Checking the generator script content..."
cat cvat/apps/organizations/rules/tests/generators/memberships_test.gen.rego.py
# Let's also check if there are any Rego files in the parent directory
echo "Checking for Rego files in the rules/tests directory..."
ls -la cvat/apps/organizations/rules/tests/
Length of output: 7741
Script:
#!/bin/bash
# Let's check the configs directory since the script reads CSV files from there
echo "Checking the configs directory content..."
ls -la cvat/apps/organizations/rules/tests/configs/
# Let's also check if there are any CSV files that define the rules
echo "Looking for memberships.csv content..."
cat cvat/apps/organizations/rules/tests/configs/memberships.csv 2>/dev/null || echo "memberships.csv not found"
Length of output: 1583
cvat/apps/engine/rules/jobs.rego (2)
Line range hint 1-300
: Confirm no breaking changes in authorization rules.
I've reviewed all the authorization rules in this file and can confirm that:
- No rules were dependent on the "business" privilege
- All existing rules remain valid with admin/user/worker privileges
- The authorization logic is unaffected by this change
The removal of the business privilege is safe for this file.
Let's verify there are no indirect dependencies on business privilege in other Rego files:
#!/bin/bash
# Search for any remaining references to business privilege in Rego files
rg -l "business.*privilege|privilege.*business" "**/*.rego"
15-15
: LGTM! Verify documentation updates.
The removal of the "business" privilege from the input structure documentation is consistent with the PR objectives. The change is safe as no rules in this file relied on the business privilege.
Let's verify if any documentation needs to be updated:
✅ Verification successful
Change approved - no documentation updates needed
The removal of "business" privilege from the input structure documentation is safe. The verification shows no remaining references to "business privilege" in the codebase, indicating that the documentation is already consistent with this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to "business" privilege in documentation
rg -i "business.*privilege|privilege.*business" "*.md" "docs/"
Length of output: 111
Script:
#!/bin/bash
# Search for any remaining references to "business" privilege in documentation and code
rg -i "business.*privilege|privilege.*business"
Length of output: 49
cvat/apps/engine/rules/tests/generators/jobs_test.gen.rego.py (2)
Line range hint 100-105
: LGTM: Privilege hierarchy remains intact.
The privilege comparison logic in eval_rule
maintains correct authorization after removing the "business" group. The hierarchy ["admin", "user", "worker", "none"]
preserves the intended access control model where:
- Admin remains the highest privilege (index 0)
- The relative ordering of other privileges is maintained
- Both strict and non-strict privilege checks continue to work as expected
53-53
: Verify the privilege hierarchy after removing "business" group.
The removal of "business" from GROUPS is consistent with the PR objective. However, let's verify that this change maintains the correct privilege hierarchy in the eval_rule
function.
✅ Verification successful
Privilege hierarchy is maintained after business group removal
The verification shows that:
- All test generators consistently define GROUPS as ["admin", "user", "worker", "none"]
- The privilege comparison logic using
GROUPS.index()
is uniform across all files - The hierarchy remains correct: admin > user > worker > none
- No references to "business" group remain in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that no other files in the test generators still reference the business group
# This helps ensure consistency across all test generators
echo "Checking for any remaining references to business group in test generators..."
rg -l "business" "cvat/apps/engine/rules/tests/generators/"
echo "Checking for any remaining business group references in Rego rules..."
rg -l "business" "cvat/apps/engine/rules/"
Length of output: 412
Script:
#!/bin/bash
# Let's verify the eval_rule function implementation and its usage of GROUPS
# to ensure the privilege hierarchy remains correct
echo "Checking the implementation of eval_rule function:"
rg -A 10 "def eval_rule" "cvat/apps/engine/rules/tests/generators/jobs_test.gen.rego.py"
echo "Checking all occurrences of GROUPS usage:"
rg "GROUPS" "cvat/apps/engine/rules/tests/generators/jobs_test.gen.rego.py"
# Let's also check if there are any other similar group definitions in related files
echo "Checking for other group definitions in test generators:"
fd . "cvat/apps/engine/rules/tests/generators/" --type f --exec rg -l "= \[.*admin.*user.*\]"
Length of output: 1902
Script:
#!/bin/bash
# Let's check the GROUPS definition in other test generators to ensure consistency
echo "Checking GROUPS definitions in other test generators:"
for file in $(fd . "cvat/apps/engine/rules/tests/generators/" --type f -e py); do
echo "=== $file ==="
rg "^GROUPS = " "$file" -A 1
done
# Let's also verify the privilege comparison logic in other files
echo "Checking privilege comparison logic:"
rg "GROUPS\.index" "cvat/apps/engine/rules/tests/generators/"
Length of output: 5704
cvat/apps/engine/rules/tasks.rego (1)
16-16
: LGTM! Verify other privilege checks in the codebase.
The removal of the "business" privilege from the enum is consistent with the PR objective. The change appears safe as no rule logic needed modification.
Let's verify that all privilege checks across the codebase have been updated:
✅ Verification successful
The privilege enum change is safe and consistent across the codebase
After examining the key files that handle privileges in the codebase:
- The IAM utils file (
utils.rego
) defines the core privilege constants asADMIN
,USER
, andWORKER
only - The jobs rules file and permissions system consistently use these three privilege levels
- No references to "business" privilege were found in the privilege-handling code
- The privilege hierarchy is properly maintained in
get_priority()
with only admin, user, and worker levels
The removal of "business" from the privilege enum is safe as the codebase already operates with just these three privilege levels.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to "business" privilege in Rego files and TypeScript interfaces
# Test 1: Check Rego files for any remaining "business" privilege references
echo "Checking Rego files for 'business' privilege..."
rg -t rego "business.*privilege|privilege.*business"
# Test 2: Check TypeScript interfaces for any remaining "business" privilege references
echo "Checking TypeScript files for 'business' privilege..."
rg -t ts "business.*privilege|privilege.*business"
# Test 3: Check Python test files for any remaining "business" role references
echo "Checking Python test files for 'business' role..."
rg -t py "business.*role|role.*business"
Length of output: 528
Script:
#!/bin/bash
# Let's try a different approach using ripgrep without file type filters
# and focus on finding any remaining "business" references in relevant files
# Search for "business" in all files under cvat/apps/engine/rules/
echo "Checking rules directory for 'business' references..."
rg "business" cvat/apps/engine/rules/
# Search for privilege-related type definitions and enums
echo "Checking for privilege type definitions..."
rg -l "type.*Privilege|enum.*Privilege|privilege.*=|PRIVILEGE"
# Search specifically in the core/types directory which typically contains type definitions
echo "Checking core/types directory..."
fd . "cvat-core" -t f -x rg "privilege|Privilege" {}
Length of output: 2083
Script:
#!/bin/bash
# Let's examine the content of key files that might contain privilege definitions
# Check the IAM utils file which seems to handle privileges
echo "Checking IAM utils for privilege definitions..."
cat cvat/apps/iam/rules/utils.rego
# Check the jobs rules file which also showed up in our search
echo "Checking jobs rules file..."
cat cvat/apps/engine/rules/jobs.rego
# Check the permissions file which likely contains privilege mappings
echo "Checking engine permissions..."
cat cvat/apps/engine/permissions.py
Length of output: 57845
cvat/apps/engine/rules/tests/generators/tasks_test.gen.rego.py (2)
46-46
: Verify privilege hierarchy after removing "business" group.
The removal of "business" from GROUPS affects privilege level comparisons in eval_rule()
. The current order ["admin", "user", "worker", "none"]
implies:
- admin > user > worker > none
Please verify this hierarchy aligns with the intended privilege levels across the application.
✅ Verification successful
Privilege hierarchy is correctly maintained after "business" group removal
After examining the test generator implementation, I can confirm that:
- The privilege hierarchy is implemented through index comparison:
GROUPS.index(privilege) <= GROUPS.index(r["privilege"])
- The current order
["admin", "user", "worker", "none"]
maintains the correct hierarchy where:- admin has highest privileges (index 0)
- user has second-highest privileges (index 1)
- worker has limited privileges (index 2)
- none has no privileges (index 3)
- The removal of "business" group doesn't impact the relative ordering of remaining groups, as the index comparison logic remains valid
The privilege hierarchy is properly preserved and the test generator will create appropriate test cases with the updated group list.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" privilege in Rego rules
# and verify the privilege hierarchy in related files
# Search for any remaining "business" references in Rego rules
echo "Checking for remaining 'business' references in Rego rules..."
rg -l 'business' 'cvat/apps/engine/rules/'
# Look for other privilege definitions to verify hierarchy consistency
echo "Checking privilege definitions in related files..."
rg -l 'privilege.*admin|privilege.*user|privilege.*worker' 'cvat/apps/engine/rules/'
Length of output: 1335
Script:
#!/bin/bash
# Let's examine the privilege hierarchy implementation in tasks.rego and the test generator
# Check tasks.rego for privilege hierarchy implementation
echo "=== Checking tasks.rego for privilege hierarchy ==="
rg -A 5 -B 5 'privilege.*admin|privilege.*user|privilege.*worker' 'cvat/apps/engine/rules/tasks.rego'
# Check the test generator's eval_rule implementation
echo -e "\n=== Checking eval_rule implementation in test generator ==="
rg -A 10 -B 10 'eval_rule' 'cvat/apps/engine/rules/tests/generators/tasks_test.gen.rego.py'
# Check if there are any remaining business group references in generated tests
echo -e "\n=== Checking for any remaining business group references in generated tests ==="
fd -e rego -e py -e json . cvat/apps/engine/rules/tests/ --exec rg -l 'business'
Length of output: 2793
Script:
#!/bin/bash
# Let's check the actual implementation of privilege hierarchy in the test generator
echo "=== Checking the full content of tasks_test.gen.rego.py ==="
cat cvat/apps/engine/rules/tests/generators/tasks_test.gen.rego.py
# Also check the generated test file to verify the test cases
echo -e "\n=== Checking the generated test file ==="
cat cvat/apps/engine/rules/tests/tasks_test.rego
Length of output: 8662
46-46
: Ensure test coverage is maintained.
The removal of "business" from GROUPS reduces the number of test cases generated via product()
. While this aligns with removing unused functionality, please verify that:
- All remaining privilege combinations are still adequately tested
- No edge cases are missed due to the reduced test matrix
✅ Verification successful
Let me gather more information about the test generation and coverage by examining the related files.
Let me check the test generation logic and the rules file that's being used to generate the tests.
Let me check the git history to understand the changes made to the GROUPS list and its impact.
Let me check the rules file that defines the actual test cases to understand the impact of removing the "business" privilege.
Based on the analysis of the code and test configuration, I can now provide a final response:
Test coverage is maintained after removing the 'business' privilege
The removal of "business" from GROUPS is safe because:
- The tasks.csv rules file shows no usage of "business" privilege in any of the access control rules
- The privilege hierarchy in the test generator uses index-based comparison (
GROUPS.index(privilege) <= GROUPS.index(r["privilege"])
) where privileges are ordered from highest (admin) to lowest (none) - All test cases are defined using only "admin", "user", "worker", and "none" privileges
The test matrix reduction is intentional and does not impact coverage since the "business" privilege was not used in any rule definitions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Compare the number of generated test cases before and after the change
# Find and analyze the generated test file
echo "Analyzing generated test cases..."
fd -t f "tasks_test.gen.rego$" 'cvat/apps/engine/rules/tests/'
Length of output: 139
Script:
#!/bin/bash
# Check the test generator implementation and its usage
echo "=== Checking test generator implementation ==="
cat cvat/apps/engine/rules/tests/generators/tasks_test.gen.rego.py
echo -e "\n=== Checking generated test file ==="
cat cvat/apps/engine/rules/tests/tasks_test.gen.rego
echo -e "\n=== Looking for related test files ==="
fd -t f "test.*\.rego$" cvat/apps/engine/rules/tests/
Length of output: 8785
Script:
#!/bin/bash
# Check the rules file and analyze test generation
echo "=== Checking tasks.csv rules file ==="
cat cvat/apps/engine/rules/tests/tasks.csv
echo -e "\n=== Checking if business group was used in rules ==="
rg "business" cvat/apps/engine/rules/tests/ -C 2
Length of output: 370
Script:
#!/bin/bash
# Check git history for changes to GROUPS and related test generation
echo "=== Checking git history for GROUPS changes ==="
git log -p --all -S "GROUPS = [" -- cvat/apps/engine/rules/tests/generators/tasks_test.gen.rego.py
echo -e "\n=== Looking for any business privilege references ==="
git log -p --all -S "business" -- "cvat/apps/engine/rules/tests/**"
echo -e "\n=== Checking current directory structure ==="
ls -R cvat/apps/engine/rules/
Length of output: 94392
Script:
#!/bin/bash
# Check the tasks rules file and look for business privilege usage
echo "=== Checking tasks.csv rules ==="
cat cvat/apps/engine/rules/tests/configs/tasks.csv
echo -e "\n=== Checking for business privilege in rules ==="
rg "business" cvat/apps/engine/rules/tests/configs/ -l
Length of output: 8700
tests/python/rest_api/test_memberships.py (1)
43-46
: LGTM! Verify test coverage for remaining roles.
The removal of "business1" from the non-admins list aligns with the PR objective of removing the business group feature. The test still maintains coverage for different user roles.
Let's verify that we have sufficient test coverage for all remaining user roles:
✅ Verification successful
Let me gather more specific information about the test coverage for memberships.
This will help us verify:
- The context around non_admins list usage
- The membership visibility test implementation
- The defined roles in the test files
Based on the gathered information, I can now provide the final response:
Test coverage is sufficient after removing "business1" user
The test coverage for membership visibility remains comprehensive:
- Test verifies all standard roles: worker, supervisor, maintainer, and owner
- Non-admin visibility test covers different user types: user1, dummy1, and worker2
- The test class includes proper role-based access verification through
_test_can_see_memberships
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test coverage for user roles
# Expected: All user roles should be covered by tests
# Check user roles defined in the codebase
rg -g '!*.{log,lock}' -A 5 'role.*=.*\[.*\]' .
# Check test coverage for different user roles
rg -g '*.py' 'role.*=.*(?:worker|supervisor|maintainer|owner)' tests/
Length of output: 17646
Script:
#!/bin/bash
# Check test coverage for memberships specifically
rg -g '*.py' -B 5 -A 5 'non_admins.*=.*\[.*\]' tests/
rg -g '*.py' '_test_can_see_memberships' tests/
rg -g '*.py' 'ROLES.*=.*\[.*\]' tests/
Length of output: 2658
cvat/apps/engine/rules/tests/generators/issues_test.gen.rego.py (1)
53-53
: LGTM! Removal of "business" group aligns with project objectives.
The change simplifies the permission model by removing the unused "business" group. This modification correctly maintains the privilege hierarchy in the eval_rule
function.
Let's verify that the test coverage remains comprehensive after removing the "business" group:
✅ Verification successful
LGTM! The GROUPS definition is consistent across all test generators
The verification shows that:
- None of the test generators include "business" in their GROUPS definition
- All test generators consistently use the same group hierarchy: ["admin", "user", "worker", "none"]
- No generated .rego files contain any references to "business"
The removal of "business" from the GROUPS list in issues_test.gen.rego.py
is consistent with the rest of the codebase and maintains the uniform permission model across all test generators.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the removal of "business" group doesn't leave any gaps in test coverage
# and that no references to "business" group remain in the test files.
# Test 1: Check if any references to "business" group remain in generated test files
echo "Checking for remaining 'business' references in test files..."
rg -l "business" "cvat/apps/engine/rules/tests/"
# Test 2: Verify the generated test file exists and contains test cases for all remaining groups
echo "Verifying test coverage for remaining groups..."
test_file="cvat/apps/engine/rules/tests/issues_test.gen.rego"
if [ -f "$test_file" ]; then
echo "Checking test cases for admin group..."
rg "privilege\": \"admin\"" "$test_file"
echo "Checking test cases for user group..."
rg "privilege\": \"user\"" "$test_file"
echo "Checking test cases for worker group..."
rg "privilege\": \"worker\"" "$test_file"
echo "Checking test cases for none group..."
rg "privilege\": \"none\"" "$test_file"
fi
Length of output: 415
Script:
#!/bin/bash
# Let's verify if the test file exists and check its content
echo "Checking if the test file exists..."
ls -l cvat/apps/engine/rules/tests/issues_test.gen.rego
echo "Checking the content of the generator script..."
cat cvat/apps/engine/rules/tests/generators/issues_test.gen.rego.py
echo "Checking if there are any other test files that might contain group definitions..."
fd . cvat/apps/engine/rules/tests/ -t f -e py -e rego
Length of output: 9679
Script:
#!/bin/bash
# Let's check if other test generators also have the same GROUPS definition
# and verify if they are consistent with this change
echo "Checking GROUPS definition in other test generators..."
for file in cvat/apps/engine/rules/tests/generators/*_test.gen.rego.py; do
echo "=== $file ==="
rg "GROUPS = \[.*\]" "$file" -A 1 -B 1
done
echo -e "\nChecking if any generated .rego files exist and contain 'business' references..."
fd . cvat/apps/engine/rules/tests/ -e rego -x rg -l "business" {}
Length of output: 4532
cvat/apps/engine/rules/tests/generators/comments_test.gen.rego.py (2)
54-54
: LGTM: Test coverage adjustment aligns with feature removal.
The removal of "business" from GROUPS appropriately reduces the generated test cases, eliminating unnecessary tests for a feature that's being removed. The test generator's logic remains intact and will continue to provide comprehensive coverage for the remaining privilege levels.
54-54
: Verify privilege hierarchy after business group removal.
The removal of "business" from GROUPS is consistent with the PR objective. However, let's verify that the remaining privilege hierarchy in eval_rule
function is still correct, as it uses GROUPS.index()
for comparisons.
✅ Verification successful
Privilege hierarchy remains correct after business group removal
The verification shows that:
- The
eval_rule
function only uses "admin" as a special case privilege, with other privilege checks handled by organization roles - All test generators consistently define GROUPS as ["admin", "user", "worker", "none"] without "business"
- The relative ordering of groups remains consistent across all test generators
The removal of "business" from GROUPS does not impact the privilege hierarchy as the privilege comparison is primarily based on organization roles rather than GROUPS.index().
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any remaining references to "business" group
# in related Rego files and verify the privilege hierarchy consistency
# Look for any remaining "business" references in Rego files
echo "Checking for remaining 'business' references in Rego files..."
rg -l "business" "cvat/apps/engine/rules/"
# Check privilege hierarchy in related test generators
echo "Checking privilege hierarchy in other test generators..."
fd --extension py --exec grep -l "GROUPS.*=.*\[.*admin.*user.*worker.*none" cvat/apps/engine/rules/tests/generators/
Length of output: 32219
Script:
#!/bin/bash
# Let's check the eval_rule function implementation and GROUPS usage in the test generator
echo "Checking eval_rule implementation in comments_test.gen.rego.py..."
rg -A 10 "def eval_rule" "cvat/apps/engine/rules/tests/generators/comments_test.gen.rego.py"
# Check if other test generators have similar GROUPS definition and eval_rule implementation
echo -e "\nChecking GROUPS definition in other test generators..."
rg "GROUPS.*=.*\[.*admin.*user.*worker.*none" "cvat/apps/engine/rules/tests/generators/"
Length of output: 1865
tests/python/shared/assets/invitations.json (1)
24-28
: LGTM! User renaming changes are consistent with business group removal.
The changes systematically rename business users to standard users, which aligns with the PR objective of removing the business group feature. The renaming pattern is consistent across all instances:
- "business1" → "user7"
- "business2" → "user8"
Let's verify that all business-related naming has been completely removed:
Also applies to: 116-120, 141-145, 166-170, 191-195, 216-220, 241-245, 249-253
✅ Verification successful
All business-related user references successfully replaced
The verification confirms that:
- All instances of "business" usernames have been replaced with "user7" and "user8"
- No remaining business-related terms exist in the file
- The changes are consistent across all occurrences in the invitation records
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining business-related terms in the codebase
# that might have been missed during the removal.
# Test: Search for any remaining "business" references in user-related contexts
rg -i "business[0-9]" --type json
rg -i "\"first_name\":\\s*\"business\"" --type json
Length of output: 84
Script:
#!/bin/bash
# Let's verify the changes in the specific file more thoroughly
cat tests/python/shared/assets/invitations.json | jq -r '.[] | select(.first_name == "Business" or .username | contains("business"))'
# Also check for any remaining business-related terms in the file
cat tests/python/shared/assets/invitations.json | grep -i "business"
Length of output: 277
Script:
#!/bin/bash
# Let's first check the structure of the JSON file
cat tests/python/shared/assets/invitations.json | head -n 10
# Then verify the content for any remaining business references
cat tests/python/shared/assets/invitations.json | grep -A 5 -i "business"
Length of output: 392
Script:
#!/bin/bash
# Let's examine the content of the results array in the JSON file
cat tests/python/shared/assets/invitations.json | jq -r '.results[] | {first_name, last_name, username}'
Length of output: 964
Script:
#!/bin/bash
# Let's try a simpler approach to check the raw content
cat tests/python/shared/assets/invitations.json | grep -B1 -A3 '"user[78]"'
Length of output: 1203
tests/python/shared/assets/users.json (3)
136-137
: LGTM: Business users successfully converted to regular users
The conversion of business users to regular users is consistent and follows the existing naming patterns. The changes align with the PR objective of removing the business group feature.
Also applies to: 139-139, 147-147, 149-149, 153-154, 156-156, 164-164, 166-166, 170-171, 173-173, 181-181, 183-183, 187-188, 190-190, 198-198, 200-200
356-356
: LGTM: JSON structure maintained correctly
The overall JSON structure remains valid with consistent formatting and accurate metadata (user count, etc.).
Line range hint 1-356
: Verify updates in related test files
Since this is a test asset file, we should verify that all test files referencing these users have been updated accordingly.
✅ Verification successful
No impact on test coverage - users.json is used as expected
The verification shows that:
- The users.json file is used as a pytest fixture in data.py that loads all users
- The test files using this fixture are properly accessing users through the fixture mechanism
- No hardcoded references to specific user emails were found in the tests
- The test_users.py validates users through usernames, not specific user data
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to business users in test files
echo "Searching for old business user references in test files..."
rg -l "business[1-4]@cvat.org|business[1-4]" tests/python/
echo "Searching for test files using users.json..."
rg -l "users\.json" tests/python/
Length of output: 361
Script:
#!/bin/bash
# Let's check how users.json is used in the data.py fixture
echo "Checking usage of users.json in data.py..."
rg -A 5 -B 5 "users\.json" tests/python/shared/fixtures/data.py
# Also check if there are any test files that import or use this fixture
echo -e "\nChecking test files using the data fixture..."
rg -l "from shared.fixtures.data|from shared.fixtures import data" tests/python/
Length of output: 732
Script:
#!/bin/bash
# Let's check how the users fixture is used in the test files
echo "Checking usage of users fixture in test files..."
rg -A 3 "def test.*users" tests/python/sdk/
# Also check direct references to specific user properties
echo -e "\nChecking references to user properties in tests..."
rg "user\[email protected]|admin\[email protected]|worker\[email protected]|dummy\[email protected]" tests/python/sdk/
Length of output: 626
cvat-core/src/server-response-types.ts (1)
50-50
: LGTM! Verify impact of removing 'business' group.
The removal of 'business' from the groups type aligns with the PR objective of removing the unused business group feature.
Let's verify the impact of this breaking change:
✅ Verification successful
Removal of 'business' group type is safe to proceed
Based on the search results, the removal of the 'business' group from the type definition is safe:
- No TypeScript/JavaScript files contain references to the 'business' group except the file being modified
- No Python files contain references to the 'business' group
- The only occurrences of 'business' are in documentation files and an unrelated node module
- No authorization rules or group-related logic references the 'business' group
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining references to 'business' group in the codebase
# that might need to be updated after this type change.
echo "Searching for 'business' group references in TypeScript/JavaScript files..."
rg -t ts -t js -t tsx -t jsx "('|\")business('|\")" -A 3 -B 3
echo "Searching for 'business' group references in Python files..."
rg -t py "('|\")business('|\")" -A 3 -B 3
echo "Searching for business group in authorization rules..."
rg -g "*.rego" "business" -A 3 -B 3
Length of output: 602
Script:
#!/bin/bash
# Let's try a different approach to search for business group references
# Search for 'business' string in all files, excluding binary and hidden files
echo "Searching for 'business' references..."
rg --type-not binary --hidden false "business"
# Search specifically for group-related business references
echo -e "\nSearching for group-related business references..."
rg --type-not binary --hidden false -i "group.*business|business.*group"
# Search for any enum or type definitions containing business
echo -e "\nSearching for type definitions containing business..."
ast-grep --pattern 'type $_ = $$$"business"$$$'
Length of output: 779
Script:
#!/bin/bash
# Let's try one more time with simpler ripgrep commands
# Search for business in all text files
echo "Searching for business references in all text files..."
rg -l "business"
# Specific search in TypeScript files
echo -e "\nSearching in TypeScript files..."
find . -name "*.ts" -o -name "*.tsx" | xargs grep -l "business" 2>/dev/null
# Specific search in JavaScript files
echo -e "\nSearching in JavaScript files..."
find . -name "*.js" -o -name "*.jsx" | xargs grep -l "business" 2>/dev/null
# Specific search in Python files
echo -e "\nSearching in Python files..."
find . -name "*.py" | xargs grep -l "business" 2>/dev/null
Length of output: 695
tests/python/shared/assets/projects.json (4)
8-12
: LGTM: User data standardization
The changes correctly update the user information, removing the business role reference in alignment with the PR objective.
386-390
: LGTM: Consistent user role update
The user data changes maintain consistency with the business group removal.
556-560
: LGTM: Owner data standardization
The owner information is correctly updated, maintaining consistency with the business group removal.
603-607
: LGTM: Final owner data update
The owner information is correctly updated. Let's verify that all business group references have been removed from the file.
✅ Verification successful
All business references have been successfully removed
After reviewing the entire content of projects.json, I can confirm that there are no remaining references to business users or groups in the file. The owner information for Project ID 1 has been correctly updated to user7, and this change is consistent with the overall cleanup of business references throughout the file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify no remaining references to business users/groups exist in the file
echo "Checking for any remaining 'business' references in the projects.json file..."
rg -i "business" "tests/python/shared/assets/projects.json"
# The command should return no results, indicating all business references have been removed
Length of output: 228
Script:
#!/bin/bash
# Let's try a different approach by checking the entire content of the file
echo "Displaying the content of projects.json..."
cat "tests/python/shared/assets/projects.json"
Length of output: 16917
cvat/settings/base.py (1)
239-239
: Verify role-based access control after business role removal
The removal of the 'business' role from IAM_ROLES aligns with the PR objective. However, we should ensure that any existing users with the 'business' role are properly migrated.
tests/python/shared/assets/tasks.json (3)
162-166
: LGTM: User assignments updated consistently.
The changes appropriately replace business user assignments with regular user assignments, aligning with the PR objective of removing the business group feature.
Also applies to: 893-897, 951-955, 1051-1055
893-897
: Verify storage configuration changes.
The storage configuration changes for task ID 11 (switching from local to cloud storage) appear unrelated to the PR objective of removing the business group feature.
893-897
: Verify bug tracker URL changes.
The addition of a bug tracker URL for task ID 14 appears unrelated to the PR objective of removing the business group feature.
tests/python/rest_api/test_webhooks.py (7)
99-99
: LGTM: Parameterization update aligns with business group removal.
The change correctly removes the "business" privilege from test parameterization, which is consistent with the PR objective of removing the business group feature.
119-119
: LGTM: Test case properly updated for non-business privileges.
The parameterization now correctly focuses on testing webhook creation permissions for worker and user roles only.
413-413
: LGTM: Get webhook test cases updated consistently.
The changes to test_project_owner_can_get_webhook
, test_webhook_owner_can_get_webhook
, and test_not_project_staff_cannot_get_webhook
properly remove business privilege testing.
Also applies to: 432-432, 450-450
634-634
: LGTM: Webhook list retrieval tests properly updated.
The parameterization changes in test_user_cannot_get_webhook_list_for_project
and test_user_can_get_webhook_list_for_project
correctly reflect the removal of business privilege testing.
Also applies to: 657-657
827-827
: LGTM: Patch webhook tests updated appropriately.
The parameterization changes in test_sandbox_user_can_update_webhook
and test_sandbox_user_cannot_update_webhook
align with the removal of business privilege testing.
Also applies to: 855-855
1033-1034
: LGTM: Delete webhook tests properly updated.
The changes correctly update the parameterization in delete webhook test cases:
test_user_can_delete_project_webhook
test_project_owner_can_delete_project_webhook
test_webhook_owner_can_delete_project_webhook
These changes maintain proper test coverage while removing business privilege testing.
Also applies to: 1104-1104, 1126-1126
Line range hint 1-1248
: Verify test coverage after business privilege removal.
While individual test changes look good, let's verify that we haven't lost any critical test coverage paths.
✅ Verification successful
Test coverage is comprehensive after business privilege removal
The test coverage analysis shows:
-
All critical role combinations are still tested:
- Admin vs regular user privileges
- Organization roles (owner, maintainer, supervisor, worker)
- Project-specific permissions (owner, assignee)
-
The test file maintains thorough coverage with parameterized tests for:
- Webhook creation permissions
- Webhook viewing/listing permissions
- Webhook modification permissions
- Webhook deletion permissions
-
All key permission boundaries are verified:
- Organization-level access control
- Project-level access control
- Individual webhook ownership rights
The removal of business privileges has not impacted the completeness of permission testing, as the remaining role-based tests provide sufficient coverage of the access control model.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to business privileges in test files
# and verify test coverage hasn't decreased significantly
# Look for any remaining "business" references in test files
echo "Checking for remaining business privilege references in tests..."
rg -l "business.*privilege" "tests/"
# Look for any TODO/FIXME comments related to privileges or roles
echo "Checking for TODO/FIXME comments related to privileges..."
rg -l "TODO.*privilege|FIXME.*privilege" "tests/"
Length of output: 350
Script:
#!/bin/bash
# Let's analyze test coverage more thoroughly by:
# 1. Looking at all privilege/role related test cases
# 2. Checking test parameterization for roles/privileges
# 3. Examining test class organization
# Check all test files containing privilege/role related tests
echo "=== Files with privilege/role tests ==="
rg -l "privilege|role.*test|test.*role" "tests/"
# Look at specific test parameterization
echo -e "\n=== Test parameterization for privileges/roles ==="
rg "@pytest.mark.parametrize.*privilege|@pytest.mark.parametrize.*role" "tests/" -A 1
# Check test class organization
echo -e "\n=== Test classes with privilege/role checks ==="
rg "class.*Test.*\:" -A 5 "tests/python/rest_api/test_webhooks.py"
Length of output: 11230
tests/python/rest_api/test_projects.py (1)
450-453
: LGTM: Test parameterization updated to remove business privilege
The modification correctly removes the 'business' privilege from test parameterization, aligning with the PR's objective of removing the business group feature. The test now appropriately covers only 'admin' and 'user' privileges.
tests/python/rest_api/test_jobs.py (4)
571-571
: LGTM: Removed business group from test parameterization.
The change correctly removes the "business" group test case while maintaining test coverage for the "user" group.
584-584
: LGTM: Removed business group from test parameterization.
The change correctly removes the "business" group test case while maintaining test coverage for both "user" and "worker" groups.
594-594
: LGTM: Removed business group from test parameterization.
The change correctly removes the "business" group test case while maintaining test coverage for access control restrictions on "user" and "worker" groups.
1094-1094
: Indentation fixes.
Minor indentation adjustments to maintain consistent code style.
Also applies to: 1192-1192
cvat/apps/lambda_manager/tests/test_lambda.py (1)
136-136
: LGTM: Group name change aligns with business group removal.
The change from "business" to "user" group name is consistent with the PR objective of removing the business group feature.
Let's verify if there are any remaining references to the "business" group in test files:
✅ Verification successful
Since the initial verification shows no remaining references to the "business" group in test files, let me check for any other potential references to this group in the codebase to ensure completeness:
Change from "business" to "user" group is properly implemented
The verification confirms that there are no remaining references to the "business" group in any Python files, including test files and migrations. The change is consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to "business" group in test files
# Expect: No occurrences of "business" group in test files
rg -g "test_*.py" "Group.*business" || echo "No references found"
Length of output: 86
Script:
#!/bin/bash
# Check for any remaining references to "business" group in Python files
rg -t py "Group.*business" || echo "No references found"
# Check for any references to "business" in group-related migrations
fd -e py -p ".*migrations.*" --exec rg -l "business" {} || echo "No migration references found"
Length of output: 204
cvat/apps/dataset_manager/tests/test_rest_api_formats.py (2)
144-144
: LGTM: Group change aligns with PR objective.
The change from "business" to "user" group is consistent with the PR's goal of removing the unused business group feature.
Line range hint 1-1000
: Verify test coverage for group-based permissions.
While the group change looks good, we should verify that all test cases properly cover the updated group-based permissions model.
Let's check for any test cases that might rely on business group permissions:
✅ Verification successful
Based on the search results, I can now provide a final assessment of the test coverage for group-based permissions:
Test coverage for group-based permissions appears adequate
The test files show comprehensive coverage of group-based permissions through:
- Base test setup that creates and assigns standard groups:
- Admin group (
group_admin
) - User group (
group_user
) - Worker/Annotator group (
group_annotator
) - Basic user group (
group_somebody
)
- Systematic group assignment testing in multiple test classes:
_DbTestBase
classes properly set up group permissions- User creation and group assignment is consistently tested
- Group-based access control is verified across API endpoints
- Test coverage spans across key areas:
- Task management permissions
- Project access controls
- Annotation capabilities
- Data import/export privileges
The test structure ensures that group-based permission changes are properly validated through the existing test suite. No additional test coverage appears necessary for the group permission changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any test cases that might still reference business group permissions
# Test: Look for business group references in test files
rg -i "business.*group" "cvat/apps/**/test_*.py"
# Test: Look for business group references in test data
rg -i "business.*group" "cvat/apps/**/assets/*.json"
Length of output: 235
Script:
#!/bin/bash
# Let's try searching with fd first to locate test files and then use rg
# Find test files
fd "test_.*\.py$"
# Look for group-related test cases
rg -l "group" -g "test_*.py"
# Look for permission-related test cases
rg -l "permission" -g "test_*.py"
# Check test base classes and fixtures
rg -A 5 "class.*Test" -g "test_*.py"
Length of output: 93921
Script:
#!/bin/bash
# Let's check for group-related test coverage
# Look for group-related test cases and assertions
rg -B 2 -A 2 "\.group" "tests/python/rest_api/test_*.py" "cvat/apps/*/tests/test_*.py"
# Look for group permission tests
rg -B 2 -A 2 "group.*permission" "tests/python/rest_api/test_*.py" "cvat/apps/*/tests/test_*.py"
# Check test setup code for group creation/assignment
rg -B 2 -A 2 "create.*group|group.*create" "tests/python/rest_api/test_*.py" "cvat/apps/*/tests/test_*.py"
# Look for group membership tests
rg -B 2 -A 2 "membership.*group|group.*membership" "tests/python/rest_api/test_*.py" "cvat/apps/*/tests/test_*.py"
Length of output: 973
Script:
#!/bin/bash
# Let's try with absolute paths from the test files we found earlier
# Look for group-related test cases and assertions
rg -B 2 -A 2 "\.group" "cvat/apps/dataset_manager/tests/test_rest_api_formats.py" "cvat/apps/engine/tests/test_rest_api.py" "cvat/apps/iam/tests/test_rest_api.py"
# Look for group permission tests
rg -B 2 -A 2 "group.*permission" "cvat/apps/dataset_manager/tests/test_rest_api_formats.py" "cvat/apps/engine/tests/test_rest_api.py" "cvat/apps/iam/tests/test_rest_api.py"
# Check test setup code for group creation/assignment
rg -B 2 -A 2 "(create_db_users|setUpTestData).*group" "cvat/apps/dataset_manager/tests/test_rest_api_formats.py" "cvat/apps/engine/tests/test_rest_api.py" "cvat/apps/iam/tests/test_rest_api.py"
# Look for group membership tests
rg -B 2 -A 2 "membership.*group|group.*membership" "cvat/apps/dataset_manager/tests/test_rest_api_formats.py" "cvat/apps/engine/tests/test_rest_api.py" "cvat/apps/iam/tests/test_rest_api.py"
Length of output: 2794
Script:
#!/bin/bash
# Let's check the test setup and group creation code more thoroughly
# Look for group creation and setup
rg -B 5 -A 5 "Group\.objects\.get_or_create" "cvat/apps/dataset_manager/tests/test_rest_api_formats.py" "cvat/apps/engine/tests/test_rest_api.py" "cvat/apps/iam/tests/test_rest_api.py"
# Check test base classes for group setup
rg -B 5 -A 5 "class.*Test.*\(.*\):" "cvat/apps/dataset_manager/tests/test_rest_api_formats.py"
# Look for group-related test methods
rg -B 5 -A 5 "def.*group" "cvat/apps/dataset_manager/tests/test_rest_api_formats.py" "cvat/apps/engine/tests/test_rest_api.py" "cvat/apps/iam/tests/test_rest_api.py"
# Check for group-related test data
rg -B 5 -A 5 "create_db_users" "cvat/apps/dataset_manager/tests/test_rest_api_formats.py" "cvat/apps/engine/tests/test_rest_api.py" "cvat/apps/iam/tests/test_rest_api.py"
Length of output: 31788
cvat/apps/engine/tests/test_rest_api.py (1)
65-65
: LGTM! Correctly updates user group assignment as part of business group removal
The change from group_business
to group_user
aligns with the PR objective of removing the unused business group feature. This modification is consistent with the broader cleanup effort across the codebase.
tests/python/shared/assets/cvat_db/data.json (10)
231-240
: LGTM: User profile updates are consistent.
The changes to user7-user10 profiles maintain consistent data structure and group assignments. All users are now assigned to the "user" group, which aligns with the PR objective of removing the business group feature.
Also applies to: 253-262, 275-284, 297-306
729-732
: LGTM: Organization membership updates are consistent.
The organization memberships and relationships have been properly updated to maintain referential integrity after the business group removal.
Also applies to: 749-752, 828-831, 876-879, 889-892, 902-905, 1019-1022
1034-1037
: LGTM: Membership invitations maintain data integrity.
All membership invitations have been updated to reflect the correct ownership relationships while maintaining historical data integrity.
Also applies to: 1046-1049, 1058-1061, 1094-1097, 1142-1145, 1166-1169
Line range hint 3891-3895
: LGTM: Project and task ownership assignments are valid.
The ownership and assignee relationships for projects and tasks have been properly maintained after the business group removal.
Also applies to: 3912-3916, 3995-3999, 4174-4178, 4276-4280, 4332-4336, 4360-4364, 4733-4737
12754-12757
: LGTM: User ratings data structure is preserved.
The user rating entries maintain the correct structure with default values and analytics access flags properly set.
Also applies to: 12765-12768, 12776-12779, 12787-12790
Line range hint 12907-12912
: LGTM: Issue tracking relationships are maintained.
Issue and comment ownership relationships have been properly preserved while removing business group associations.
Also applies to: 12923-12928, 12997-13001, 13010-13014
Line range hint 13094-13099
: LGTM: Cloud storage and webhook configurations are valid.
Cloud storage and webhook configurations maintain proper ownership relationships after the business group removal.
Also applies to: 13558-13563
13677-13679
: LGTM: Organization membership details are consistent.
The organization membership details maintain proper user information and invitation data.
Also applies to: 13766-13768
19005-19008
: LGTM: Audit log entries accurately reflect changes.
The audit log entries properly document the user profile changes and group assignments, maintaining a clear history of the modifications.
Also applies to: 19023-19026, 19041-19044, 19059-19062, 19077-19080, 19095-19098, 19113-19116, 19131-19134, 19149-19152
Line range hint 1-19152
: Verify data consistency across the database.
While the changes in this file look correct, it's important to ensure that all database constraints and relationships are maintained across the entire system.
@@ -11,7 +11,7 @@ export default class User { | |||
public readonly email: string; | |||
public readonly firstName: string; | |||
public readonly lastName: string; | |||
public readonly groups: ('user' | 'business' | 'admin')[]; | |||
public readonly groups: ('user' | 'admin')[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Breaking change found: 'business' group removal needs documentation
The git history clearly shows the removal of the 'business' group from the User type (groups: ('user' | 'business' | 'admin')[]
-> groups: ('user' | 'admin')[]
), but this breaking change is not documented in the CHANGELOG.md or any migration guides. This needs to be addressed by:
- Adding an entry in CHANGELOG.md under a breaking changes section
- Documenting the migration path for users currently using the 'business' group
- Updating relevant documentation to reflect the current valid group types
🔗 Analysis chain
Breaking Change: Update CHANGELOG
The removal of the 'business' group from the User type is a breaking change that could affect API consumers.
Let's verify if this is documented:
Consider adding a migration guide for users who might be relying on the 'business' group in their applications.
Let me gather more information to verify this change comprehensively.
Let me check one more thing to verify if this is a recent change and if there are any related commits.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if breaking change is documented in changelog
# Look for changelog entries
echo "Checking changelog entries..."
fd -g 'CHANGELOG*' | xargs rg -l "business"
Length of output: 160
Script:
#!/bin/bash
# Check git history for changes related to 'business' group
echo "Checking git history..."
git log -p | rg -B 5 -A 5 "business.*group"
# Check for current usage of 'business' group in the codebase
echo "Checking current codebase for 'business' references..."
rg "business.*group" -B 2 -A 2
# Look specifically in CHANGELOG.md at the root
echo "Checking CHANGELOG.md..."
if [ -f "CHANGELOG.md" ]; then
cat CHANGELOG.md | rg -B 2 -A 2 "(group|business|breaking)"
fi
Length of output: 3011
Script:
#!/bin/bash
# Check git history specifically for the user.ts file
echo "Checking git history for user.ts..."
git log -p cvat-core/src/user.ts | rg -B 5 -A 5 "groups.*:"
# Check if there are any migration guides or documentation
echo "Checking for migration guides..."
fd -g "*.md" | xargs rg -l "migration|upgrade|breaking.?change"
Length of output: 2756
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added changelog entry, relevant documentation updated
Though some users on app.cvat.ai still have business group. We can change group for them manually, or make a migration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Eldies: Great, thank you for adding the changelog entry and updating the documentation.
Regarding the users on app.cvat.ai who still have the 'business' group, updating their groups manually or via a migration sounds like a good plan to ensure consistency.
(^‿^)
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
cvat/apps/engine/rules/tests/generators/annotationguides_test.gen.rego.py
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8607 +/- ##
===========================================
- Coverage 74.32% 74.31% -0.01%
===========================================
Files 401 401
Lines 43319 43319
Branches 3925 3925
===========================================
- Hits 32198 32194 -4
- Misses 11121 11125 +4
|
I think you need to add a migration that will remove the |
added migration |
Quality Gate failedFailed conditions |
Motivation and context
It is not used for anything
How has this been tested?
Checklist
develop
branch(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
Chores