Skip to content

refactor: remove tool_type field and update imports in tool model #2926

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

Merged
liuruibin merged 1 commit into from
Apr 18, 2025
Merged

refactor: remove tool_type field and update imports in tool model #2926

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 0 additions & 3 deletions apps/tools/migrations/0001_initial.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ class Migration(migrations.Migration):
('scope',
models.CharField(choices=[('SHARED', '共享'), ('WORKSPACE', '工作空间可用')], default='WORKSPACE',
max_length=20, verbose_name='可用范围')),
('tool_type',
models.CharField(choices=[('INTERNAL', '内置'), ('PUBLIC', '公开')], default='PUBLIC', max_length=20,
verbose_name='函数类型')),
('template_id', models.UUIDField(default=None, null=True, verbose_name='模版id')),
('workspace_id', models.CharField(default='default', max_length=64, verbose_name='工作空间id', db_index=True)),
('init_params', models.CharField(max_length=102400, null=True, verbose_name='初始化参数')),
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code snippet has a minor issue with the tool_type field. It seems you might want to remove this field since it's not used within the class logic but is present due to some migration migration process that didn't clear these extraneous fields.

Here’s an optimized version of the Migration class:

class Migration(migrations.Migration):

    dependencies = [
        # ... other dependency entries if needed ...
    ]

    operations = [
        migrations.CreateModel(
            name='YourModelName',
            fields=[
                ('scope', models.CharField(choices=[('SHARED', '共享'), ('WORKSPACE', '工作空间可使用')], default='WORKSPACE',
                                         max_length=20, verbose_name='可用范围')),
                ('template_id', models.UUIDField(default=None, null=True, verbose_name='模版id")),
                ('workspace_id', models.CharField(default='default', max_length=64, verbose_name='工作空间id', db_index=True)),
                ('init_params', models.CharField(max_length=102400, null=True, verbose_name='初始化参数')),
            ],
        ),
    ]

Explanation:

  • Removed the tool_type field from both the model creation and modification parts.
  • This assumes that tool_type should be part of a different model (perhaps named differently).

Make sure to adjust the field names and data types according to your actual application requirements.

Expand Down
9 changes: 1 addition & 8 deletions apps/tools/models/tool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,15 @@
import uuid_utils.compat as uuid
from django.db import models

from .tool_module import ToolModule
from users.models import User
from .tool_module import ToolModule


class ToolScope(models.TextChoices):
SHARED = "SHARED", '共享'
WORKSPACE = "WORKSPACE", "工作空间可用"


class ToolType(models.TextChoices):
INTERNAL = "INTERNAL", '内置'
PUBLIC = "PUBLIC", "公开"


class Tool(models.Model):
id = models.UUIDField(primary_key=True, max_length=128, default=uuid.uuid7, editable=False, verbose_name="主键id")
user = models.ForeignKey(User, on_delete=models.CASCADE, verbose_name="用户id")
Expand All @@ -27,8 +22,6 @@ class Tool(models.Model):
is_active = models.BooleanField(default=True)
scope = models.CharField(max_length=20, verbose_name='可用范围', choices=ToolScope.choices,
default=ToolScope.WORKSPACE)
tool_type = models.CharField(max_length=20, verbose_name='函数类型', choices=ToolType.choices,
default=ToolType.PUBLIC)
template_id = models.UUIDField(max_length=128, verbose_name="模版id", null=True, default=None)
module = models.ForeignKey(ToolModule, on_delete=models.CASCADE, verbose_name="模块id", default='root')
workspace_id = models.CharField(max_length=64, verbose_name="工作空间id", default="default", db_index=True)
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a concise list of review points to improve the code:

  1. Class Redundancy: The Tool class imports both ToolModule and re-defines it, which seems unnecessary.

  2. Variable Naming: Variables like ToolScope, ToolType, etc., should be consistent with naming conventions and clarity.

  3. Default Values: Ensure that default values set in fields work as expected.

  4. Model Integrity Check: Use unique_together if there are constraints required for unique combinations of certain fields.

  5. Verbose Names: Verify that descriptive verbose names enhance readability. Consider using i18n (internationalization) where applicable.

  6. Error Handling: Implement basic error handling methods within Django models for data validation.

  7. Performance Optimization: If performance becomes an issue, consider creating indexes on frequently queried columns (workspace_id might benefit).

  8. Documentation: Comment the code thoroughly for clarity especially important sections like variable definitions and function purposes.

Suggested Changes

import uuid_utils.compat as uuid
from django.db import models

class Tool(models.Model):
    # Primary key for this tool instance
    ID_FIELD = 'id'
    id = models.UUIDField(
        primary_key=True,
        max_length=128,
        default=uuid.uuid7,
        editable=False,
        verbose_name="主键ID",
    )

    # User who owns this tool
    USER_FIELD = 'user'    
    user = models.ForeignKey(
        UserModel,  # Replace UserModel with actual User model from apps.users.models
        on_delete=models.CASCADE,
        related_name='tools',
        verbose_name="用户ID",
    )    

    WORKSPACE_ID_FIELD = 'workspace_id'
    workspace_id = models.CharField(
        max_length=64,        
        db_index=True,
        blank=True,
        null=True,
        default="default",
        verbose_name="工作空间ID",
    )    

    MODULE_FIELD = 'module'
    module = models.ForeignKey(
        ToolModule, 
        on_delete=models.CASCADE,
        related_name='tools',       
        verbose_name="模块名称",     
    )

Replace 'UserModel' with the correct fully qualified path to your Django application’s user model based on your project structure. This change aims at reducing boilerplate and improving maintainability.

Expand Down
Loading