scam icon indicating copy to clipboard operation
scam copied to clipboard

`quoted_id` not implemented

Open mrcljx opened this issue 14 years ago • 2 comments

If used in a AR query-condition, i.e.

status = Status[3]
Question.where("status_id = ?", status)

the query will have the Scam inserted as Yaml. This can be solved by adding def quoted_id id.to_s end

Dangerous when the id isn't numeric though.

mrcljx avatar Apr 07 '11 11:04 mrcljx

I am guessing this is more related to scam not conforming to active model's interface. Can you do more research and see if that is indeed the issue? Happy to pull something that fixes it. On Thursday, April 7, 2011 at 7:39 AM, sirlantis wrote:

If used in a AR query-condition, i.e.

status = Status[3] Question.where("status_id = ?", status)

the query will have the Scam inserted as Yaml. This can be solved by adding def quoted_id id.to_s end

Dangerous when the id isn't numeric though.

Reply to this email directly or view it on GitHub: https://github.com/jnunemaker/scam/issues/2

jnunemaker avatar Apr 07 '11 13:04 jnunemaker

I assumed that this would be part of the ActiveModel interface, but somehow it is not. Assuming status would be a regular AR object, methods are called in this order:

Q.where(["status_id = ?", status])
Q.sanitize_sql_array
Q.replace_bind_variables
Q.quote_bound_value
Q.connection.quote(status, column[:status_id])

# It then checks respond_to?(:quoted_id), see: 
# http://apidock.com/rails/ActiveRecord/ConnectionAdapters/Quoting/quote

status.quoted_id
status.quoted_value(1, column[:id])
status.class.connection.quote(1, column[:id])
1.to_s # because column[:id]-type is a Fixnum

This seems to be AR-only, AR#quoted_id in http://apidock.com/rails/ActiveRecord/Base/quoted_id - I don't like the decision that the other model is responsible for quoting it's id (since the Q model knows that column[:status_id] is a Fixnum). The only possible solution I would know of would be to use

def quoted_id
  ActiveRecord::Base.connection.quote(id)
end

Now that I think about it I wouldn't want to include that into Scam.

If you think otherwise I would create a pull request, which would require AR for the specs and have a Scam::ActiveRecord module.

mrcljx avatar Apr 08 '11 00:04 mrcljx