-
Notifications
You must be signed in to change notification settings - Fork 2.1k
fix: The document list status filtering is incorrect. #1822
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
Conversation
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
).filter(task_type_status__in=[State.PENDING.value, State.STARTED.value]).filter( | ||
document_id=document_id).values('id'), | ||
TaskType(instance.get('type')), | ||
State.REVOKE) | ||
ListenerManagement.update_status(QuerySet(Document).annotate( | ||
reversed_status=Reverse('status'), | ||
task_type_status=Substr('reversed_status', TaskType(instance.get('type')).value, | ||
TaskType(instance.get('type')).value), | ||
1), | ||
).filter(task_type_status__in=[State.PENDING.value, State.STARTED.value]).filter( | ||
id=document_id).values('id'), | ||
TaskType(instance.get('type')), |
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.
在上述代码中,存在以下几个需要检查和优化的地方:
-
get_request_body_api
方法中的items
部分:'source_url_list': openapi.Schema(type=openapi.TYPE_ARRAY, title="文档地址列表", description="文档地址列表", items=openapi.Schema(type=openapi.TYPE_STRING))
这里定义了
source_url_list
字段为数组类型,并且每个元素是字符串类型。虽然当前这样定义没有问题,但在未来维护或扩展时,可能有必要考虑是否需要限制单个URL的最大长度或其他验证规则。 -
多个类中重复的部分:
class DocumentInstanceSerializer(ApiMixin, serializers.ModelSerializer): ... class AnnotationInstanceSerializer(serializers.ModelSerializer): ... ...
在不同地方使用
serializers.ModelSerializer
比较频繁,可以考虑提取一个通用的基类来避免冗余代码。 -
DocumentInstanceSerializer
中的数据验证逻辑:for item in validated_data.get('annotation_type_list'): if item not in set(annotation_type_options) + {'其他'}: raise ValidationError(f"Invalid annotation type: {item}")
这部分代码通过将允许的注解类型与
'其他'
合并到同一个集合内进行快速查找。如果需要更复杂的过滤条件或更多的数据验证,可以适当调整这个方法。 -
cancel
方法中的查询设置:ListenerManagement.update_status(QuerySet(Paragraph).annotate( ... ), TaskType(instance.get('type')), State.REVOKE) ListenerManagement.update_status(QuerySet(Document).annotate( ... ), TaskType(instance.get('type')), State.REVOKE)
可以考虑将这些相同的操作合并成一个函数,以便于重用和调试。
-
API 文档字段信息:
确保所有 API 的文档字段描述(如@staticmethod get_request_body_api()
)准确无误,并提供适当的错误消息。
以下是针对以上发现的一些建议性修改:
@@ -143,20 +143,22 @@ def get_request_params_api():
required=True,
description='知识库id'),
@staticmethod
def get_request_body_api():
return openapi.Schema(
type=openapi.TYPE_OBJECT,
required=['source_url_list'],
+ properties={
'source_url_list': openapi.Schema(type=openapi.TYPE_ARRAY, title="文档地址列表", description="文档地址列表",
- items=openapi.Schema(type=openapi.TYPE_STRING)),
+ items=openapi.Schema(type=openapi.TYPE_STRING, min_length=1, max_length=255)), # 添加最小、最大长度限制
'selector': openapi.Schema(type=openapi.TYPE_STRING, title="选择器", description="选择器")
}
)
@@ -677,16 +678,17 @@ def cancel(self, instance, with_valid=True):
).values('id')
else:
if status != State.SUCCESS.value:
query_set = query_set.filter(status__icontains=status)
+ document_id = QuerySet(Document).annotate(reversed_status=Reverse('status')).get(id=instance['document_id']).id
+
ListenerManagement.update_status(
QuerySet(Paragraph).annotate(
reversed_status=Reverse('status'),
- task_type_status=Substr('reversed_status', TaskType(instance.get('type')).value,
- TaskType(instance.get('type')).value),
+ task_type_status=Substr('reversed_status', TaskType.instance(instance.get('type')).value, 1), # 使用实例获取值
).filter(task_type_status__in=[State.PENDING.value, State.STARTED.value]),
TaskType(instance.get('type')),
State.REVOKE)
ListenerManagement.update_status(
QuerySet(Document).annotate(
reversed_status=Reverse('status'),
- task_type_status=Substr('reversed_status', TaskType(instance.get('type')).value,
- TaskType(instance.get('type')).value),
+ task_type_status=Substr('reversed_status', TaskType.instance(instance.get('type')).value, 1), # 使用实例获取值
).filter(task_type_status__in=[State.FINISHED.value, State.CANCELED.value]).
values('id')))
通过这些建议性的修改,可以使代码更加清晰、易于维护和验证。
@@ -136,7 +136,7 @@ | |||
" | |||
class="justify-center" | |||
:command="beforeCommand('status', State.STARTED, TaskType.GENERATE_PROBLEM)" | |||
>生成问题中</el-dropdown-item | |||
>生成中</el-dropdown-item | |||
> | |||
</el-dropdown-menu> | |||
</template> |
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.
在修改 text
属性时,从 "生成问题中"
改为 "生成中"
是一个较小的调整,但需要注意的是,在某些情况下,这可能会导致代码逻辑和用户界面显示上的不同体验。
推荐的做法是统一使用单字母缩写或更简洁的文本表达方法,如 "生"
. 这可以提高代码的可读性和一致性。同时,确保所有相同用途的按钮和菜单项都使用相同的格式,以便于维护和理解。
(cherry picked from commit 36f1a3b)
fix: The document list status filtering is incorrect.