-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Move 'Distinct' out of select string and use .uniq instead #461
Conversation
joins(joins.join(" ")). | ||
where(conditions.join(" AND ")). | ||
group(group). | ||
having(having). | ||
order(options[:order]). | ||
readonly(false) | ||
|
||
request = request.uniq unless (context and tag_types.one?) && options.delete(:any) |
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.
Looks good. Can you perhaps extract the logic into a well-named method(s)?
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.
You can use use :
request.uniq! unless (context and tag_types.one?) && options.delete(:any)
Seems like travis failing not because of my changes but because of previous one. |
Edit : Ignore my previous comment. |
Could you rebase and make the tests pass ? Master is passing the tests. |
Silly mistake. I use uniq! as i was previously suggested, but it exists only in rails 4.0.2. Now, everything is okay. |
Seem fine to me, i think we should merge it. |
Awesome! tried it from @developer88 repo. It fixed my problem colliding with wvanbergen/scoped_search 👍 |
Busy plus travel -> not reviewing many at this time |
@developer88 could you add your change in the change-log.md and rebase ? |
@seuros It's done. |
Sorry to ask again, but could you rebase and force push to remove the extra commit ? |
@seuros My mistake. It's done. |
Fix : Move 'Distinct' out of select string and use .uniq instead
Is there a way to pull a branch that includes this fix? I've been having a problem that looks like it would be solved by this, and I'd like to see if it fixes before reporting anything different? Alternatively, when would this make it into master? |
Apologies. I got this by specifying the git in my gemfile, and it appears to fix my issue. Looks like it has not yet propogated to Rubygems yet.... |
I already merged it. You can use master for now. A new version will be |
@seuros Do you need help prepping a release, or would you like me to just make a pre-release while you prep it? |
I wanted to merge #390 before the release. |
In the meantime, I did just release a 3.1.0.rc1 https://github.com/mbleigh/acts-as-taggable-on/blob/master/CHANGELOG.md#310rc1--2014-02-26 |
Fix : Move 'Distinct' out of select string and use .uniq instead
Fix issue #357
When using :any => true option in tagged_with method along with other ActiveRecord query get and error like:
"PG::Error: ERROR: syntax error at or near "DISTINCT"
So i move 'Distinct' out to the end of tagged_with method definition and use .uniq method instead.