Refactor validates_uniqueness_to_tenant

[artplan1]

Hi,
I refactored this validation method because it didn't work for global records with mobility. Queries used in validates_uniqueness_to_tenant didn't work with mobility's fields.
But mobility overrides UniquenessValidator to validate translated fields.

I decided to use validates_uniqueness_of in the case of global model, because validates_uniqueness_of runs same query by itself.

For global records:

• In 1st validates_uniqueness_of we're validating uniqueness in tenant (query: tenant = value)
• in 2nd validates_uniqueness_of we're validating global uniqueness (query: tenant = nil)

in perfect world it should be done with 1 query validating tenant = nil OR value, but rails validations don't allow it.

[artplan1]

I don't like this, because it discards conditions from args, not sure how to merge lambdas here

[excid3]

Only thing I could imagine working for that would be an array of lambdas?

[artplan1]

hi again @excid3 ,
while trying to figure out what is wrong in our codebase I found out one more case that wasn't covered in validations/tests here:
when user is trying to create global record, but record with same name exist in some tenant.

I splitted validations to 3:

1. validate within tenant scope (old validation that is working and is correct)
2. if model is global and tenant is not set on instance - validating globally (tenant id can be nil or filled)
   1st validation doesn't cover this, because when tenant is set it filters within tenant, if it's not set it filters within nil. But we need to search within all records if current record is global
3. if model is global and tenant is set on instance - validating within global records (searching non-unique records with tenant id = nil)
   1st validation covers half of this, because when tenant is set it filters within tenant, if it's not set it filters within nil. But we need to search within records with tenant = nil or tenant = current (covered by 1st) if current record has tenant = current
   2nd doesn't cover this we need scoping

Also I've added calls for conditions and if if they are passed to validates_uniqueness_to_tenant

Also I think if ActsAsTenant.models_with_global_records.include?(self) is redundant here. If record is not global - it shouldn't have empty tenant in this validation. This way we can reduce number of validations by adding if: ->(instance) { instance[fkey].present? } to 1st one, because it only makes sense if tenant is set

[excid3]

I think this sounds good. I haven't used any global models like this in an app so I'll have to refresh myself on it.

Could any of these changes be breaking for existing users?

[artplan1]

Could any of these changes be breaking for existing users?

I hope no

1. I think small percentage of gem users use global records
2. tests are green
3. tests are green in our project with changes from my branch :D

[dmc0re]

lgtm, plz 🚢 it