DmitryTsepelev / store_model

Work with JSON-backed attributes as ActiveRecord-ish models
MIT License
1.08k stars 89 forks source link

Introduce #assign_preserving method #41

Open flvrone opened 5 years ago

flvrone commented 5 years ago

Which will preserve existing nested attributes :) For me, it's quite a boilerplate code now, and it's useful, so I think it might be handy to be here out of the box. Here's an example of something I already have:

class Site
  class Settings
    include StoreModel::Model

    NESTED_MODELS_TO_PRESERVE = %i[colours style].freeze

    attribute :twitter_handle, :string
    # ...

    attribute :colours, ColourScheme.to_type, default: -> { ColourScheme.new }
    attribute :style, Styling.to_type, default: -> { Styling.new }
    attribute :footer_pages, FooterPage.to_array_type, default: -> { [] }

    def assign_preserving(attrs)
      attrs = attrs.dup

      NESTED_MODELS_TO_PRESERVE.each do |model_key|
        next unless (nested_model = send(model_key))
        next unless (nested_attrs = attrs.delete(model_key))

        nested_model.assign_preserving(nested_attrs)
      end

      # NOTE: footer_pages are not being "preserved"
      #       and are overwritten if given, because it is an array
      assign_attributes(attrs)
    end
  end
end

And all the nested models have the same assign_preserving method, however their NESTED_MODELS_TO_PRESERVE might be blank. Yeah, and I guess we might have something more sophisticated than a constant of all nested models.

DmitryTsepelev commented 5 years ago

Hi @FunkyloverOne!

Thanks for the reaching out, I like the idea! Let's design the API for it 🙂

I think it should be possible to patch the .attributedefinition to accept preserve: argument:

class Site
  class Settings
    include StoreModel::Model

    attribute :twitter_handle, :string
    # ...

    attribute :colours, ColourScheme.to_type, default: -> { ColourScheme.new }, preserve: true
    attribute :style, Styling.to_type, default: -> { Styling.new }, preserve: true
    attribute :footer_pages, FooterPage.to_array_type, default: -> { [] }
  end
end

In this case #assign_preserving can be moved to StoreModel::Model, and we should also make sure that if preserve: true is passed than type is either JsonType or ArrayType. What do you think?

flvrone commented 5 years ago

Hey @DmitryTsepelev, one important thing is that we also need to have that #assign_preserving on a parent ActiveRecord model.

Arrays preservation is not that easy as singular JSONs. You see, we need a way of identifying each individual element, updating/removing it, adding new ones. It is supposed to be similar to ActiveRecord's accepts_nested_attributes_for API.

But we could start without arrays preservation support, it's already good enough 😄

DmitryTsepelev commented 5 years ago

Makes sense. We should probably change the name to something that can be easily associated with StoreModel (like assign_preserving_nested_store_models but less verbose)