Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Documentation for where method on relations #6697

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

https://groups.google.com/d/topic/rubyonrails-core/IquCr3-vNVE/discussion generated some good discussion, but also some plain noise because the "where" method in Active Record isn't documented.

I decided to fix that. This commit adds rdoc for #where, based on both the Rails Guide and a perusal of the Active Record test suite. I'm opening a pull request, rather than just committing to docrails, because I want some input on style and substance for documenting this important method.

I also found examples in the test suite of condition templates like this:

User.where("field = '%s'", "value")

I don't see this printf-style mentioned anywhere in the guides or other external documentation; I'm wondering if it's still valid/considered in good taste. If it is, then I'd appreciate a pointer to which %-escapes are supported.

@ghost ghost assigned fxn Jun 10, 2012

Bump. If no response in another 3 days, I'll just commit to docrails.

@carlosantoniodasilva carlosantoniodasilva commented on the diff Jun 14, 2012

activerecord/lib/active_record/relation/query_methods.rb
+ #
+ # A single string, without additional arguments, is passed to the query
+ # constructor as a SQL fragment, and used in the where clause of the query.
+ #
+ # Client.where("orders_count = '2'")
+ # # SELECT * from cliens where orders_count = '2';
+ #
+ # Note that building your own string from user input may expose your application
+ # to injection attacks if not done properly. As an alternative, it is recommended
+ # to use one of the following methods.
+ #
+ # === array
+ #
+ # If an array is passed, then the first element of the array is treated as a template, and
+ # the remaining elements are inserted into the template to generate the condition.
+ # Active Record takes care of building the query to avoid injection
@carlosantoniodasilva

carlosantoniodasilva Jun 14, 2012

Owner

This line break looks a little bit weird :)

Docs look really good to me. I'd just ask you to check the hashes to keep a consistency with spaces:

# The example is like this
User.joins(:posts).where({"posts.published" => true})
User.joins(:posts).where({ :posts => { :published => true } })

# We could use spaces in all hash examples to keep consistency and readability
User.joins(:posts).where({ "posts.published" => true })
User.joins(:posts).where({ :posts => { :published => true } })

Thank you!
/cc @vijaydev

@dpassage and yeah, please feel free to send new doc patches straight to docrails if you want :). Of course that shouldn't stop you to send to here as well, both are going to be reviewed before getting merged to Rails master. Thanks.

Uh, what happened? I just tried to add another commit, should I close this pull request and start over?

@dpassage weird.. you can try rebasing from rails master again, and make sure you squash your commits into one (in case you have more). If you have any problems with that rebasing just go ahead and open another pull request.

OK, seem to have the commit fixed, and reflects the feedback.

For the question about %s-escapes, what's the best channel to get that answered? -core?

Apparently %s is still valid, as we can see in the sanitization methods, although I've never used it myself. It should work similarly as using ?, with the difference that arguments are converted to to_s and always quoted as strings, as far as I could see.

Wow, you're right. Bit of a mess; seems like it's more prone to injection if not used properly, and just confusing. But it is what it is from a doc perspective... Adding coverage.

David Paschich Documentation for where and where! methods on relations. Based on exa…
…mples seen

in the Rails test suite. [ci skip]
12215b1

OK, this commit should be ready to go.

@carlosantoniodasilva carlosantoniodasilva commented on the diff Jun 15, 2012

activerecord/lib/active_record/relation/query_methods.rb
+ # User.where(["name = ? and email = ?", "Joe", "joe@example.com"])
+ # # SELECT * FROM users WHERE name = 'Joe' AND email = 'joe@example.com';
+ #
+ # Alternatively, you can use named placeholders in the template, and pass a hash as the
+ # second element of the array. The names in the template are replaced with the corresponding
+ # values from the hash.
+ #
+ # User.where(["name = :name and email = :email", { name: "Joe", email: "joe@example.com"}])
+ # # SELECT * FROM users WHERE name = 'Joe' AND email = 'joe@example.com';
+ #
+ # This can make for more readable code in complex queries.
+ #
+ # Lastly, you can use sprintf-style % escapes in the template. This works slightly differently
+ # than the previous methods; you are responsible for ensuring that the values in the template
+ # are properly quoted. The values are passed to the connector for quoting, but the caller
+ # is repsonsible for ensuring they are enclosed in quotes in the resulting SQL. After quoting,
@carlosantoniodasilva

carlosantoniodasilva Jun 15, 2012

Owner

Typo in responsible :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment