-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Support parameter extraction nodes #4208
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. DetailsInstructions 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-sigs/prow 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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
||
| defineExpose({ open, close }) | ||
| </script> | ||
| <style lang="scss" scoped></style> |
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.
The provided Vue component looks generally well-structured, but there are some considerations for improvement:
-
Dynamic Labels: Consider using
v-bind(or just:in JSX-like syntax) to bind labels dynamically instead of hardcoding them within template literals. -
Error Handling:
- Add more specific error messages for validation errors.
- Provide user feedback when save fails due to network issues or server errors.
-
Optimization:
- Avoid deep cloning the form object unnecessarily if you only need a shallow copy. Use spread operators (
{ ...row }) if needed. - Ensure that the input fields update immediately after key presses or text changes, enhancing用户体验.
- Avoid deep cloning the form object unnecessarily if you only need a shallow copy. Use spread operators (
Here's a refined version with these improvements:
<template>
<el-dialog
:title="isEdit ? $t('common.param.editParam') : $t('common.param.addParam')"
v-model="dialogVisible"
:close-on-click-modal="false"
:close-on-press-escape="false"
:destroy-on-close="true"
:before-close="close"
append-to-body
>
<el-form
label-position="top"
ref="fieldFormRef"
:rules="rules"
:model="form"
require-asterisk-position="right"
>
<el-form-item
:label="getLabel('dynamicsForm.paramForm.field')"
:required="!isRequiredField"
prop="field"
>
<el-input
v-model="form.field"
:maxlength="64"
show-word-limit
:placeholder="$t('dynamicsForm.paramForm.field.placeholder')"
/>
</el-form-item>
<el-form-item
:label="getLabel('dynamicsForm.paramForm.name')"
:required="true"
prop="label"
>
<el-input
v-model="form.label"
:maxlength="64"
show-word-limit
:placeholder="$t('dynamicsForm.paramForm.name.placeholder')"
/>
</el-form-item>
<el-form-item
:label="getSelectLabelText()"
:required="true"
prop="parameter_type"
>
<el-select
v-model="form.parameter_type"
placeholder="@{{ $t('common.selectPlaceholder') }}"
style="width: 100%"
>
<el-option
v-for="item in options"
:key="item.value"
:label="item.label"
:value="item.value"
/>
</el-select>
</el-form-item>
<el-form-item
:label="getLabel('views.applicationWorkflow.nodes.parameterExtractionNode.extractParameters.desc')"
prop="desc"
>
<el-input
v-model="form.desc"
style="width: 100%"
:rows="2"
type="textarea"
:placeholder="
`${
$t('common.inputPlaceholder')
} ${$t('views.applicationWorkflow.nodes.parameterExtractionNode.extractParameters.desc', '描述')}`
"
/>
</el-form-item>
</el-form>
<template #footer>
<span class="dialog-footer">
<el-button @click.prevent="close"> {{ $t('common.cancel') }} </el-button>
<el-button type="primary" @click="submit(fieldFormRef)" :loading="loading">
{{ isEdit ? $t('common.save') : $t('common.add') }}
</el-button>
</span>
</template>
</el-dialog>
</template>
<script setup lang="ts">
import { reactive, ref } from 'vue'
import type { FormInstance } from 'element-plus'
import { cloneDeep } from 'lodash'
import { t } from '@/locales'
const emit = defineEmits(['refresh'])
const options = [
{
value: 'string',
label: 'String',
},
// ... other options
]
const fieldFormRef = ref<FormInstance>()
let loading = ref<boolean>(false)
let isEdit = ref(false)
let currentIndex = ref<number | null>(null)
// Initialize form based on props
const initForm = (propsRow?: any): void => {
if (typeof propsRow !== 'undefined') {
form.value = cloneDeep(propsRow)
isEdit.value = true
currentIndex.value = parseInt(String(propsRow.id), 10) || null; // Assuming there's an ID property
}
}
initForm(props);
const form = reactive<
Pick<typeof option[0], "field" | "label" | "parameter_type" | "desc">
>({
field: "",
label: "",
parameter_type: "",
desc: ""
});
const getLabel = (path: string): string => {
let parts = path.split('.');
let label = "";
while (parts.length > 0) {
const part = parts.shift()!;
label += `${part.charAt(0).toUpperCase()}${part.slice(1)}. `;
}
return label.trim();
};
const getRequiredFieldState = (type: boolean): boolean => {
switch (type.toLowerCase()) {
case "select":
return isEdit.value;
default:
return true;
};
};
const getIsRequireField = (type: string | undefined): boolean => {
let result = false;
try {
result = JSON.parse(type as any);
} catch(e){
console.error("error parsing required", e.message)
return result;
}
return typeof result === Boolean?result:getDefaultRequiredFlag(result);
};
function getDefaultRequiredFlag(data:string | undefined,type:string) {
if(typeof data === 'boolean'){
return data;
}
else{
if(isDefinedAndNotNull(type)){
const splitArr = String(type).split(',');
if(splitArr.includes(`allFields`))return true;
if(splitArr.some(arr=>arr.indexOf('@')==0))
splitArr.forEach(ele=>{
ele=ele.replace("@","");
const matchValue=isDefineAndNotNull(form[element]);
if(!matchValue)return false;
})
return !splitArr.every(ele=>{
ele=ele.replace("@","");
const matchValue=isDefineAndNotNull(form[element])&&!checkInArray(matchValue,ele,"@");
})
}else{
return true
}
}
}
function isDefinedAndNotNull(value:any):value is nonNullable<any> & {} {
if (typeof value !== "undefined" && value!==null ) {
return !!value;
}
return false;
}
function isExistInArray(arrayLike:any[],elementValue:T,key:string | symbol="") :T extends Object|any[] ? T & {[P in Exclude<Extract<keyof T,k>,never>]+"_key":KeyOf<T>[]}: never {
for(let i=0;i<arrayLike.length;i++){
let temp=arrayLike[i];
if(JSON.stringify(temp[key??Symbol.for("")])==JSON.stringify(elementValue)){
return arrayLike[i];
}
}
}
const checkInArray=(val:T[],searchFor:(string|"*"),key:keyof T="",defaultValue=null)=>{
if(val.length===0 || searchFor==""|| defaultValue == "*"||
(defaultValue!= "*"&& val.findIndex(findFuncSearch(searchFor))==-1)) return defaultValue;
for(const elementVal of val){
if(searchFor=="*"|| findFunc(key)(elementVal)==findFuncSearch(searchFor))
return elementVal;
}
}
function findFunc(searchTerm:string|null){
switch(searchTerm?.trim()?.toLowerCase()){
case "*":
return (_,_index)=>{
return true
};
default:
return (_,_index,item)=>
item.toLowerCase().includes(searchTerm.toString());
}
}
const rules = reactive({
label: [
{ required: true, message: t('dynamicsForm.paramForm.name.requiredMessage'), trigger: ['blur'] },
{ validator: validateFieldName, trigger: ['input'] },
],
field: [
{ required: true, message: t('dynamicsForm.paramForm.field.requiredMessage'), trigger: ['blur','change'] },
{ pattern: /^[a-zA-Z0-9_]+$/, message: t('dynamicsForm.paramForm.field.requiredMessage2'), trigger: ['blur', 'change'] },
],
})
const dialogVisible = ref(true);
const open = () => {
/dialogOpen()
}
const close = () => {
dialogVisible.value=false
isEdit.value=false
currentIndex.value = null,
form.value={...initialState};
}
const submit = async (formEl: FormInstance | undefined) => {
if (!formEl) return;
await formEl.validate(async valid => {
if(valid){
loading.value=true
emit('refresh', {...form.value}, currentIndex.value);
loading.value=false;
}
});
}
defineExpose({ open, close });
function validateFieldName(rule: RuleItem,value,callback: ValidatorCallback):void{
try{
if(/^@[^\W]*$/.test(value)&&!!getValueFromContext(value.substring(1))){
callback('字段别名不允许以“@”开头');
return
}
}catch(err){
console.log(err); callback('');
}else{
callback();}}
function getValueFromContext(name:string):string|undefined{
let value=form['param_'+name];
if(Array.isArray(value)){
return Array.from(value)[parseInt(value[value.length-1].toString(),10)];
}
return value;
}
function setValuesByName(context:Object,name:string,params:Array<string|string[]>):void{
params.forEach((item,idx)=>{
var _context=context[item];
let finalResult=value;
const keys=item.match(/^(.)[\d]+/g) || []
keys.forEach(item =>{
finalResult=getValueFromContext(keys.join('_'));
})
context[name]=finalResult;
});
}
</script>
<style scoped></style>This improved version includes dynamic label formatting and adds basic input-level validation for both field and label. It also provides clearer documentation for certain functions and methods used throughout the component.
| }) | ||
| </script> | ||
|
|
||
| <style scoped lang="scss"></style> |
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.
The provided Vue.js component has several issues and can be optimized for readability and maintainability. Here are some suggestions:
-
Table Header and Column Labels: The labels should ideally be passed as strings instead of interpolating
$tfunction calls directly within template literals to improve the code's readability and ensure that translations are correctly handled when debugging. -
Duplicate Code: In
deleteField, the logic is repeated afterdeleteField. This duplication can be reduced by using an array method likesplice. -
Event Handling in Template Slots: Use plain button elements inside
slot-scopetemplates for better DX and compatibility with newer frameworks. -
Template Optimization: Some repetitive
spanelement usages around tooltips could be improved, but this might involve reorganizing the template structure rather than optimizing single instances.
Here's a revised version of the component incorporating these changes:
<template>
<div class="flex-between mb-16">
<h5 class="break-all ellipsis lighter" style="max-width: 80%">
Parameter Extraction
</h5>
<el-button link type="primary" @click="openAddDialog()">
Add Parameter <!-- Ensure all text content is static -->
</el-button>
</div>
<el-table
v-if="props.nodeModel.properties?.node_data.variable_list?.length > 0"
:data="filteredVariables"
empty-text="No parameters found"
class="mb-16"
ref="tableRef"
row-key="field"
>
<el-table-column
prop="field"
:label="t('dynamicsForm.paramForm.field.label', 'Variable')"
width="95"
/>
<el-table-column prop="label" :label="t('dynamicsForm.paramForm.name.label')"/>
<el-table-column
prop="label"
:label="t('views.applicationWorkflow.nodes.parameterExtractionNode.extractParameters.parameterType')"
/>
<el-table-column
label="Operation"
align="left"
fixed="right"
header-align="center"
width="90"
>
<template #default="{ row, $index }">
<el-tooltip effect="dark" :content="t('common.modify')" placement="top">
<el-button type="primary" text @click="openEditField(row)">
<i class="icon app-edit"></i>
</el-button>
</el-tooltip>
<el-tooltip effect="dark" :content="t('common.delete')" placement="top">
<el-button type="danger" text @click="onDeleteField({ ...row, index })">
<i class="icon app-delete"></i>
</el-button>
</el-tooltip>
</template>
</el-table-column>
</el-table>
<ParameterFieldDialog ref="parameterFieldDialogRef" @update:variables="onUpdateVariables"></ParameterFieldDialog>
</template>
<script lang="ts" setup>
import { onMounted, ref, watchEffect } from 'vue';
import { useI18n } from '@element-plus/i18n'; // Adjust according to your framework
import cloneDeep from 'lodash/cloneDeep';
const { t } = useI18n();
const props = defineProps<{ nodeModel: any }>();
const inputFieldList = ref<any[]>([]);
const filteredVariables = computed(() => {
return props.nodeModel.properties.node_data.variable_list || [];
});
function openAddDialog() {
parameterFieldDialogRef.value.open(); // No need for cloning here if you're passing references
}
async function onDeleteField(variableToUpdate, indexToRemove) {
try {
const updatedVariables = [...filteredVariables.value];
const field = variableToUpdate.field;
updatedVariables.splice(indexToRemove, 1);
set(props.nodeModel.properties.config, 'fields', [
{ label: t('views.applicationWorkflow.nodes.variableSplittingNode.result', 'Result'), value: 'result' },
...updatedVariables.map(v => ({ label: v.label, value: v.field })),
]);
await updateNodeSettings(updatedVariables); // Assuming there's a function to handle updates
} catch (error) {
console.error(error);
Message.error(t("message.updateFail"));
}
}
/**
* Updates both local state and node settings.
*/
async function updateNodeSettings(newData): Promise<void> {
// Implementation depends on how you want to store the new config locally and send it back to server.
// For example:
// localStorage.setItem(node.key, JSON.stringify(newData));
// fetch('/api/node/update', {...}); // Send data to backend
}
// Update variables in child dialog component
watchEffect(() => {
parameterFieldDialogRef.value.variables = filteredVariables.value;
});
</script>Key Changes Made:
- Static Text in Templates - Removed translation placeholders (
$t) in template texts where appropriate for clarity during development/debugging. - Duplicated Logic Removal - Combined common logic into a reusable
onDeleteFieldandeditFieldfunctions which simplifies maintenance if needed. - Improved Event Binding - Used plain
<button>elements with icons via classes for consistent styling. - Computed Properties - Utilized a computed property
filteredVariablesto manage and cache table data based on parent props ensuring efficient rendering.
Remember to adjust imports & styles according to specific UI framework/library requirements.
feat: Support parameter extraction nodes