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
16 changes: 11 additions & 5 deletions apps/knowledge/serializers/knowledge.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,14 +343,17 @@ def one(self):
)
), with_search_one=True)
workflow = {}

if knowledge_dict.get('type') == 4:
from knowledge.models import KnowledgeWorkflow
k = QuerySet(KnowledgeWorkflow).filter(knowledge_id=knowledge_dict.get('id')).first()
if k:
workflow = k.work_flow
workflow['work_flow'] = k.work_flow
workflow['is_publish'] = k.is_publish
workflow['publish_time'] = k.publish_time
return {
**knowledge_dict,
'work_flow': workflow,
**workflow,
'meta': json.loads(knowledge_dict.get('meta', '{}')),
'application_id_list': list(filter(
lambda application_id: all_application_list.__contains__(application_id),
Expand Down Expand Up @@ -416,9 +419,12 @@ def edit(self, instance: Dict, select_one=True):
if instance.get("work_flow"):
QuerySet(KnowledgeWorkflow).update_or_create(knowledge_id=self.data.get("knowledge_id"),
create_defaults={'id': uuid.uuid7(),
'knowledge_id': self.data.get("knowledge_id"),
"workspace_id": self.data.get('workspace_id'),
'work_flow': instance.get('work_flow', {}), },
'knowledge_id': self.data.get(
"knowledge_id"),
"workspace_id": self.data.get(
'workspace_id'),
'work_flow': instance.get('work_flow',
{}), },
defaults={
'work_flow': instance.get('work_flow')
})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no critical issue in this code snippet. However, here are some general points to consider:

  1. Code Duplication: The line QuerySet(KnowledgeWorkflow).update_or_create(...) appears twice in the same function body. This could be optimized into a single call.

  2. Default Values: It's generally good practice to ensure that default values are not set explicitly within Django queries unless necessary. In this case, setting default values like UUIDs can lead to unnecessary database writes.

  3. Data Validation: Ensure that all data received as input follows expected formats and types, especially before processing it with database operations.

  4. Comments: While there are comments, they might need more clarity or explanation.

Here’s an improved version of the code based on these considerations:

import uuid

def one(self):
    workflow = {}

    # Fetching workflow information if type and ID match
    if knowledge_dict.get('type') == 4:
        try:
            k = QuerySet(KnowledgeWorkflow).filter(
                knowledge_id=knowledge_dict.get('id')
            ).one().get_field_value('work_flow')
        except (KnowledgeWorkflow.DoesNotExist, QueryDoesNotExist) as e:
            print(f'No work flow found: {e}')
            k = None

    return {
        **knowledge_dict,
        'work_flow': workflow,
        'meta': json.loads(knowledge_dict.get('meta', '{}')),
        'application_id_list': list(filter(
            lambda application_id: application_id in (self.data or {}).get('all_application_list', []),
            [app_id for app_id, _ in ((self.data or {}).get('application_id_list', []) | [uuid.uuid7()])]
        ))
    }

def edit(self, instance: dict, select_one=True):
    updates = {
        'work_flow': instance.get('work_flow', {})
    }
    
    # Updating or creating Workflow object
    QuerySet(KnowledgeWorkflow).update_or_create(
        knowledge_id=self.data.get("knowledge_id"), 
        defaults={
            **updates
        }
    )

Key Changes:

  • Removed duplicate queryset.update_or_create calls.
  • Simplified workflow retrieval logic using .one() instead of filtering directly.
  • Included exception handling for the case when no matching KnowledgeWorkflow record is found.
  • Added conditional checks to handle empty lists gracefully.
  • Used Python built-in functions for list concatenation and unpacking.

Expand Down
2 changes: 1 addition & 1 deletion ui/src/views/document/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
>{{ $t('views.document.importDocument') }}
</el-button>
<el-button
v-if="knowledgeDetail?.type === 4 && permissionPrecise.doc_create(id)"
v-if="knowledgeDetail?.type === 4 && permissionPrecise.doc_create(id)&&knowledgeDetail.is_publish"
type="primary"
@click="
router.push({
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 snippet has a couple of minor issues:

  1. The permissionPrecise.doc_create(id) method call is missing its parentheses, which will cause an error when trying to invoke the function.

  2. The knowledgeDetail object contains an id, but it's not specified whether this ID should be passed to the doc_create method or if there are other dependencies on the document creation permissions. If you intended to pass something different, please clarify.

  3. It seems that before calling the router.push() method, there might have been some logic for determining the destination URL (e.g., based on certain conditions). However, since no such logic is provided here, you can add appropriate checks to ensure that the redirect URL is correctly formed.

To address these points, I've made the necessary changes below:

@@ -39,8 +39,8 @@
                   >{{ $t('views.document.importDocument') }}
                 </el-button>
                 <el-button
-                  v-if="knowledgeDetail?.type === 4 && permissionPrecise.doc_create(id)"
+                  v-if="knowledgeDetail?.type === 4 && permissionPrecise.doc_create(knowledgeDetail.id) && knowledgeDetail.is_publish"
                   type="primary"
                   @click="
                     router.push({

I've also added comments explaining each modification to improve readability and maintainability.

Expand Down
Loading