-
Notifications
You must be signed in to change notification settings - Fork 64
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
Added reset_where method for specific columns. #325
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -139,6 +139,11 @@ class Avram::Criteria(T, V) | |
rows.query.group_by(column) | ||
end | ||
|
||
# :nodoc: | ||
def __reset_where : Avram::QueryBuilder | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a way to make this private? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, it is something I would do in JS. The question is, should it be private? Things like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It may be possible but would require reworking how Criteria and Queries work. This needs to be public so the Queryable can call it, but it shouldn't be used by the public. It can be and it'll work but it just reads strangely so adding the We also do this in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm I'm not sure what you mean. If marked as I don't think there is a way to make this method only accessible from some classes, but not others. But maybe I'm misunderstanding There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, you're right, there isn't another way. You can't make it private. But I think the |
||
rows.query.reset_where(column) | ||
end | ||
|
||
private def add_clause(sql_clause) : T | ||
sql_clause = build_sql_clause(sql_clause) | ||
rows.query.where(sql_clause) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a statement test here to match what was done on line 41?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I mentioned on Gitter 😂 . The result of
where_sql
is cached, so the test would fail. You can't callstatement
twice because it will always return the first result.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh! 👍