Skip to content

Fixes #38597 - Add 'Scheduled' option#989

Merged
MariaAga merged 1 commit intotheforeman:masterfrom
Lukshio:38597-add_scheduled
Oct 9, 2025
Merged

Fixes #38597 - Add 'Scheduled' option#989
MariaAga merged 1 commit intotheforeman:masterfrom
Lukshio:38597-add_scheduled

Conversation

@Lukshio
Copy link
Copy Markdown
Contributor

@Lukshio Lukshio commented Jul 22, 2025

#988 Is required for this one

@adamruzicka
Copy link
Copy Markdown
Contributor

Blocked on #988 (comment) and a rebase

@Lukshio Lukshio force-pushed the 38597-add_scheduled branch from 27acfef to e690124 Compare July 23, 2025 18:14
@Lukshio Lukshio force-pushed the 38597-add_scheduled branch from e690124 to 6e174a0 Compare August 1, 2025 10:30
@Lukshio Lukshio force-pushed the 38597-add_scheduled branch 2 times, most recently from 9e1145b to 474336e Compare August 12, 2025 11:47
@Lukshio Lukshio force-pushed the 38597-add_scheduled branch 2 times, most recently from 4f4c603 to 7625ac2 Compare August 13, 2025 14:32
@Lukshio Lukshio force-pushed the 38597-add_scheduled branch from 7625ac2 to 8b842a7 Compare September 24, 2025 10:57
@Lukshio
Copy link
Copy Markdown
Contributor Author

Lukshio commented Sep 24, 2025

Rebase

@Lukshio Lukshio requested a review from MariaAga September 24, 2025 10:58
@MariaAga
Copy link
Copy Markdown
Member

image The chart should reflect this as well, currently clicking on the chart changes the filter to "success"

@MariaAga
Copy link
Copy Markdown
Member

Also, not sure if its in the scope of this PR, but searching for name ~ * gives empty results for "all results" and "Scheduled" (if there are only scheduled hosts)

@Lukshio
Copy link
Copy Markdown
Contributor Author

Lukshio commented Sep 30, 2025

Also, not sure if its in the scope of this PR, but searching for name ~ * gives empty results for "all results" and "Scheduled" (if there are only scheduled hosts)

After discussion with @adamruzicka about this. We decided to do just this "quick fix" for the scheduled option. Because the filter itself is written in some way, that does not allow us to quickly add this option without breaking things or do lot of work to get it done. So the answer is: Yes, it's out of scope of this PR

@Lukshio
Copy link
Copy Markdown
Contributor Author

Lukshio commented Sep 30, 2025

image The chart should reflect this as well, currently clicking on the chart changes the filter to "success"

I took a look at this issue, and it actually looks like, that it is a bug, because I don't think it should filter 'success' while all of the jobs are 'awaiting'. Or does it have some special behavior? That is also question for @kmalyjur

I think the issue is here:

const onChartClick = (_evt, { index }) => {
const statusKeys = Object.keys(STATUS_TITLES);
const selectedKey = statusKeys[index + 1]; // first status is ALL_STATUSES
const selectedFilter = selectedKey ? STATUS_TITLES[selectedKey]?.id : null;
if (onFilterChange && selectedFilter) {
onFilterChange(selectedFilter);
}
};

The index skips "ALL_STATUSES", but it skips it also when there are not any finished jobs.

Then there is a question what is the correct behavior:
When all of the jobs are awaiting clicking on chart:

  1. Filter only scheduled jobs
  2. Filter all statuses

@kmalyjur
Copy link
Copy Markdown
Contributor

kmalyjur commented Oct 1, 2025

it is a bug, because I don't think it should filter 'success' while all of the jobs are 'awaiting'

You're right, it's a bug, it probably happened because I and others forgot about the scheduling option.

  1. Filter only scheduled jobs
  2. Filter all statuses

The first option makes more sense for me.

@Lukshio
Copy link
Copy Markdown
Contributor Author

Lukshio commented Oct 6, 2025

Copy link
Copy Markdown
Member

@MariaAga MariaAga left a comment

Choose a reason for hiding this comment

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

Thank you!

@MariaAga MariaAga merged commit 4175552 into theforeman:master Oct 9, 2025
25 checks passed
@Lukshio Lukshio deleted the 38597-add_scheduled branch October 20, 2025 14:45
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.

5 participants