Skip to content
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

Don't use self.class in ActiveRecord#sanitize_sql_hash_for_conditions. #10473

Closed
wants to merge 1 commit into from
Closed

Don't use self.class in ActiveRecord#sanitize_sql_hash_for_conditions. #10473

wants to merge 1 commit into from

Conversation

schuetzm
Copy link
Contributor

@schuetzm schuetzm commented May 5, 2013

self is already a class here, so don't call class on it again, otherwise
the PredicateBuilder will call the non-existant Class#reflect_on_association.
This makes Cancan's accessible_by useable with multiple rules involving
polymorphic relations.

self is already a class here, so don't call class on it again, otherwise
the PredicateBuilder will call the non-existant Class#reflect_on_association.
This makes Cancan's accessible_by useable with multiple rules involving
polymorphic relations.
def test_sanitize_sql_hash_handles_models_as_values
treasure = Treasure.new
treasure.id = 1
assert_nothing_raised do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think is better to assert the return of the sanitize_sql instead of assert_nothing_raised

@zohlgren
Copy link
Contributor

zohlgren commented May 5, 2013

Submitted this a few days ago: #10352

@rafaelfranca
Copy link
Member

😅 how we will handle this?

#10352 needs a rebase but has better tests. The CHANGELOG message here seems better.

@zohlgren
Copy link
Contributor

zohlgren commented May 5, 2013

Updated CHANGELOG. #10352

@schuetzm
Copy link
Contributor Author

schuetzm commented May 5, 2013

I hadn't seen @zohlgren's PR; I will close this one.

@schuetzm schuetzm closed this May 5, 2013
@schuetzm schuetzm deleted the fix-sanitize_sql_hash branch August 28, 2013 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants