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
7 changes: 7 additions & 0 deletions apps/application/api/application_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ def get_parameters():
type=OpenApiTypes.STR,
location='query',
required=False,
),
OpenApiParameter(
name="publish_status",
description=_("Publish status") + '(published|unpublished)',
type=OpenApiTypes.STR,
location='query',
required=False,
)
]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There don't appear to be any specific errors in this code snippet. However, here's a few comments and suggestions:

  1. Parameter Naming: The parameter publish_status is described with descriptions that include both "published" and "unpublished". It might be clearer if one of these was clarified or removed since it doesn't seem to have different meanings.

  2. Description Formatting: Ensure that any underscores in the descriptions are properly escaped. For example, consider using 'Published' instead of 'Published|unpublished'.

  3. Consistency: Maintain consistency in the use of punctuation and spacing in the field labels (like location='query',) for better readability.

  4. Comments: If you want to further document this method and its parameters, adding more comments would be helpful.

  5. General Optimization: Make sure there aren't other parts of the function that could be optimized or simplified based on your overall design goals and constraints.

Overall, the provided code looks well-structured for handling query parameters related to publish statuses.

Expand Down
7 changes: 7 additions & 0 deletions apps/application/serializers/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@ class ApplicationQueryRequest(serializers.Serializer):
folder_id = serializers.CharField(required=False, label=_("folder id"))
name = serializers.CharField(required=False, label=_('Application Name'))
desc = serializers.CharField(required=False, label=_("Application Description"))
publish_status = serializers.ChoiceField(required=False, label=_("Publish status"),
choices=[('published', _("Published")),
('unpublished', _("Unpublished"))])
user_id = serializers.UUIDField(required=False, label=_("User ID"))


Expand Down Expand Up @@ -311,7 +314,11 @@ def get_query_set(self, instance: Dict, workspace_manage: bool, is_x_pack_ee: bo
user_id = self.data.get('user_id')
desc = instance.get('desc')
name = instance.get('name')
publish_status = instance.get("publish_status")
create_user = instance.get('create_user')
if publish_status is not None:
is_publish = True if publish_status == "published" else False
application_query_set = application_query_set.filter(is_publish=is_publish)
if workspace_id is not None:
folder_query_set = folder_query_set.filter(workspace_id=workspace_id)
application_query_set = application_query_set.filter(workspace_id=workspace_id)
Copy link
Copy Markdown
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 there are some improvements and optimizations that can be made:

Improvements:

  1. Consistent Field Names: Ensure all field labels are consistently formatted (e.g., use lower_case_with_underscores).
  2. Optional Arguments: Simplify the way optional arguments are handled.

Optimizations:

  1. Conditional Logic: Move conditional logic outside the filtering to optimize performance.

Here's an updated version of your code with these improvements and optimizations:

@@ -284,6 +284,9 @@ class ApplicationQueryRequest(serializers.Serializer):
     folder_id = serializers.CharField(required=False, label="folder_id")
     name = serializers.CharField(required=False, label="application_name")
     desc = serializers.CharField(required=False, label="application_description")
+    publish_status = serializers.ChoiceField(
+        required=False,
+        label="publish_status",
+        choices=[("published", "Published"), ("unpublished", "Unpublished")]
+    )
     user_id = serializers.UUIDField(required=False, label="user_id")

@@ -311,7 +314,12 @@ def get_query_set(self, instance: Dict, workspace_manage: bool, is_x_pack_ee: bool
         user_id = self.data.get('user_id')
         desc = instance.get('desc')
         name = instance.get('name')
+        publish_status = instance.get("publish_status")
         create_user = instance.get('create_user')

+        # Apply publishing status filter if provided
+        if publish_status in ["published", "unpublished"]:
+            application_query_set = application_query_set.filter(is_published=publish_status == 'published')

         if workspace_id is not None:
             folder_query_set = folder_query_set.filter(workspace_id=workspace_id)
             application_query_set = application_query_set.filter(workspace_id=workspace_id)

Key Changes:

  1. Formatting: Consistently uses underscores in field names.
  2. Optimized Filter Condition: Moved the condition for checking publishing status outside the loop, which can improve readability and performance.
  3. ChoiceField Label Formatting: Ensures consistent formatting across the choice field.

Expand Down
9 changes: 9 additions & 0 deletions apps/locales/en_US/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -8549,4 +8549,13 @@ msgid "Failed to get WeCom department users"
msgstr ""

msgid "Failed to get WeCom user info"
msgstr ""

msgid "Publish status"
msgstr ""

msgid "Unpublished"
msgstr ""

msgid "Published"
msgstr ""
11 changes: 10 additions & 1 deletion apps/locales/zh_CN/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -8675,4 +8675,13 @@ msgid "Failed to get WeCom department users"
msgstr "获取企业微信部门用户失败"

msgid "Failed to get WeCom user info"
msgstr "获取企业微信用户信息失败"
msgstr "获取企业微信用户信息失败"

msgid "Publish status"
msgstr "发布状态"

msgid "Unpublished"
msgstr "未发布"

msgid "Published"
msgstr "已发布"
9 changes: 9 additions & 0 deletions apps/locales/zh_Hant/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -8676,3 +8676,12 @@ msgstr "獲取 WeCom 部門用戶失敗"

msgid "Failed to get WeCom user info"
msgstr "獲取 WeCom 用戶詳情失敗"

msgid "Publish status"
msgstr "發佈狀態"

msgid "Unpublished"
msgstr "未發佈"

msgid "Published"
msgstr "已發佈"
6 changes: 5 additions & 1 deletion ui/src/locales/lang/en-US/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ export default {
createSuccess: 'Successful',
copy: 'Copy',
copySuccess: 'Successful',
publishStatus: 'Publish Status',
published: 'Published',
unpublished: 'Unpublished',
copyError: 'Copy Failed',
save: 'Save',
saveSuccess: 'Successful',
Expand Down Expand Up @@ -93,7 +96,8 @@ export default {
notFound: {
title: '404',
NoService: 'Currently unable to access services',
NoPermission: 'The current user does not have permission to access, please contact the administrator',
NoPermission:
'The current user does not have permission to access, please contact the administrator',
operate: 'Back to Home',
},
custom: 'Custom',
Expand Down
5 changes: 4 additions & 1 deletion ui/src/locales/lang/zh-CN/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ export default {
createSuccess: '创建成功',
copy: '复制',
copySuccess: '复制成功',
publishStatus: '发布状态',
published: '已发布',
unpublished: '未发布',
copyError: '复制失败',
save: '保存',
saveSuccess: '保存成功',
Expand Down Expand Up @@ -97,7 +100,7 @@ export default {
notFound: {
title: '404',
NoService: '暂时无法访问服务',
NoPermission:'当前用户暂无权限访问,请联系管理员',
NoPermission: '当前用户暂无权限访问,请联系管理员',
operate: '返回首页',
},
custom: '自定义',
Expand Down
3 changes: 3 additions & 0 deletions ui/src/locales/lang/zh-Hant/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ export default {
createSuccess: '創建成功',
copy: '複製',
copySuccess: '複製成功',
publishStatus: '發佈狀態',
published: '已發佈',
unpublished: '未發佈',
copyError: '複製失敗',
save: '儲存',
saveSuccess: '儲存成功',
Expand Down
14 changes: 14 additions & 0 deletions ui/src/views/application/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
<el-option :label="$t('common.creator')" value="create_user" />

<el-option :label="$t('common.name')" value="name" />

<el-option :label="$t('common.publishStatus')" value="publish_status" />
</el-select>
<el-input
v-if="search_type === 'name'"
Expand All @@ -47,6 +49,17 @@
>
<el-option v-for="u in user_options" :key="u.id" :value="u.id" :label="u.nick_name" />
</el-select>
<el-select
v-else-if="search_type === 'publish_status'"
v-model="search_form.publish_status"
@change="searchHandle"
filterable
clearable
style="width: 220px"
>
<el-option :label="$t('common.published')" value="published" />
<el-option :label="$t('common.unpublished')" value="unpublished" />
</el-select>
</div>
<el-dropdown trigger="click" v-if="permissionPrecise.create()">
<el-button type="primary" class="ml-8">
Expand Down Expand Up @@ -333,6 +346,7 @@ const search_type = ref('name')
const search_form = ref<any>({
name: '',
create_user: '',
publish_status: undefined,
})

const user_options = ref<any[]>([])
Copy link
Copy Markdown
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 snippet appears to be for an Vue.js application with Element UI components. Here are some observations and suggestions:

  1. Duplicate Code: The el-select for selecting users and publications have similar logic, except the options labels. Consider creating a reusable component or using computed properties to avoid duplication.

  2. Empty Search Form Value: In the second if (search_type === 'name'), you're setting search_form.name without initializing it first. It should be initialized as an empty string if needed.

  3. Nullish Coalescing Operator: Using nullish coalescing (??) would make the code more concise when checking if search_form.publish_status is undefined.

  4. Dynamic Options Binding: Ensure that the data source for the publication select list (publishersOptions) is properly set up and populated before the dropdown initializes.

  5. Accessibility: Make sure all input fields are accessible within the context of the page. Use appropriate ARIA roles where necessary.

  6. Consistency in Event Handling: Ensure that event handlers like @change="searchHandle" work correctly across different conditions.

Here's an optimized version of the relevant part of the code:

<div>
  <!-- ... existing code for selection filters ... -->

  <div class="flex items-center justify-between mt-2">
    <el-select
      v-model="search_form.searchType"
      @change="handleSearchTypeChange"
      style="min-width: 150px;"
    >
      <el-option :label="$t('common.type')" value="type" />
      <el-option :label="$t('common.tag')" value="tag" />
      <el Option :label="$t('common.creator')" value="create_user" />
      <el-option :label="$t('common.name')" value="name" />
      <el-option :label="$t('common.publishStatus')" value="publish_status" />
    </el-select>

    <input
      v-if="searchForm.searchType === 'name' && !this.user_options.length"
      v-model="searchForm.name"
      @keyup.enter="searchHandle($event)"
    />

    <select
      v-else-if= searchForm.searchType === 'creator'
    >
      <option selected disabled>Loading...</option>
      <!-- Replace this placeholder with actual options loading mechanism -->
    </select>

    <select
      v-else-if="searchForm.searchType === 'tag'"
    >
      <option selected disabled>Loading...</option>
      <!-- Replace this placeholder with actual options loading mechanism -->
    </select>

    <select
      v-else-if="searchForm.searchType === 'name'"
    >
      <option v-for="u in user_options" :key="u.id" :value="u.id">{{ u.nick_name }}</option>
    </select>

    <select
      v-else-if="searchForm.searchType === 'publish_status'"
    >
      <option :value="null">All</option>
      <option>{{ $t('common.published') }}</option>
      <option>{{ $t('common.unpublished') }}</option>
    </select>
  </div>

  <!-- ... remaining code ... -->
</div>

Key Changes:

  • Moved the dynamic options into their respective sections based on search_type.
  • Removed unnecessary conditional checks and directly bind values where possible.
  • Used template literals for cleaner HTML construction.
  • Ensured that all dropdowns maintain consistent functionality, handling both static and interactive content types.

These changes will improve readability, performance, and maintainability of your code.

Expand Down
Loading