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

Fixed generated SQL for tagged_with :any #570

Merged
merged 1 commit into from
Sep 16, 2014
Merged

Conversation

stiff
Copy link
Contributor

@stiff stiff commented Jul 30, 2014

Closes #519.

@seuros
Copy link
Collaborator

seuros commented Jul 30, 2014

Thank you. Now the tests are failing.

@stiff
Copy link
Contributor Author

stiff commented Jul 30, 2014

Agree, it's not good but I needed an urgent fix, maybe later I'll take a look at the tests

@stiff
Copy link
Contributor Author

stiff commented Jul 30, 2014

Fixed wrong merging, now two tests failing:

  • one which abuses the forgiving treatment of invalid SQL by Sqlite and Mysql, which should be removed (or group by should be added explicitly). But it signifies that the public api has slightly changed, which should be noted and version should be bumped?
  • another which groups by unexistent column, and the only reason it was not failing before is because the column with same name occasionally appeared in join'd table.

group = "#{table_name}.#{primary_key}"
select_clause << group
end
conditions << "EXISTS (#{tagging_cond})"
Copy link
Collaborator

Choose a reason for hiding this comment

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

That work only in PG .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What exactly doesn't work and in which DB? Can you provide the error message, or something more specific

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nevermind, i used the wrong database. Sorry.

@seuros
Copy link
Collaborator

seuros commented Sep 6, 2014

If the tests are invalid, we should remove/fix them, i think we can fix correct the api if we don't remove the feature. WDYT ?

cc @bf4

@stiff
Copy link
Contributor Author

stiff commented Sep 8, 2014

Take a look at latest (7af0ed3) commit: it fixes the test, and implements :order_by_matching_tag_count

The 'not covered by test' in 15ab99d refers only to sqlite3, see 7af0ed3 travis build results. Very strange.

@seuros
Copy link
Collaborator

seuros commented Sep 15, 2014

@stiff can you squash your commits ? ping me when it done.

@stiff
Copy link
Contributor Author

stiff commented Sep 16, 2014

Done

seuros added a commit that referenced this pull request Sep 16, 2014
Fixed generated SQL for tagged_with :any
@seuros seuros merged commit 3564bab into mbleigh:master Sep 16, 2014
@seuros
Copy link
Collaborator

seuros commented Sep 16, 2014

Thank you

tekniklr pushed a commit to tekniklr/acts-as-taggable-on that referenced this pull request Mar 19, 2021
Fixed generated SQL for tagged_with :any
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.

PG error with v3.2.0
2 participants