search

mountetna / magma

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

Add rename_attribute action #200

Closed alimi closed 4 years ago

alimi commented 4 years ago

This PR builds off of https://github.com/mountetna/magma/pull/199. Magma::RenameAttributeAction will change the given attribute's attribute_name without changing its column_name.

To facilitate this change, Magma::Attribute's primary key (via Sequel::Model) was updated so it uses column_name instead of attribute_name.

https://github.com/mountetna/magma/blob/333cf5a0b680949272b3f042a83e4436d18b45c2/lib/magma/attribute.rb#L1-L7

column_name will never change so it should be treated as part of the attribute's primary key.

This PR also changes the rollback behavior in Magma::ModelUpdateActions. Instead of rollback actions in the order they came in, it'll roll them back in reverse so we reset things back to their original state. https://github.com/mountetna/magma/commit/333cf5a0b680949272b3f042a83e4436d18b45c2

This PR does not update model dictionaries.

alimi commented 4 years ago

Instead of having every action define #rollback to roll back in memory changes, Magma::ModelUpdateActions will now restart the server if Magma::ModelUpdateActions fails. This will always get us back to the state before the actions were run without any added maintenance burden.https://github.com/mountetna/magma/pull/200/commits/35b95e8d993c97f91968477b0946a64a84e257cf

graft commented 4 years ago

I added a couple of fixes, one involving the use of _id in foreign key columns, which is for some reason how Sequel likes to do things. This will be different in the future since we now set column_name = attribute_name; I don't think this will cause trouble, but we shall see.

Another issue is that this action doesn't really work for link attributes. It turns out that changing the name of a link attribute is caught by some validation, since it must point to a valid model and there is no way to specify link_model_name; this isn't exactly the best way to catch this error, but good enough.

The actual behavior of moving a link attribute is a more delicate issue best left resolved elsewhere; since there is a reciprocal link relationship to deal with, I feel the best way forward is probably more like unlink_model and link_model actions.