Skip track without modifier

[duhast]

Hi,

This update introduces the possibility to not save tracks when modifier of the model being changed is not set. This comes handy in case the models get updated in many places on backend, but we only need to track changes made by users.

[dblock]

Why wouldn't one add a presence validator that makes the modifier required instead of doing all this work?

Assuming that doesn't do the job, this needs tests to be merged. Also squash the commits, please.

[duhast]

@dblock it's not about validation of modifier presence, it's about not saving tracks when modifier is unset.

[dblock]

Right, if the object is not valid? it won't save. That seems easier to express the same thing, no? Or what am I missing?

[reedlaw]

This is useful for testing when you are creating Factories that don't need history tracking. I believe the intent of this pull request is to allow models to be updated without tracking history. I would also like to make validation optional and if the modifier is missing then skip saving history.

[dblock]

All I am saying is that this logic should be expressed differently than with an if. You should be able to save something with no modifier and disabling validation the "standard" way, with validate: false.

[reedlaw]

I'm having a terrible time getting this to work with FactoryGirl and embedded documents. I've tried adding validate: false as suggested here but that only works on parent documents without embedded documents.

This is my suggestion: updates without modifiers should work without validate: false. Only if modifier is set does the HistoryTracker get created.

[dblock]

@reedlaw Could you write a spec showing what you mean by "having a terrible time getting it to work with ..."?

[reedlaw]

@dblock I could write a spec for FactoryGirl that shows the problem with creating invalid embedded documents, but the change I made to mongoid-history solves my particular problem. I realize it only works with mongoid v6 and changes expected behavior so you don't need to merge it. Maybe this PR will help others in my situation.

[dblock]

@reedlaw LMK if you're interested in finishing this, I can help out. The implementation as it stands I don't think is what we want, see comments above.

[h8rry]

@dblock thanks for following up.

here's a case where the intend of this PR will be useful: records are automatically created by the system (hence, there should be no modifier)

Currently, it requires modifier to be present. Yes, we don't want to disable all validations with validate: false. So, if we can add a params where we can disable validates :modified, presence: true, then it should work.

For example,

Post.create(title: 'LOL', track_modifier: false)

Still quite new to this repo. I tried to edit some code but haven't wrapped my head around it

[dblock]

One thought would be to allow Post.create!(title: 'LOL', updated_by: nil), so when it's set explicitly it can be saved. But then people who do Post.create!(title: 'LOL', updated_by: current_user) will miss the accidental current user being nil.

So I would be ok with track_modifier: false passed into save, someone just needs to PR that.