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

Add tests to make sure find_by methods are case sensitive. #23690

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@tgxworld
Contributor

tgxworld commented Feb 15, 2016

In Rails 4.2.5.1, I get

User.create!(username: 'tgx')

User.find_by_username('tgx') # Returns record
User.find_by_username('TGX') # Returns record

User.find_by(username: 'tgx') # Returns record
User.find_by(username: 'TGX') # Returns nil 

Tests are according to the current behavior in Rails 5

@rails-bot

This comment has been minimized.

Show comment
Hide comment
@rails-bot

rails-bot Feb 15, 2016

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

rails-bot commented Feb 15, 2016

r? @eileencodes

(@rails-bot has picked a reviewer for you, use r? to override)

@vipulnsward

View changes

Show outdated Hide outdated activerecord/test/cases/finder_test.rb

tgxworld added a commit to discourse/discourse that referenced this pull request Feb 15, 2016

@tgxworld

This comment has been minimized.

Show comment
Hide comment
@tgxworld

tgxworld Feb 15, 2016

Contributor

Hmm different behavior for MySQL....... It is getting late here. I'll have a look tomorrow

Contributor

tgxworld commented Feb 15, 2016

Hmm different behavior for MySQL....... It is getting late here. I'll have a look tomorrow

@tgxworld

This comment has been minimized.

Show comment
Hide comment
@tgxworld

tgxworld Feb 16, 2016

Contributor

r? @sgrif

Contributor

tgxworld commented Feb 16, 2016

r? @sgrif

@rails-bot rails-bot assigned sgrif and unassigned eileencodes Feb 16, 2016

@tgxworld

This comment has been minimized.

Show comment
Hide comment
@tgxworld

tgxworld Feb 17, 2016

Contributor

Build failure is not related.

Contributor

tgxworld commented Feb 17, 2016

Build failure is not related.

@yangshun

This comment has been minimized.

Show comment
Hide comment
@yangshun

yangshun Feb 17, 2016

typo: dynamic

typo: dynamic

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Feb 17, 2016

Member

Is there a reason we'd ever expect this to fail given reasonable code? This seems like a really unnecessary test to me. There's no way a bug like that would be introduced without breaking find_by in other easily visible ways

Member

sgrif commented Feb 17, 2016

Is there a reason we'd ever expect this to fail given reasonable code? This seems like a really unnecessary test to me. There's no way a bug like that would be introduced without breaking find_by in other easily visible ways

@tgxworld tgxworld closed this Feb 18, 2016

@tgxworld

This comment has been minimized.

Show comment
Hide comment
@tgxworld

tgxworld Feb 18, 2016

Contributor

@sgrif Apologies. I just confirmed that the bug we're facing is not related to Rails.

Contributor

tgxworld commented Feb 18, 2016

@sgrif Apologies. I just confirmed that the bug we're facing is not related to Rails.

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