Skip to content
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.

Sign up for GitHub

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

Jump to bottom

removing business group #8607

Merged
merged 16 commits into from
Oct 31, 2024
Merged

removing business group #8607

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
ddecf49
remove business
Eldies Oct 29, 2024
494a862
removing analytics_visibility from settings
Eldies Oct 29, 2024
fceb9ac
removing business
Eldies Oct 29, 2024
f61b0f3
removing group business in test db and renaming business users
Eldies Oct 29, 2024
7104541
fixes
Eldies Oct 29, 2024
abd4aab
changelog entry
Eldies Oct 29, 2024
5570e8b
fixing group pks in test db
Eldies Oct 29, 2024
e1ec109
fixing tests
Eldies Oct 29, 2024
6b1471b
fixing tests
Eldies Oct 29, 2024
19ef42e
Merge branch 'refs/heads/develop' into dl/remove-business
Eldies Oct 29, 2024
ee5f1b5
fixes
Eldies Oct 30, 2024
3368181
migration for removing business group
Eldies Oct 30, 2024
f2890fa
Merge branch 'refs/heads/develop' into dl/remove-business
Eldies Oct 30, 2024
410cf67
fix
Eldies Oct 30, 2024
e11796b
fixing migration
Eldies Oct 31, 2024
5c79372
Merge branch 'refs/heads/develop' into dl/remove-business
Eldies Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog.d/20241029_120317_dmitrii.lavrukhin_remove_business.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
### Removed

- Removed unused business group
(<https://github.com/cvat-ai/cvat/pull/8607>)
2 changes: 1 addition & 1 deletion cvat-core/src/server-response-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export interface SerializedUser {
first_name: string;
last_name: string;
email?: string;
groups?: ('user' | 'business' | 'admin')[];
groups?: ('user' | 'admin')[];
is_staff?: boolean;
is_superuser?: boolean;
is_active?: boolean;
Expand Down
2 changes: 1 addition & 1 deletion cvat-core/src/user.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')[];
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 29, 2024 •
edited
Loading

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

Copy link
Contributor Author

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

Copy link
Contributor

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!

public readonly lastLogin: string;
public readonly dateJoined: string;
public readonly isStaff: boolean;
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/analytics_report/rules/analytics_reports.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
Eldies marked this conversation as resolved.
Show resolved Hide resolved
# },
# "organization": {
# "id": <num>,
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/dataset_manager/tests/test_rest_api_formats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def setUpTestData(cls):
@classmethod
def create_db_users(cls):
(group_admin, _) = Group.objects.get_or_create(name="admin")
(group_user, _) = Group.objects.get_or_create(name="business")
(group_user, _) = Group.objects.get_or_create(name="user")

user_admin = User.objects.create_superuser(username="admin", email="",
password="admin")
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/cache.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
ZipChunkWriter,
ZipCompressedChunkWriter,
)
from cvat.apps.engine.utils import md5_hash, load_image
from cvat.apps.engine.utils import load_image, md5_hash
SpecLad marked this conversation as resolved.
Show resolved Hide resolved
from utils.dataset_manifest import ImageManifestManager

slogger = ServerLogManager(__name__)
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/annotationguides.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/cloudstorages.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/comments.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/issues.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/jobs.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/labels.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
15 changes: 1 addition & 14 deletions cvat/apps/engine/rules/projects.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down Expand Up @@ -59,19 +59,6 @@ allow if {
organizations.has_perm(organizations.SUPERVISOR)
}

allow if {
input.scope in {utils.CREATE, utils.IMPORT_BACKUP}
utils.is_sandbox
utils.has_perm(utils.BUSINESS)
}

allow if {
input.scope in {utils.CREATE, utils.IMPORT_BACKUP}
input.auth.organization.id == input.resource.organization.id
utils.has_perm(utils.BUSINESS)
organizations.has_perm(organizations.SUPERVISOR)
}

allow if {
input.scope == utils.LIST
utils.is_sandbox
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/server.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import data.utils
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
29 changes: 1 addition & 28 deletions cvat/apps/engine/rules/tasks.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down Expand Up @@ -93,19 +93,6 @@ allow if {
organizations.has_perm(organizations.SUPERVISOR)
}

allow if {
input.scope in {utils.CREATE, utils.IMPORT_BACKUP}
utils.is_sandbox
utils.has_perm(utils.BUSINESS)
}

allow if {
input.scope in {utils.CREATE, utils.IMPORT_BACKUP}
input.auth.organization.id == input.resource.organization.id
utils.has_perm(utils.BUSINESS)
organizations.has_perm(organizations.SUPERVISOR)
}

allow if {
input.scope == utils.CREATE_IN_PROJECT
utils.is_sandbox
Expand All @@ -128,20 +115,6 @@ allow if {
is_project_staff
}

allow if {
input.scope == utils.CREATE_IN_PROJECT
utils.is_sandbox
utils.has_perm(utils.BUSINESS)
is_project_staff
}

allow if {
input.scope == utils.CREATE_IN_PROJECT
input.auth.organization.id == input.resource.organization.id
utils.has_perm(utils.BUSINESS)
organizations.has_perm(organizations.SUPERVISOR)
}

allow if {
input.scope == utils.LIST
utils.is_sandbox
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/annotationguides_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def read_rules(name):
"job:assignee",
"none",
]
GROUPS = ["admin", "business", "user", "worker"]
GROUPS = ["admin", "user", "worker"]
Eldies marked this conversation as resolved.
Show resolved Hide resolved
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]
SAME_ORG = [True, False]

Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/cloudstorages_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def read_rules(name):
SCOPES = {rule["scope"] for rule in simple_rules}
CONTEXTS = ["sandbox", "organization"]
OWNERSHIPS = ["owner", "none"]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]
SAME_ORG = [False, True]

Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/comments_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def read_rules(name):
"owner",
"none",
]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]
SAME_ORG = [True, False]
HAS_PROJ = [True, False]
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/issues_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def read_rules(name):
"assignee",
"none",
]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]
SAME_ORG = [True, False]
HAS_PROJ = [True, False]
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/jobs_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def read_rules(name):
"assignee",
"none",
]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]
SAME_ORG = [True, False]

Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/projects_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def read_rules(name):
SCOPES = {rule["scope"] for rule in simple_rules}
CONTEXTS = ["sandbox", "organization"]
OWNERSHIPS = ["owner", "assignee", "none"]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]
SAME_ORG = [False, True]

Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/server_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def read_rules(name):
SCOPES = {rule["scope"] for rule in simple_rules}
CONTEXTS = ["sandbox", "organization"]
OWNERSHIPS = ["none"]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]


Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/tasks_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def read_rules(name):
SCOPES = list({rule["scope"] for rule in simple_rules})
CONTEXTS = ["sandbox", "organization"]
OWNERSHIPS = ["project:owner", "project:assignee", "owner", "assignee", "none"]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]
SAME_ORG = [True, False]

Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/tests/generators/users_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def read_rules(name):
SCOPES = {rule["scope"] for rule in simple_rules}
CONTEXTS = ["sandbox", "organization"]
OWNERSHIPS = ["self", "none"]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
Eldies marked this conversation as resolved.
Show resolved Hide resolved
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]


Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/engine/rules/users.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
3 changes: 1 addition & 2 deletions cvat/apps/engine/tests/test_rest_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@

def create_db_users(cls):
(group_admin, _) = Group.objects.get_or_create(name="admin")
(group_business, _) = Group.objects.get_or_create(name="business")
(group_user, _) = Group.objects.get_or_create(name="user")
(group_annotator, _) = Group.objects.get_or_create(name="worker")
(group_somebody, _) = Group.objects.get_or_create(name="somebody")
Expand All @@ -63,7 +62,7 @@ def create_db_users(cls):
password="admin")
user_admin.groups.add(group_admin)
user_owner = User.objects.create_user(username="user1", password="user1")
user_owner.groups.add(group_business)
user_owner.groups.add(group_user)
user_assignee = User.objects.create_user(username="user2", password="user2")
user_assignee.groups.add(group_annotator)
user_annotator = User.objects.create_user(username="user3", password="user3")
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/events/rules/events.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/events/rules/tests/generators/events_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def read_rules(name):
SCOPES = list({rule["scope"] for rule in simple_rules})
CONTEXTS = ["sandbox", "organization"]
OWNERSHIPS = ["none"]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]
SAME_ORG = [True, False]

Expand Down
6 changes: 0 additions & 6 deletions cvat/apps/iam/rules/utils.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import rego.v1

# Groups
ADMIN := "admin"
BUSINESS := "business"
USER := "user"
WORKER := "worker"

Expand Down Expand Up @@ -65,7 +64,6 @@ UPDATE_VALIDATION_LAYOUT := "update:validation_layout"

get_priority(privilege) := {
ADMIN: 0,
BUSINESS: 50,
USER: 75,
WORKER: 100,
null: 1000
Expand All @@ -79,10 +77,6 @@ is_admin if {
input.auth.user.privilege == ADMIN
}

is_business if {
input.auth.user.privilege == BUSINESS
}

is_user if {
input.auth.user.privilege == USER
}
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/lambda_manager/rules/lambda.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import data.organizations
# "auth": {
# "user": {
# "id": <num>,
# "privilege": <"admin"|"business"|"user"|"worker"> or null
# "privilege": <"admin"|"user"|"worker"> or null
# },
# "organization": {
# "id": <num>,
Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/lambda_manager/rules/tests/generators/lambda_test.gen.rego.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def read_rules(name):
SCOPES = list({rule["scope"] for rule in simple_rules})
CONTEXTS = ["sandbox", "organization"]
OWNERSHIPS = ["none"]
GROUPS = ["admin", "business", "user", "worker", "none"]
GROUPS = ["admin", "user", "worker", "none"]
ORG_ROLES = ["owner", "maintainer", "supervisor", "worker", None]


Expand Down
2 changes: 1 addition & 1 deletion cvat/apps/lambda_manager/tests/test_lambda.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def _invoke_function(self, func, payload):
@classmethod
def _create_db_users(cls):
(group_admin, _) = Group.objects.get_or_create(name="admin")
(group_user, _) = Group.objects.get_or_create(name="business")
(group_user, _) = Group.objects.get_or_create(name="user")

user_admin = User.objects.create_superuser(username="admin", email="",
password="admin")
Expand Down
4 changes: 1 addition & 3 deletions cvat/apps/log_viewer/permissions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,4 @@ def get_scopes(request, view, obj):
}[view.action]]

def get_resource(self):
return {
'visibility': 'public' if settings.RESTRICTIONS['analytics_visibility'] else 'private',
}
return None
Loading
Loading