feat: show history about completed runs in each cron workflow #11790

yunwoo-yu · 2023-09-09T17:31:06Z

Fixes #11706

Motivation

You can view completed workflows on the cron-workflows detail page.
You can explore workflows that you want users to see more easily. Better UX
We've taken the component that was applied to the workflow view and reused it.

Modifications

before
after

Verification

Signed-off-by: juijeong8324 <juijeong8324@gmail.com> Signed-off-by: Justice <juijeong8324@gmail.com>

Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com> Signed-off-by: Justice <juijeong8324@gmail.com>

Signed-off-by: GeunSam2 <rootiron96@gmail.com>

…juijeong8324 Fix/docs lint error for juijeong8324

Add artifact-visualization-demo.png at docs/assets Signed-off-by: Justice <juijeong8324@gmail.com>

Signed-off-by: Justice <juijeong8324@gmail.com>

Signed-off-by: YunCow <skypnal12@gmail.com>

agilgur5

Thanks for implementing this!

There are some issues with the implementation right now, as not everything is transferable from the Workflows List. See some comments below

agilgur5 · 2023-09-09T19:39:56Z

ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx

@@ -62,6 +73,16 @@ export const CronWorkflowDetails = ({match, location, history}: RouteComponentPr

    useEffect(() => setEdited(true), [cronWorkflow]);

+    useEffect(() => {
+        services.workflows.list(namespace, selectedPhases, selectedLabels, pagination).then(res => {


Ideally we can filter in the request to not retrieve unnecessary workflows, although that might not be possible

this is possible (and is already done in the Workflows List filter for Cron Workflows).
you can pass in a label with the Cron Workflow name: workflows.argoproj.io/cron-workflow=<name-of-cron-workflow>

That is the only label that should be passed as there is otherwise no label filter here (that is similarly only in the sidebar on the Workflows List page)

also should use async/await per #11740

Thanks for the feedback, would it be ok to change to async/await and get the data via label names like on the workflows List page?

You can replace selectedLabels (the 3rd argument to list) with the cron-workflow label from above

agilgur5 · 2023-09-09T19:43:51Z

ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx

@@ -25,6 +30,12 @@ export const CronWorkflowDetails = ({match, location, history}: RouteComponentPr
    const [name] = useState(match.params.name);
    const [sidePanel, setSidePanel] = useState(queryParams.get('sidePanel'));
    const [tab, setTab] = useState(queryParams.get('tab'));
+    const [workflows, setWorkflows] = useState<Workflow[]>([]);
+    const [{selectedPhases, selectedLabels, paginationLimit}, setSelectedData] = useState<WorkflowListRenderOptions>(JSON.parse(localStorage.getItem('ListOptions/options')));


there are no phases or labels here. those are from the sidebar filter in the Workflows List which is not present here.

pagination does not seem to be present either. there seems to be some logic here, but no UI elements for pagination.

also the localStorage call would not apply here as it is specific to the Workflows list page

Thanks I was wondering this, would it be a good idea to put a selection or pagination in the detail cron page?

I don't think filtering is needed right now. pagination would be nice to have, but for a first-pass MVP, I think we can leave out pagination as well.

You may have not noticed, but the main overall Cron Workflows page itself actually doesn't implement pagination yet 😅 : #10846

agilgur5 · 2023-09-09T19:48:55Z

ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx

@@ -183,6 +204,17 @@ export const CronWorkflowDetails = ({match, location, history}: RouteComponentPr

        return items;
    };
+    const updateCurrentlySelectedAndBatchActions = (newSelectedWorkflows: Map<string, Workflow>): void => {


So the checkbox is unused here. No batch actions were added to this page, so it's not useable currently

The workflows-row component has fixed selection and checkboxes. What do you think about creating a new cron-workflows-row component?

So ideally we would factor out a workflows-row without checkboxes and then use that in both places.
In particular, this history view you've created could also be useful for Workflow Templates as well, so making it reusable would be useful.

That being said, that requires a bit of refactoring. As a first-pass, we could just make the checkboxes no-op for now and refactor later -- that was why I commented on the function itself and not the UI elements.
I'll leave that decision up to you for if you want to try to tackle a refactor or just leave the checkboxes non-functional for now.

yunwoo-yu

Thanks for the feedback, I've got a few questions and will try to revise it in the direction of your feedback 👍🙇‍♂️

yunwoo-yu · 2023-09-10T05:26:04Z

ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx

@@ -62,6 +73,16 @@ export const CronWorkflowDetails = ({match, location, history}: RouteComponentPr

    useEffect(() => setEdited(true), [cronWorkflow]);

+    useEffect(() => {
+        services.workflows.list(namespace, selectedPhases, selectedLabels, pagination).then(res => {


Thanks for the feedback, would it be ok to change to async/await and get the data via label names like on the workflows List page?

yunwoo-yu · 2023-09-10T05:28:04Z

ui/src/app/cron-workflows/components/cron-workflow-details/cron-workflow-details.tsx

@@ -25,6 +30,12 @@ export const CronWorkflowDetails = ({match, location, history}: RouteComponentPr
    const [name] = useState(match.params.name);
    const [sidePanel, setSidePanel] = useState(queryParams.get('sidePanel'));
    const [tab, setTab] = useState(queryParams.get('tab'));
+    const [workflows, setWorkflows] = useState<Workflow[]>([]);
+    const [{selectedPhases, selectedLabels, paginationLimit}, setSelectedData] = useState<WorkflowListRenderOptions>(JSON.parse(localStorage.getItem('ListOptions/options')));


Thanks I was wondering this, would it be a good idea to put a selection or pagination in the detail cron page?

Signed-off-by: YunCow <skypnal12@gmail.com>

feat/design : Apply feedback about PR #11790

Revert "feat/design : Apply feedback about PR #11790"

yunwoo-yu · 2023-09-13T13:08:34Z

@agilgur5

hello 😄

I'll hide the checkbox for now, thanks for the advice

The PR got stuck in the commit log, so I'll repromote it again

Thanks!

agilgur5 · 2023-09-13T14:29:57Z

The PR got stuck in the commit log, so I'll repromote it again

You can rebase the branch and force push. No need to open a new PR.

Or if you create a new branch with the original commits cherry-picked, you can just name it the same as the old branch and force push.

yunwoo-yu · 2023-09-13T15:07:54Z

Uncontributed commits from collaborators got mixed up in the middle, and it was really hard to pick them out 😅

And I deleted the already forked repository. Maybe juijeong8324 will PR it again!

agilgur5 · 2023-09-13T15:22:17Z

And I deleted the already forked repository. Maybe juijeong8324 will PR it again!

Gotcha. Ok.

Uncontributed commits from collaborators got mixed up in the middle, and it was really hard to pick them out 😅

You can always use git reflog to find the reference before merging and checkout back to that. Then do a cleaner rebase as opposed to a merge. reflog is always your friend if you screw something up.

I've had to teach a lot of engineers how to properly use git before. It can do almost everything you need, just need to learn how 🙂 rebase -i is definitely my favorite 😄

agilgur5 · 2023-09-13T15:43:05Z

Replaced by / followed up in #11811

agilgur5 · 2023-09-14T03:47:09Z

@yunwoo-yu I noticed in this PR and in #11788 that you used your fork's master branch. You usually want to use feature branches. That makes it easier to separate your work and could make it easier to rebase and cherry-pick as well.

yunwoo-yu · 2023-09-14T08:38:19Z

@agilgur5

For the next PR, I'll create a separate feat branch and upload it. Thanks 😁

And like you said, I was able to get rid of the merged fact with reflog. I had already deleted the repository so I didn't get to try it, but I'll do it the way you suggested next time. 👍

juijeong8324 and others added 13 commits September 3, 2023 17:34

docs: Add description about img in artifact-visualization.md

2a11703

Signed-off-by: juijeong8324 <juijeong8324@gmail.com> Signed-off-by: Justice <juijeong8324@gmail.com>

docs: modify img to image in artifact-visualization.md

95e1128

Signed-off-by: juijeong8324 <juijeong8324@gmail.com> Signed-off-by: Justice <juijeong8324@gmail.com>

docs: modify docs/artifact-visualization.md

8d5be3a

Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com> Signed-off-by: Justice <juijeong8324@gmail.com>

docs: add line break

086bb9b

Signed-off-by: GeunSam2 <rootiron96@gmail.com>

docs: replace inline html to duble space

2748ce9

Signed-off-by: GeunSam2 <rootiron96@gmail.com>

Merge pull request #1 from ArgoWorkflows-OSS/fix/docs-lint-error-for-…

1d3e875

…juijeong8324 Fix/docs lint error for juijeong8324

Merge branch 'argoproj:master' into master

5c993a6

Merge branch 'argoproj:master' into master

6732b5f

docs : Add image file at docs/assets

ab52986

Add artifact-visualization-demo.png at docs/assets Signed-off-by: Justice <juijeong8324@gmail.com>

docs: modify demo image artifact-visualization.md

666fc25

Signed-off-by: Justice <juijeong8324@gmail.com>

Merge branch 'argoproj:master' into master

503be03

Merge branch 'argoproj:master' into master

120d1fc

feat: show history about completed runs in each cron workflow

e70b3cf

Signed-off-by: YunCow <skypnal12@gmail.com>

agilgur5 added the area/ui label Sep 9, 2023

agilgur5 requested changes Sep 9, 2023

View reviewed changes

This was referenced Sep 9, 2023

refactor(ui): migrate several components from class to functional #11791

Merged

feat: View the history of completions in cron-workflows #11788

Closed

yunwoo-yu commented Sep 10, 2023

View reviewed changes

juijeong8324 and others added 9 commits September 10, 2023 18:01

Merge branch 'argoproj:master' into master

69d925e

Merge branch 'argoproj:master' into master

8be5e72

design : Add cron-workflows.details.sass

789e437

feat : Filter in the request to pass cron-workflow

7c32559

Merge branch 'argoproj:master' into master

a752691

Merge branch 'master' into master

8a3b7df

Signed-off-by: YunCow <skypnal12@gmail.com>

Merge pull request #1 from juijeong8324/master

6679692

feat/design : Apply feedback about PR #11790

Revert "feat/design : Apply feedback about PR #11790"

bb6b8e0

Merge pull request #2 from yunwoo-yu/revert-1-master

0341a92

Revert "feat/design : Apply feedback about PR #11790"

yunwoo-yu closed this Sep 13, 2023

agilgur5 mentioned this pull request Sep 13, 2023

feat: show history about completed runs in each cron workflow #11811

Merged

agilgur5 mentioned this pull request Sep 22, 2023

REQUEST: Promotion to Reviewer for @agilgur5 argoproj/argoproj#242

Closed

8 tasks

agilgur5 added the solution/superseded This PR or issue has been superseded by another one (slightly different from a duplicate) label Feb 17, 2024

agilgur5 mentioned this pull request Jun 30, 2024

feat(ui): add history to WorkflowTemplate & ClusterWorkflowTemplate details #13283

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: show history about completed runs in each cron workflow #11790

feat: show history about completed runs in each cron workflow #11790

yunwoo-yu commented Sep 9, 2023

agilgur5 left a comment

agilgur5 Sep 9, 2023

agilgur5 Sep 10, 2023 •

edited

Loading

agilgur5 Sep 10, 2023

yunwoo-yu Sep 10, 2023

agilgur5 Sep 10, 2023

agilgur5 Sep 9, 2023 •

edited

Loading

yunwoo-yu Sep 10, 2023

agilgur5 Sep 10, 2023 •

edited

Loading

agilgur5 Sep 9, 2023

yunwoo-yu Sep 10, 2023 •

edited

Loading

agilgur5 Sep 10, 2023

yunwoo-yu left a comment

yunwoo-yu Sep 10, 2023

yunwoo-yu Sep 10, 2023

yunwoo-yu commented Sep 13, 2023

agilgur5 commented Sep 13, 2023 •

edited

Loading

yunwoo-yu commented Sep 13, 2023

agilgur5 commented Sep 13, 2023

agilgur5 commented Sep 13, 2023 •

edited

Loading

agilgur5 commented Sep 14, 2023

yunwoo-yu commented Sep 14, 2023

feat: show history about completed runs in each cron workflow #11790

feat: show history about completed runs in each cron workflow #11790

Conversation

yunwoo-yu commented Sep 9, 2023

Motivation

Modifications

Verification

agilgur5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agilgur5 Sep 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agilgur5 Sep 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agilgur5 Sep 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yunwoo-yu Sep 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yunwoo-yu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yunwoo-yu commented Sep 13, 2023

agilgur5 commented Sep 13, 2023 • edited Loading

yunwoo-yu commented Sep 13, 2023

agilgur5 commented Sep 13, 2023

agilgur5 commented Sep 13, 2023 • edited Loading

agilgur5 commented Sep 14, 2023

yunwoo-yu commented Sep 14, 2023

agilgur5 Sep 10, 2023 •

edited

Loading

agilgur5 Sep 9, 2023 •

edited

Loading

agilgur5 Sep 10, 2023 •

edited

Loading

yunwoo-yu Sep 10, 2023 •

edited

Loading

agilgur5 commented Sep 13, 2023 •

edited

Loading

agilgur5 commented Sep 13, 2023 •

edited

Loading