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.

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

Merged
merged 16 commits into from
Oct 31, 2024
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
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
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

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
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
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/rules/annotationguides.rego
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
31 changes: 31 additions & 0 deletions cvat/apps/iam/migrations/0001_remove_business_group.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Generated by Django 4.2.16 on 2024-10-30 12:03
from django.conf import settings
from django.db import migrations


BUSINESS_GROUP_NAME = "business"
USER_GROUP_NAME = "user"


def delete_business_group(apps, schema_editor):
Group = apps.get_model('auth', 'Group')
User = apps.get_model(settings.AUTH_USER_MODEL)

if user_group := Group.objects.filter(name=USER_GROUP_NAME).first():
user_group.user_set.add(*User.objects.filter(groups__name=BUSINESS_GROUP_NAME))

Group.objects.filter(name=BUSINESS_GROUP_NAME).delete()


class Migration(migrations.Migration):

dependencies = [
migrations.swappable_dependency(settings.AUTH_USER_MODEL),
]

operations = [
migrations.RunPython(
delete_business_group,
reverse_code=migrations.RunPython.noop,
),
]
3 changes: 3 additions & 0 deletions cvat/apps/iam/migrations/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Copyright (C) 2024 CVAT.ai Corporation
#
# SPDX-License-Identifier: MIT
6 changes: 0 additions & 6 deletions cvat/apps/iam/rules/utils.rego
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
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
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
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
Loading
Loading