Skip to content

feat: Switch to cron expression#4843

Merged
zhanweizhang7 merged 1 commit intov2from
pr@v2@feat_switch_cron
Mar 5, 2026
Merged

feat: Switch to cron expression#4843
zhanweizhang7 merged 1 commit intov2from
pr@v2@feat_switch_cron

Conversation

@shaohuzhang1
Copy link
Contributor

feat: Switch to cron expression

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Mar 5, 2026

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.

Details

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
Copy link

f2c-ci-robot bot commented Mar 5, 2026

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

Details Needs approval from an approver in each of these files:

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


Promise.all([
...(toolParameterRef.value ? toolParameterRef.value.map((item) => item.validate()) : []),
...(applicationParameterRef.value
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 are no irregularities or significant issues in the provided code snippet. However, there are some general optimizations and improvements that can be made:

  1. Code Duplication: The el-cascader component and the input field for Cron expressions have some duplicate code. You could refactor this to reduce redundancy by creating a common method that handles both options.

  2. Event Listeners: The @focusout listener on the input should also call validateCron() to ensure immediate validation when focus is lost.

  3. Form Reset Logic: Ensure that any reset logic (like setting default values) works correctly after switching schedules types. This includes maintaining old preset settings if changing back to cron.

  4. Input Cleanup Before Submission: Add cleanup logic before submission to remove any unnecessary data or empty fields.

  5. Performance Considerations: For performance-intensive tasks like real-time validation of inputs, consider optimizing these operations without affecting user experience. In this case, simple DOM manipulation might be sufficient.

Here's a possible refactored version with additional improvements:

// Refactor el-cascader and input components
<template>
  <!-- ... -->
</template>

<script setup lang="ts">
// ...

 function switchScheduleType() {
   // ...
 }
 
 const updateScheduledFields = (type: string) => {
   const isCron = type === 'cron';
   form.value.trigger_setting.schedule_type = type;
   scheduled.value = [];
   
   if (!isCron) {
     lastPresetSetting.value = cloneDeep({
       schedule_type: form.value.trigger_setting.schedule_type,
       interval_unit: form.value.trigger_setting.interval_unit,
       interval_value: form.value.trigger_setting.interval_value,
       days: form.value.trigger_setting.days,
       time: form.value.trigger_setting.time,
     });
     
     form.value.trigger_setting.interval_unit = undefined;
     form.value.trigger_setting.interval_value = undefined;
     form.value.trigger_setting.days = undefined;
     form.value.trigger_setting.time = undefined;
     return;
   }
 }

<-- ...

onMounted(() => {
  // Initialize/update triggeredCycleOptions here...
})
</script>

By refactoring and adding event listeners, you enhance maintainability and usability while ensuring all necessary validations and conversions are handled accurately.

@zhanweizhang7 zhanweizhang7 merged commit cb2143c into v2 Mar 5, 2026
3 checks passed
@zhanweizhang7 zhanweizhang7 deleted the pr@v2@feat_switch_cron branch March 5, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants