Project

General

Profile

Refactor #16718

Replace modifications to host list relation in host multiple action code

Added by Dominic Cleal over 2 years ago. Updated 11 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Rails
Target version:
Team Backlog:
Fixed in Releases:
Found in Releases:

Description

On Rails 5, some of the host multiple action tests fail as the controller states that all hosts failed, instead of all succeeding:

HostsControllerTest::submit actions with multiple hosts#test_0001_build without reboot [/home/dcleal/code/foreman/foreman/test/controllers/hosts_controller_test.rb:686]:
--- expected
+++ actual
@ -1 +1 @
-"The selected hosts will execute a build operation on next reboot"
+nil

To obtain a list of hosts where updating their build status etc. didn't work, the controller relied on calling .to_a.delete_if on the host list relation and then checking the result later. This doesn't work under Rails 5, it simply returns the cached result of the relation again as it hasn't actually changed.

Rather than trying to delete entries out of the Rails relation, the code should keep a separate list of hosts that failed the action and use this for display purposes. The code should be clearer as a result too.

Associated revisions

Revision ca59bac0 (diff)
Added by Dominic Cleal over 2 years ago

fixes #16718 - remove relation.to_a.delete_if in multiple actions

To obtain a list of hosts where updating their build status etc. didn't
work, the controller relied on calling .to_a.delete_if on a relation and
then checking the result later. This doesn't work under Rails 5, it
simply returns the cached result of the relation again as it hasn't
actually changed.

The results are now saved in a different array instead of affecting the
relation.

History

#1 Updated by The Foreman Bot over 2 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/3898 added

#2 Updated by Dominic Cleal over 2 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100

#3 Updated by Dominic Cleal over 2 years ago

  • Legacy Backlogs Release (now unused) set to 189

#4 Updated by The Foreman Bot almost 2 years ago

  • Pull request https://github.com/theforeman/foreman/pull/4613 added

Also available in: Atom PDF