Skip to content

Fixes #38636 - JobInvocationHostTable doesnt load the template with auto reload#995

Merged
adamruzicka merged 1 commit intotheforeman:masterfrom
kmalyjur:job-inv-template-reload
Aug 13, 2025
Merged

Fixes #38636 - JobInvocationHostTable doesnt load the template with auto reload#995
adamruzicka merged 1 commit intotheforeman:masterfrom
kmalyjur:job-inv-template-reload

Conversation

@kmalyjur
Copy link
Copy Markdown
Contributor

@kmalyjur kmalyjur commented Aug 5, 2025

The index page now passes finished to the JobInvocationHostTable, and a useEffect is added to refetch the data when finished becomes true. I'm not sure if it's the best solution, even though it works as expected, so I'm open to new ideas.

@MariaAga
Copy link
Copy Markdown
Member

MariaAga commented Aug 6, 2025

I'm getting Uncaught Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. Reac on host expand

@kmalyjur kmalyjur force-pushed the job-inv-template-reload branch 2 times, most recently from 6871a94 to e94db24 Compare August 7, 2025 13:42
@kmalyjur
Copy link
Copy Markdown
Contributor Author

kmalyjur commented Aug 7, 2025

@MariaAga Thank you, fixed now

@MariaAga
Copy link
Copy Markdown
Member

MariaAga commented Aug 7, 2025

The error doesnt happen, but the issue from the pr isnt resolved, if a user schedules a job for the future (2 mins) and stays on the page the table stays on »A task for this host has not been started« even if the tasks has started

@kmalyjur
Copy link
Copy Markdown
Contributor Author

kmalyjur commented Aug 7, 2025

@MariaAga I tested it exactly like how you described, and in my opinion, it works:
https://github.com/user-attachments/assets/c61b8e2e-66e8-4962-b1ed-88c241d5367b

}, [allResponse]);

useEffect(() => {
if (finished && !refreshedOnFinish.current) {
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.

Might be because your job goes immediately to finished and mine goes to running

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right! I changed it so it reloads when the status changes. Is that okay?

@kmalyjur kmalyjur force-pushed the job-inv-template-reload branch from e94db24 to 020f338 Compare August 11, 2025 10:10
@MariaAga
Copy link
Copy Markdown
Member

Opening a job invocation in a new tab comes back empty job_invocations_detail/x/host_invocation/y
image

@kmalyjur kmalyjur force-pushed the job-inv-template-reload branch from 020f338 to 3ed6a7a Compare August 12, 2025 12:59
@kmalyjur
Copy link
Copy Markdown
Contributor Author

@MariaAga I'm sorry, I tested it properly now, I hope.

Comment on lines -60 to -62
isVisible,
hostName,
hostProxy,
isExpanded,
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.

do we need a new prop? can we use just some isExpanded || !isInTableView?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I changed it back to isExpanded

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 as expected now!

@kmalyjur kmalyjur force-pushed the job-inv-template-reload branch from 3ed6a7a to 6b8d3c8 Compare August 13, 2025 10:19
@kmalyjur
Copy link
Copy Markdown
Contributor Author

@adamruzicka could you please merge it?

@adamruzicka adamruzicka merged commit ffc0839 into theforeman:master Aug 13, 2025
25 checks passed
@adamruzicka
Copy link
Copy Markdown
Contributor

Thank you @kmalyjur & @MariaAga !

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