Skip to content
This repository has been archived by the owner on Nov 9, 2021. It is now read-only.

Adding where parameter substitution #16

Merged
merged 1 commit into from
May 13, 2013
Merged

Adding where parameter substitution #16

merged 1 commit into from
May 13, 2013

Conversation

diwu1989
Copy link
Contributor

Allow the use of .where(condition, val1, val2, val3, ...) for parameter substitution.

example:
.where('a = ?', 'hello world')
--- where a = 'hello world'

.where('a in ?', [1,2,3])
--- where a in (1, 2, 3)

.where('a in ? and b = ?', ['x', 'y'], 123)
--- where a in ('x', 'y') and b = 123

@hiddentao
Copy link
Owner

Nice. Just one thing...

Please add tests for this new functionality into the WhereBlock tests inside blocks.test.coffee. That's where the tests for this should really sit.

@diwu1989
Copy link
Contributor Author

Moved and added more tests at the right place.

@hiddentao
Copy link
Owner

The tests are in the right place now but it would be better if they tested the calls made to both _sanitizeValue and _formatValue. For an example of what I mean look at:

https://github.com/diwu1989/squel/blob/b778b8bbd23e39717838b348f49f2214c2e26056/test/blocks.test.coffee#L753

If you stub these two functions then you can check that they're being called for every value. This means you don't have to test the where() call with every different date type (string, null, integer, etc.)

If you're not sure how to do this then just leave things as they are and I'll rewrite the test code once I've merged.

Thanks.

@diwu1989
Copy link
Contributor Author

Updated

@hiddentao
Copy link
Owner

That looks great. Just one last thing... in the substitutes variadic arguments test you've added please assert at the end that @inst.wheres contains the expected values. Thanks.

@hiddentao
Copy link
Owner

Ah, one more small change on this line:

https://github.com/diwu1989/squel/commit/1739d9ad93a87174dbca87c615f0b6b2d1f991f3#L0R720

Set the stub to return '[' + val + ']' or something similar so that we are testing that the result of calling _formatValue is actually getting used. I've done something similar at:

https://github.com/diwu1989/squel/blob/1739d9ad93a87174dbca87c615f0b6b2d1f991f3/test/blocks.test.coffee#L518

@diwu1989
Copy link
Contributor Author

Updated and squashed all changes related to this feature

@hiddentao
Copy link
Owner

Thanks.

hiddentao added a commit that referenced this pull request May 13, 2013
Adding where parameter substitution
@hiddentao hiddentao merged commit 2f5b711 into hiddentao:master May 13, 2013
@hiddentao hiddentao mentioned this pull request May 13, 2013
@hiddentao hiddentao mentioned this pull request May 29, 2013
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 5aa852a on diwu1989:whereClause into * on hiddentao:master*.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants