feat(ui): add previous runs section to sensor details page#14851
feat(ui): add previous runs section to sensor details page#14851puretension wants to merge 8 commits intoargoproj:mainfrom
Conversation
Signed-off-by: puretension <[email protected]>
Signed-off-by: puretension <[email protected]>
Signed-off-by: puretension <[email protected]>
Signed-off-by: puretension <[email protected]>
… details page Signed-off-by: puretension <[email protected]>
… details page Signed-off-by: puretension <[email protected]>
… details page Signed-off-by: puretension <[email protected]>
| @@ -0,0 +1,9 @@ | |||
| Component: UI | |||
There was a problem hiding this comment.
This file shouldn't be in this PR.
Signed-off-by: puretension <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a "Previous Runs" section to the Sensor details page to display workflows triggered by sensors, improving UX consistency with CronWorkflow details page.
- Added workflow fetching using
workflows.argoproj.io/sensorlabel to retrieve sensor-triggered workflows - Implemented Previous Runs UI using WorkflowDetailsList component matching CronWorkflow pattern
- Added label constant for sensor workflows to shared models
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ui/src/sensors/sensor-details.tsx | Added workflow list fetching and Previous Runs section display with ZeroState handling |
| ui/src/shared/models/workflows.ts | Added sensor label constant for filtering sensor-triggered workflows |
| ui/src/workflow-templates/workflow-template-list.tsx | Unrelated changes: Added label persistence and URL query param handling |
| .features/pending/14809-sensor-previous-runs.md | Feature documentation for the sensor Previous Runs addition |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /> | ||
| )} | ||
| <> | ||
| {!workflows || workflows.length === 0 ? ( |
There was a problem hiding this comment.
The condition !workflows will always be false since workflows is initialized to [] on line 36. The check should be workflows.length === 0 only, matching the pattern in cron-workflow-details.tsx which checks !workflows where workflows is initialized to undefined.
| {!workflows || workflows.length === 0 ? ( | |
| {workflows.length === 0 ? ( |
| const workflowList = await services.workflows.list(namespace, null, [`${models.labels.sensor}=${name}`], {limit: 50}); | ||
| const workflowsInfo = await services.info.getInfo(); | ||
|
|
||
| setWorkflows(workflowList.items); | ||
| setColumns(workflowsInfo.columns); |
There was a problem hiding this comment.
Missing error handling for the async workflow fetching. If services.workflows.list() or services.info.getInfo() fails, the error is not caught or displayed to the user. Add .catch(setError) or wrap in try-catch to handle potential errors, similar to the sensor fetching useEffect on line 66-72.
| const workflowList = await services.workflows.list(namespace, null, [`${models.labels.sensor}=${name}`], {limit: 50}); | |
| const workflowsInfo = await services.info.getInfo(); | |
| setWorkflows(workflowList.items); | |
| setColumns(workflowsInfo.columns); | |
| try { | |
| const workflowList = await services.workflows.list(namespace, null, [`${models.labels.sensor}=${name}`], {limit: 50}); | |
| const workflowsInfo = await services.info.getInfo(); | |
| setWorkflows(workflowList.items); | |
| setColumns(workflowsInfo.columns); | |
| setError(null); | |
| } catch (err) { | |
| setError(err); | |
| } |
|
Hi, thanks for working on it. Any update? |
Fixes #14809
Motivation
Sensor details page lacks Previous Runs visibility, making it difficult for users to locate and monitor workflows triggered by sensor events. This creates inconsistent UX compared to CronWorkflow which provides clear Previous Runs section.
Modifications
workflows.argoproj.io/sensorlabel filteringrecoreded_video.mov
Verification
workflows.argoproj.io/sensor=sensor-namelabelDocumentation
No documentation changes needed as this follows existing CronWorkflow Previous Runs pattern that users are already familiar with.