Skip to content

Fixes #39018 - Fix infinite loop on job invocations page#1019

Merged
MariaAga merged 1 commit intotheforeman:masterfrom
jeremylenz:39018-fix-job-invocations-infinite-loop
Jan 30, 2026
Merged

Fixes #39018 - Fix infinite loop on job invocations page#1019
MariaAga merged 1 commit intotheforeman:masterfrom
jeremylenz:39018-fix-job-invocations-infinite-loop

Conversation

@jeremylenz
Copy link
Contributor

Summary

  • Fixes infinite loop that occurred when visiting /legacy/job_invocations/:id
  • The issue was caused by unstable dependencies in a useEffect hook creating a feedback loop with Redux state
  • Refactored the effect to only respond to external statusFilter changes (chart clicks) and removed dependencies that caused the loop

Test plan

  • Visit /legacy/job_invocations/:id in the browser and verify the page loads without freezing
  • Click on different status filters in the aggregate status chart (success, failed, pending, cancelled)
  • Verify the host list updates correctly when clicking status filters
  • Use the search box to search for hosts and verify results appear correctly
  • Verify pagination works as expected
  • Run the new unit tests: npm test -- WrappedTargetingHosts.test.js

🤖 Generated with Claude Code

Copy link
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.

The fix works, thanks. Left comments for the tests

jest.clearAllMocks();
});

it('should render without infinite loop', () => {
Copy link
Member

Choose a reason for hiding this comment

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

Tests pass even with the infinite loop

@jeremylenz
Copy link
Contributor Author

Addressed your feedback:

  • Removed the webpack/mocks/foremanReact mock files
  • Removed all inline jest mocks from the test
  • Simplified the tests to a basic rendering check

You were right that the tests weren't actually validating the infinite loop fix - with a mock store, we can't simulate the Redux state changes that would trigger it.

@jeremylenz jeremylenz requested a review from MariaAga January 26, 2026 21:29
@jeremylenz
Copy link
Contributor Author

@MariaAga Updated, removed the enzyme test and replaced it with a react-testing-library test. However I still can't get it to actually fail with the old code, so do you think we should remove it or keep it?

@MariaAga
Copy link
Member

If they dont fail with the old code I dont think they are relevant to this PR then, and shouldnt be included in here

@jeremylenz
Copy link
Contributor Author

ok, removed the test

Copy link
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.

LGTM, tested with packit as well.
@jeremylenz Could you squash the commits?

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@jeremylenz jeremylenz force-pushed the 39018-fix-job-invocations-infinite-loop branch from 6524546 to 03c445b Compare January 29, 2026 13:09
@jeremylenz
Copy link
Contributor Author

squashed

@MariaAga MariaAga merged commit 76f82cf into theforeman:master Jan 30, 2026
12 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.

2 participants