mountetna / magma

Data server with friendly data loaders
GNU General Public License v2.0
5 stars 2 forks source link

Hide type in attributes/split base Attribute #113

Closed graft closed 4 years ago

graft commented 4 years ago

Currently the Magma::Model.attribute method accepts a :type option. This allows you to set the database column type (via the Sequel ORM, which will set the postgres data type for you). This allows for some interesting non-desirable behavior in the model space, if an attribute can be set with a column type that Magma does not directly support.

Especially in the era of #104 where we will not be programming models (and thus allowing all sorts of non-standard shenanigans like this) but instantiating from data, we should instead have only one exposed set of types, that is, magma types.

The current 'type' columns in use are DateTime, Float, String and Integer. There is actually quite a lot of code in Magma and Timur splitting on Magma::Attribute#type as a result of this division, which forks the basic Attribute into these four subtypes.

The change we wish to make is:

  1. Make Magma::Model.attribute private
  2. Add new attribute classes (Magma::DateTimeAttribute, Magma::FloatAttribute, Magma::StringAttribute, Magma::IntegerAttribute) and their corresponding Magma::Model methods (e.g. Magma::Model.integer). These methods should set type, but not expose it as an option.
  3. Update model specs to use 'integer :att_name' instead of 'attribute :att_name, type: Integer'
  4. Fix ColumnPredicate, etc. to use new attribute classes rather than Magma::Attribute.
graft commented 4 years ago

Completed by #120