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

Convert unsafe SQL to pure active record methods (rails 5.2) #19366

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Oct 4, 2019

DEPRECATION WARNING: Dangerous query method (method whose arguments are used as raw SQL) called with non-attribute argument(s): "storage_id, COUNT(*) AS vm_count". Non-attribute arguments will be disallowed in Rails 6.0. This method should not be called with user-provided values, such as request parameters or model attributes. Known-safe values can be passed by wrapping them in Arel.sql().

Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just use standard rails here

app/models/storage.rb Outdated Show resolved Hide resolved
app/models/mixins/compliance_mixin.rb Outdated Show resolved Hide resolved
@kbrock kbrock force-pushed the wrap_known_safe_SQL_in_arel_sql branch from ab1758a to a147213 Compare October 4, 2019 20:28
Copy link
Member

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made those changes - lets see how the tests work on this batch

@kbrock kbrock changed the title Wrap known-safe raw SQL with Arel.sql to suppress deprecation (rails 5.2) Convert unsafe SQL to pure active record methods (rails 5.2) Oct 4, 2019
DEPRECATION WARNING: Dangerous query method
(method whose arguments are used as raw SQL)
called with non-attribute argument(s):
"storage_id, COUNT(*) AS vm_count".
Non-attribute arguments will be disallowed in Rails 6.0.
This method should not be called with user-provided values,
such as request parameters or model attributes.
Known-safe values can be passed by wrapping them in Arel.sql().

---

Since the order is a standard query, using order(:timestamp)
passing :timestamp => "DESC" is the same thing in descending order

group.count is a count for nodes with a group by
@kbrock kbrock force-pushed the wrap_known_safe_SQL_in_arel_sql branch from a147213 to eb3e88d Compare October 4, 2019 20:42
@miq-bot
Copy link
Member

miq-bot commented Oct 4, 2019

Checked commit jrafanie@eb3e88d with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

Copy link
Member Author

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jrafanie
Copy link
Member Author

jrafanie commented Oct 4, 2019

Merging Keenan's code on my PR 🤣

@jrafanie jrafanie merged commit a16b4ce into ManageIQ:master Oct 4, 2019
@jrafanie jrafanie added this to the Sprint 122 Ending Oct 14, 2019 milestone Oct 4, 2019
@jrafanie jrafanie self-assigned this Oct 4, 2019
@jrafanie jrafanie deleted the wrap_known_safe_SQL_in_arel_sql branch October 4, 2019 21:08
@kbrock
Copy link
Member

kbrock commented Oct 4, 2019

<3 thanks

@chessbyte chessbyte mentioned this pull request Mar 31, 2020
38 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants