Don't quote ID's as Arel will quote them -- follow same conventions as the delete method. #332

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

cmeiklejohn commented Apr 28, 2011

Don't quote IDs as Arel will quote them -- follow same conventions as the delete method, which is already in place. This causes problems when working with strings as primary keys which are then quoted twice.

Included is a patch to resolve this issue. The delete method uses ID, not quoted ID, as ARel will quote it.

See both:

https://rails.lighthouseapp.com/projects/8994/tickets/6230-optimistic-locking-quoting-id-applied-twice-for-update-statement
https://rails.lighthouseapp.com/projects/8994/tickets/6578-optimistic-locking-and-double-quoting-of-strings

Contributor

josevalim commented May 7, 2011

Member

jonleighton commented May 7, 2011

@cmeiklejohn can you add a test please?

Contributor

cmeiklejohn commented May 8, 2011

will do.

Contributor

cmeiklejohn commented May 8, 2011

Added. Had to add a new model to represent the table with a string for primary key -- also added coverage to the destroy call as well. Let me know if you'd like to see anything renamed or changed.

jonleighton added a commit that referenced this pull request May 8, 2011

@jonleighton jonleighton closed this May 8, 2011

Member

jonleighton commented May 8, 2011

Oops, meant to say the following at the same time as closing:

Merged, thanks! I condensed it into a single commit so it's clearer to other people that the tests are associated with the fix.

Merge commit: a8daea4
Commit: 4fbd8ad

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