SchemaPlus / schema_validations

Automatically creates validations basing on the database schema.
Other
174 stars 34 forks source link

Support case-insensitive uniqueness validations under MySQL #39

Open dmeranda opened 8 years ago

dmeranda commented 8 years ago

This is a proposed fix for issue #38

I'm not sure how to proceed writing a test spec for this. It should only be run with against a MySQL database, and furthermore may require creating a table using raw SQL (execute) rather than as a rails schema migration/load; because a proper test will rely on the MySQL COLLATION type qualifier which is not exposed by the schema loader/dumper (though it is a read-only property of the Column type).

The example code presented in the issue would probably serve as a good basis for a test.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.01%) to 99.275% when pulling 89f54f032b98eeb582a925343c55c66fe84882a8 on dmeranda:mysql-case-insensitive into b3e8be26a076e7e138a8c73df2be8266fc13ad63 on SchemaPlus:master.

coveralls commented 8 years ago

Coverage Status

Coverage increased (+0.01%) to 99.275% when pulling ac3ed285e88fbc0aa6279780547938b650a306d9 on dmeranda:mysql-case-insensitive into b3e8be26a076e7e138a8c73df2be8266fc13ad63 on SchemaPlus:master.

ronen commented 8 years ago

Thanks for the PR. Good catch regarding :collation. Arguably, schema_plus_columns case_sensitive? method is wrong or misleading in its logic. Maybe it should take an optional scope argument. I'll open an issue over there.

I'm not sure how to proceed writing a test spec for this. It should only be run with against a MySQL database,

Schema_dev adds the ability for you to specify, e.g. :mysql => :only or :mysql => :skip as an option on any rspec context or test. See for example validations_spec.rb#L112 or schema_plus_views/spec/named_schemas_spec.rb#L63

and furthermore may require creating a table using raw SQL (execute) rather than as a rails schema migration/load

schema_plus_views/spec/named_schemas_spec.rb also has examples of creating schemas via connection.execute, which you ought to be able to mimic -- and this case should be simpler than that since it'd be mysql only.

If you can fit it into validations_spec.rb i guess that'd be best, otherwise it could be in a separate file. I actually don't like validations_spec's setup of having one big ugly schema shared by all tests; each test or context should define a small schema containing just the bits that are needed for that specific test. This big-ugly-schema style is legacy code that I've been gradually moving away from in the schema plus family. So you shouldn't feel the need to shoehorn the collation stuff into that big schema, just define something specific to the new the case(s) you're testing.

the MySQL COLLATION type qualifier which is not exposed by the schema loader/dumper (though it is a read-only property of the Column type).

That seems like a natural thing for schema_plus_columns to add! I'll open up an issue there for that too.

Thanks again for taking the time with this!