Fixes #20105 - index search not retained after deleting host

[kgaikwad]

As hosts controller extension file is available in katello project, created PR-7047.

[theforeman-bot]

Issues: #20105

[tbrisker]

This works when deleting a host from the hosts index page, but breaks when deleting a host from the host show page. Also note that a test is failing - which tests this behavior exactly.

[tbrisker]

@kgaikwad a different test seems to be failing now, can you please take a look?

[kgaikwad]

@tbrisker ,
Yes, I have checked test result.
I am looking into it but not able to get why it is failing with error - NoMethodError: undefined method encoding' for nil:NilClassin methodredirection_url_on_host_deletion`.

[tbrisker]

[test foreman]

[kgaikwad]

@tbrisker,
Got the reason.
On my local setup Katello feature is enabled that's why I was not able to reproduce test failure.
After disabling it, I am able to reproduce error - NoMethodError: undefined method encoding' for nil:NilClass`

encoding is used inside routes.recognize_path method.
Now I have handled nil value for session["redirect_to_url_#{controller_name}"], hopefully it will fix the test cases.

[ohadlevy]

thanks for your patch two questions:

1. why is this file in the host_details concern? I think it should be in the host controller?
2. can you please add tests to demonstrate the various conditions you are targeting?

thanks!

[kgaikwad]

@ohadlevy,

1. why is this file in the host_details concern? I think it should be in the host controller?

Yes, your right! It should be in host controller. But when I tried to put method redirection_url_on_host_deletion in host controller.
After running a rubocop I am getting below error -
app/controllers/hosts_controller.rb:1:1: C: Class has too many lines. [769/760]
That's why I moved it to host_details concern.
Please let me know your suggestions on it.

2. can you please add tests to demonstrate the various conditions you are targeting?

ok, I will add more tests and will update the PR.

[xprazak2]

@kgaikwad, could you rebase pls?

[kgaikwad]

@tbrisker, @xprazak2 ,

I have updated this PR.
Added two test cases. Also, added a change in Katello as hosts_controller extended in it through PR-7671

[jturel]

@kgaikwad can you rebase this?

[kgaikwad]

Test failure seems irrelevant to this pull-request. Any suggestions?

Error Message
Capybara::Poltergeist::StatusFailError: Request to 'http://{ip}:41197/puppetclass_lookup_keys' failed to reach server, check DNS and/or server status...

[jturel]

I'm looking to move forward on Katello/katello#7671 and that needs to start here :)

[kgaikwad]

@jturel, @tbrisker ,
Done with rebase. Got one test failed but I think it's irrelevant to my changes.

Test failure with error - Capybara::ElementNotFound: Unable to find link "Interfaces"...
Stacktrace

Capybara::ElementNotFound: Unable to find link "Interfaces"
    test/integration/shared/host_finders.rb:8:in `go_to_interfaces_tab'
    test/integration/host_js_test.rb:699:in `block (3 levels) 

[xprazak2]

@tbrisker, @ohadlevy any additional comments?

[ares]

[test foreman]

stale review

[ares]

@kgaikwad test failures seems unrelated and it fixes the issue for single host deletion. The original bug though asked for keeping the filter for bulk deletion I believe. I think the similar change should happen for submit_multiple_destroy controller action. The only change required is needed at https://github.com/theforeman/foreman/pull/4969/files#diff-1339af15c5b21b5bc6e557bbbda860c6R530, instead of redirect_to(hosts_path) you need to redirect_to(saved_redirect_url_or(send("#{controller_name}_url")))

I'm happy to merged then.

[ares]

ACK pending jenkins

[ares]

Thank you @kgaikwad, I'm happy for this to be merged, sorry for long waiting time.