Project

General

Profile

Bug #13217

Discovery Rules order by name, not priority

Added by Steve Parker about 7 years ago. Updated about 7 years ago.

Status:
Closed
Priority:
Urgent
Category:
Image
Target version:
Difficulty:
Triaged:
No
Bugzilla link:
Fixed in Releases:
Found in Releases:
Red Hat JIRA:

Description

Discovery rules are processed in order of :name, not :priority.

A simple fix is in line 9 of foreman_discovery-4.0.0/app/controllers/concerns/foreman/controller/discovered_extensions.rb:

-    DiscoveryRule.where(:enabled => true).reorder(:priority).each do |rule|
+    DiscoveryRule.where(:enabled => true).order(:priority).each do |rule|

I don't know for sure, but think it could be related to foreman_discovery-4.0.0/app/models/discovery_rule.rb, which sets: order("discover_rules.name") around line 29:

  # with proc support, default_scope can no longer be chained
  # include all default scoping here
  default_scope lambda {
                  with_taxonomy_scope do
                    order("discovery_rules.name")
                  end
                }


Related issues

Related to Discovery - Bug #12765: Inconsistent rule ordering in the UIClosed2015-12-10

Associated revisions

Revision b1d56894 (diff)
Added by Lukas Zapletal about 7 years ago

Fixes #13217 - rule ordering fixed

History

#1 Updated by Steve Parker about 7 years ago

My apologies, reverse diff format in the original report. The fix is to use ".reorder" instead of ".order"

#2 Updated by Lukas Zapletal about 7 years ago

  • Category set to Image
  • Assignee set to Lukas Zapletal
  • Priority changed from Normal to Urgent
  • Target version set to Discovery Plugin 6.0

Oh thanks!

#3 Updated by Lukas Zapletal about 7 years ago

  • Related to Bug #12765: Inconsistent rule ordering in the UI added

#4 Updated by Lukas Zapletal about 7 years ago

  • Target version changed from Discovery Plugin 6.0 to Discovery Plugin 5.0.2

What a nasty bug, Steve. Thanks for investigation!

#5 Updated by Lukas Zapletal about 7 years ago

For the record, I am clearing our documentation which was also a bit fuzzy around this: https://github.com/theforeman/theforeman.org/pull/556

#6 Updated by The Foreman Bot about 7 years ago

  • Status changed from New to Ready For Testing
  • Pull request https://github.com/theforeman/foreman_discovery/pull/254 added

#7 Updated by Lukas Zapletal about 7 years ago

  • Target version changed from Discovery Plugin 5.0.2 to Discovery Plugin 5.0.3

#8 Updated by Anonymous about 7 years ago

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

#9 Updated by Lukas Zapletal about 7 years ago

  • Bugzilla link set to 1320492

Also available in: Atom PDF