Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dirty Tracking #386

Open
johnmeehan opened this issue Jul 26, 2016 · 9 comments
Open

Dirty Tracking #386

johnmeehan opened this issue Jul 26, 2016 · 9 comments

Comments

@johnmeehan
Copy link

RE: Dirty Tracking

I have a great big Reform Form that creates/updates a product that is composed of a number models. The form gathers dimensions for the different components that all change size depending on the overall size of the product.

Create action with Reform works fantasticly! Way better than the "Rails Way".

When editing these models with the form I need to update the dimensions ( and if I change one they all have to recalculate). Currently I have been using a before_validation :recalculate_dimensions call back in the models which will update the associated models if one of them is edited. It works but its messy.

I wish to now extract this before_validation call back to a service object.

For dirty tracking Reform currently only has a form.changed?(:title) #=> true method.

The code I am trying to pull out of the models is:

  # Rails model code.
  before_validation :recalculate_dimensions, unless: :new_record?

  def recalculate_dimensions
    if changed?
      if height_changed?
        some_long_calculation  
      end
      if width_changed?
        width_diffrence = width_was - width_change[1]
        width_recalculation(width_diffrence)
      end
    end
  end


What am I getting at?

To me it looks like I am missing from the Active Model Dirty Tracking is the xxx_was and xxx_change methods.

  1. Can I in the Reform world do something simular to these methods? Or do I need to/should I do a pull request to add them to Reform...
  2. Is there a way to get the original form value and the about to be saved value? i.e @form.height_was @form.height_change
  3. Can I update the other dimensions through reform before the validations get run?

Also discussed: #117

@fran-worley
Copy link
Collaborator

fran-worley commented Jul 26, 2016

Lets take these one at a time:

  1. You can use the change-tracking from the disposable API as your reform instance is just a Twin.
  2. Reform makes no changes to the model until you call save. Validate just populates the twin ready for saving to your model so calling reform_form.model will give you the model you started with which you could use to access the original values.
  3. If you want to make changes to things before running validations I suggest you use a populator

@dnd
Copy link

dnd commented Dec 20, 2016

I think the biggest part of what @johnmeehan is suggesting is the xxx_was(AM) or xxx_initial(Sequel) methods. Just knowing that a field changed such as what disposable provides is not enough. I haven't worked on disposable at all, but it seems like this might be a fairly easy change?

@fran-worley
Copy link
Collaborator

@dnd because of the way that Reform works, you can access the xxx_was value simply by getting the model value. (Before sync/save obviously) such a method is necessary in AR because values of overwritten directly on the model instance, as we don't do that in Reform we've never seen the need to have a special method simply as an alias for model.attr ...

@dnd
Copy link

dnd commented Dec 20, 2016

@fran-worley yes, of course that is possible to do, but it feels like it is more of a workaround than anything. This seems like such a common operation, that having a more declarative way of doing it would lead to better code that more expresses the intent of what is happening.

Also, as it stands you have to know the state of the form to use this method. If the model hasn't been sync'd, then this works just fine. If the model has been sync'd, then you have to use its methods. So now you can't just have one way of doing it, even though reform should easily be able to know and provide this information.

Further, what if you're using reform with some type of ORM model, but also PORO models that don't implement any dirty tracking themselves.

Basically if reform implements this functionality you can have one way of doing it no matter what the scenario.

@apotonick
Copy link
Member

I don't mind one before- and after accessor in Disposable, but this is absolutely not code for a form, but must go to the update operation wrapping this.

@fran-worley
Copy link
Collaborator

@dnd maybe I'm misunderstanding you. I don't see how accessing the form value vs the model value is a work around its kind the whole point of Reform.

Even ActiveModel::Dirty requires some knowledge of state as once you're record is saved your changed? And other methods resent.

I just worry about tagging on another round of methods to make Reform behave more like AR/AM when just tweaking your workflow to better embrace Reform might result in better code and a smaller gem.

if you really really want a _was as an alias to call model.attr then it would end up in Reform-rails in a seperate module, but I'm really not convinced it's worth it.

@apotonick
Copy link
Member

@fran-worley We can put this into Disposable into the Twin::Changed module, this should be fine there. Or you think it's wrong?

Agreeing that this ain't no stinky Reform thing, though!

@dnd
Copy link

dnd commented Dec 21, 2016

It definitely wouldn't belong in reform-rails. Heck, I'm not even using Rails, and I need it. I think this aligns very much with the semantics of what one would expect from a form. The form should be able to tell me about the changes that are going to occur, not just that something changed.

Given what Disposable is, and that it is already partially implemented there that is definitely the right place for the rest of the functionality though.

The problem is mostly in the event of needing to do any dirty tracking after a sync has happened.

Here's an example to try and demonstrate.

# Using a Sequel model
user = User.create(name: 'tom')
form.validate(name: 'bob')
form.name => 'bob'
user.name => 'tom'
form.sync
form.name => 'bob'
user.name => 'bob'

# At this point you might say you could use the dirty tracking on the model
user.initial_value(:name) => 'tom'


# What about if you're using a PORO, that doesn't have any dirty tracking?
# It's the same scenario as above, but after the sync you have no option. 
# Meaning if you use a PORO model, you have to then implement your own dirty 
# tracking to be able to use it.

# If the form implements this functionality then it works no matter what is used, and 
# what the current context of the form is.
form.name => 'bob'
form.initial_value(:name) => 'tom'
form.sync
form.name => 'bob'
form.initial_value(:name) => 'tom'

@emaglio
Copy link
Member

emaglio commented Jul 14, 2019

@apotonick let’s move this to the disposable gem as you suggested

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants