Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #36906 - fix bulk Compliance Policy for all hosts #552

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

MariaAga
Copy link
Member

@MariaAga MariaAga commented Nov 8, 2023

Updating the function to match
https://github.com/theforeman/foreman/blob/8930302fa46af140b636c6e4776d1caa0e9dbdf9/app/controllers/hosts_controller.rb#L780
I didn't find a way to just use it from the host controller since its private.
The issue for all hosts selected was that the params was search= and it just redirected to the hosts page inside the action modal, instead of putting all the hosts into @hosts

Copy link
Member

@ofedoren ofedoren left a comment

Choose a reason for hiding this comment

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

Thanks, @MariaAga, seems to fix... an unexpected behavior?..

Before:
ScreenShot-1699546117641
After:
ScreenShot-1699546611884

In both cases it seemed to be working, but... differently.

I mean, it seems to unassign the policy, but currently it sends the host ids to the server, but with the fix I can't find any host ids in the request logs...

@adamruzicka, could you take a look as well?

@adamruzicka
Copy link
Contributor

Seems to work for me. If I select hosts explicitly, host_id parameter(s) get sent over. If I select all without searching, there's no parameter at all, if I search and then select all, the search is sent over as search. The backend honors these parameters (or lack of thereof) and (un)assigns the policy to the matched hosts

Copy link
Member

@ofedoren ofedoren left a comment

Choose a reason for hiding this comment

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

If so, I don't see anything to be blocked on, let's get this in.

Thanks, @MariaAga and @adamruzicka.

@ofedoren ofedoren merged commit ed97737 into theforeman:master Nov 10, 2023
3 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