Skip to content

Fixes #38630 - Add expand all rows to table in job invocation detail#993

Merged
MariaAga merged 1 commit intotheforeman:masterfrom
kmalyjur:job-inv-expand-table-2
Oct 21, 2025
Merged

Fixes #38630 - Add expand all rows to table in job invocation detail#993
MariaAga merged 1 commit intotheforeman:masterfrom
kmalyjur:job-inv-expand-table-2

Conversation

@kmalyjur
Copy link
Copy Markdown
Contributor

@kmalyjur kmalyjur commented Aug 1, 2025

#10632 is needed for this PR

image

@kmalyjur kmalyjur changed the title Add expand all rows to table in job invocation detail Add expand all rows of table in job invocation detail Aug 1, 2025
@ofedoren ofedoren changed the title Add expand all rows of table in job invocation detail Fixes #38630 - Add expand all rows to table in job invocation detail Aug 29, 2025
Comment on lines +326 to +331
const allAreExpanded = areAllPageRowsExpanded;

setExpandedHost(prevExpanded => {
const pageIdsSet = new Set(pageHostIds);

if (allAreExpanded) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

const allAreExpanded = areAllPageRowsExpanded;
Is this const needed? can the code use areAllPageRowsExpanded?

const pageIdsSet = new Set(pageHostIds);

if (allAreExpanded) {
return prevExpanded.filter(hostId => !pageIdsSet.has(hostId));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if all expended I would expect that the new value of expandedHost will be just []

return prevExpanded.filter(hostId => !pageIdsSet.has(hostId));
}

const newExpanded = new Set([...prevExpanded, ...pageHostIds]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

and if its not expended than the new value will be just the page ids, not sure what is the prevExpanded purpose here?

@MariaAga
Copy link
Copy Markdown
Member

Foreman PR merged, this PR needs a rebase 🙏

@adamruzicka
Copy link
Copy Markdown
Contributor

Please bump the requires_foreman version in lib/foreman_remote_execution/plugin.rb

@kmalyjur kmalyjur force-pushed the job-inv-expand-table-2 branch from 3d491ec to 496cd61 Compare October 20, 2025 14:07
@kmalyjur
Copy link
Copy Markdown
Contributor Author

@MariaAga Thank you, could you please re-review? It's rebased.

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.

Works well, thanks

@MariaAga MariaAga merged commit 8842dd2 into theforeman:master Oct 21, 2025
25 checks passed
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.

3 participants