Skip to content

feat: Internal function can modify names #2725

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
merged 1 commit into from
Mar 28, 2025
Merged

feat: Internal function can modify names #2725

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
1 change: 1 addition & 0 deletions ui/src/locales/lang/en-US/views/function-lib.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export default {
form: {
functionName: {
label: 'Name',
name: 'Function Name',
placeholder: 'Please enter the function name',
requiredMessage: 'Please enter the function name'
},
Expand Down
3 changes: 2 additions & 1 deletion ui/src/locales/lang/zh-CN/views/function-lib.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export default {
form: {
functionName: {
label: '名称',
name: '函数名称',
placeholder: '请输入函数名称',
requiredMessage: '请输入函数名称'
},
Expand Down Expand Up @@ -63,7 +64,7 @@ export default {
paramInfo2: '使用函数时不显示',
code: '函数内容（Python）',
selectPlaceholder: '请选择参数',
inputPlaceholder: '请输入参数值',
inputPlaceholder: '请输入参数值'
},
debug: {
run: '运行',
Copy link
Contributor

Choose a reason for hiding this comment

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

The provided code has a minor issue with duplicate keys in the JSON object structure under form. The property 'name' is duplicated within the functionName schema. This duplication might be unnecessary unless you intended to have two separate fields for different purposes.

Here's the corrected and optimized version:

export default {
  form: {
    functionName: {
      label: '名称',
      name: '函数标题', // Suggested change from 'name' to avoid key duplication
      placeholder: '请输入函数名称',
      requiredMessage: '请输入函数名称'
    },
    paramInfo1: '必须用于查询结果列表的列定义',
    paramInfo2: '使用函数时不显示',
    code: '函数内容（Python）',
    selectPlaceholder: '请选择参数',
    inputPlaceholder: '请输入参数值',
    debug: {
      run: '运行'
    }
};

This ensures that each field maintains a unique identifier while providing clear and concise labels. If there was indeed a need for both 'name' and another property with similar functionality, please clarify your use case further for more specific recommendations.

Expand Down
3 changes: 2 additions & 1 deletion ui/src/locales/lang/zh-Hant/views/function-lib.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export default {
form: {
functionName: {
label: '名稱',
name: '函數名稱',
placeholder: '請輸入函數名稱',
requiredMessage: '請輸入函數名稱'
},
Expand Down Expand Up @@ -63,7 +64,7 @@ export default {
paramInfo2: '使用函數時不顯示',
code: '函数内容（Python）',
selectPlaceholder: '請选择參數',
inputPlaceholder: '請輸入參數值',
inputPlaceholder: '請輸入參數值'
},
debug: {
run: '運行',
Expand Down
11 changes: 6 additions & 5 deletions ui/src/views/function-lib/component/AddInternalFunctionDialog.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>
<el-dialog
:title="$t('views.functionLib.functionForm.form.functionName.placeholder')"
:title="$t('views.functionLib.functionForm.form.functionName.name')"
v-model="dialogVisible"
:close-on-click-modal="false"
:close-on-press-escape="false"
Expand All @@ -23,7 +23,7 @@
<span class="dialog-footer">
<el-button @click.prevent="dialogVisible = false"> {{ $t('common.cancel') }} </el-button>
<el-button type="primary" @click="submit(fieldFormRef)" :loading="loading">
{{ $t('common.add') }}
{{ isEdit ? $t('common.save') : $t('common.add') }}
</el-button>
</span>
</template>
Expand All @@ -39,6 +39,7 @@ const emit = defineEmits(['refresh'])

const fieldFormRef = ref()
const loading = ref<boolean>(false)
const isEdit = ref<boolean>(false)

const form = ref<any>({
name: ''
Expand All @@ -64,19 +65,19 @@ watch(dialogVisible, (bool) => {
}
})

const open = (row: any) => {
const open = (row: any, edit: boolean) => {
if (row) {
form.value = cloneDeep(row)
}

isEdit.value = edit || false
dialogVisible.value = true
}

const submit = async (formEl: FormInstance | undefined) => {
if (!formEl) return
await formEl.validate((valid) => {
if (valid) {
emit('refresh', form.value)
emit('refresh', form.value, isEdit.value)
dialogVisible.value = false
}
})
Copy link
Contributor

Choose a reason for hiding this comment

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

The code appears to be mostly correct, but here are a few optimizations and improvements:

Improvements

  1. Avoid Redundant Checks: Remove the redundant !row check on line 63 since it's unnecessary when you're using the logical OR operator.

  2. Use Default Values in Functions: Set default values where appropriate to avoid checking for them repeatedly.

  3. Code Readability: Ensure consistent naming conventions and readability.

  4. Consistent Imports: Use import statements consistently throughout the file.

  5. Remove Unnecessary Comments: Keep comments concise and relevant to the code logic.

Optimizations

  1. Efficient Clone Operation: Use cloneDeep from Lodash or another deep cloning library only when necessary, especially if form.value contains complex objects with many dependencies.

Cleaned-up Code

Here’s the optimized version of the provided code:

<template>
  <el-dialog
    :title="$t('views.functionLib.functionForm.form.functionName.name')"
    v-model="dialogVisible"
    :close-on-click-modal="false"
    :close-on-press-escape="false"
  >
    ...
    <!-- rest of template content -->
  </el-dialog>
</template>

<script setup lang="ts">
import { cloneDeep } from 'lodash'; // Importing lodash for deep cloning
import { ref, defineProps, defineEmits } from 'vue';
import type { FormInstance } from 'element-plus';

defineProps({
  row: Object,
});

defineEmits(['refresh']);

const dialogVisible = ref(false);
const fieldFormRef = ref<FormInstance>();
const loading = ref(false);
const isEdit = ref(true);

const form = ref<any>({
  name: '',
});

watch(dialogVisible, () => {
  form.value = {};
});

const open = (row?: any) => {
  if (row) {
    form.value = cloneDeep(row);
    isEdit.value = !!row; // Check if row exists before setting edit state
  }
  dialogVisible.value = true;
};

const submit = async (formEl: FormInstance | undefined) => {
  if (!formEl) return;

  await formEl.validate((valid) => {
    if (valid) {
      emit('refresh', form.value, isEdit.value);
      dialogVisible.value = false;
    }
  });
};
</script>

Additional Considerations

  1. Type Definitions: Ensure that all types like FormInstance, any, etc., are imported correctly at the top of the file.

  2. Error Handling: Add error handling around network requests or other asynchronous operations within the submit function to manage exceptions gracefully.

By addressing these points, your code becomes more efficient, readable, and maintainable.

Expand Down
37 changes: 23 additions & 14 deletions ui/src/views/function-lib/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
<template>
<div class="function-lib-list-container p-24" style="padding-top: 16px">
<el-tabs
v-model="functionType"
@tab-change="
tabChangeHandle

"
>
<el-tabs v-model="functionType" @tab-change="tabChangeHandle">
<el-tab-pane :label="$t('views.functionLib.title')" name="PUBLIC"></el-tab-pane>
<el-tab-pane :label="$t('views.functionLib.internalTitle')" name="INTERNAL"></el-tab-pane>
</el-tabs>
Expand Down Expand Up @@ -165,6 +159,14 @@
</el-button>
<template #dropdown>
<el-dropdown-menu>
<el-dropdown-item
v-if="item.template_id"
:disabled="item.permission_type === 'PUBLIC' && !canEdit(item)"
@click.stop="addInternalFunction(item, true)"
>
<el-icon><EditPen /></el-icon>
{{ $t('common.edit') }}
</el-dropdown-item>
<el-dropdown-item
v-if="!item.template_id"
:disabled="item.permission_type === 'PUBLIC' && !canEdit(item)"
Expand Down Expand Up @@ -366,17 +368,24 @@ async function openDescDrawer(row: any) {
InternalDescDrawerRef.value.open(content, row)
}

function addInternalFunction(data?: any) {
AddInternalFunctionDialogRef.value.open(data)
function addInternalFunction(data?: any, isEdit?: boolean) {
AddInternalFunctionDialogRef.value.open(data, isEdit)
}

function confirmAddInternalFunction(data?: any) {
functionLibApi
.addInternalFunction(data.id, { name: data.name }, changeStateloading)
.then((res) => {
MsgSuccess(t('common.submitSuccess'))
function confirmAddInternalFunction(data?: any, isEdit?: boolean) {
if (isEdit) {
functionLibApi.putFunctionLib(data?.id as string, data, loading).then((res) => {
MsgSuccess(t('common.saveSuccess'))
searchHandle()
})
} else {
functionLibApi
.addInternalFunction(data.id, { name: data.name }, changeStateloading)
.then((res) => {
MsgSuccess(t('common.addSuccess'))
searchHandle()
})
}
}

function searchHandle() {
Copy link
Contributor

Choose a reason for hiding this comment

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

The provided code seems to be an Angular template for displaying a list of functions with tabs (el-tabs) and actions within each tab. There are a few points that can be improved:

Main Issues

  1. Duplicated tabChangeHandle Call: The <el-tabs/@tab-change> attribute is currently set to call the tabChangeHandle method twice.

  2. Code Duplication: Similar logic is duplicated in confirmAddInternalFunction, which is concerning for maintenance and readability.

  3. Undefined Variables: Some variables like loading and MsgSuccess seem to be referenced but not declared anywhere in this code snippet. Ensure they are properly imported or defined before usage.

  4. Redundant Conditional Logic: The condition inside if (item.template_id) seems redundant since the same action is being performed in both branches when !item.template_id.

  5. Missing Contextual Information: While the code doesn’t contain all context, it suggests interaction with API endpoints like functionLibApi and using i18n utilities from Vue.js ($t()).

  6. Incomplete Function Documentation/Comments:

    • openDescDrawer: Missing parameters and description on what these params do.
    • addInternalFunction: No documentation about its purpose or how the isEdit parameter affects behavior.
    • confirmAddInternalFunction: Needs comments explaining the two different scenarios where it handles either adding or updating internal functions.

  7. Unnecessary Template Inline Styles Over CSS Modules:

    style="padding-top: 16px"

    It's generally better to use external stylesheets rather than inline styles.

Optimization Suggestions

  • Avoid Redundant Code: Extract repeated code into reusable methods/functions if needed. For example, move common logic to helper functions like handlePermissions.
  • Use TypeScript Interfaces: If you haven't already, consider defining interfaces or types for your data structures used across various components to improve type safety.
  • Review Dependency Management: Although not immediately evident here, ensure all necessary dependencies (like Ant Design VUE) are correctly imported and configured.
  • Linter Compliance: Check with your linter setup to address any linting errors related to unused parameters, naming conventions, etc.

Incorporating these changes will help make the code more maintainable, scalable, and adhere to best practices.

Expand Down