feat/show scheduled flow status#6330
Conversation
HenryHengZJ
commented
May 1, 2026
HenryHengZJ
commented
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a ScheduleStatusBadge component to visualize the scheduling status of agent flows within the ItemCard and FlowListTable views. The Agentflows view was updated to detect scheduled flows and fetch their status from the backend. Review feedback highlights a potential race condition and lack of error handling in the asynchronous status fetching logic, which could lead to inconsistent states. Additionally, suggestions were made to use optional chaining for safer property access on node data and to incorporate the loading state into the badge UI to prevent misleading 'Paused' statuses during data retrieval.
| Promise.allSettled( | ||
| scheduleConfiguredIds.map((id) => chatflowsApi.getScheduleStatus(id).then((res) => ({ id, data: res.data }))) | ||
| ).then((settled) => { | ||
| const next = { ...initialStatuses } | ||
| settled.forEach((r) => { | ||
| if (r.status === 'fulfilled') { | ||
| const { id, data } = r.value | ||
| next[id] = { | ||
| isScheduled: true, | ||
| enabled: data?.enabled === true, | ||
| nextRunAt: data?.record?.nextRunAt || null, | ||
| cronExpression: data?.record?.cronExpression || null, | ||
| loading: false | ||
| } | ||
| } | ||
| }) | ||
| setScheduleStatuses(next) | ||
| }) |
There was a problem hiding this comment.
There are two issues here:
- Race Condition: If getAllAgentflows.data changes while the requests are pending, the results from an older execution might overwrite the state of a newer one.
- Error Handling: If a request fails, the flow remains in a loading: true state indefinitely because allSettled results are only processed if fulfilled.
It is recommended to handle errors within the map and use a functional update for setScheduleStatuses to ensure state consistency.
if (scheduleConfiguredIds.length > 0) {
Promise.all(
scheduleConfiguredIds.map((id) =>
chatflowsApi
.getScheduleStatus(id)
.then((res) => ({ id, data: res.data }))
.catch(() => ({ id, error: true }))
)
).then((results) => {
setScheduleStatuses((prev) => {
const next = { ...prev }
results.forEach(({ id, data }) => {
if (next[id]) {
next[id] = {
...next[id],
enabled: data?.enabled === true,
nextRunAt: data?.record?.nextRunAt || null,
cronExpression: data?.record?.cronExpression || null,
loading: false
}
}
})
return next
})
})
}| if (node.data.name === 'stickyNote' || node.data.name === 'stickyNoteAgentflow') continue | ||
| const foundIcon = AGENTFLOW_ICONS.find((icon) => icon.name === node.data.name) | ||
| if (foundIcon) { | ||
| icons[agentflows[i].id].push(foundIcon) | ||
| } else { | ||
| const imageSrc = `${baseURL}/api/v1/node-icon/${nodes[j].data.name}` | ||
| const imageSrc = `${baseURL}/api/v1/node-icon/${node.data.name}` | ||
| if (!images[agentflows[i].id].some((img) => img.imageSrc === imageSrc)) { | ||
| images[agentflows[i].id].push({ | ||
| imageSrc, | ||
| label: nodes[j].data.label | ||
| label: node.data.label | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
The code accesses node.data.name and node.data.label without optional chaining. Since node.data?.name is used on line 153, it implies that node.data could potentially be undefined. Accessing properties directly on a potentially undefined object will cause a runtime error. This ensures consistency with existing patterns in the file.
| if (node.data.name === 'stickyNote' || node.data.name === 'stickyNoteAgentflow') continue | |
| const foundIcon = AGENTFLOW_ICONS.find((icon) => icon.name === node.data.name) | |
| if (foundIcon) { | |
| icons[agentflows[i].id].push(foundIcon) | |
| } else { | |
| const imageSrc = `${baseURL}/api/v1/node-icon/${nodes[j].data.name}` | |
| const imageSrc = `${baseURL}/api/v1/node-icon/${node.data.name}` | |
| if (!images[agentflows[i].id].some((img) => img.imageSrc === imageSrc)) { | |
| images[agentflows[i].id].push({ | |
| imageSrc, | |
| label: nodes[j].data.label | |
| label: node.data.label | |
| }) | |
| } | |
| } | |
| if (node.data?.name === 'stickyNote' || node.data?.name === 'stickyNoteAgentflow') continue | |
| const foundIcon = AGENTFLOW_ICONS.find((icon) => icon.name === node.data?.name) | |
| if (foundIcon) { | |
| icons[agentflows[i].id].push(foundIcon) | |
| } else { | |
| const imageSrc = baseURL + "/api/v1/node-icon/" + node.data?.name | |
| if (!images[agentflows[i].id].some((img) => img.imageSrc === imageSrc)) { | |
| images[agentflows[i].id].push({ | |
| imageSrc, | |
| label: node.data?.label | |
| }) | |
| } | |
| } |
References
- To maintain consistency, new functionality should follow existing patterns within the same file.
| const ScheduleStatusBadge = ({ scheduleStatus, size = 'md' }) => { | ||
| const theme = useTheme() | ||
| const customization = useSelector((state) => state.customization) | ||
|
|
||
| if (!scheduleStatus?.isScheduled) return null | ||
|
|
||
| const isActive = scheduleStatus.enabled === true | ||
| const palette = customization.isDarkMode ? 'dark' : 'light' | ||
| const colors = isActive ? ACTIVE[palette] : PAUSED[palette] | ||
|
|
||
| const tooltipText = isActive | ||
| ? scheduleStatus.nextRunAt | ||
| ? `Schedule active — next run ${moment(scheduleStatus.nextRunAt).format('MMM D, YYYY h:mm A')}` | ||
| : 'Schedule active' | ||
| : 'Schedule configured but turned off' |
There was a problem hiding this comment.
The loading property is defined in propTypes but not used in the component logic. Currently, while the schedule status is being fetched, the badge defaults to showing "Paused" because enabled is false in the initial state. This is misleading to the user. You should use the loading state to show a loading indicator or a "Loading..." label.
1 participant
