mountetna / magma

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

Support add_attribute #169

Closed graft closed 3 years ago

graft commented 4 years ago

In /update_model, we may add an attribute to an existing model using the add_attribute action. This is the simplest possible 'safe-migrating' action, which requires us only to add a new column.

We already insert attributes in a couple of places, for example in the load_projects command: https://github.com/mountetna/magma/blob/fb3154d877708d9f74fce257cf016b6ac08dd96f/lib/magma/commands.rb#L33 The requirements of the action are therefore the same as here, a valid input row include model_name, attribute_name and all of the Magma::Attribute.options (except maybe creation/update stuff).

The action should fail if the attribute exists - while we could update the existing attribute instead, the ambiguity is worth avoiding in such fraught situations.

In the non-migrating action we had a single job to do, to update the attributes table. This action, however, must also modify the data tables, in addition to inserting into the database and loading the new attribute. The migration here is pretty simple, just add a column to the corresponding model table with the specified attribute name.

How should we run the migration itself? Sequel provides a migration class which we use for running the migrations we normally use - we can (?) do the same here. In this case we might make a service class inheriting from Sequel::Migration which has an #up method that calls alter_table and add_column with the instance variables @model_name, @attribute_name, @type, which we can initialize with our arguments and then immediately apply. We don't need a down method because we should undo this action by running another action like remove_attribute, so we don't need a concept of reversibility here.

Once we've run the migration, the new column and attribute should be available to add data into through the usual Magma methods.

graft commented 4 years ago

Note that in some cases (link attributes like collection) the migration need not run because no column is required. Conversely other link attributes would require validating that link_model_name exists.

graft commented 4 years ago

We should be validating each of the attribute inputs for sure, but some validations here would include:

  1. project_name, attribute_name and model_name are all in snake_case.
  2. project_name and model_name have existing referents.
  3. attribute_name does not collide with an existing attribute.
  4. link_model_name is in snake_case and references an existing model. If link_model_name is null, attribute_name is used instead, so in this case should reference an existing model (e.g. adding a child :tumbo should determine there is a model named tumbo.)