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

Move 'Distinct' out of select string and use .uniq instead #461

Merged
merged 1 commit into from
Jan 29, 2014

Conversation

developer88
Copy link
Contributor

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.

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)
Copy link
Collaborator

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)?

Copy link
Collaborator

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)

@developer88
Copy link
Contributor Author

Seems like travis failing not because of my changes but because of previous one.

@seuros
Copy link
Collaborator

seuros commented Jan 21, 2014

Edit : Ignore my previous comment.

@seuros
Copy link
Collaborator

seuros commented Jan 23, 2014

Could you rebase and make the tests pass ? Master is passing the tests.

@developer88
Copy link
Contributor Author

Silly mistake. I use uniq! as i was previously suggested, but it exists only in rails 4.0.2. Now, everything is okay.

@seuros
Copy link
Collaborator

seuros commented Jan 24, 2014

Seem fine to me, i think we should merge it.
/cc @bf4

@simonmd
Copy link

simonmd commented Jan 26, 2014

Awesome! tried it from @developer88 repo. It fixed my problem colliding with wvanbergen/scoped_search 👍

@bf4
Copy link
Collaborator

bf4 commented Jan 26, 2014

Busy plus travel -> not reviewing many at this time

@seuros
Copy link
Collaborator

seuros commented Jan 27, 2014

@developer88 could you add your change in the change-log.md and rebase ?

developer88 added a commit to developer88/acts-as-taggable-on that referenced this pull request Jan 28, 2014
@developer88
Copy link
Contributor Author

@seuros It's done.

@seuros
Copy link
Collaborator

seuros commented Jan 28, 2014

Sorry to ask again, but could you rebase and force push to remove the extra commit ?

@developer88
Copy link
Contributor Author

@seuros My mistake. It's done.

seuros added a commit that referenced this pull request Jan 29, 2014
Fix : Move 'Distinct' out of select string and use .uniq instead
@seuros seuros merged commit de6af63 into mbleigh:master Jan 29, 2014
@Halloran
Copy link

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?

@Halloran
Copy link

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....

@seuros
Copy link
Collaborator

seuros commented Feb 26, 2014

I already merged it. You can use master for now. A new version will be
released soon.

@bf4
Copy link
Collaborator

bf4 commented Feb 26, 2014

@seuros Do you need help prepping a release, or would you like me to just make a pre-release while you prep it?

@seuros
Copy link
Collaborator

seuros commented Feb 26, 2014

I wanted to merge #390 before the release.

@bf4
Copy link
Collaborator

bf4 commented Feb 27, 2014

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

tekniklr pushed a commit to tekniklr/acts-as-taggable-on that referenced this pull request Mar 19, 2021
Fix : Move 'Distinct' out of select string and use .uniq instead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants