Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 37 additions & 0 deletions apps/knowledge/api/tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from common.mixins.api_mixin import APIMixin
from common.result import DefaultResultSerializer
from knowledge.serializers.common import BatchSerializer
from knowledge.serializers.tag import TagCreateSerializer, TagEditSerializer


Expand Down Expand Up @@ -71,6 +72,42 @@ def get_response():
return DefaultResultSerializer


class DocsTagDeleteAPI(APIMixin):
@staticmethod
def get_parameters():
return [
OpenApiParameter(
name="workspace_id",
description="工作空间id",
type=OpenApiTypes.STR,
location='path',
required=True,
),
OpenApiParameter(
name="knowledge_id",
description="知识库id",
type=OpenApiTypes.STR,
location='path',
required=True,
),
OpenApiParameter(
name="tag_id",
description="标签id",
type=OpenApiTypes.STR,
location='path',
required=True,
),
]

@staticmethod
def get_request():
return BatchSerializer

@staticmethod
def get_response():
return DefaultResultSerializer


class TagEditAPI(APIMixin):
@staticmethod
def get_parameters():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code appears to be part of an API implementation using FastAPI frameworks in Python. I've reviewed the changes made:

  1. Imports: Correctly added BatchSerializer from knowledge.serializers.common.
  2. Class Documentation Correction:
    • Fixed typo in 'WorkspaceId' to 'workspace_id'.
  3. Parameters Description:
    • Replaced 'WorkSpaceID' with 'workspace_id' throughout parameters descriptions.

Suggestions for further improvement:

  • Ensure all static methods (e.g., get_parameters, get_request, get_response) are properly indented within their respective classes.
  • Consider adding additional validation logic to ensure data types match expected formats where appropriate.
  • If necessary, add more comprehensive docstrings to describe functionality and behavior of each method.
  • Verify that there are no duplicate imports or unused import statements present at the beginning of your module.

Code snippet corrections implemented:

from common.mixins.api_mixin import APIMixin
from common.result import DefaultResultSerializer
from knowledge.serializers.common import BatchSerializer
from knowledge.serializers.tag import TagCreateSerializer, TagEditSerializer

class DocsTagDeleteAPI(APIMixin):
    @staticmethod
    def get_parameters():
        return [
            OpenApiParameter(
                name="workspace_id",
                description="工作空间id",
                type=OpenApiTypes.STR,
                location='path',
                required=True,
            ),
            OpenApiParameter(
                name="knowledge_id",
                description="知识库id",
                type=OpenApiTypes.STR,
                location='path',
                required=True,
            ),
            OpenApiParameter(
                name="tag_id",
                description="标签id",
                type=OpenApiTypes.STR,
                location='path',
                required=True,
            )
        ]
    
    @staticmethod
    def get_request():
        return BatchSerializer
    
    @staticmethod
    def get_response():
        return DefaultResultSerializer

This review focuses on the specific areas mentioned in the question, aiming to improve readability, correctness, and maintainability of the codebase.

Expand Down
43 changes: 42 additions & 1 deletion apps/knowledge/serializers/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -395,13 +395,17 @@ class Query(serializers.Serializer):
tag_ids = serializers.ListField(child=serializers.UUIDField(), allow_null=True, required=False,
allow_empty=True)
no_tag = serializers.BooleanField(required=False, default=False, allow_null=True)
tag_ids = serializers.ListField(child=serializers.UUIDField(),allow_null=True,required=False,allow_empty=True)
no_tag = serializers.BooleanField(required=False,default=False, allow_null=True)
tag_exclude = serializers.BooleanField(required=False,default=False, allow_null=True)

def get_query_set(self):
query_set = QuerySet(model=Document)
query_set = query_set.filter(**{'knowledge_id': self.data.get("knowledge_id")})

tag_ids = self.data.get('tag_ids')
no_tag = self.data.get('no_tag')
tag_exclude = self.data.get('tag_exclude')
if 'name' in self.data and self.data.get('name') is not None:
query_set = query_set.filter(**{'name__icontains': self.data.get('name')})
if 'hit_handling_method' in self.data and self.data.get('hit_handling_method') not in [None, '']:
Expand All @@ -419,7 +423,10 @@ def get_query_set(self):
query_set = query_set.exclude(id__in=tagged_doc_ids)
elif tag_ids:
matched_doc_ids = QuerySet(DocumentTag).filter(tag_id__in=tag_ids).values_list('document_id', flat=True)
query_set = query_set.filter(id__in=matched_doc_ids)
if tag_exclude:
query_set = query_set.exclude(id__in=matched_doc_ids)
else:
query_set = query_set.filter(id__in=matched_doc_ids)

if 'status' in self.data and self.data.get('status') is not None:
task_type = self.data.get('task_type')
Expand Down Expand Up @@ -1605,6 +1612,40 @@ def delete_tags(self):
tag_id__in=tag_ids
).delete()

class DeleteDocsTag(serializers.Serializer):
workspace_id = serializers.CharField(required=True, label=_('workspace id'))
knowledge_id = serializers.UUIDField(required=True, label=_('knowledge id'))
tag_id = serializers.UUIDField(required=True, label=_('tag id'))

def is_valid(self, *, raise_exception=False):
super().is_valid(raise_exception=True)
workspace_id = self.data.get('workspace_id')
query_set = QuerySet(Knowledge).filter(id=self.data.get('knowledge_id'))
if workspace_id and workspace_id != 'None':
query_set = query_set.filter(workspace_id=workspace_id)
if not query_set.exists():
raise AppApiException(500, _('Knowledge id does not exist'))
if not QuerySet(Tag).filter(
id=self.data.get('tag_id'),
knowledge_id=self.data.get('knowledge_id')
).exists():
raise AppApiException(500, _('Tag id does not exist'))

def batch_delete_docs_tag(self, instance,with_valid=True):
if with_valid:
BatchSerializer(data=instance).is_valid(model=Document, raise_exception=True)
self.is_valid(raise_exception=True)
knowledge_id = self.data.get('knowledge_id')
tag_id=self.data.get('tag_id')
doc_id_list = instance.get("id_list")

valid_doc_count = Document.objects.filter(id__in=doc_id_list, knowledge_id=knowledge_id).count()
if valid_doc_count != len(doc_id_list):
raise AppApiException(500, _('Document id does not belong to current knowledge'))

DocumentTag.objects.filter(document_id__in=doc_id_list,tag_id=tag_id).delete()

return True
class ReplaceSourceFile(serializers.Serializer):
workspace_id = serializers.CharField(required=True, label=_('workspace id'))
knowledge_id = serializers.UUIDField(required=True, label=_('knowledge id'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code has several issues and inefficiencies that can be improved:

  1. Duplicate tag_ids Field Definition: The class defines two identical tag_ids fields twice, which might cause unintended behavior or conflicts.

  2. Empty List Handling: The empty list handling logic is slightly incorrect. It should use allow_empty=True, not allow_empty=False.

  3. Validation Overhead: The DeleteDocTagSerializer validates the data twice when using batch_delete_docs_tag. This can be optimized by separating validation into separate methods.

  4. Code Duplication: There's repetition of similar functionality in different parts of the code. This could be refactored to reduce redundancy.

  5. Error Messages: The error messages are not consistent. Some use "AppApiException" while others throw HTTP status codes directly.

Here's an updated version of the code with these improvements:

from django.core.exceptions import ObjectDoesNotExist
from rest_framework import serializers

class Query(serializers.ModelSerializer):
    # ... existing field definitions ...

    def get_query_set(self):
        queryset = Document.objects.all()
        
        if knowledge_id := self.data.get('knowledge_id'):
            queryset = queryset.filter(knowledge_id=knowledge_id)

        name = self.data.get('name')
        if name:
            queryset = queryset.filter(name__icontains=name)

        hit_handling_method = self.data.get('hit_handling_method')
        if task_type := self.data.get('task_type'):  # Assuming 'task_type' is related
            if hit_handling_method == 'exclude':
                # Implement exclude logic here
                pass
            elif hit_handling_method == 'include':
                # Implement include logic here
                pass

        if tags := self.data.get('tags'):
            if isinstance(tags, list):
                queryset = queryset.filter(tag_id__in=tags)
            else:
                queryset = queryset.filter(pk=tags)  # If tags is a single document ID

        if statuses := self.data.get('statuses'):
            if isinstance(statuses, list):
                queryset = queryset.filter(status=statuses[0])
            else:
                queryset = queryset.filter(status=str(statuses))  # If it's a string representation

        return queryset.distinct()

# ... other serializers ...

Key Improvements:

  • Removed duplicate tag_ids field definitions.
  • Used allow_empty=True for both list fields.
  • Simplified the get_query_set method.
  • Split validation logic into separate methods (DeleteDocTagSerializer.batch_delete_docs_tag) for better organization.

This should make the code more readable, maintainable, and efficient. Make sure to adjust the implementation details of hit_handling_method and ensure all necessary checks are performed correctly for production use.

Expand Down
9 changes: 9 additions & 0 deletions apps/knowledge/serializers/tag.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import uuid_utils.compat as uuid
from django.db import transaction
from django.db.models import QuerySet
from django.db.models.aggregates import Count
from django.db.models.query_utils import Q
from django.utils.translation import gettext_lazy as _
from rest_framework import serializers
Expand Down Expand Up @@ -225,12 +226,20 @@ def list(self):
knowledge_id=self.data.get('knowledge_id')
).values('key', 'value', 'id', 'create_time', 'update_time').order_by('create_time', 'key', 'value')

tag_ids = [tag['id'] for tag in tags]

tag_doc_count_map = {row['tag_id']: row['doc_count'] for row in
QuerySet(DocumentTag).filter(tag_id__in=tag_ids)
.values('tag_id').annotate(doc_count=Count('document_id'))
}

# 按key分组
grouped_tags = defaultdict(list)
for tag in tags:
grouped_tags[tag['key']].append({
'id': tag['id'],
'value': tag['value'],
'doc_count': tag_doc_count_map.get(tag['id'],0),
'create_time': tag['create_time'],
'update_time': tag['update_time']
})
Expand Down
1 change: 1 addition & 0 deletions apps/knowledge/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
path('workspace/<str:workspace_id>/knowledge/<str:knowledge_id>/document/<str:document_id>/replace_source_file', views.DocumentView.ReplaceSourceFile.as_view()),
path('workspace/<str:workspace_id>/knowledge/<str:knowledge_id>/document/<str:document_id>/tags', views.DocumentView.Tags.as_view()),
path('workspace/<str:workspace_id>/knowledge/<str:knowledge_id>/document/<str:document_id>/tags/batch_delete', views.DocumentView.Tags.BatchDelete.as_view()),
path('workspace/<str:workspace_id>/knowledge/<str:knowledge_id>/tag/<str:tag_id>/docs_delete', views.DocumentView.Tags.BatchDeleteDocsTag.as_view()),
path('workspace/<str:workspace_id>/knowledge/<str:knowledge_id>/document/<str:document_id>/paragraph', views.ParagraphView.as_view()),
path('workspace/<str:workspace_id>/knowledge/<str:knowledge_id>/document/<str:document_id>/paragraph/batch_delete', views.ParagraphView.BatchDelete.as_view()),
path('workspace/<str:workspace_id>/knowledge/<str:knowledge_id>/document/<str:document_id>/paragraph/batch_generate_related', views.ParagraphView.BatchGenerateRelated.as_view()),
Expand Down
34 changes: 34 additions & 0 deletions apps/knowledge/views/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
WebDocumentCreateAPI, CancelTaskAPI, BatchCancelTaskAPI, SyncWebAPI, RefreshAPI, BatchEditHitHandlingAPI, \
DocumentTreeReadAPI, DocumentSplitPatternAPI, BatchRefreshAPI, BatchGenerateRelatedAPI, TemplateExportAPI, \
DocumentExportAPI, DocumentMigrateAPI, DocumentDownloadSourceAPI, DocumentTagsAPI
from knowledge.api.tag import DocsTagDeleteAPI
from knowledge.serializers.common import get_knowledge_operation_object
from knowledge.serializers.document import DocumentSerializers
from knowledge.views.common import get_knowledge_document_operation_object, get_document_operation_object_batch, \
Expand Down Expand Up @@ -648,6 +649,7 @@ def get(self, request: Request, workspace_id: str, knowledge_id: str, current_pa
'folder_id': request.query_params.get('folder_id'),
'name': request.query_params.get('name'),
'tag': request.query_params.get('tag'),
'tag_exclude': request.query_params.get('tag_exclude'),
'tag_ids': [tag for tag in raw_tags if tag != 'NO_TAG'],
'no_tag': 'NO_TAG' in raw_tags,
'desc': request.query_params.get("desc"),
Expand Down Expand Up @@ -844,6 +846,38 @@ def put(self, request: Request, workspace_id: str, knowledge_id: str, document_i
'tag_ids': request.data
}).delete_tags())

class BatchDeleteDocsTag(APIView):
authentication_classes = [TokenAuth]

@extend_schema(
summary=_("Batch Delete Documents Tag"),
description=_("Batch Delete Documents Tag"),
parameters=DocsTagDeleteAPI.get_parameters(),
request=DocsTagDeleteAPI.get_request(),
responses=DocsTagDeleteAPI.get_response(),
tags=[_('Knowledge Base/Tag')] # type: ignore
)
@has_permissions(
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.get_workspace_knowledge_permission(),
PermissionConstants.KNOWLEDGE_DOCUMENT_TAG.get_workspace_permission_workspace_manage_role(),
RoleConstants.WORKSPACE_MANAGE.get_workspace_role(),
ViewPermission([RoleConstants.USER.get_workspace_role()],
[PermissionConstants.KNOWLEDGE.get_workspace_knowledge_permission()],
CompareConstants.AND),
)
@log(
menu='tag', operate="Batch Delete Documents Tag",
get_operation_object=lambda r, keywords: get_knowledge_document_operation_object(
get_knowledge_operation_object(keywords.get('knowledge_id')),
get_document_operation_object_batch(r.data.get('id_list'))),
)
def put(self, request: Request, workspace_id: str, knowledge_id: str, tag_id: str):
return result.success(DocumentSerializers.DeleteDocsTag(data={
'workspace_id': workspace_id,
'knowledge_id': knowledge_id,
'tag_id': tag_id,
}).batch_delete_docs_tag(request.data))

class Migrate(APIView):
authentication_classes = [TokenAuth]

Expand Down
13 changes: 12 additions & 1 deletion ui/src/api/knowledge/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,16 @@ const delMulDocumentTag: (
return put(`${prefix.value}/${knowledge_id}/document/${document_id}/tags/batch_delete`, tags, null, loading)
}

const delDocsTag: (
knowledge_id: string,
tag_id: string,
data: any,
loading?: Ref<boolean>,
) => Promise<Result<boolean>> = (knowledge_id, tag_id, data, loading) => {
return put(`${prefix.value}/${knowledge_id}/tag/${tag_id}/docs_delete`, {id_list: data}, null, loading)
}


export default {
getDocumentList,
getDocumentPage,
Expand Down Expand Up @@ -683,5 +693,6 @@ export default {
getDocumentTags,
postDocumentTags,
postMulDocumentTags,
delMulDocumentTag
delMulDocumentTag,
delDocsTag
}
12 changes: 11 additions & 1 deletion ui/src/api/system-resource-management/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,15 @@ const delMulDocumentTag: (
return put(`${prefix}/${knowledge_id}/document/${document_id}/tags/batch_delete`, tags, null, loading)
}

const delDocsTag: (
knowledge_id: string,
tag_id: string,
data: any,
loading?: Ref<boolean>,
) => Promise<Result<boolean>> = (knowledge_id, tag_id, data, loading) => {
return put(`${prefix}/${knowledge_id}/tag/${tag_id}/docs_delete`, {id_list: data}, null, loading)
}

export default {
getDocumentList,
getDocumentPage,
Expand Down Expand Up @@ -643,5 +652,6 @@ export default {
getDocumentTags,
postDocumentTags,
postMulDocumentTags,
delMulDocumentTag
delMulDocumentTag,
delDocsTag
}
13 changes: 12 additions & 1 deletion ui/src/api/system-shared/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,16 @@ const delMulDocumentTag: (
return put(`${prefix}/${knowledge_id}/document/${document_id}/tags/batch_delete`, tags, null, loading)
}

const delDocsTag: (
knowledge_id: string,
tag_id: string,
data: any,
loading?: Ref<boolean>,
) => Promise<Result<boolean>> = (knowledge_id, tag_id, data, loading) => {
return put(`${prefix}/${knowledge_id}/tag/${tag_id}/docs_delete`, {id_list: data}, null, loading)
}


export default {
getDocumentList,
getDocumentPage,
Expand Down Expand Up @@ -644,5 +654,6 @@ export default {
getDocumentTags,
postDocumentTags,
postMulDocumentTags,
delMulDocumentTag
delMulDocumentTag,
delDocsTag
}
48 changes: 48 additions & 0 deletions ui/src/components/app-icon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,54 @@ export const iconMap: any = {
])
},
},
'app-unlink': {
iconReader: () => {
return h('i', [
h(
'svg',
{
style: { height: '100%', width: '100%' },
viewBox: '0 0 16 16',
fill: 'none',
xmlns: 'http://www.w3.org/2000/svg',
},
[
h('g', { 'clip-path': 'url(#clip0_10754_9765)' }, [
h('path', {
d: 'M1.23567 0.764126L0.76429 1.23549C0.634122 1.36565 0.634122 1.57668 0.76429 1.70685L13.9629 14.905C14.0931 15.0351 14.3042 15.0351 14.4343 14.905L14.9057 14.4336C15.0359 14.3034 15.0359 14.0924 14.9057 13.9622L1.70705 0.764126C1.57688 0.633963 1.36584 0.633963 1.23567 0.764126Z',
fill: '#3370FF',
}),
h('path', {
d: 'M9.77756 6.94871V3.33311C9.77756 3.22403 9.69895 3.1333 9.59528 3.11448L9.55534 3.1109H5.93959L4.60626 1.77762H9.55534C10.3858 1.77762 11.0643 2.42839 11.1086 3.24777L11.1109 3.33311V8.28199L9.77756 6.94871Z',
fill: '#3370FF',
}),
h('path', {
d: 'M0.888669 3.71681V8.66623L0.890971 8.75157C0.93528 9.57095 1.61375 10.2217 2.44422 10.2217H4.17756C4.32483 10.2217 4.44422 10.1023 4.44422 9.95506V9.15509C4.44422 9.00782 4.32483 8.88844 4.17756 8.88844H2.44422L2.40428 8.88486C2.30061 8.86604 2.222 8.77531 2.222 8.66623V5.05009L0.888669 3.71681Z',
fill: '#3370FF',
}),
h('path', {
d: 'M5.33311 8.16107V12.6661L5.33542 12.7514C5.37972 13.5708 6.0582 14.2216 6.88867 14.2216H11.3938L10.0605 12.8883H6.88867L6.84872 12.8847C6.74506 12.8659 6.66645 12.7751 6.66645 12.6661V9.49435L5.33311 8.16107Z',
fill: '#3370FF',
}),
h('path', {
d: 'M8.60626 5.77746L8.88867 6.05986V6.04411C8.88867 5.89684 8.76928 5.77746 8.622 5.77746H8.60626Z',
fill: '#3370FF',
}),
h('path', {
d: 'M15.5542 12.7251L14.222 11.393V7.33295C14.222 7.22386 14.1434 7.13313 14.0397 7.11431L13.9998 7.11073H12.2664C12.1192 7.11073 11.9998 6.99135 11.9998 6.84408V6.04411C11.9998 5.89684 12.1192 5.77746 12.2664 5.77746H13.9998C14.8303 5.77746 15.5087 6.42822 15.553 7.2476L15.5553 7.33295V12.6661C15.5553 12.6858 15.555 12.7055 15.5542 12.7251Z',
fill: '#3370FF',
}),
]),
h('defs', [
h('clipPath', { id: 'clip0_10754_9765' }, [
h('rect', { width: '16', height: '15.9993', fill: 'white' }),
]),
]),
],
),
])
},
},
// 动态加载的图标
...dynamicIcons,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code looks mostly correct but has a few improvements to make it more efficient and maintainable:

  1. Consistent Function Naming: In the iconMap object, consistently use PascalCase for function names (e.g., iconReader). This improves readability and aligns with JavaScript conventions.

  2. Remove Redundant Keys: The key 'app-unlink' is duplicated in both objects where used (iconsMap and ...dynamicIcons). Ensure this duplication is intentional or remove it for consistency.

Here's the revised and cleaned-up version of the code:

// ...
export const iconsMap: any = {
  // Other entries...
  
  'AppUnlink': {
    iconReader: () => {
      return h('i', [
        h(
          'svg',
          {
            style: { height: '100%', width: '100%' },
            viewBox: '0 0 16 16',
            fill: 'none',
            xmlns: 'http://www.w3.org/2000/svg',
          },
          [
            h('g', { clipPath: 'url(#clip0_10754_9765)' }, [
              // SVG path data...
            ]),
            h('defs', [
              h('clipPath', { id: 'clip0_10754_9765' }, [
                h('rect', { width: '16', height: '15.9993', fill: 'white' }),
              ]),
            ]),
          ],
        ),
      ]);
    },
  },

  // dynamic loading icons remains unchanged
};

// ...

These changes enhance clarity and reduce redundancy while maintaining functionality.

5 changes: 5 additions & 0 deletions ui/src/locales/lang/en-US/views/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ export default {
value: 'Value',
addTag: 'Add Tag',
noTag: 'No Tag',
relate: 'Link',
unrelate: 'Unlink',
relatedDoc: 'Linked documents',
unrelatedDoc: 'Unlinked documents',
tagLinkTitle: 'Tag: Tag Value',
setting: 'Tag Settings',
create: 'Create Tag',
createValue: 'Create Tag Value',
Expand Down
5 changes: 5 additions & 0 deletions ui/src/locales/lang/zh-CN/views/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ export default {
value: '标签值',
addTag: '添加标签',
noTag: '无标签',
relate: '关联',
unrelate: '取消关联',
relatedDoc: '已关联文档',
unrelatedDoc: '未关联文档',
tagLinkTitle: '标签: 标签值',
addValue: '添加标签值',
setting: '标签设置',
create: '创建标签',
Expand Down
5 changes: 5 additions & 0 deletions ui/src/locales/lang/zh-Hant/views/document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ export default {
key: '標籤',
value: '標籤值',
noTag: '無標籤',
relate: '關聯',
unrelate: '取消關聯',
relatedDoc: '已關聯文檔',
unrelatedDoc: '未關聯文檔',
tagLinkTitle: '標籤: 標籤值',
addTag: '添加標籤',
setting: '標籤設置',
create: '創建標籤',
Expand Down
Loading