Fixes #30375 - fix os_minor search to not show empty minor #7816
Conversation
|
Issues: #30375 |
| if os_y.to_i.public_send(operator + operator_addition1, y.to_i) | ||
| if os_y == y | ||
| if os_z.to_i.public_send(operator + operator_addition2, z.to_i) | ||
| unless os.minor.empty? |
There was a problem hiding this comment.
wouldn't this be better and save us the additional indentation?
| unless os.minor.empty? | |
| next if os.minor.empty? |
Just a nitpick, I know I'm bit of a indentation freak 🤦 xD
There was a problem hiding this comment.
done :) thansk! @ezr-ondrej
|
Would it make sense to add a test in https://github.com/theforeman/foreman/blob/5de05e8581dff572ea10745d09fd58ecea514e4e/test/models/concerns/hostext/search_test.rb ? |
| Operatingsystem.all.find_each do |os| | ||
| next if os.minor.empty? |
There was a problem hiding this comment.
You can pre-filter the empty ones when pulling them from the db
| Operatingsystem.all.find_each do |os| | |
| next if os.minor.empty? | |
| Operatingsystem.where.not(minor: nil).find_each do |os| |
There was a problem hiding this comment.
Unless minor can be '' empty string, I thing this is better solution :)
There was a problem hiding this comment.
how about:
| Operatingsystem.all.find_each do |os| | |
| next if os.minor.empty? | |
| Operatingsystem.where.not(minor: [nil, '']).find_each do |os| |
There was a problem hiding this comment.
Unless minor can be
''empty string, I thing this is better solution :)
I think we should think about empty strings, but it's a bigger effort. Debian doesn't really have minor versions anymore. It used to really have supported minor versions but now a minor is just a roll up of patches to a release. Also with CentOS there are no supported minors (only the latest). Unless you sync content, you can't really install an older minor version.
tl;dr: the Foreman model is complex, may not match reality anymore, needs discussion with whiteboards and/or beer
There was a problem hiding this comment.
Yeah, the whole inventory would deserve a brainstorm session, but definitely out of scope of this PR.
|
[test katello] is broken again |
|
2.1- 9a917db |
The issue:
when we search for os_minor < 0, all hosts with empty minor versions are shown.