Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(Slugged+History+Scoped): fix SQL errors when using the 3 plugins together #625

Closed
wants to merge 1 commit into from

Conversation

eturino
Copy link

@eturino eturino commented Nov 24, 2014

scope_for_slug_generator method on not new records, with History and Scoped enabled, produced invalid SQL statements.

SELECT "restaurants".* FROM "restaurants"  
  WHERE "restaurants"."city_id" = 1 
  AND ("restaurants"."id" != 1) 
  AND (sluggable_id <> 1) 
  AND "restaurants"."scope" = 'city_id:1'

SCOPE is part of the Slug model, not the Restaurant model:

SELECT "restaurants".* FROM "restaurants"  
  WHERE "restaurants"."city_id" = 1 
  AND ("restaurants"."id" != 1) 
  AND (sluggable_id <> 1) 
  AND "friendly_id_slugs"."scope" = 'city_id:1'

…together

`scope_for_slug_generator` method on not new records, with History and Scoped enabled, produced invalid SQL statements.

SELECT "restaurants".* FROM "restaurants"  WHERE "restaurants"."city_id" = 1 AND ("restaurants"."id" != 1) AND (sluggable_id <> 1) AND "restaurants"."scope" = 'city_id:1'

SCOPE is part of the Slug model, not the Restaurant model:

SELECT "restaurants".* FROM "restaurants"  WHERE "restaurants"."city_id" = 1 AND ("restaurants"."id" != 1) AND (sluggable_id <> 1) AND "friendly_id_slugs"."scope" = 'city_id:1'
@norman
Copy link
Owner

norman commented Dec 16, 2014

Thanks for this, and sorry for taking so long to respond. I've looked into the issue the the real problem is that the scoped and history modules are order-dependent (which they should not be). That is, if you load :scoped before :history if fails, but if you load :history before :scoped then it's fine. There's a similar but simpler fix in #588, I'll merge that today and will push out a new version some time this week (possibly today depending on my day goes).

@norman norman closed this Dec 16, 2014
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants