-
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
Let '.count' work when tagged_with is accompanied by a group clause #417
Conversation
Can you rebase against master and force push? |
@bf4 Just saw this now. I have rebase and forced-push the branch again. |
Thanks, will review. Can you update your spec to the expect syntax? |
@bf4 done! |
Just got bit by this one. Would love to see the fix merged into master. Thanks. |
@rgould Can you add your change to changelog.md and force push ? I will merge it once you do that. |
@seuros Done! Thanks! |
@rgould could you rebase ? the Actual change-log is different than yours. Thanks |
@seuros Derp, sorry about that. I've rebased and force-pushed. |
Fix: Let '.count' work when tagged_with is accompanied by a group clause
* This fixes the issue where the Mapping.with_traffic_summary scope would not chain with Mapping.tagged_with, generating invalid SQL when a `count` was required (falling over on `any?` and the like) * While we initially patched AATO ourselves, it turns out a fix was already merged for AATO 3.1.0 (mbleigh/acts-as-taggable-on#417) * 3.1.1 also had a tag counting performance improvement, which in truth doesn't do much for us but requires a migration
* This fixes the issue where the Mapping.with_traffic_summary scope would not chain with Mapping.tagged_with, generating invalid SQL when a `count` was required (falling over on `any?` and the like) * While we initially patched AATO ourselves, it turns out a fix was already merged for AATO 3.1.0 (mbleigh/acts-as-taggable-on#417) * 3.1.1 also had a tag counting performance improvement, which in truth doesn't do much for us but requires a migration
* This fixes the issue where the Mapping.with_traffic_summary scope would not chain with Mapping.tagged_with, generating invalid SQL when a `count` was required (falling over on `any?` and the like) * While we initially patched AATO ourselves, it turns out a fix was already merged for AATO 3.1.0 (mbleigh/acts-as-taggable-on#417) * 3.1.1 also had a tag counting performance improvement, which in truth doesn't do much for us but requires a migration
* This fixes the issue where the Mapping.with_traffic_summary scope would not chain with Mapping.tagged_with, generating invalid SQL when a `count` was required (falling over on `any?` and the like) * While we initially patched AATO ourselves, it turns out a fix was already merged for AATO 3.1.0 (mbleigh/acts-as-taggable-on#417) * 3.1.1 also had a tag counting performance improvement, which in truth doesn't do much for us but requires a migration
Fix: Let '.count' work when tagged_with is accompanied by a group clause
Give
group
a default of[]
, which will later evaluate into a no-op by Rails. Previously this would benil
, which Rails translates to " AS " in SQL, generating an invalid statement.