Scoped support?

Question

Scoped support?

bricesanchez opened this issue 7 years ago · 9 comments

Thanks for your awesome work on mobility!

I'm in the process of switching from globalize to mobility for refinerycms refinery/refinerycms#3360

I have a lot of fails specs due to this config: https://github.com/refinery/refinerycms/blob/master/pages/app/models/refinery/page.rb#L36-L37

I'm not sure if it's because friendly_id-mobility gem does not support scoped feature or if i have another bug.

Do you have any idea?

Answer 1 · 2018-03-29T12:26:07.000Z

I made a comment on refinery/refinerycms#3360, but basically FriendlyId::Mobility doesn't do anything in particular for scoped and I don't believe it should have to.

However, because Mobility doesn't monkey-patch ActiveRecord, the order in which things happen in your model is important. For history (as mentioned in the readme), you need to have :mobility come before :history in the array passed to use:, like this:

friendly_id :title, use: [:history, :mobility]

so I believe the same is true for scoped (if so I need to add this to the readme on this repo):

friendly_id :title, use: [:scoped, :mobility]

It seems that in that other PR, the order is reversed, which means that scoped wouldn't get the scope overrides that Mobility uses to handle translated attributes, which would explain the failures.

Answer 2 · 2018-03-29T12:27:58.000Z

I'm not sure though that that will fix it. If not, I'll add some specs here on scoped and check that it works with Mobility, and if not try and track down the problem. (Probably we should have those specs here anyway.)

Answer 3 · 2018-03-29T12:34:55.000Z

Actually, thinking about this again, I don't think that alone will fix it. The problem is that unlike Globalize, Mobility doesn't patch AR query methods globally, only through the i18n scope (available on translated models). So perhaps if after calling translates in the model, if you added default_scope { i18n } that might work. But this gem should probably handle this so you don't need to do that.

Answer 4 · 2018-03-29T13:29:38.000Z

Thanks for your quick answer !

Actually, thinking about this again, I don't think that alone will fix it. The problem is that unlike Globalize, Mobility doesn't patch AR query methods globally, only through the i18n scope (available on translated models). So perhaps if after calling translates in the model, if you added default_scope { i18n } that might work. But this gem should probably handle this so you don't need to do that.

It looks like this helps a little: refinery/refinerycms@836e050

It fixes 20 specs but not all slug specs! :)

Answer 5 · 2018-03-29T13:59:01.000Z

It fixes 20 specs but not all slug specs! :)

Ok that's good! Ideally though, this gem should allow you to use scoped without having to apply a default scope like that. I'll look into it.

Answer 6 · 2018-04-04T07:48:24.000Z

I'm pretty busy this week, will try to get to this next week. I don't think it should be much work to get scoped support. Ideally I'd like to just import scoped specs from FriendlyId and test against them, with changes to test that Mobility finds work (similar to how other tests are done in this gem right now).

Answer 7 · 2018-04-04T07:48:49.000Z

Of course if you want to give it a shot, that's also always welcome 😄

Answer 8 · 2018-04-04T11:41:53.000Z

I'm made my early tests the last night, i will try to send you a PR if i find something interesting.

Answer 9 · 2018-09-25T11:14:13.000Z

Any chance to fix this issue? Scoped does not work for me. I am using it in a self referential relationship ie. categories with sub categories.