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 #23644 - Fix > >= < <= version/release searches #7381

Merged
merged 1 commit into from Jun 14, 2018

Conversation

Projects
None yet
4 participants
@PaulSD
Copy link
Member

commented May 18, 2018

No description provided.

@theforeman-bot

This comment has been minimized.

Copy link

commented May 18, 2018

Issues: #23644

self.scoped_search_sortable('version', operator, value)
end

def self.scoped_search_release(key, operator, value)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 18, 2018

Lint/UnusedMethodArgument: Unused method argument - key. If it's necessary, use _ or _key as an argument name to indicate that it won't be used.

@@ -30,6 +30,40 @@ def self.repository_association_class
RepositoryRpm
end

def self.scoped_search_version(key, operator, value)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 18, 2018

Lint/UnusedMethodArgument: Unused method argument - key. If it's necessary, use _ or _key as an argument name to indicate that it won't be used.

ids = JSON.parse(response.body)['results'].map { |p| p['id'] }
assert_includes ids, katello_rpms(:one_two).id
assert_includes ids, katello_rpms(:three).id
end

This comment has been minimized.

Copy link
@jlsherrill

jlsherrill May 21, 2018

Member

putting this in rpm_test.rb would be a better place, and if you could add more searches there, that'd be great. I don't see any searches in rpm_test.rb, but here's what one would look like:

https://github.com/Katello/katello/blob/master/test/models/erratum_test.rb#L27-L29

e = e.to_i # nil or blank becomes 0
condition = "#{self.table_name}.epoch #{operator} ? AND #{self.table_name}.version #{operator} ?"
parameters = [e, v]
unless r.nil? or r.blank?

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Style/AndOr: Use || instead of or.

parameters = [e, e] + parameters
return { :conditions => condition, :parameter => parameters }
else # if ['=', '<>'].include?(operator)
e = e.to_i # nil or blank becomes 0

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Layout/ExtraSpacing: Unnecessary spacing detected.

condition += " OR (#{self.table_name}.version_sortable = ? AND #{self.table_name}.release_sortable #{operator} ?)"
parameters += [parameters[0], Util::Package.sortable_version(r)]
end
e = e.to_i # nil or blank becomes 0

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Layout/ExtraSpacing: Unnecessary spacing detected.

if ['>', '>=', '<', '<='].include?(operator)
condition = "#{self.table_name}.version_sortable #{operator} ?"
parameters = [Util::Package.sortable_version(v)]
unless r.nil? or r.blank?

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Style/AndOr: Use || instead of or.

condition += " AND #{self.table_name}.release #{operator} ?"
parameters += [r]
end
unless e.nil? or e.blank?

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Style/AndOr: Use || instead of or.

op = (operator == 'IN' ? '=' : '<>')
conditions = []
parameters = []
for val in value.split(',').collect { |v| v.strip }

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Style/For: Prefer each over for.

end
end

def self.scoped_search_evr(_key, operator, value)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Metrics/AbcSize: Assignment Branch Condition size for scoped_search_evr is too high. [81/63]
Metrics/CyclomaticComplexity: Cyclomatic complexity for scoped_search_evr is too high. [18/14]
Metrics/MethodLength: Method has too many lines. [56/30]
Metrics/PerceivedComplexity: Perceived complexity for scoped_search_evr is too high. [20/18]

assert_equal expected, results.map(&:uuid).sort
end

def test_search_compare

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Metrics/AbcSize: Assignment Branch Condition size for test_search_compare is too high. [72.11/63]
Metrics/MethodLength: Method has too many lines. [36/30]

self.epoch = parsed[:epoch].to_i # nil or blank becomes 0
self.version = Package.sortable_version(parsed[:version])
r = parsed[:release]
self.release = r.nil? or r.blank? ? nil : Package.sortable_version(r)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Style/AndOr: Use || instead of or.

self.epoch = match[:epoch] rescue nil
self.release = (match[:release] rescue nil) ? Package.sortable_version(match[:release]) : nil
parsed = Package.parse_evr
self.epoch = parsed[:epoch].to_i # nil or blank becomes 0

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Layout/ExtraSpacing: Unnecessary spacing detected.

condition += " AND #{self.table_name}.release #{op} ?"
parameters += [r]
end
conditions += ['('+condition+')']

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Layout/SpaceAroundOperators: Surrounding space missing for operator +.

self.epoch = parsed[:epoch].to_i # nil or blank becomes 0
self.version = Package.sortable_version(parsed[:version])
r = parsed[:release]
self.release = r.nil? || r.blank? ? nil : Package.sortable_version(r)

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Lint/RequireParentheses: Use parentheses in the method call to avoid confusion about precedence.

parameters += [v]
end
unless r.nil? || r.blank?
condition += ["#{self.table_name}.release #{operator} ?"]

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Lint/UselessAssignment: Useless assignment to variable - condition.

else
conditions =
"convert_to(#{self.table_name}.version_sortable, 'SQL_ASCII') #{operator[0]} convert_to(?, 'SQL_ASCII') OR " +
"(#{self.table_name}.version_sortable = ? AND " +

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Style/LineEndConcatenation: Use \ instead of + or << to concatenate those strings.

parameters = [sv]
else
conditions =
"convert_to(#{self.table_name}.version_sortable, 'SQL_ASCII') #{operator[0]} convert_to(?, 'SQL_ASCII') OR " +

This comment has been minimized.

Copy link
@houndci-bot

houndci-bot May 30, 2018

Style/LineEndConcatenation: Use \ instead of + or << to concatenate those strings.

@jlsherrill

This comment has been minimized.

Copy link
Member

commented Jun 14, 2018

apologies for the delay on this. Tested and looks good. The code is a bit more complex than i'd like but that seems to be the reality of scoped_search and what this is doing :)

@jlsherrill jlsherrill merged commit 10a1281 into Katello:master Jun 14, 2018

3 checks passed

Hound No violations found. Woof!
katello Build finished. 3706 tests run, 9 skipped, 0 failed.
Details
prprocessor Commit message style is correct
Details

@PaulSD PaulSD deleted the PaulSD:version_search branch Jun 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.