Skip to content

Fixes #38538 - Add bulk change owner to all hosts page - #10589

Merged
jeremylenz merged 1 commit into
theforeman:developfrom
chris1984:bulk-owner
Jul 16, 2025
Merged

Fixes #38538 - Add bulk change owner to all hosts page#10589
jeremylenz merged 1 commit into
theforeman:developfrom
chris1984:bulk-owner

Conversation

@chris1984

@chris1984 chris1984 commented Jul 1, 2025

Copy link
Copy Markdown
Member
  • Added new method in the bulk_action_controller
  • Added a new route and permissions
  • Added tests for the host bulk actions controller for my change
  • Added the UI bits modal, actions, etc

Screenshot 2025-07-01 142106
Screenshot 2025-07-01 at 14-21-29 Hosts
Screenshot 2025-07-01 at 14-21-21 Hosts

@github-actions github-actions Bot added the UI label Jul 1, 2025
@chris1984

chris1984 commented Jul 4, 2025

Copy link
Copy Markdown
Member Author

Test failures are unrelated. @jeremylenz want to take a look?

jeremylenz
jeremylenz reviewed Jul 7, 2025
View reviewed changes
Comment thread .../javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js Outdated
Comment thread .../javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js Outdated
Comment thread test/controllers/api/v2/hosts_bulk_actions_controller_test.rb Outdated
Comment thread test/controllers/api/v2/hosts_bulk_actions_controller_test.rb Outdated
Comment thread test/controllers/api/v2/hosts_bulk_actions_controller_test.rb Outdated
Comment thread test/controllers/api/v2/hosts_bulk_actions_controller_test.rb Outdated
Comment thread .../javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js Outdated
Comment thread .../javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js Outdated
lfu
lfu reviewed Jul 8, 2025
View reviewed changes
Comment thread app/controllers/api/v2/hosts_bulk_actions_controller.rb Outdated
lfu
lfu reviewed Jul 8, 2025
View reviewed changes
Comment thread config/routes/api/v2.rb Outdated
@chris1984
chris1984 requested a review from jeremylenz July 14, 2025 02:38
@jeremylenz

jeremylenz commented Jul 14, 2025

Copy link
Copy Markdown
Contributor

I think test failures are due to needing a rebase

jeremylenz
jeremylenz reviewed Jul 14, 2025
View reviewed changes

@jeremylenz jeremylenz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tests need another look. In addition to my specific comments, I would go through and make sure all the tests are not duplicated in bulk_hosts_extension_test or somewhere else.

Comment thread test/controllers/api/v2/hosts_bulk_actions_controller_test.rb Outdated
Comment thread test/controllers/api/v2/hosts_bulk_actions_controller_test.rb Outdated
Comment thread .../javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js Outdated
@chris1984

chris1984 commented Jul 15, 2025

Copy link
Copy Markdown
Member Author

@jeremylenz rebased and made the suggestions you put in the comments. Test failure looks unrelated. I went through the tests I made and the ones in bulk_hosts_extensions_test and don't see any more duplicates. Let me know how it looks now.

@jeremylenz

jeremylenz commented Jul 15, 2025

Copy link
Copy Markdown
Contributor

Snapshot failures are unrelated.

@chris1984
chris1984 force-pushed the bulk-owner branch 2 times, most recently from 014530e to c3727f8 Compare July 16, 2025 15:30
@chris1984

chris1984 commented Jul 16, 2025

Copy link
Copy Markdown
Member Author

snapshot and test failures are unrelated

jeremylenz
jeremylenz reviewed Jul 16, 2025
View reviewed changes

@jeremylenz jeremylenz left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sentence case unless it's a page title ;)

Works well, except I did find one issue when using select all mode. But it's also an issue for host groups, so opened a separate issue for it: https://projects.theforeman.org/issues/38585

Once Katello/katello#11441 is in, tests should hopefully be green after rebase.

Comment thread .../javascripts/react_app/components/HostsIndex/BulkActions/changeOwner/BulkChangeOwnerModal.js Outdated
@chris1984
Fixes #38538 - Add bulk change owner to all hosts page
2c21ed1 
* Added new method in the bulk_action_controller
* Added a new route and permissions
* Added tests for the host bulk actions controller for my change
@jeremylenz jeremylenz mentioned this pull request Jul 16, 2025
Merged
@chris1984

chris1984 commented Jul 16, 2025

Copy link
Copy Markdown
Member Author

@jeremylenz all green :)

jeremylenz
jeremylenz approved these changes Jul 16, 2025
View reviewed changes

@jeremylenz jeremylenz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

ACK 👍

@jeremylenz
jeremylenz merged commit 8a5505b into theforeman:develop Jul 16, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeremylenz jeremylenz jeremylenz approved these changes
+1 more reviewer
@lfu lfu lfu left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

UI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chris1984 @jeremylenz @lfu