graft
commented
4 years ago Very nice. A few remarks:
1) Could we use parent_model_name instead of parent or parent_model? The latter suggests a Magma::Model object to me, the former is ambiguous.
2) In line 10 in model.rb we define the array of types. It seems a bit ignominious to have this canonical type definition nestled in this iterator. Options might be: 1) move it to a constant or 2) collect it from the attribute class names. But per (3) below, not all attribute classes might be exposed, so we would probably need to define some sort of flag indicating that an attribute class also represents an exposed type. The advantage of (2) is not having to maintain a list in model.rb, though (1) is obviously easier to implement and maybe clearer in the end.
3) Also in line 10 in model.rb, while I might be mistaken, I don't think foreign_key should actually be an exposed type, as it is just a base class for parent and link types.
125
This will refactor identifier, parent, and link to match the other attribute patterns. The only holdover was that the previous #parent method was a getter and setter method. There is an instance variable set in the define method group in order to set the parent on the model.