Skip to content

feat: 子流程支持循环执行以及HTTP回调 --story=130003551 --story=132632590 #711

Open
Mianhuatang8 wants to merge 37 commits intoTencentBlueKing:masterfrom
Mianhuatang8:merge_dev_subflow_http_to_master
Open

feat: 子流程支持循环执行以及HTTP回调 --story=130003551 --story=132632590 #711
Mianhuatang8 wants to merge 37 commits intoTencentBlueKing:masterfrom
Mianhuatang8:merge_dev_subflow_http_to_master

Conversation

@Mianhuatang8
Copy link
Copy Markdown
Collaborator

No description provided.

# Reviewed, transaction id: 74500
# Reviewed, transaction id: 74762
# Reviewed, transaction id: 74765
* feat: 循环变量协议修改 --story=130003551
# Reviewed, transaction id: 74970

* fix: 代码优化 --story=130003551
# Reviewed, transaction id: 74976

* fix: 代码优化 --story=130003551
# Reviewed, transaction id: 74987
# Reviewed, transaction id: 75024
# Reviewed, transaction id: 75036
* fix: 验收问题处理 --story=130003551
# Reviewed, transaction id: 75549

* fix: 验收问题处理 --story=130003551
# Reviewed, transaction id: 75555

* fix: 验收问题处理 --story=130003551
# Reviewed, transaction id: 75559
* fix: 验收问题处理 --story=130003551
# Reviewed, transaction id: 76446

* fix: 将循环次数选择抽出为公共组件 --story=130003551
# Reviewed, transaction id: 76615
# Reviewed, transaction id: 75033
# Reviewed, transaction id: 77443
# Reviewed, transaction id: 79251
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@01be4b0). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #711   +/-   ##
=========================================
  Coverage          ?   81.38%           
=========================================
  Files             ?      288           
  Lines             ?    16689           
  Branches          ?        0           
=========================================
  Hits              ?    13582           
  Misses            ?     3107           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Code Review 总结

本 PR 为子流程添加了循环执行和 HTTP 回调功能,涉及前端模板编辑、任务执行、变量选择等多个模块的大量改动。整体功能方向清晰,变量列表搜索/分组的 UI 重构和循环执行配置面板的实现较为完整。以下是主要发现:

⚠️ 逻辑问题

  1. VariableList 直接修改 prop 数据onSearchtoggleGroup 直接修改了来自 prop 的 group.isCollapse,违反 Vue 单向数据流原则,可能导致父组件状态被意外篡改。
  2. forEach(async ...) 不被 awaitfilterHistoryData 中的 breadcrumbData.forEach(async ...) 的异步回调不会被外层等待,loading 状态会提前结束,异步异常无法被 catch 捕获。
  3. activitiesArray.find(...).id 可能 NPE — find 可能返回 undefined,直接访问 .id 会抛异常。

⚡ 性能建议

  1. loadSubprocessOutput 每次加载子流程都会请求 — 如果输出参数不常变,建议做缓存。

✨ 代码整洁

  1. document.querySelector 全局选择器 — formMixins 中 handleListShow 使用全局选择器可能匹配到错误元素。
  2. TemplateData 未使用 — 已导入但未在模板中引用。
  3. 缩进不一致 — NodeConfig.vue 中部分代码缩进跌落,视觉上容易产生误解。
  4. 魔法数字 — loopConfig 默认值中的 loop_times: 3 等建议抽常量。

整体改动质量不错,UI 交互体验有明显提升。上述 1-3 项建议优先处理。

if (!this.isListOpen) {
return;
}
const listPanel = document.querySelector('.rf-select-list');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ document.querySelector('.rf-select-list') 只能匹配到页面上第一个 .rf-select-list 元素。当页面同时渲染多个 Tag 组件时,点击关闭逻辑可能作用在错误的下拉面板上。建议改用 this.$el.querySelector 或通过 $refs 定位当前组件实例内的面板。

onSearch() {
// 搜索时自动展开所有分组
this.filteredGroupList.forEach((group) => {
group.isCollapse = false;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ onSearchtoggleGroup 直接修改了 prop varList 引用的 group 对象的 isCollapse,违反 Vue 单向数据流原则。建议将折叠状态维护在组件自身 data 中(如用 collapsedGroups Set 跟踪)。

version: has.call(output, 'version') ? output.version : 'legacy',
};
});
// 输出变量
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✨ Lines 777-799 缩进从 10 空格跌落到 8 空格,视觉上看起来像跳出了 try 块。建议统一缩进以避免维护时产生误解。

// 从接口获取子流程标准输出参数
const subprocessPlugin = this.atomList.find(item => item.code === 'subprocess_plugin');
const subprocessPluginVersion = subprocessPlugin?.list?.[0]?.version || '';
const res = await this.loadSubprocessOutput({ space_id: this.spaceId, version: subprocessPluginVersion });
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

loadSubprocessOutput 在每次 loadSubflowData 时都会发起网络请求。如果 subprocess_plugin 输出参数不频繁变化,建议缓存结果避免重复请求。

import { getOrderNodeToNodeTree } from '@/utils/orderCanvasNodeToNodeTree.js';
import JumpLinkBKFlowOrExternal from '@/components/common/JumpLinkBKFlowOrExternal.vue';
import SubStageCanvas from '../../../components/canvas/StageCanvas/SubStageCanvas.vue';
import TemplateData from './TemplateData.vue';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TemplateData 已导入并注册但未在 <template> 中使用,请移除未使用的 import 和组件注册。

}
} else {
const { template_node_id: templateNodeId } = this.subCanvsActivityCollection[this.nodeDetailConfig.node_id];
curNewNodeId = activitiesArray.find(item => item.template_node_id === templateNodeId).id;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ activitiesArray.find(item => item.template_node_id === templateNodeId) 可能返回 undefined,直接访问 .id 会抛异常。建议加空值判断。

component_code: this.nodeDetailConfig.component_code,
};
const res = await this.getNodeActDetail(query);
this.breadcrumbData.forEach(async (item) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ breadcrumbData.forEach(async ...) 中的 async 回调不会被外层 await,导致 isBreadCrumbLoading = false(line 788)在异步操作完成前执行,且异常无法被 catch 捕获。建议改用 for...of + awaitPromise.all

|| (Array.isArray(loopParams) && loopParams.length === 0)
|| (!Array.isArray(loopParams) && typeof loopParams === 'object' && Object.keys(loopParams).length === 0)) {
this.formData.loopConfig.loop_params = [{ name: '', value: '', is_quote: false }];
} else if (!Array.isArray(loopParams)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

loop_times: 3 等默认值属于魔法数字,按项目规范建议抽取为命名常量(如 DEFAULT_LOOP_TIMES)。

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Code Review 总结(增量审查)

本次增量审查针对子流程循环执行功能的最新提交。大部分代码改动围绕面包屑选择器重构、循环/重试/执行次数选择、以及循环输出参数处理,整体实现思路清晰。

上一轮问题状态

  • ⚠️ activitiesArray.find(...).id NPE — 已通过前置判断部分缓解,但在 filterHistoryData else 分支中 find 仍可能返回 undefined(line 757),风险仍存
  • ⚠️ VariableList 直接修改 prop — 未修复,onSearchtoggleGroup 仍直接修改 prop 对象的 isCollapse
  • ⚠️ forEach(async ...) — 未修复,filterHistoryData (line 767) 和 loadBreadCrumbData (line 1524) 中仍存在
  • TemplateData 未使用 — 未修复,仍在 import 和注册
  • document.querySelector('.rf-select-list') — 未修复,formMixins.js line 367 仍使用全局选择器
  • loop_times: 3 魔法数字 — 未修复
  • ✨ NodeConfig.vue 缩进问题 — 未修复

新发现问题

  1. 🚨 filterHistoryDataactivitiesArray.find(...) 未做空值保护(line 757),直接 .id 可能 NPE
  2. ⚠️ filterHistoryDatataskInfo = outputs.find(...) || {}|| {} 始终为 truthy,if (taskInfo) 永远为 true,else 分支成为死代码(line 738)
  3. ⚠️ selectBreadcrumExecuteCountprevItemtargetIndex <= 0 时为 null,直接访问 .allExecutedInfo 会抛 NPE(line 816)

}
} else {
const { template_node_id: templateNodeId } = this.subCanvsActivityCollection[this.nodeDetailConfig.node_id];
curNewNodeId = activitiesArray.find(item => item.template_node_id === templateNodeId).id;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ activitiesArray.find(item => item.template_node_id === templateNodeId) 可能返回 undefined,直接访问 .id 会抛异常。建议使用 optional chaining:.find(...)?.id

if (outputs && outputs.task_id) {
taskId = outputs.task_id;
} else if (Array.isArray(outputs)) {
const taskInfo = outputs.find(item => item.key === 'task_id') || {};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ outputs.find(...) || {} 结果始终为 truthy(空对象),if (taskInfo) 永远为 true,后续 else 分支成为死代码。应去掉 || {},改为 const taskInfo = outputs.find(item => item.key === 'task_id') 后再判断。

component_code: this.nodeDetailConfig.component_code,
};
const res = await this.getNodeActDetail(query);
this.breadcrumbData.forEach(async (item) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ breadcrumbData.forEach(async ...) 中的 async 回调不会被外层 await,导致后续 isBreadCrumbLoading = false 在异步操作完成前执行。建议改用 for...of + awaitPromise.all

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Code Review 总结(增量审查 - HTTP回调功能)

本次增量审查主要关注新增的 HTTP 回调(Webhook)功能相关提交,包括 HttpCallbackConfig.vueWebhookCallback.vueTabTemplateConfig.vue 改动,以及 TaskOperation.vue / index.vue 中的回调记录获取逻辑。整体功能完整,调试能力和表单校验做得较好。

上一轮问题状态

  • ⚠️ activitiesArray.find(...).id NPE(line 757)— 未修复,仍存在
  • ⚠️ VariableList prop 直接修改 — 未修复
  • ⚠️ forEach(async ...) 不被 await — 未修复(line 767, 1524)
  • ⚠️ outputs.find(...) || {} 死代码 — 未修复(line 738, 807)
  • TemplateData 未使用 — 未修复
  • ✨ 其余 Minor 项 — 未修复

新发现问题

详见行内评论(共 5 条)。

// 请求获取回调记录
if (['FINISHED', 'FAILED'].includes(this.state)) {
const instanceData = await this.getTaskInstanceData(this.taskId);
this.webhookHistory = instanceData.webhook_delivery_history;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ instanceData.webhook_delivery_history 未做防御性处理,如果接口未返回该字段会导致 webhookHistoryundefined,进而传给 WebhookCallback 组件引发渲染异常。建议改为:

this.webhookHistory = instanceData.webhook_delivery_history || [];

this.triggerMethod = triggerMethod;
this.parentTaskInfo = parentTaskInfo || {};
// 查询关联流程权限,用于判断是否展示查看流程按钮
const templateData = await this.loadTemplateData({ templateId, common: templateSource === 'common' });
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ loadTemplateData 失败(如用户无模板查看权限)会导致整个 getTaskData 进入 catch,后续 setPipelineTree 不会执行,整个任务页面无法渲染。建议将这段逻辑移到 try/catch 外层或独立 try/catch 中,避免影响核心任务数据加载。

import NoData from '@/components/common/base/NoData.vue';
import Vue from 'vue';
import { bkTooltips } from 'bk-magic-vue';
Vue.use(bkTooltips);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Vue.use(bkTooltips) 放在模块顶层会在 import 时产生全局副作用。如果其他组件已经注册了该指令,这里的重复注册虽不会报错,但不符合单一职责原则。建议改为在组件内通过 directives: { bkTooltips } 局部注册。

return Promise.all(validations).then(results => results.every(valid => valid))
.catch(() => false);
}
return true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 debugMock 方法在发送调试请求时,会将认证信息(token、username、password)直接发送到后端。请确认后端 verify_webhook_configuration 接口对这些敏感字段有妥善处理(不写入日志、不明文存储),否则存在凭据泄露风险。

trigger: 'blur',
},
{
validator: val => val <= 10,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ timeoutintervalretry_times 的上限校验(val <= 10val <= 600val <= 5)使用的是字符串与数字的隐式比较。当用户输入 "2" 时因字符串比较会得到正确结果,但输入如 "1e2" 可能绕过 /^[0-9]+$/ 前置校验。建议校验前先 Number(val) 转为数字再比较,或在 bk-input 上添加 type="number" 约束。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants