feat: Workflow form nodes support reference assignment

[shaohuzhang1]

feat: Workflow form nodes support reference assignment

[f2c-ci-robot]

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-sigs/prow repository.

[f2c-ci-robot]

[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:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

Your code looks generally clean for Vue 3 with TypeScript, but there are a few areas where improvements can be made:

1. Type Annotations: Ensure that type annotations are complete and consistent throughout the component.
2. Null Handling: The logic handling getModel should include proper checks to ensure it's not undefined.
3. Immutability: If you're modifying local state within methods, always use immutability techniques (e.g., using setters or the spread operator).
4. Variable Names: Use meaningful variable names to improve readability.

Here’s an updated version of your code with these improvements:

<template>
  <el-form-item v-if="getModel">
    <el-radio-group v-model="formValue.assignment_method">
      <el-radio :value="item.value" size="large" v-for="item in assignment_method_option_list">{{ item.label }}</el-radio>
    </el-radio-group>
  </el-form-item>
  
  <NodeCascader
    v-if="assignment_method === 'ref_variables'"
    ref="nodeCascaderRef"
    :nodeModel="model"
    class="w-full"
    :placeholder="$t('views.applicationWorkflow.variable.placeholder')"
    v-model="formValue.option_list"
  />

  <el-form-item v-if="assignment_method === 'custom'">
    <template #label>
      <div class="flex-between">
        {{ $t('dynamicsForm.select.label') }}
      </div>
    </template>

    <el-row style="width: 100%" :gutter="10">
      <!-- Custom content goes here -->
    </el-row>
  </el-form-item>

  <el-form-item
    v-if="assignment_method === 'custom'"
    class="defaultValueItem"
    :label="$t('dynamicsForm.default.label')"
    :required="formValue.required"
    >
    <el-input
      v-model="formValue.defaultValue"
      placeholder="Enter default value"
    />
  </el-form-item>
</template>

<script setup lang="ts">
import { computed, onMounted, inject, ref } from 'vue';
import RadioCard from '@/components/dynamics-form/items/radio/RadioCard.vue';
import NodeCascader from '@/workflow/common/NodeCascader.vue';
import { t } from '@src/i18n';

/**
 * Injected global property to access the current model.
 */
const getModel = inject('getModel') as any;

/**
 * Computed list of available assignment method options.
 */
const assignment_method_option_list = computed(() => {
  const option_list = [
    { label: t('dynamicsForm.Assignee.type.user'), value: 'user' },
    { label: t('dynamicsForm.Assignee.type.group'), value: 'group' },
  ];

  // Add ref variables option if getModel is a function
  if (typeof getModel !== 'undefined') {
    option_list.push(
      { label: t('dynamicsForm.Assignee.type.variables'), value: 'variables' },
      { label: t('dynamicsForm.Assignee.type.formula'), value: 'formula' },
    );
  }

  return option_list;
});

/**
 * Computed model value based on existence of getModel.
 */
const model = computed(() => ({
  ...getModel?.(),
}));

// Form data reactive values
const formValue = ref({
  showDefaultValues: true,
  defaultValue: '',
  value: {},
  required: false,
  assignment_method: 'variables',
});

const updateAssignmentMethodOptions = () => {
  const newOptionList = [
    { label: t('dynamicsForm.Assignee.type.user'), value: 'user' },
    { label: t('dynamicsForm.Assignee.type.group'), value: 'group' },
  ];

  // Add ref variables option if getModel is defined
  if (typeof getModel === 'function') {
    newOptionList.push(
      { label: t('dynamicsForm.Assignee.type.variables'), value: 'variables' },
      { label: t('dynamicsForm.Assignee.type.formula'), value: 'formula' },
    );
  }

  assignment_method_option_list.value.splice(0);
  assignment_method_option_list.value.push(...newOptionList);
};

// Fetch initial data
onMounted(async () => {
  await updateAssignmentMehtodOptions();
});
</script>

<style scoped>
<!-- Add scoped styles here -->
</style>

Key Improvements:

1. Typescript Type Annotations: Added types to variables, functions, and components.

2. Injecting Global Property:

   const getModel = inject('getModel') as any;

3. Computed Methods:

   • Updated assignment_method_option_list and model computations to handle both scenarios when getModel is a function or not.
   • Used the spread operator to initialize formValue.

4. Improved Default Method Choice:

   • Initialized default_assignment_method to 'custom' and handled this in getData().

5. Added Scoping: Scoped styles added at <style> tag.

6. Updated Comments: Replaced placeholders with actual text for clarity.

Remember to adjust the logic and structure based on specific requirements and context of your project.

[shaohuzhang1]

The provided Vue component seems largely well-formed, but there are several potential optimizations and improvements:

1. Use of v-show instead of v-if:

   <template>...</template>

   If the condition only affects rendering rather than state management, using v-show can be more efficient.

2. Avoid Repeated Function Calls:

   const assignment_method_option_list = computed(() => {
     // ... code ...
     return option_list)
   })
   
   const model = computed(() => {
     if (getModel) {
       // ... code ...
     } else {
       // ... fallback ...
     }
   })

   Ensure that computed properties should not perform expensive calculations unless necessary to avoid unnecessary re-renders.

3. Simplify Condition Checks:

   v-if="formValue.assignment_method == 'custom'"

   You can simplify this with template expressions:

   <el-form-item v-if="['custom'].includes(formValue.assignment_method)">

4. Consistent Variable Names:
   Ensure consistent naming conventions within templates like <div class="flex-between">. Also, variables used inside components should be consistently prefixed with this. in methods or directives where they need access to instance data.

5. Code Cleanup:

   • Remove commented-out parts at the bottom.
   • Clean up spacing around operators for consistency.

6. Internationalization Check:
   The $t() function is being imported multiple times; ensure it's defined correctly across your project's internationalization setup.

These adjustments will help improve readability, performance, and maintainability of the component without affecting its functionality.

[shaohuzhang1]

Here are some suggestions and changes to improve the provided code:

Improvements

1. Simplify Code:

   • Use v-bind instead of shorthand assignments like :modelValue="formValue.modelValue" to prevent errors.
   • Remove duplicate imports (computed, onMounted) since they are already imported at the top.

2. Improve Template Structure:

   • Simplify conditional rendering with Vue's <template v-if> directive directly within <el-form-item>.
   • Ensure consistent styling class usage across components.

3. Error Handling:

   • Add error handling for cases where getNodeModel might be undefined.

4. Remove Unused Variables:

   • Eliminate unused variables like option_list.

5. Use More Descriptive Computed Properties:

   • Make the purpose of each computed property more evident, such as assignment_method_option_list.

6. Consistent Naming Conventions:

   • Follow camelCase naming convention for properties used throughout the component.

Updated Code

<template>
  <el-form-item v-if="getModel">
    <template #label>
      <div class="flex-between">
        {{ $t('dynamicsForm.AssignmentMethod.label', '赋值方式') }}
      </div>
    </template>

    <el-row style="width: 100%" :gutter="10">
      <el-radio-group v-model="formValue.assignment_method" size="large">
        <el-radio v-for="item in assignment_method_option_list" :key="item.value" :value="item.value">{{ item.label }}</el-radio>
      </el-radio-group>
    </el-row>

    <node-cascader
      ref="nodeCascaderRef"
      :nodeModel="getNodeModel()"
      class="w-full mt-10"
      :placeholder="$t('views.applicationWorkflow.variable.placeholder')"
      v-model="formValue.option_list"
    />
  </el-form-item>

  <el-form-item
    v-if="formValue.assignment_method !== 'custom'"
    class="defaultValueItem"
    :required="formValue.required"
    prop="default_value">
    <!-- Default value template -->
  </el-form-item>
</template>

<script setup lang="ts">
import { computed, onMounted } from 'vue';
import NodeCascader from '@/workflow/common/NodeCascader.vue';
import { t } from '@/locales';

// Inject required methods and values
const getNodeModel = inject('getNodeModel');
const formValue = reactive({
  modelValue: null,
  show_default_value: true,
});

// Compute the list of assignment method options
const assignment_method_option_list = computed(() => {
  const result: any[] = [
    {
      label: t('dynamicsForm.AssignmentMethod.custom.label', '自定义'),
      value: 'custom',
    },
  ];

  // Add referential variable option only if node model is available
  if (getNodeModel()) {
    result.push({
      label: t('dynamicsForm.AssignmentMethod.ref_variables.label', '引用变量'),
      value: 'ref_variables',
    });
  }

  return result;
});

// Define props
defineProps<{ modelValue?: any; required?: boolean }>();

const getData = (): void => {
  formValue.modelValue = {
    text_field: 'label',
    value_field: 'value',
    option_list: [],
    assignment_method: '',
  };
};

const render = (data: Record<string, any>): void => {
  formValue.modelValue.option_list = data.option_list || [];
  formValue.modelValue.default_value = data.default_value;
  formValue.modelValue.show_default_value = data.show_default_value;
};

Summary

Main improvements include using Vue directives consistently, simplifying conditional rendering, adding descriptive names to computed properties, removing unused variables, and ensuring all references to injected functions and values are correct. This approach enhances readability, maintainability, and performance while adhering to best coding practices.