search

ErwinM / acts_as_tenant

Easy multi-tenancy for Rails in a shared database setup.
MIT License
1.53k stars 263 forks source link

Honor the scope passed to associations when performing validations #282

Closed adrian-gomez closed 1 year ago

adrian-gomez commented 2 years ago

Hi @ErwinM, thanks for the awesome gem!

We recently started to migrate our whole app to use acts_as_tenant and found a small issue for our use case where validations performed by acts_as_tenant are failing when checking for the existence of belongs_to associations. We have been using https://github.com/rubysherpas/paranoia for a long time (we have plans to migrate away from it in the future but it's not an easy task). So the issue is the default scope generated by paranoia which makes deleted records to be skipped during validation, that makes sense since those are deleted! but in some scenarios we do want our models to access deleted associations, the way to do that is to pass a scope to the association:

belongs_to :other, -> { with_deleted }

right now acts_as_tenant is ignoring that scope so the validation fails.

The PR will use the scope if given.

An extra addition (that I can move to another PR if you like) is allowing to pass a scope to act_as_tenant and just pass the scope directly to belongs_to (it seems that's the only modification needed for that to work)

adrian-gomez commented 2 years ago

@excid3 do you think this is something you might want to add to the gem? As we keep using we are coming across some other changes we would like to suggest/implement is a PR the best way to do those? or should we open an issue first? Thanks!

excid3 commented 1 year ago

Thanks for this @adrian-gomez

acts_as_tenant should match the belongs_to interface since it defines the same thing behind the scenes so it would be great to support scopes. 👍

Would you be able to fix the merge conflicts and I can merge this in?

adrian-gomez commented 1 year ago

@excid3 sorry for the delay! I just updated our fork with the latest changes and resolved the merge conflicts

adrian-gomez commented 1 year ago

@excid3 thanks for merging this!

khiav223577 commented 9 months ago

Hi @excid3,

I recently came across this issue and it seems to be impacting users. Would it be possible to consider a release for v0.6.2 to address this? Or is there a recommended workaround to bypass this issue?